Skip to content

Conversation

@JafarAbdi
Copy link
Member

3b83598 introduced a bug where MoveRelative tries to move the link's origin to ik_frame despite link_T_ik_frame isn't the identity matrix, the problem is in the calculation we assume target_eigen to be world_T_ik_frame_new_pose which isn't the case (see move_relative.cpp#L270-L273 and move_relative.cpp#L243-L249)

I think the bug existed even before the refactor, it's just that we didn't have a case where link_T_ik_frame isn't the identity matrix

@JafarAbdi JafarAbdi requested review from rhaschke and v4hn December 12, 2021 19:13
@codecov
Copy link

codecov bot commented Dec 12, 2021

Codecov Report

Merging #318 (be1c59e) into master (b4a9e20) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #318      +/-   ##
==========================================
- Coverage   53.00%   52.99%   -0.00%     
==========================================
  Files         102      102              
  Lines        7599     7598       -1     
==========================================
- Hits         4027     4026       -1     
  Misses       3572     3572              
Impacted Files Coverage Δ
core/src/stages/move_relative.cpp 69.40% <ø> (-0.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4a9e20...be1c59e. Read the comment docs.

@v4hn
Copy link
Contributor

v4hn commented Dec 14, 2021

This patch is still broken because the direction property will be computed based on link_pose instead of ik_pose_world. I will provide a proper fix + test in a bit.

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.

2 participants