fix(joint_state_broadcaster): suppress confusing warning for standard interfaces by greencookie-afk · Pull Request #2276 · ros-controls/ros2_controllers

greencookie-afk · 2026-04-01T14:28:24Z

Description

This PR fixes the confusing warning message from JointStateBroadcaster when using standard interfaces like velocity with the default mapping.

Problem

When configuring the broadcaster with standard interfaces:

rotor_state_broadcaster:
  ros__parameters:
    joints: [rotor/1, rotor/2, rotor/3, rotor/4]
    interfaces: [velocity]

The following confusing warning was displayed:

Mapping from 'velocity' to interface 'velocity' will not be done, because 'velocity' is defined in 'interface' parameter.

This warning is confusing because:

The user didn't configure any custom mapping
The data is being published correctly to the velocity field
The warning suggests something is wrong when everything works as expected

Solution

The warning is now only displayed when there's an actual custom mapping being ignored (i.e., when the interface name differs from the JointState field name).

For example, if someone had:

interfaces: [velocity]
map_interface_to_joint_state:
  velocity: custom_velocity_interface

Then the warning would still be shown because the custom mapping custom_velocity_interface -> velocity would be ignored.

But for the standard case where interfaces: [velocity] and the default mapping velocity -> velocity, no warning is shown because no mapping is needed.

Changes

Modified joint_state_broadcaster.cpp to only print the warning when interface != interface_to_map

Related Issue

Fixes #2261

greencookie-afk · 2026-04-01T14:31:42Z

@christophfroehlich, I opened a PR for this issue. Would you be able to review when you have a chance? I am a beginner so any feedback will be appreciated.

thedevmystic

Looks good!

thedevmystic · 2026-04-03T07:44:33Z

@christophfroehlich, I opened a PR for this issue. Would you be able to review when you have a chance? I am a beginner so any feedback will be appreciated.

We're here for the community! Either beginner or expert. Welcome aboard to the project, @greencookie-afk!

greencookie-afk · 2026-04-03T07:46:55Z

@thedevmystic thanks buddy i updated it.!!

codecov · 2026-04-11T11:32:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.94%. Comparing base (ede5b33) to head (1835ec7).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2276      +/-   ##
==========================================
- Coverage   85.01%   84.94%   -0.08%     
==========================================
  Files         145      145              
  Lines       15234    15359     +125     
  Branches     1336     1336              
==========================================
+ Hits        12951    13046      +95     
- Misses       1791     1822      +31     
+ Partials      492      491       -1

Flag	Coverage Δ
unittests	`84.94% <100.00%> (-0.08%)`	⬇️

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

Files with missing lines	Coverage Δ
..._state_broadcaster/src/joint_state_broadcaster.cpp	`70.21% <ø> (+0.53%)`	⬆️
..._broadcaster/test/test_joint_state_broadcaster.cpp	`97.46% <100.00%> (-0.45%)`	⬇️

... and 17 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

christophfroehlich

Thank you. The changes LGTM, but I ask you to

clarify the wrong configuration (i.e., when the warning is printed) in the parameter description of interfaces and joint_state_broadcaster_parameter_context.yml
Add a test for this case to have coverage. You could copy TestCustomInterfaceMapping but run it with a velocity interface parameter.

bmagyar · 2026-04-22T18:01:43Z

@greencookie-afk ☝️ we still miss the follow up action on this

greencookie-afk · 2026-04-23T10:07:04Z

@greencookie-afk ☝️ we still miss the follow up action on this

i am working on it.

mergify · 2026-04-24T11:55:47Z

This pull request is in conflict. Could you fix it @greencookie-afk?

… interfaces When using standard interfaces like 'velocity' with the default mapping, the warning 'Mapping from velocity to interface velocity will not be done' is confusing because no mapping is actually needed. This change only shows the warning when there's a custom mapping being ignored (i.e., when interface name differs from the JointState field name). Fixes ros-controls#2261

Co-authored-by: Surya! <thedevmystic@gmail.com>

- Add TestStandardVelocityInterfaceNoWarning test that verifies no warning is printed when using standard interface (velocity) without custom mapping - Add TestCustomInterfaceMappingIgnoredWhenVelocityInterfaceIsRequested test that verifies warning IS printed when custom mapping is ignored - Update interfaces parameter description to clarify when warning is printed - Update parameter_context.yml with same clarification Addresses review feedback from christophfroehlich

christophfroehlich · 2026-04-25T08:03:13Z

@greencookie-afk

Once a PR got reviewed, please don't do force pushes as this will break github UI and subsequent reviews will take more time.

christophfroehlich

