Skip to content
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

enable to read custom limb name from .yaml file (fix for #249) #250

Merged
merged 7 commits into from
Apr 16, 2023

Conversation

Naoki-Hiraoka
Copy link
Contributor

#249 に、

  • 複数yamlを読み込む機能
  • replace_xmlsなどのキーをlimbと誤って解釈しないようにする修正

を追加しました。

euscollada/src/collada2eus_urdfmodel.cpp Outdated Show resolved Hide resolved
euscollada/src/collada2eus_urdfmodel.cpp Outdated Show resolved Hide resolved
euscollada/src/collada2eus_urdfmodel.cpp Outdated Show resolved Hide resolved
euscollada/src/collada2eus_urdfmodel.cpp Outdated Show resolved Hide resolved
@@ -0,0 +1,79 @@
##
Copy link

@pazeshun pazeshun Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

気づくの遅れて申し訳ない、このsamplerobot1.yaml, samplerobot2.yamlって、euslisp/jskeus#598 で追加してくれたみたいなケンタウロス型ロボットになったりします?そうしておくと、このPRで追加した機能が後の人たちに伝わりやすくなる気がしました。難しそうなら教えて下さい

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ケンタウロスはちょうどよいcolladaモデルが見つからなかったので、PR2モデルのハンドをcustom limb化するテストを追加しました。6505add
目的1: custom limb対応 に対応するテストがgenerate-pr2-custom-limb-test

目的2: replace_xmlsなど、eusで使わないキーを別のyamlにしたい に対応するテストがgenerate-sample-robot-multi-yaml-test

です

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとう。そうか、colladaモデルが必要だったね、申し訳ない。
元々の @Naoki-Hiraoka のモチベーションはハンドだったと思うので、結果的によりクリアになりました。
Hiroで動くか確認したりしてみようと思っているので、お待ちください

Copy link
Contributor Author

@Naoki-Hiraoka Naoki-Hiraoka Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hiroで動くか確認したりしてみようと思っているので、お待ちください

既存のhrpsysにつなげる場合には start-jsk/rtmros_common#1121 が必要になるかもしれません

@Naoki-Hiraoka Naoki-Hiraoka force-pushed the custom_limb-checkjoint branch from 07896f2 to 6505add Compare March 14, 2023 03:40
@k-okada
Copy link
Member

k-okada commented Mar 17, 2023

@pazeshun https://gist.github.com/k-okada/5d1b7bfbfa3f84792ee97cfdb3431eee にdiff を添付します.
master のソースツリーに
#250 を加えたもの https://gist.github.com/k-okada/5d1b7bfbfa3f84792ee97cfdb3431eee#file-custom_limb-checkjoint-diff
start-jsk/rtmros_common#1130 も加えたもの https://gist.github.com/k-okada/5d1b7bfbfa3f84792ee97cfdb3431eee#file-src-scripts_custom_limb-diff
です.タイムスタンプだけでなくて,eusのコードも変わっているけど,そういもの???

ロボットモデルの見た目は同じかなと思いました.
スクリーンショット 2023-03-17 161123

@pazeshun
Copy link

pazeshun commented Apr 12, 2023

大変遅くなりました。
@k-okada 自分の手元でもdiffが出てしまうことを確認しました。
@Naoki-Hiraoka このPRとstart-jsk/rtmros_common#1130 を使ってhironxjsk.lを生成すると、使わない場合と比べて以下のようなdiffが生じます。
https://gist.githubusercontent.com/pazeshun/7952f16ede3aa9229e08a314c60bc030/raw/05858c10d557564a03d519c258cdf686ae5b7a97/hironxjsk_hiraoka_diff.txt
linkの宣言順番とかが変わってるみたいでdiffが大きく、チェックが難しいのですが、簡単な修正でdiffが小さくなるようにできたりしないでしょうか。
簡単じゃなければ教えてください。
一応、hironxjskで:start-grasp/:stop-grasp/:angle-vectorができるのは確認したのと、PR2のeusモデルのテストは通るので、最悪モデルファイル変わっていても大丈夫そうではあるのですが・・・。

確認方法

mkdir -p ~/ros/ws_hiraoka_before/src
cd ~/ros/ws_hiraoka_before/src
wstool init
wstool merge https://gist.githubusercontent.com/pazeshun/3c653fae2159ec1372c42f91ff88328e/raw/29db1aff911494ce587fd352a79dd771b7d7a467/ws_hiraoka_before.rosinstall
wstool update
cd ..
catkin build hironx_tutorial

mkdir -p ~/ros/ws_hiraoka_after/src
cd ~/ros/ws_hiraoka_after/src
wstool init
wstool merge https://gist.githubusercontent.com/pazeshun/a6910235b56fb833fd0cd39392618015/raw/38a5330c8420cbd40692e8c44e88b9ca9cf521de/ws_hiraoka_after.rosinstall
wstool update
cd ..
catkin build hironx_tutorial

cd ~/ros
diff -u ws_hiraoka_before/src/rtmros_tutorials/hrpsys_ros_bridge_tutorials/models/hironxjsk.l ws_hiraoka_after/src/rtmros_tutorials/hrpsys_ros_bridge_tutorials/models/hironxjsk.l > ~/hironxjsk_hiraoka_diff.txt

source ~/ros/ws_hiraoka_after/devel/setup.bash
rostest pr2eus make-pr2-model-file-test.launch

@pazeshun
Copy link

pazeshun commented Apr 12, 2023

@Naoki-Hiraoka HIRONXJSK_controller_config.yamlには以下のdiffが出ていて、明らかに間違ってそうなのですが、どうでしょうか。
https://gist.githubusercontent.com/pazeshun/dbc7d2c6a3897c76e4f4dd957b3b9c7d/raw/c3b58149841595541ddb54b59f4ba59a8747bf26/HIRONXJSK_controller_config_hiraoka_diff.txt

確認方法

前のコメントで作った環境で、

cd ~/ros
diff -u ws_hiraoka_before/src/rtmros_tutorials/hrpsys_ros_bridge_tutorials/models/HIRONXJSK_controller_config.yaml ws_hiraoka_after/src/rtmros_tutorials/hrpsys_ros_bridge_tutorials/models/HIRONXJSK_controller_config.yaml > ~/HIRONXJSK_controller_config_hiraoka_diff.txt

@Naoki-Hiraoka
Copy link
Contributor Author

ご確認くださりありがとうございます。

挙動には影響を与えませんが、今回改めて変更内容を確認したときに気になった箇所をリファクタリングしました。6cb9aa6

@Naoki-Hiraoka HIRONXJSK_controller_config.yamlには以下のdiffが出ていて、明らかに間違ってそうなのですが、どうでしょうか。 https://gist.githubusercontent.com/pazeshun/dbc7d2c6a3897c76e4f4dd957b3b9c7d/raw/c3b58149841595541ddb54b59f4ba59a8747bf26/HIRONXJSK_controller_config_hiraoka_diff.txt

間違いでした。start-jsk/rtmros_common@c4aec1b で修正しました。

@Naoki-Hiraoka このPRとstart-jsk/rtmros_common#1130 を使ってhironxjsk.lを生成すると、使わない場合と比べて以下のようなdiffが生じます。
https://gist.githubusercontent.com/pazeshun/7952f16ede3aa9229e08a314c60bc030/raw/05858c10d557564a03d519c258cdf686ae5b7a97/hironxjsk_hiraoka_diff.txt
linkの宣言順番とかが変わってるみたいでdiffが大きく、チェックが難しいのですが、簡単な修正でdiffが小さくなるようにできたりしないでしょうか。

行数が多いので全ては見れていませんが、リンク名だけ目視で確認したところ、リンクの宣言順番は変化していますが、全体は変わっていないようです。

このPull Requestで変更した部分では無いのですが、

#if URDFDOM_1_0_0_API
for (map<LinkConstSharedPtr, Pose >::iterator it = m_link_coords.begin();
#else
for (map<boost::shared_ptr<const Link>, Pose >::iterator it = m_link_coords.begin();
#endif


#if URDFDOM_1_0_0_API
for(map <LinkConstSharedPtr, MapVisual >::iterator it = m_link_visual.begin();
#else
for(map <boost::shared_ptr<const Link>, MapVisual >::iterator it = m_link_visual.begin();
#endif

で、linkのshared_ptrをキーとするmapにlinkが格納されていて、mapのイテレータを回したときにどのような順番になっているかによって、linkの宣言順番が決まっています。恐らくmapの順番は、linkのメモリを確保するときにどのアドレスに確保されたかに依存すると思われます。

そのため、今回変更した部分はlinkの宣言順番と直接関係しない部分なのですが、今回変更した部分で行うメモリ確保・開放がlinkのアドレスに何らかの影響を与えて、linkの宣言順番が変化してしまっているのではないかと思います。

今後も何か変更するたびにlinkの宣言順番が変化してしまうのを防ぐためには、shared_ptrをキーとするmapのイテレータを回してlinkを宣言する代わりに、例えばlink名の文字列をキーとするmapのイテレータを回して宣言するという方法に変える、といった方法が考えられます。このPull Requestとはまた別の話になるかと思いますが、この変更を行うPull Requestを出すことは可能です。

@pazeshun
Copy link

pazeshun commented Apr 14, 2023

@Naoki-Hiraoka

そのため、今回変更した部分はlinkの宣言順番と直接関係しない部分なのですが、今回変更した部分で行うメモリ確保・開放がlinkのアドレスに何らかの影響を与えて、linkの宣言順番が変化してしまっているのではないかと思います。

今後も何か変更するたびにlinkの宣言順番が変化してしまうのを防ぐためには、shared_ptrをキーとするmapのイテレータを回してlinkを宣言する代わりに、例えばlink名の文字列をキーとするmapのイテレータを回して宣言するという方法に変える、といった方法が考えられます。このPull Requestとはまた別の話になるかと思いますが、この変更を行うPull Requestを出すことは可能です。

なるほど、宣言順番を固定化するように変更するにしても、今回は順番が変わってしまうことが避けられなさそうな感じですね。
変更しなくていいかと思います。

挙動には影響を与えませんが、今回改めて変更内容を確認したときに気になった箇所をリファクタリングしました。6cb9aa6

@Naoki-Hiraoka HIRONXJSK_controller_config.yamlには以下のdiffが出ていて、明らかに間違ってそうなのですが、どうでしょうか。 https://gist.githubusercontent.com/pazeshun/dbc7d2c6a3897c76e4f4dd957b3b9c7d/raw/c3b58149841595541ddb54b59f4ba59a8747bf26/HIRONXJSK_controller_config_hiraoka_diff.txt

間違いでした。start-jsk/rtmros_common@c4aec1b で修正しました。

@Naoki-Hiraoka このPRとstart-jsk/rtmros_common#1130 を使ってhironxjsk.lを生成すると、使わない場合と比べて以下のようなdiffが生じます。
https://gist.githubusercontent.com/pazeshun/7952f16ede3aa9229e08a314c60bc030/raw/05858c10d557564a03d519c258cdf686ae5b7a97/hironxjsk_hiraoka_diff.txt
linkの宣言順番とかが変わってるみたいでdiffが大きく、チェックが難しいのですが、簡単な修正でdiffが小さくなるようにできたりしないでしょうか。

行数が多いので全ては見れていませんが、リンク名だけ目視で確認したところ、リンクの宣言順番は変化していますが、全体は変わっていないようです。

今回の変更後のdiffは以下のようになりました。
hironxjsk.l:
https://gist.githubusercontent.com/pazeshun/7952f16ede3aa9229e08a314c60bc030/raw/d6166cf5d9cf6f7faa631c2ed184a76a4135c61a/hironxjsk_hiraoka_diff.txt
HIRONXJSK_controller_config.yaml:
https://gist.githubusercontent.com/pazeshun/dbc7d2c6a3897c76e4f4dd957b3b9c7d/raw/0fb096c5346f57f7e5eccdcd42f97443d210906f/HIRONXJSK_controller_config_hiraoka_diff.txt

HIRONXJSK_controller_config.yamlについては治ったようで、リンクの宣言順番に関するdiffしかありません。ありがとうございます。
hironxjsk.lについては、全体をざっと見ましたが、日付やファイルパス、リンクの宣言順番に関するdiff以外のdiffは発見できませんでした。
追加で、以下のように、URDFは変わっていないことを確認しました。

diff -u ws_hiraoka_before/src/rtmros_tutorials/hrpsys_ros_bridge_tutorials/models/HIRONXJSK_SENSORS.urdf ws_hiraoka_after/src/rtmros_tutorials/hrpsys_ros_bridge_tutorials/models/HIRONXJSK_SENSORS.urdf

また、Hiro実機に接続されたPC上でこのPRとrtmros_commonのPRが入ったworkspaceを作り、実機で立ち上げるlaunchをそのworkspaceから起動して、正常に起動することと、rviz上でエラーなくロボットモデルや点群が見えること、roseusから:angle-vector/:start-grasp/:stop-graspできることを確認しました。
以上から、恐らく大丈夫であろうと判断します。

なお、以下のPR2のeusモデルのテストも通りました。

source ~/ros/ws_hiraoka_after/devel/setup.bash
rostest pr2eus make-pr2-model-file-test.launch

@k-okada k-okada merged commit f327d15 into jsk-ros-pkg:master Apr 16, 2023
@k-okada
Copy link
Member

k-okada commented Apr 16, 2023

このPull Requestとはまた別の話になるかと思いますが、この変更を行うPull Requestを出すことは可能です。

mapの順番が違うから結果がも違う,というのが,案外近い将来に起こりそうなので,
URDFDOM_1_0_0_API の場合だけでいいので,対応してたORを作ってください.

https://thispointer.com/stdmap-tutorial-part-2-stdmap-and-external-sorting-criteria-comparator/
でいいのかな.
@Naoki-Hiraoka

@Naoki-Hiraoka
Copy link
Contributor Author

mapの順番が違うから結果がも違う,というのが,案外近い将来に起こりそうなので,
URDFDOM_1_0_0_API の場合だけでいいので,対応してたORを作ってください.

https://thispointer.com/stdmap-tutorial-part-2-stdmap-and-external-sorting-criteria-comparator/
でいいのかな.

#252 に作成しました。変更点が少なかったため、URDFDOM_1_0_0_APIの場合とそうで無い場合の両方を変更しました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants