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

Add eusbullet for using bullet collision function (implement with c defun) #555

Merged
merged 3 commits into from
Oct 27, 2019

Conversation

mmurooka
Copy link
Member

reimplement #538 according to advice of #538 (comment).
Defference with previous PR is this PR uses C defun and does not use defforeign.

Correspondence between PQP and new Bullet is:
pqp.l <-> bullet.l
euspqp.c <-> eusbullet.c
CPQP.C <-> CBULLET.cpp
PQP (directory) <-> None (bullet is installed by apt)

@k-okada
Copy link
Member

k-okada commented Sep 19, 2019

Thank you for contributing jskeus documentation

Please check latest documents before merging

PDF version of Japanese jmanual: jmanual.pdf
HTML version of Japanese manual: jmanual.html
Sphinx (ReST) version of Japanese manual: jmanual.rst

@mmurooka
Copy link
Member Author


(defun collision-check
(model1 model2 &rest args &key &allow-other-keys)
"Check collision between model1 and model2 using Bullet.
Copy link
Contributor

@Naoki-Hiraoka Naoki-Hiraoka Sep 24, 2019

Choose a reason for hiding this comment

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

I think using Bullet should be removed.
Please consider mmurooka#2

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, merged.

(send *sarm* :joint0 :joint-angle -1 :relative t)
(send *irtviewer* :draw-objects))
\end{verbatim}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This demo does not work, and pqp-collision-check-objects is PQP specific.
Please consider mmurooka#3

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, merged. Checked that demo works.

;; move object
(incf cnt)
(send obj1 :newcoords (make-coords :pos (float-vector (* 500.0 (sin (/ cnt 20.0))) 50 0)))
(send *irtviewer* :draw-objects)
Copy link
Contributor

Choose a reason for hiding this comment

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

In the visualization, lines are flickering heavily and hard to see.
Please consider mmurooka#4

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, merged.

@mmurooka mmurooka force-pushed the jskeus-bullet-c-defun branch from 8b0a420 to b265bc9 Compare September 24, 2019 09:15
@mmurooka
Copy link
Member Author

@Naoki-Hiraoka thank you for your review. I merged your PR and rebased commits.

@k-okada
Copy link
Member

k-okada commented Sep 24, 2019

Thank you for contributing jskeus documentation

Please check latest documents before merging

PDF version of Japanese jmanual: jmanual.pdf
HTML version of Japanese manual: jmanual.html
Sphinx (ReST) version of Japanese manual: jmanual.rst

irteus/bullet.l Outdated