The newly added tests do not even build, please have a look

greencookie-afk · 2026-04-25T13:44:24Z

The newly added tests do not even build, please have a look

I'm sorry I'll have a look at it again in a cleaner and better way. Kind of new here so it takes time to adjust.

christophfroehlich · 2026-04-26T08:22:13Z

The newly added tests do not even build, please have a look

I'm sorry I'll have a look at it again in a cleaner and better way. Kind of new here so it takes time to adjust.

We are happy to help, but please check as a minimum if your changes build and tests succeed. See https://control.ros.org/rolling/doc/contributing/contributing.html

saikishor

LGTM

christophfroehlich

Thank you!

greencookie-afk · 2026-04-28T06:15:49Z

Thank you!

Thank you so much everyone

… interfaces (backport #2276) (#2333)

… interfaces (backport #2276) (#2334)

github-actions Bot requested review from MarqRazz, Maverobot, bijoua29, christophfroehlich and xguay April 1, 2026 14:28

thedevmystic suggested changes Apr 3, 2026

View reviewed changes

Comment thread joint_state_broadcaster/src/joint_state_broadcaster.cpp Outdated

christophfroehlich requested changes Apr 11, 2026

View reviewed changes

greencookie-afk requested review from christophfroehlich and thedevmystic April 23, 2026 10:17

greencookie-afk force-pushed the fix/confusing-warning-joint-state-broadcaster branch from 444d294 to e0149f8 Compare April 24, 2026 11:54

greencookie-afk and others added 3 commits April 24, 2026 17:29

Update joint_state_broadcaster/src/joint_state_broadcaster.cpp

1846cce

Co-authored-by: Surya! <thedevmystic@gmail.com>

greencookie-afk force-pushed the fix/confusing-warning-joint-state-broadcaster branch from e0149f8 to c5653ad Compare April 24, 2026 11:59

christophfroehlich requested changes Apr 25, 2026

View reviewed changes

greencookie-afk added 2 commits April 27, 2026 13:43

test(joint_state_broadcaster): trim velocity mapping coverage

97d9b73

test(joint_state_broadcaster): fix velocity mapping expectation

997109e

saikishor requested changes Apr 27, 2026

View reviewed changes

Comment thread joint_state_broadcaster/src/joint_state_broadcaster.cpp Outdated

refactor(joint_state_broadcaster): use warn expression macro

4f00329

greencookie-afk requested a review from saikishor April 27, 2026 09:24

greencookie-afk requested a review from christophfroehlich April 27, 2026 09:24

saikishor approved these changes Apr 27, 2026

View reviewed changes

Merge branch 'master' into fix/confusing-warning-joint-state-broadcaster

1835ec7

christophfroehlich added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Apr 27, 2026

christophfroehlich approved these changes Apr 27, 2026

View reviewed changes

christophfroehlich merged commit f6ebc8b into ros-controls:master Apr 27, 2026
17 of 21 checks passed

This was referenced Apr 27, 2026

fix(joint_state_broadcaster): suppress confusing warning for standard interfaces (backport #2276) #2333

Merged

fix(joint_state_broadcaster): suppress confusing warning for standard interfaces (backport #2276) #2334

Merged

christophfroehlich pushed a commit that referenced this pull request Apr 28, 2026

fix(joint_state_broadcaster): suppress confusing warning for standard…

6fc9bc0

… interfaces (backport #2276) (#2333)

christophfroehlich pushed a commit that referenced this pull request Apr 28, 2026

fix(joint_state_broadcaster): suppress confusing warning for standard…

c2a182f

… interfaces (backport #2276) (#2334)

greencookie-afk deleted the fix/confusing-warning-joint-state-broadcaster branch April 28, 2026 14:54

Conversation

greencookie-afk commented Apr 1, 2026

Description

Problem

Solution

Changes

Related Issue

Uh oh!

greencookie-afk commented Apr 1, 2026

Uh oh!

thedevmystic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thedevmystic commented Apr 3, 2026

Uh oh!

greencookie-afk commented Apr 3, 2026

Uh oh!

codecov Bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

bmagyar commented Apr 22, 2026

Uh oh!

greencookie-afk commented Apr 23, 2026

Uh oh!

mergify Bot commented Apr 24, 2026

Uh oh!

christophfroehlich commented Apr 25, 2026

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

greencookie-afk commented Apr 25, 2026

Uh oh!

christophfroehlich commented Apr 26, 2026

Uh oh!

Uh oh!

saikishor left a comment

Choose a reason for hiding this comment

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greencookie-afk commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Apr 11, 2026 •

edited

Loading