New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to use sensor_data qos setting for short queue sizes. #815
Conversation
Humm yeah it's not immediately clear to me why the QoS settings break the test. I checked that the plugin receives the command and is publishing odom messages, but the callback on the test is never called. Maybe you have an idea? Otherwise I can try to poke deeper. |
a6f16fc
to
062829d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline, the test was changed to use the same QoS settings as the plugin and now it passes: 315bfde. I'll add a note to the migration page.
I'll merge this if CI comes back clean.
Changing to the sensor data profile changes the depth but also the reliability (which I think you've spoken about offline). This PR has the following side-effects wrt reliability that we may want to reconsider:
|
Thanks for taking a look at this, @dhood . I don't understand much about QoS settings yet, but all your comments sound reasonable. Would you be up for making a quick PR with the suggestions? |
@chapulina I think that this is the best default profile to use. It's not exactly the 1 queue size but it's close at 5.