-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[hrpiob] Upgrade for hrpsys 315.4.0 onward. #481
base: indigo-devel
Are you sure you want to change the base?
[hrpiob] Upgrade for hrpsys 315.4.0 onward. #481
Conversation
a7cccfa
to
be738c3
Compare
Relevant PR fkanehiro/hrpsys-base#598 |
better to have backward compatibility |
確認ですが,次のように理解しています.
上記の理解で正しいとすると,バージョン互換性を保つためのコードを加えるべきは,正論としては下流の hrpsys と思っています. |
逆で、このコードがhrpsysにソースコードとしても作られたライブラリとしても依存しています.ソースコードとしては
```
#include <hrpsys/io/iob.h>
```
があるのと、ライブラリとしては、hrpsysが315.4.0
以前の環境だとread_batteryとかを定義した..soでないとロードできないというこおとで、hrpsysのバージョンにおうじて、ここのソースの書き方が変わるので、このコードがhrpsysに依存しています.
fkanehiro/hrpsys-base#597
と表れ方は違いますが、原因はおなじです.
```
k-okada@p40-yoga:/tmp/rtmros_hironx/hironx_ros_bridge/robot/hrpiob$ cat
hoge.txt
diff --git a/hironx_ros_bridge/robot/hrpiob/CMakeLists.txt
b/hironx_ros_bridge/robot/hrpiob/CMakeLists.txt
index 7077046..3331d20 100644
--- a/hironx_ros_bridge/robot/hrpiob/CMakeLists.txt
+++ b/hironx_ros_bridge/robot/hrpiob/CMakeLists.txt
@@ -38,15 +38,29 @@ include_directories(/opt/jsk/include /usr/pkg/include)
link_directories(/opt/jsk/lib /usr/pkg/lib)
#endif()
-# hrpsys-base
-#include(FindPkgConfig)
+# Can not use this????
+# find_package(PkgConfig)
+# pkg_check_modules(HRPSYSBASE REQUIRED hrpsys-base)
+# include_directories(${HRPSYSBASE_INCLUDE_DIRS})
+# hrpsys-base
+include(FindPkgConfig)
execute_process(
COMMAND pkg-config --cflags-only-I hrpsys-base
OUTPUT_VARIABLE HRPSYSBASE_CXX_FLAGS
RESULT_VARIABLE RESULT)
+execute_process(
+ COMMAND pkg-config --modversion hrpsys-base
+ OUTPUT_VARIABLE HRPSYSBASE_VERSION
+ RESULT_VARIABLE RESULT)
message("HRPSYSBASE_CXX_FLAGS: ${HRPSYSBASE_CXX_FLAGS}")
+message("HRPSYSBASE_VERSION: ${HRPSYSBASE_VERSION}")
+if(NOT "${HRPSYSBASE_VERSION}" VERSION_LESS "315.4.0")
+ add_definitions(-DROBOT_IOB_VERSION=2)
+ message(STATUS "USE VERSION 2 OF IOB")
+ message(STATUS "ADD DEFINITION -DROBOT_IOB_VERSION=2")
+endif()
add_definitions(${HRPSYSBASE_CXX_FLAGS})
diff --git a/hironx_ros_bridge/robot/hrpiob/iob.cpp
b/hironx_ros_bridge/robot/hrpiob/iob.cpp
index 529c51c..931f596 100644
--- a/hironx_ros_bridge/robot/hrpiob/iob.cpp
+++ b/hironx_ros_bridge/robot/hrpiob/iob.cpp
@@ -734,6 +734,7 @@ int read_digital_output(char *doutput)
return FALSE;
}
+#if defined(ROBOT_IOB_VERSION) && ROBOT_IOB_VERSION >= 2
/**
* @brief Needed for hrpsys 315.4.0 onward (added at
fkanehiro/hrpsys-base#598).
* TODO Implement if we want to utilize this.
@@ -763,3 +764,4 @@ int read_battery(char *battery)
std::fprintf(stdout, "read_battery not implemented. See
#481\n");
return 0;
}
+#endif
diff --git a/hironx_ros_bridge/robot/hrpiob/nextage-open.hpp
b/hironx_ros_bridge/robot/hrpiob/nextage-open.hpp
index c91e2e2..dce6744 100644
--- a/hironx_ros_bridge/robot/hrpiob/nextage-open.hpp
+++ b/hironx_ros_bridge/robot/hrpiob/nextage-open.hpp
@@ -68,9 +68,11 @@ namespace NEXTAGE_OPEN
virtual int length_digital_output(void) = 0;
virtual int read_digital_output(char *doutput) = 0;
+#if defined(ROBOT_IOB_VERSION) && ROBOT_IOB_VERSION >= 2
virtual int number_of_thermometers(void) = 0;
virtual int number_of_batteries(void) = 0;
virtual int read_battery(char *battery) = 0;
+#endif
};
}
```
ちなみに、iob.hは全体がextern "C"でかこまれていて、-DROBOT_IOB_VERSION>=2
をしなにと、number_of_batteiresとかの定義がされないから、できた..soでC++スタイルの定義になっていて、
hrpsys側からloadした時にエラーになりますね.なので、nextage-open.hpp でこれらの関数を
定義しているのが間違いで、iob.hにちゃんと定義があるから、-D.. をつかってこれを利用できるようにする
というのが正解ですね.
参考:
http://stackoverflow.com/questions/1041866/in-c-source-what-is-the-effect-of-extern-c
http://www.tldp.org/HOWTO/C++-dlopen/thesolution.html
…--
◉ Kei Okada
2016年12月16日 11:43 Isaac I.Y. Saito <[email protected]>:
better to have backward compatibility
確認ですが,次のように理解しています.
- "backward compatibility" は hrpsys の古いバージョンのこと.
- hrpsys が hrpiob に依存する (つまり hrpiob が上流).
上記の理解で正しいとすると,バージョン互換性を保つためのコードを加えるべきは,正論としては下流の hrpsys と思っています.
よくわかっていませんが,両者は密接な関係がある等の理由で,正論通りにはいかず,hrpiob 側に互換コードを入れるのがこの場合正しい,
という状況でしょうか?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#481 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAeG3J5nJCjIX5qff9yFrYKg9Tc49EjAks5rIfrTgaJpZM4LLAIk>
.
|
15275ba
to
372c9e1
Compare
Thank you very much for the deep insight and a patch. Commits are now updated, and I'll test the .so with this change on the real robot.
|
Trying the binary
|
Not sure if this is related, but in I assume the
|
Looking at older ModelLoader log files, the less output started exactly when hrpsys was updated to 315.9.0. Now I also switched to hrpsys 315.7.0 but the symptom in both ModelLoader.log and rtcd.log remain the same.. |
Looks like this is also something that started with newer hrpsys (as you see below no rtcd-* from last month where the controller was still 315.1.8 has it).
|
I found an old ticket start-jsk/rtmros_common#294 (comment) where I see the same |
Switched back to
|
372c9e1
to
92baa5e
Compare
I've added a couple of commits, and now I'm able to build with I noticed, however, that cmake failed to find
|
Since hrpsys 315.4.0 with updated
RobotHardware
, rtcd on QNX fails on NEXTAGE Open as follows.hrpiob
with this PR hopefully fixes the issue.Please review @534o @emijah