Skip to content
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

fix: Race condition on quickly setting and getting config #188

Conversation

rokusottervanger
Copy link
Contributor

@rokusottervanger rokusottervanger commented Jan 27, 2022

fixes #187 by using the server's response to the set service to update the latest configuration.

fixes ros#187 by using the server's response to the set service to
update the latest configuration.
@gbiggs
Copy link
Contributor

gbiggs commented Feb 3, 2022

Can you please provide a minimal, reproducible example that can be used to test this PR fixes the problem?

@gbiggs
Copy link
Contributor

gbiggs commented Feb 3, 2022

Thanks for the test, which I got to fail without the fix and pass with the fix. Thanks also for the contribution!

@gbiggs gbiggs merged commit 0e371b6 into ros:noetic-devel Feb 3, 2022
@rokusottervanger
Copy link
Contributor Author

I need this fix on a ROS Melodic system as well. Should I make a new PR backporting this fix? What is the best way forward?

@rokusottervanger rokusottervanger deleted the fix/187-race-condition-when-quickly-setting-and-getting-config branch February 4, 2022 09:09
rokusottervanger added a commit to nobleo/dynamic_reconfigure that referenced this pull request Feb 4, 2022
* fix: Race condition on quickly setting and getting config

fixes ros#187 by using the server's response to the set service to
update the latest configuration.

* test: add test for single threaded set and get using c++ client
@gbiggs
Copy link
Contributor

gbiggs commented Feb 6, 2022

We'll handle backporting to melodic.

gbiggs pushed a commit that referenced this pull request Feb 19, 2022
* fix: Race condition on quickly setting and getting config

fixes #187 by using the server's response to the set service to
update the latest configuration.

* test: add test for single threaded set and get using c++ client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C++ client: race condition when calling setConfiguration and getCurrentConfiguration from callbacks
2 participants