Add subscription options to odometry subscriptions (main)#4968
Add subscription options to odometry subscriptions (main)#4968pele1410 wants to merge 3 commits intoros-navigation:mainfrom
Conversation
|
CI failed unusually. I retriggered it to see if that resolves the issue. In the meantime, please sign off your commits with DCO -- check out the failed DCO job for instructions how |
Signed-off-by: Christopher Thompson <cthompson@metalsharkboats.com>
|
@pele1410 the same test is failing repeatedly, I think there's a problem here. I just checked our nightly jobs and PRs I merged today and those builds have all passed, so I'm fairly confident this is introducing a subtle bug. I'm seeing a ticket with this mentioned ros2/rclcpp#2741. It looks like this touches upon a recently reported issue with rclcpp - which you would probably hit eventually with Jazzy as well (good thing we ran this across Nav2's extensive CI 😉 ) |
|
Well that's unfortunate. I can see an easy way around re-creating the Or we wait for that issue/PR to be resolved, which I assume is the path forward here. |
|
I think so, there is a PR open, so hopefully it shouldn’t be too long! I commented in the ticket that we ran into it as well in Nav2, maybe that’ll help push it forward a bit 🙂 |
|
I see ros2/rclcpp#2741 has been completed and landed on Rolling. Any chance we get an updated test here? |
|
Has that been released into the binaries? I just retriggered CI, but its not been pushed into rolling binaries yet, then this won't work until it is released and available. |
|
@pele1410 can you pull in main's updates? I think there's a caching related issue preventing the builds to all succeed since this is out of date |
|
This throws an exception still:
Is this not out into binaries yet? |
I'll be honest, I'm not entirely sure how to tell. |
|
It looks like it's in 29.5.0 based on this commit which was made 14 hours ago This might have been just after we re-ran the tests |
|
The last rolling release was April 16th https://discourse.ros.org/t/new-packages-for-ros-2-rolling-ridley-2025-04-16/43256 which released 29.4.0 which was cut 3 weeks ago. Your PR was merged in 2 weeks ago, so its not available in binaries yet, but should be the next sync in approximately 2-3 weeks |
Codecov ReportAll modified and coverable lines are covered by tests ✅
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
@pele1410 funny enough this is covered under my recent PR #5288 that'll be merged later today. It exposes all subscribers / publishers QoS profiles using the parameter Thus, I'm going to close this since it is a specialization for a single topic for what the other PR is a general complete solution for. Let me know if you have any questions 😄 Sorry about the closure - I appreciate your efforts here greatly, it just happens to overlap of a larger refactoring I've been up to for some weeks. |
Basic Info
Description of contribution in a few bullet points
Added
rclcpp::SubscriptionOptionsto the odometry topic subscribers to allow qos overrides in the configuration. We run our odometry topics with Unreliable QoS to prevent network issues that are common with high-rate, reliable DDS transmissions. This change allows us to override the Reliable QoS of the odom subscriptions to allow matching.Description of documentation updates required from your changes
I would expect that this should be documented as a new configuration option, but I'm not entirely sure where that would happen.
Description of how this change was tested
Testing was done against the Jazzy branch with #4961
Future work that may be required in bullet points
N/A
For Maintainers: