Skip to content

Conversation

@christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Oct 15, 2025

I changed the storage of the component to be handled by the resource manager instead of inside of the component (deprecated).
The initial values of the handles are set by the RM -> removes some code here.

Mimic handling is similar to ros-controls/ros2_control#2571

I added a mimicked gripper integration test (similar to gz_ros2_control_demos) and also extended the gmock test to actually test some stuff (adapted from GenericSystem).

I fixed the behavior to accept joint_states from the robot with missing position interfaces, see topic_based_system_2dof_velocity_only. But it throws and deactivates if a topic with position fields is received but no state interfaces are configured, see topic_based_system_2dof_velocity_only_inconsistent_topic. Is this the intended behavior?

Base automatically changed from fix/integration/test to main October 16, 2025 08:39
@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.61%. Comparing base (cd86811) to head (68f39dc).

Files with missing lines Patch % Lines
...rface/src/joint_state_topic_hardware_interface.cpp 62.00% 11 Missing and 8 partials ⚠️
...test/joint_state_topic_hardware_interface_test.cpp 90.09% 9 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #44       +/-   ##
===========================================
+ Coverage   61.44%   75.61%   +14.17%     
===========================================
  Files           3        4        +1     
  Lines         249      324       +75     
  Branches       42       35        -7     
===========================================
+ Hits          153      245       +92     
+ Misses         75       55       -20     
- Partials       21       24        +3     
Flag Coverage Δ
unittests 75.61% <83.33%> (+14.17%) ⬆️

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

Files with missing lines Coverage Δ
...c_hardware_interface/test/gripper/test_gripper.cpp 100.00% <100.00%> (ø)
...test/joint_state_topic_hardware_interface_test.cpp 90.26% <90.09%> (+7.91%) ⬆️
...rface/src/joint_state_topic_hardware_interface.cpp 63.21% <62.00%> (+8.88%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@MarqRazz MarqRazz left a comment

Choose a reason for hiding this comment

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

Overall I think these changes work great and I was able to test it on my robots Isaac config and everything still works as expected.

How do we want to handle all of the CI failures till the upstream changes get synced?

@saikishor
Copy link
Member

Overall I think these changes work great and I was able to test it on my robots Isaac config and everything still works as expected.

How do we want to handle all of the CI failures till the upstream changes get synced?

As semi-binary CI is green, it should be fine

@christophfroehlich christophfroehlich merged commit f617d2a into main Oct 22, 2025
20 of 29 checks passed
@christophfroehlich christophfroehlich deleted the rewrite/handles branch October 22, 2025 06:46
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.

4 participants