Skip to content

Conversation

@Naoki-Hiraoka
Copy link
Contributor

#540

:calc-static-balance-pointの引数について、

マニュアルでは

force-list [N] and moment-list [Nm] are list of force and moment at target-points.

となっていますが、

現在の実装では
force-list: ロボットが受ける力 [N]
moment-list: ロボットが与えるモーメント [Nmm]
となっています。

:calc-static-balance-pointの引数が
force-list: ロボットが受ける力 [N]
moment-list: ロボットが受けるモーメント [Nm]
となるように変更し、テストを追加しました。

Copy link
Member

@mmurooka mmurooka left a comment

Choose a reason for hiding this comment

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

Please fix the method document
https://github.com/euslisp/jskeus/pull/541/files#diff-80b7b3bf7c65ce7da90574530d5cd5b1R1020
so that we can know these forces and moments are robot receiving forces and moments.

(concatenate float-vector
(mapcan #'(lambda (f m) (concatenate cons f m))
force-list moment-list)
(v- (scale (* 1e-6 (send *robot* :weight)) *g-vec*)) #F(0 0 0)
Copy link
Member

Choose a reason for hiding this comment

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

*robot* -> robot. Also the next line.

(and result
(> centroid-thre (distance (apply #'midpoint 0.5 (send robot :legs :end-coords :worldpos))
(send robot :calc-static-balance-point :force-list force-list :moment-list moment-list)))
(> 1.0
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment about the meaning of this condition checking.

Copy link
Member

Choose a reason for hiding this comment

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

I follow this code, and now understand this condition check and think this is OK. (But still add comment please.)

@Naoki-Hiraoka
Copy link
Contributor Author

Thank you for reviewing.
I fixed the document, added the comment, and fixed my code as suggested.

@mmurooka
Copy link
Member

OK, and rebase into one commit and force push, please.

…atic-balance-point. add a test related to this.
@Naoki-Hiraoka Naoki-Hiraoka force-pushed the fix-calc-static-balance-point branch from e7ae594 to 1f4ef9e Compare May 26, 2019 15:14
@Naoki-Hiraoka
Copy link
Contributor Author

force pushed.

@mmurooka
Copy link
Member

Thanks, approved.
Same fix with euslisp/EusLisp#376 is necessary for travis test passing. (maybe another new PR should be created for this.)

@k-okada
Copy link
Member

k-okada commented Jul 4, 2019

@Naoki-Hiraoka
Copy link
Contributor Author

I tried adding documents in #549 .

Should I update https://github.com/jsk-ros-pkg/euslisp-docs?

@k-okada
Copy link
Member

k-okada commented Jul 8, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants