Skip to content

Conversation

@delipl
Copy link
Contributor

@delipl delipl commented Jun 7, 2025

Added frame_id parameter to fill the joint_states message in the Joint State Broadcaster. Thanks to this parameter rviz2 allows to visualize the effort of the joints.

image

Signed-off-by: Jakub Delicat <delicat.kuba@gmail.com>
Signed-off-by: Jakub Delicat <delicat.kuba@gmail.com>
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

@delipl you have access to the URDF model in the joint state broadcaster. Why not use it to get its root link and publish by default, instead of setting a custom frame_id from the user?

AFAIK, the visualization is same irrespective of the type of the frame you choose. Am I right?

@delipl
Copy link
Contributor Author

delipl commented Jun 7, 2025

@delipl you have access to the URDF model in the joint state broadcaster. Why not use it to get its root link and publish by default, instead of setting a custom frame_id from the user?

AFAIK, the visualization is same irrespective of the type of the frame you choose. Am I right?

Good point. I can change the frame id to the root of the urdf.

Edit:
I added the parameter because I'm using humble. There is no model_.

@saikishor
Copy link
Member

Good point. I can change the frame id to the root of the urdf.

Edit:
I added the parameter because I'm using humble. There is no model_.

Sure. But for Jazzy upon we have a URDF Model. May be you can use these changes to open another PR for Humble, but for this one, please use the URDF root link

@codecov
Copy link

codecov bot commented Jun 7, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.29%. Comparing base (97dd175) to head (13ae298).
⚠️ Report is 66 commits behind head on master.

Files with missing lines Patch % Lines
..._state_broadcaster/src/joint_state_broadcaster.cpp 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1746      +/-   ##
==========================================
- Coverage   86.30%   86.29%   -0.01%     
==========================================
  Files         123      123              
  Lines       11942    11948       +6     
  Branches      995      996       +1     
==========================================
+ Hits        10306    10310       +4     
- Misses       1333     1334       +1     
- Partials      303      304       +1     
Flag Coverage Δ
unittests 86.29% <66.66%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...oint_state_broadcaster/joint_state_broadcaster.hpp 100.00% <ø> (ø)
..._broadcaster/test/test_joint_state_broadcaster.cpp 96.93% <100.00%> (+<0.01%) ⬆️
..._state_broadcaster/src/joint_state_broadcaster.cpp 85.10% <60.00%> (-0.69%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bmagyar
Copy link
Member

bmagyar commented Jun 9, 2025

@saikishor what if we merge this as-is as a first version, backport all the way to humble and then modify things to use the URDF instead? Is there any chance you'd want to not use the absolute root of the URDF?

@saikishor
Copy link
Member

@saikishor what if we merge this as-is as a first version, backport all the way to humble and then modify things to use the URDF instead?

@bmagyar I checked locally, it is not directly backportable, so I opened a new PR : #1751 cherry-picking @delipl's changes and refining a bit.

Is there any chance you'd want to not use the absolute root of the URDF?

I don't think the RViZ2 visualization changes based on the frame_id. I think it only check that it should be a valid frame

Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Thank you!

@bmagyar bmagyar merged commit fc576ee into ros-controls:master Jun 11, 2025
18 of 27 checks passed
@zimmerman-pliant
Copy link

Is it right that this is not backported and released via Jazzy binaries yet?

@christophfroehlich christophfroehlich added the backport-jazzy Triggers PR backport to ROS 2 jazzy. label Sep 9, 2025
mergify bot pushed a commit that referenced this pull request Sep 9, 2025
christophfroehlich pushed a commit that referenced this pull request Sep 9, 2025
Co-authored-by: Jakub "Deli" Delicat <delicat.kuba@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants