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

Fix init-ending overwrite #509

Closed
wants to merge 2 commits into from
Closed

Conversation

snozawa
Copy link
Collaborator

@snozawa snozawa commented Apr 18, 2017

#508

@YoheiKakiuchi
please check.

@snozawa
Copy link
Collaborator Author

snozawa commented Apr 18, 2017

この直し方じゃない気がしてきました。

@YoheiKakiuchi
Copy link
Member

この直し方じゃない気がしてきました。

どうしてかな?

@snozawa
Copy link
Collaborator Author

snozawa commented Apr 18, 2017

euslisp/jskeus#428
にPRしてみました。

joint min/max table自体はupstreamのjskeus本体の機能なので、そこを修正すると良い気がしました。
upstreamの:init-endingでdefine-min-max-tableをよぶことで、hrp2jsk.lなどのファイルの中で:init-endingをそもそも定義する必要がなくなり、それに伴うアヤシイコード(このPR)も必要なくなるような気がしました。

@snozawa
Copy link
Collaborator Author

snozawa commented Apr 18, 2017

このPRの代わりに、euslisp/jskeus#428 に対応した別途rtmros_tutorials側の修正はいりそうです。
ちなみに、あんまりよくない副作用?ですが、上記別途の対応がなくても euslisp/jskeus#428 が取り込まれれば、#508 の問題は治るっぽいです。
誤って:init-endingが上書きされても、:define-min-max-tableされるのが上の:init-endingなので。

@k-okada
Copy link
Member

k-okada commented Apr 18, 2017

euslisp/jskeus#428 (comment) > so best way is to set min-max-table when we make each joint,

#508 でよくないかな.KISS, keep simple and stupid とこれまで思っていたんだけど、wikipediaみるとkeep simple, stupid なんだね.そういうつもりはなくて、http://www.shoeisha.com/book/hp/pc/book/hsl/linus.html 読んで感銘を受けて、いつも心がけていたつもりなんだけど、勘違いだったか.「KISSとは “Keep it Simple, Stupid”、あるいは “Keep it Simple and Small” の略なんだけど」となっている.linuxはstupid なコードは書いていないんだな.ムムム.

長期的には、そもそもstaro/jaxon系で:init-endingに追加している部分がよくなくて、bodyはmodelかえたらいいし、coordsはどうするんだろうね.リンクを作ればいいのかな.というか、別リンクになっていたら、(:hogehoge-link ()) でアクセスできないっけ?

@snozawa
Copy link
Collaborator Author

snozawa commented Apr 18, 2017

長期的には、そもそもstaro/jaxon系で:init-endingに追加している部分がよくなくて、bodyはmodelかえたらいいし、coordsはどうするんだろうね.リンクを作ればいいのかな.というか、別リンクになっていたら、(:hogehoge-link ()) でアクセスできないっけ?

そうですね。これはおっしゃるとおりだと思います。
モデル形状は、VRMLでかえるべきですね

他、coordsなどは:init-endingに書く以外のアイデアが浮かんでない状態です。
リンクを作る、は、euslispの段階で追加するのであれば、:init-endingに書くという点では同じっぽいですし、
それ以前(convert前、VRMLなど)で追加すると仮想のリンクが増えることによるキネマティクスやシミュレータの計算が増えそうで、影響が大きくKISSでないかな、とちょっと思いました。

euslisp/jskeus#428 (comment) > so best way is to set min-max-table when we make each joint,

こちらは、もしmake-min-max-tableが超はやければ確かにbest wayだと思います。
モデルのインスタンスを生成するときのjointインスタンス生成時に実行すると、
現状(とrbrainのころから)、やや時間がかかってたようなので、極力モデルインスタンス生成時間を減らすためには
make-min-max-tableを事前にモデルファイルconversion時に行うように、分けるのかなあと思ってます。

一旦#509, #508などをみた挙句、euslisp/jskeus#428 を作ったのもKISSを考えたためです。
:init-endingのoverwriteをたくみにやるよりは、joint-min-max-talbe自体が存在するirteusのレベルで行う方が、
この辺に必要になってくるコードの量、メンテしないといけないファイル数がへるためです。

ただ、#509 (comment)
にコメントいただいて改めて思い返してみると、モデルに:init-endingを追加する部分(https://github.com/start-jsk/rtmros_tutorials/blob/master/hrpsys_ros_bridge_tutorials/euslisp/make-joint-min-max-table.l#L164-L169)をなくして、
#508
のようにutilsでinit-endingを追加するだけにしておけば、
:init-endingが2箇所にバッティングすることもないので、
#508の路線でいくならそれが一番シンプルな気がしました。

なので、#508をちょい修正するPR作ってみようと思います

@k-okada
Copy link
Member

k-okada commented Apr 18, 2017 via email

@snozawa
Copy link
Collaborator Author

snozawa commented Apr 18, 2017

Close according to #509.
Moved to #511.

@snozawa snozawa closed this Apr 18, 2017
@snozawa snozawa deleted the fix_initending branch April 18, 2017 14:14
itohdak pushed a commit to itohdak/rtmros_tutorials that referenced this pull request Jan 10, 2019
…627.patch

【hrpsys-base.20180627.patch】現時点でのhrpsysをhrp2で利用するためのpatch
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