-
Notifications
You must be signed in to change notification settings - Fork 304
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 the namespaced controller_manager spawner + added tests #1640
Fix the namespaced controller_manager spawner + added tests #1640
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1640 +/- ##
==========================================
+ Coverage 87.95% 88.04% +0.08%
==========================================
Files 108 108
Lines 9913 9985 +72
Branches 889 891 +2
==========================================
+ Hits 8719 8791 +72
+ Misses 875 874 -1
- Partials 319 320 +1
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.
Tested with ros-controls/ros2_control_demos#502 but I still failed.
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.
Sounds like a good idea but let's deprecate the namespace parameter first, loudly
Make sense. You are right. I shouldn't have removed it abruptly hahHa 😅😅 |
Signed-off-by: Jakub Delicat <jakub.delicat@husarion.com>
… controller_manager
4fe868f
to
49498c3
Compare
@bmagyar I've added the @christophfroehlich I've made some changes on ros-controls/ros2_control_demos#502 for the new namespacing and also the missing |
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.
Tested now successfully with 1ac5e13 of ros-controls/ros2_control_demos#502.
May I ask for updating the docs, and adding migration notes?
Sire, I can update the docs. |
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.
Thanks :)
Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
* [ResourceManager] Make destructor virtual for use in derived classes (#1607) * Fix typos in test_resource_manager.cpp (#1609) * [CM] Remove support for the description parameter and use only `robot_description` topic (#1358) --------- Co-authored-by: Felix Exner <exner@fzi.de> Co-authored-by: Christoph Froehlich <christoph.froehlich@ait.ac.at> * move critical variables to the private context (#1623) * Fix controller starting with later load of robot description test (#1624) * Fix the duplicated restart of the controller_manager services initialization * Scope the ControllerManagerRunner to avoid malloc and other test issues * reorder the tests for consistent log at the end * Remove noqa (#1626) * Unused header cleanup (#1627) * Create debugging.rst (#1625) --------- Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com> Co-authored-by: Christoph Froehlich <christoph.froehlich@ait.ac.at> * Update changelogs * 4.14.0 * add missing rclcpp logging include for Humble compatibility build (#1635) * [CM] Remove deprecated spawner args (#1639) * Add a pytest launch file to test ros2_control_node (#1636) * Fix rst markup (#1642) * Fix rqt_cm paragraph * Fix indent * CM: Add missing includes (#1641) * [RM] Add `get_hardware_info` method to the Hardware Components (#1643) * Fix the namespaced controller_manager spawner + added tests (#1640) * Bump version of pre-commit hooks (#1649) Co-authored-by: christophfroehlich <3367244+christophfroehlich@users.noreply.github.com> * Add missing include for executors (#1653) * Update changelogs * 4.15.0 * refactor SwitchParams to fix the incosistencies in the spawner tests (#1638) * Modify test with missing CM to have a timeout * Catch exception when CM services are not found And print the error and exit in the application * Exit with code 1 on unreached CM --------- Co-authored-by: Sai Kishor Kothakota <sai.kishor@pal-robotics.com> Co-authored-by: Parker Drouillard <parker@pepcorp.ca> Co-authored-by: Dr. Denis <denis@stoglrobotics.de> Co-authored-by: Christoph Froehlich <christoph.froehlich@ait.ac.at> Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com> Co-authored-by: Henry Moore <henry.moore@picknik.ai> Co-authored-by: Lennart Nachtigall <firesurfer127@gmail.com> Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com> Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: christophfroehlich <3367244+christophfroehlich@users.noreply.github.com>
This PR aims to fix the spawner to work with namespace controller manager and added tests for this particular case. It is started from #1547, however after looking into the spawner, it seems like the
--namespace
parameter is no longer required as you could pass it in the arg--controller-manager
as how it is done for the unspawner, if this is fine, then we can get rid of the--namespace
argcloses: #1547
fixes: #1506