-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add empty set_atomically parameter service #354
Conversation
Signed-off-by: acuadros95 <[email protected]>
Signed-off-by: acuadros95 <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #354 +/- ##
==========================================
+ Coverage 69.20% 69.63% +0.42%
==========================================
Files 16 16
Lines 2715 2760 +45
Branches 765 766 +1
==========================================
+ Hits 1879 1922 +43
- Misses 450 452 +2
Partials 386 386
|
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.
LGTM
If it's at all helpful I've checkout out and built this branch and it fixes the ros2cli issues for me. |
@Mergifyio backport rolling iron |
✅ Backports have been created
|
* Add empty set_atomically parameter service Signed-off-by: acuadros95 <[email protected]> * Fix cpplint Signed-off-by: acuadros95 <[email protected]> --------- Signed-off-by: acuadros95 <[email protected]> (cherry picked from commit 688bd28)
* Add empty set_atomically parameter service Signed-off-by: acuadros95 <[email protected]> * Fix cpplint Signed-off-by: acuadros95 <[email protected]> --------- Signed-off-by: acuadros95 <[email protected]> (cherry picked from commit 688bd28)
@Acuadros95 I added a backport for rolling and iron distribution. Do you want/need this fix on Humble and Foxy distribution as well? |
@JanStaschulat No, this is just needed on Iron and Rolling. Thanks! |
* Add empty set_atomically parameter service (#354) * Add empty set_atomically parameter service Signed-off-by: acuadros95 <[email protected]> * Fix cpplint Signed-off-by: acuadros95 <[email protected]> --------- Signed-off-by: acuadros95 <[email protected]> (cherry picked from commit 688bd28) * trigger CI-pipeline Signed-off-by: Jan Staschulat <[email protected]> * Uncrustify Signed-off-by: acuadros95 <[email protected]> --------- Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: acuadros95 <[email protected]> Co-authored-by: Antonio Cuadros <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> Co-authored-by: acuadros95 <[email protected]>
* Add empty set_atomically parameter service (#354) * Add empty set_atomically parameter service Signed-off-by: acuadros95 <[email protected]> * Fix cpplint Signed-off-by: acuadros95 <[email protected]> --------- Signed-off-by: acuadros95 <[email protected]> (cherry picked from commit 688bd28) * Trigger ci-pipeline again. Signed-off-by: Jan Staschulat <[email protected]> * Fix uncrustify Signed-off-by: acuadros95 <[email protected]> --------- Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: acuadros95 <[email protected]> Co-authored-by: Antonio Cuadros <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> Co-authored-by: acuadros95 <[email protected]>
New parameter client implementations check the availability of all the related services before executing a command.
We were missing the
/set_parameters_atomically
, making ROS 2 Cli parameter commands fail.This PR adds a empty service implementation to fix this issue. Related: micro-ROS/micro-ROS-Agent#195
Note: This fix shall be included on the Iron release.