Skip to content

Unify transmission tests using shared minimal robot URDF in test assets#3031

Open
naitikpahwa18 wants to merge 4 commits intoros-controls:masterfrom
naitikpahwa18:move-test-urdf-648
Open

Unify transmission tests using shared minimal robot URDF in test assets#3031
naitikpahwa18 wants to merge 4 commits intoros-controls:masterfrom
naitikpahwa18:move-test-urdf-648

Conversation

@naitikpahwa18
Copy link

This PR extends the shared minimal_robot_urdf in ros2_control_test_assets with transmission definitions (Simple, Differential, and FourBar) and updates the transmission loader tests to use this canonical URDF instead of locally embedded XML strings. It also introduces reusable transmission snippets (including an invalid variant) for parser-related tests. The goal is to consolidate test URDF definitions into a single fully-featured robot description, reduce duplication, and align with the architectural intent discussed in issue #648.

Signed-off-by: Naitik Pahwa <naitikpahwa18@users.noreply.github.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates transmission-related test URDF definitions by extending the shared ros2_control_test_assets::minimal_robot_urdf with multiple transmission definitions (Simple, Differential, FourBar) and updating transmission loader tests to use this canonical URDF instead of embedding XML strings.

Changes:

  • Extend minimal_robot_urdf test asset with ros2_control transmission tags for Simple, Differential, and FourBar transmissions.
  • Add reusable “classic URDF transmission” snippets (minimal_robot_transmissions and an invalid variant) for parser-focused tests.
  • Update transmission loader tests to consume minimal_robot_urdf and (where needed) locate the correct transmission by type.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
transmission_interface/test/simple_transmission_loader_test.cpp Switch FullSpec test URDF source to shared minimal_robot_urdf.
transmission_interface/test/differential_transmission_loader_test.cpp Switch FullSpec test to shared minimal_robot_urdf and find the Differential transmission by type.
transmission_interface/test/four_bar_linkage_transmission_loader_test.cpp Switch FullSpec test to shared minimal_robot_urdf and find the FourBar transmission by type.
ros2_control_test_assets/include/ros2_control_test_assets/descriptions.hpp Add transmission definitions/snippets and include them in minimal_robot_urdf.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

Thanks for taking this old issue.

Please consider our contributing guidelines and have a look at the failing pre-commit job.

Signed-off-by: Naitik Pahwa <naitikpahwa18@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Feb 15, 2026

Codecov Report

❌ Patch coverage is 82.60870% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.29%. Comparing base (cb9a983) to head (c28dd85).

Files with missing lines Patch % Lines
...ace/test/differential_transmission_loader_test.cpp 81.81% 0 Missing and 2 partials ⚠️
...test/four_bar_linkage_transmission_loader_test.cpp 81.81% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3031      +/-   ##
==========================================
+ Coverage   89.28%   89.29%   +0.01%     
==========================================
  Files         158      158              
  Lines       19296    19308      +12     
  Branches     1560     1568       +8     
==========================================
+ Hits        17228    17241      +13     
+ Misses       1424     1422       -2     
- Partials      644      645       +1     
Flag Coverage Δ
unittests 89.29% <82.60%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
...interface/test/simple_transmission_loader_test.cpp 84.21% <100.00%> (-0.28%) ⬇️
...ace/test/differential_transmission_loader_test.cpp 92.00% <81.81%> (-2.12%) ⬇️
...test/four_bar_linkage_transmission_loader_test.cpp 92.00% <81.81%> (-2.12%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM

@christophfroehlich christophfroehlich added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Feb 15, 2026
@christophfroehlich christophfroehlich moved this to Needs review in Review triage Feb 15, 2026
Signed-off-by: Naitik Pahwa <naitikpahwa18@users.noreply.github.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. backport-kilted Triggers PR backport to ROS 2 kilted.

Projects

Status: Needs review

Development

Successfully merging this pull request may close these issues.

Move test-URDF from transmission interface to ros2_control_test_assets

4 participants