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

[euscollada] Add size check to end-coords translation/rotation #98

Merged
merged 2 commits into from
Apr 30, 2015

Conversation

orikuma
Copy link
Contributor

@orikuma orikuma commented Feb 23, 2015

docのメンバからNodeを作ったあとにNodeのサイズを確認するように変更をしました.

euscolladaのend-coordsの生成部分に関して, 現在はtranslate, rotateの記述がない場合docの該当メンバにアクセスした時点でExceptionが出ることを期待している実装になっているようですが,
yaml-cpp-0.5ではdocのメンバにアクセスした時点ではExceptionが出ず, nodeのインデックスにアクセスした段階でExceptionが出るようになっています.
したがって中途半端にend-coordsの部分のeusのコードが生成されてしまい, 括弧対応が取れず正しいeusのモデルファイルになりません.

…e undefiend limb end-coords transformation/rotation breaks matching of parentheses in yaml-cpp 0.5.
@YoheiKakiuchi
Copy link
Member

インデント変わってしまっていませんか?

@orikuma
Copy link
Contributor Author

orikuma commented Feb 23, 2015

ifが入ったのでインデントは1段ずれていると思います.

@YoheiKakiuchi
Copy link
Member

n.size() == 0

のときに,exceptionをスローすればいいということはない?

@orikuma
Copy link
Contributor Author

orikuma commented Feb 23, 2015

fprintf(output_fp, " (send %s-end-coords :translate (float-vector", limb_name.c_str());
が呼ばれる前に抜けられればいいので, Exceptionを投げても良い気がします.

@YoheiKakiuchi
Copy link
Member

これでいいんじゃないかな?

value = n[i].as<double>(); // check element existing

@orikuma
Copy link
Contributor Author

orikuma commented Feb 23, 2015

n[i]を使うならfor内で代入する必要がありますが, それだとfprintf(output_fp, " (send %s-end-coords :translate (float-vector", limb_name.c_str());より後になるのでダメだと思います.
fprintf前にn[0]にアクセスしに行く方法もありますが, それよりはsizeをチェックしてExceptionを投げるほうが良い気がします.

@YoheiKakiuchi
Copy link
Member

fprintf前にn[0]にアクセスしに行く方法もありますが, それよりはsizeをチェックしてExceptionを投げるほうが良い気がします.

どうして?

@orikuma
Copy link
Contributor Author

orikuma commented Feb 23, 2015

やりたいことが"Nodeが空であるかどうかの確認"なので, exceptionを期待してn[0]にアクセスする
よりはsizeを確認したほうがコードとして意味がわかりやすいかと思ったのですが, どうでしょうか.

@k-okada
Copy link
Member

k-okada commented Apr 28, 2015

いろいろ議論が続いていますが,マージしても大丈夫でしょうか?それとも直す方向性があれば教えて下さい.
テストとしては,end-coordsがあるけど,rotation/translationがどっちかしかない,みたいなものがあればいいのかな?そういうyamlでパッと有るのはどれだろう.

@YoheiKakiuchi
Copy link
Member

ぱっと見直すと、サイズチェックのときに n.size() == 3 にするように思えてきました。
yamlが不正, translationのサイズが2だったときなどに、正しくエラーにできないなどがあるので。

@k-okada
Copy link
Member

k-okada commented Apr 29, 2015

#114
でテストコードをつくってみました.まだtravis先生は何も言っていませんが,こちらの手者では,ちゃんと動いているように見えますね.
サイズのチェックはtranslate, rotateなどの指定があるかどうかで,translateそのもののサイズが3以外のチェックは前も今もされていないので,今回のPRでは良いかという気がしました.

@k-okada
Copy link
Member

k-okada commented Apr 29, 2015

テストはhrpsys_ros_bridge_tutorialsからもってきた,
https://github.com/k-okada/jsk_model_tools/blob/709880f0334fda2e0244591360d4c8194ea1aca0/euscollada/test/sample1.yaml
をつかっていて,

##
## end-coords
##
larm-end-coords:
  translate : [0, 0, -0.12]
  rotate    : [0, 1,  0, 90]
  parent    : LARM_LINK6
rarm-end-coords:
  translate : [0, 0, -0.12]
  rotate    : [0, 1,  0, 90]
  parent    : RARM_LINK6
lleg-end-coords:
  translate : [0.00, 0, -0.07]
rleg-end-coords:
  translate : [0.00, 0, -0.07]
head-end-coords:
  translate : [0.08, 0, 0.13]
  rotate    : [0, 1, 0, 90]

と,一応前パターン(torsoはない)がある気がします.rotateだけが指定されている,というのは無いですが.

k-okada added a commit that referenced this pull request Apr 30, 2015
[euscollada] Add size check to end-coords translation/rotation
@k-okada k-okada merged commit 81534c8 into jsk-ros-pkg:master Apr 30, 2015
aginika pushed a commit to aginika/jsk_model_tools that referenced this pull request Jul 30, 2015
Move all the pointcloud processing to c2
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