[jazzy] Fix QoS overrides ignored when topic name has no leading slash (backport #2394)#2413
Merged
Merged
Conversation
…sh (backport #2394) (#2405) * Fix QoS overrides ignored when topic name has no leading slash (#2394) * Fix QoS overrides ignored when topic name has no leading slash Signed-off-by: root <lakhmanisahil8@gmail.com> * Address review feedback Signed-off-by: root <lakhmanisahil8@gmail.com> Co-authored-by: Michael Orlov <morlovmr@gmail.com> * change timeout to 5s Signed-off-by: root <lakhmanisahil8@gmail.com> Co-authored-by: Michael Orlov <morlovmr@gmail.com> * Use expanded_topic_name for logging in subscription_qos_for_topic Also fixed code alignment in a couple other places with topic names are expanded. Signed-off-by: Michael Orlov <morlovmr@gmail.com> --------- Signed-off-by: root <lakhmanisahil8@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> Co-authored-by: Michael Orlov <morlovmr@gmail.com> (cherry picked from commit d2c4d6d) # Conflicts: # rosbag2_transport/src/rosbag2_transport/recorder.cpp * Address merge conflicts Signed-off-by: Michael Orlov <morlovmr@gmail.com> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Co-authored-by: Sahil Lakhmani <126493645+lakhmanisahil@users.noreply.github.com> Co-authored-by: Michael Orlov <morlovmr@gmail.com> (cherry picked from commit ba5a75d)
MichaelOrlov
changed the title
MichaelOrlov
approved these changes
Apr 23, 2026
Contributor
MichaelOrlov
left a comment
MichaelOrlov
left a comment
There was a problem hiding this comment.
LGTM with green CI.
Contributor
|
Pulls: #2413
|
Contributor
|
Merging with yellow Windows CI, because:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
QoS override keys are compared against bag topic names using exact string matching. ROS 2 allows different levels of qualification for topic names
my_topicand/my_topicare different strings but expand to the same fully-qualified name/my_topic. This means a user writingtalker:in their YAML override file gets no match against/talkerstored in the bag, and the override is silently ignored with no warning or error.Fix: normalize topic names to their fully-qualified form (prepend
/if absent) on both sides of the QoS override map — at insertion time in the constructor/record()and at lookup time inpublisher_qos_for_topic()/subscription_qos_for_topic().Fixes #1240
Is this user-facing behavior change?
Yes. Previously only an exact string match worked. Now both forms are accepted:
Likewise if the bag stores
my_topic, then/my_topicin the YAML also works correctly now.Did you use Generative AI?
Yes, Claude Sonnet 4.6
Additional Information
ros2 bag play(player.cpp) andros2 bag record(recorder.cpp)/or is empty it is returned unchanged — no double-slash risktopic_qos_profile_override_matches_unqualified_nameis added totest_play.cppto cover the case from the issue completion criteriaDurability: VOLATILE(broken) vsDurability: TRANSIENT_LOCAL(fixed) with the same unqualifiedtalker:YAML keyBefore
After
This is an automatic backport of pull request #2394 done by Mergify.
This is an automatic backport of pull request #2405 done by Mergify.