-
Notifications
You must be signed in to change notification settings - Fork 299
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
refactor SwitchParams to fix the incosistencies in the spawner tests (backport #1638) #1660
Conversation
Cherry-pick of a1ad523 has failed:
To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
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.
this breaks ABI of an internal class, so this should not be a big deal?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## iron #1660 +/- ##
=======================================
Coverage 87.45% 87.46%
=======================================
Files 101 101
Lines 8078 8099 +21
Branches 711 712 +1
=======================================
+ Hits 7065 7084 +19
- Misses 717 719 +2
Partials 296 296
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
looks good
This PR aims to fix the inconsistencies that are observed with the spawner_unspawner tests and could be reproducible at higher frequencies as well. It seems to be that due to the wall_timer etc the manager_switch was trigger twice and causing the crashes, and upon checking there are missing guards and mutexes to protect this context.
The changes are also tested on the real hardware! :)
The following failures are caused with the current setup and is solved with the changes proposed in this PR:
Fixes: #1637
Fixes: #1368
Fixes: #1647
Fixes: #1657
Fixes: #1644
Fixes: ros-controls/ros2_control_ci#105
Fixes: ros-controls/ros2_control_ci#106
Fixes: ros-controls/ros2_control_ci#107
Fixes: ros-controls/ros2_control_ci#108
This is an automatic backport of pull request #1638 done by [Mergify](https://mergify.com).