(defmethod cascaded-coords
(:make-btmodel
(&key (fat 0) vs m)
Copy link
Contributor

@Naoki-Hiraoka Naoki-Hiraoka Sep 24, 2019

Choose a reason for hiding this comment

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

Arguments vs and m are local variables.
Please consider mmurooka#5

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, merged.

irteus/bullet.l Outdated
(export '(bt-collision-distance bt-collision-check))

(defun bt-make-model-from-body
(b &key (csg (send b :csg)) (margin nil) m)
Copy link
Contributor

Choose a reason for hiding this comment

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

m is local variable.
Please consider mmurooka#6

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, merged.

@mmurooka mmurooka force-pushed the jskeus-bullet-c-defun branch from 5db6479 to 6cb08aa Compare September 24, 2019 11:22
@mmurooka
Copy link
Member Author

@Naoki-Hiraoka thank you again for your review. I merged your PR and rebased commits.

@k-okada
Copy link
Member

k-okada commented Sep 24, 2019

Thank you for contributing jskeus documentation

Please check latest documents before merging

PDF version of Japanese jmanual: jmanual.pdf
HTML version of Japanese manual: jmanual.html
Sphinx (ReST) version of Japanese manual: jmanual.rst

btVector3 m_pointOnA;
btVector3 m_pointOnB;
btVector3 m_normalBtoA;
btScalar m_distance;
Copy link
Contributor

Choose a reason for hiding this comment

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

Bullet_User_Manual.pdf によれば,btScalar型はデフォルトではfloatになっており,コンパイル時にBT_USE_DOUBLE_PRECISIONをONにするとdoubleになるそうです.このオプションを付けなくても十分に機能していますので必須ではありませんが,64bitマシンの場合にはこのオプションをつけてコンパイルした方が正確になりそうです.


long BT_MakeMeshModel(double *verticesPoints, long numVertices)
{
btConvexHullShape* pshape = new btConvexHullShape();
Copy link
Contributor

Choose a reason for hiding this comment

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

よく使用されるロボットモデルで簡単に試した限りでは,現在の実装でも安定性・計算時間ともにpqpと同程度には問題無く機能しているので必須ではありませんが,
Bullet_User_Manual.pdfによれば,パフォーマンスや安定性のためには

Use less then 100 vertices in a convex mesh

とするのが良く,btShapeHull utilityを用いてconvex hullを単純化するべきだそうです.

Copy link
Contributor

@Naoki-Hiraoka Naoki-Hiraoka left a comment

Choose a reason for hiding this comment

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

MakeFileは私の知識不足により確認できませんでした.

@mmurooka
Copy link
Member Author

Review approved. @k-okada Please tell if more refine necessary.

Copy link
Member

@k-okada k-okada left a comment

Choose a reason for hiding this comment

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

please add one more comment

@@ -51,6 +51,7 @@ for test_l in irteus/test/*.l; do

# osrf/ubuntu_arm64:trusty takes >50 min, skip irteus-demo.l
[[ "$DOCKER_IMAGE" == *"arm64:trusty"* && $test_l =~ irteus-demo.l ]] && continue;
[[ ( "$DOCKER_IMAGE" == *"trusty"* || "$DOCKER_IMAGE" == *"jessie"* ) && $test_l =~ test-collision.l ]] && continue;
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 comment why we have to skip this test.

Copy link
Member Author

Choose a reason for hiding this comment

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

added comment: 1f4da2f#diff-b4553af39acae935d505c76e139d4aa3R54-R55
(rebase commit into previous)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@mmurooka mmurooka force-pushed the jskeus-bullet-c-defun branch from 6cb08aa to 0b8c0e2 Compare October 26, 2019 04:50
@k-okada
Copy link
Member

k-okada commented Oct 26, 2019

Thank you for contributing jskeus documentation

Please check latest documents before merging

PDF version of Japanese jmanual: jmanual.pdf
HTML version of Japanese manual: jmanual.html
Sphinx (ReST) version of Japanese manual: jmanual.rst

@k-okada
Copy link
Member

k-okada commented Oct 27, 2019

Thank you for contributing jskeus documentation

Please check latest documents before merging

PDF version of Japanese jmanual: jmanual.pdf
HTML version of Japanese manual: jmanual.html
Sphinx (ReST) version of Japanese manual: jmanual.rst

@k-okada k-okada self-requested a review October 27, 2019 07:09
@k-okada k-okada merged commit b3d64f0 into euslisp:master Oct 27, 2019
@mmurooka mmurooka deleted the jskeus-bullet-c-defun branch October 27, 2019 14:34
@k-okada
Copy link
Member

k-okada commented Oct 31, 2019

@mmurooka all test in EusLisp failing, is this related to this change ? https://travis-ci.org/euslisp/EusLisp/builds/604932545

@mmurooka
Copy link
Member Author

it seems so... I'll check.

One of the problem is that, in all test cases, jskeus is compiled without bullet.
https://api.travis-ci.org/v3/job/604932547/log.txt

jskeus is compiled without bullet, so you can not use function eusinteger_t C_BT_CalcCollisionDistance(eusinteger_t, eusinteger_t, eusfloat_t*, eusfloat_t*, eusfloat_t*, eusfloat_t*, eusfloat_t*, eusfloat_t*, eusfloat_t*, eusfloat_t*). Please install bullet >= 2.83.
...
RESULT: test-collision-sphere-analytical-pqp
  TEST-NUM: 1
    PASSED:   1
    FAILURE:  0
RESULT: test-collision-sphere-analytical-bullet
  TEST-NUM: 1
    PASSED:   0
�[31m    FAILURE:  1�[0m
RESULT: test-collision-cube-approx-pqp
  TEST-NUM: 1
    PASSED:   1
    FAILURE:  0
RESULT: test-collision-cube-approx-bullet
  TEST-NUM: 1
    PASSED:   0
�[31m    FAILURE:  1�[0m
RESULT: test-collision-cylinder-approx-pqp
  TEST-NUM: 1
    PASSED:   1
    FAILURE:  0
RESULT: test-collision-cylinder-approx-bullet
  TEST-NUM: 1
    PASSED:   0
�[31m    FAILURE:  1�[0m
RESULT: test-collision-mesh-approx-pqp
  TEST-NUM: 1
    PASSED:   1
    FAILURE:  0
RESULT: test-collision-mesh-approx-bullet
  TEST-NUM: 1
    PASSED:   0
�[31m    FAILURE:  1�[0m

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.

None yet

3 participants