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.

Sign up for GitHub

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

Jump to bottom

Enable controller manager services to control hardware lifecycle #abi-breaking #637

Merged
merged 9 commits into from
Feb 23, 2022
Merged

Enable controller manager services to control hardware lifecycle #abi-breaking #637

merged 9 commits into from
Feb 23, 2022

Conversation

destogl
Copy link
Member

@destogl destogl commented Feb 5, 2022

This is the last part of #559.
It implements service logic in controller manager to utilize hardware lifecycle.
It also add parameters to "configure_components_on_start" and "activate_components_on_start" to configure and activate hardware when controller manager is started.
If the parameters are empty or not existent "the old" behavior is preserved where all hardware components are activated at start of the controller manager.

@destogl destogl requested a review from bmagyar February 5, 2022 21:39
@destogl destogl self-assigned this Feb 5, 2022
@destogl destogl changed the title Enable controller manager services to control hardware lifecycle Enable controller manager services to control hardware lifecycle #abi-breaking Feb 5, 2022
@destogl destogl added the request-reviews This label should be used by maintaines only! Label triggers adding of reviewers to PR. label Feb 11, 2022
@destogl destogl requested review from bijoua29 and removed request for harderthan and ShawnSchaerer February 11, 2022 08:28
harderthan
harderthan reviewed Feb 11, 2022
View reviewed changes
Copy link
Contributor

@harderthan harderthan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add two comments.

This is first time to review on ros2_control community.
If there are things to point out,, please feel free to give feedback.

controller_manager/src/controller_manager.cpp Show resolved Hide resolved
controller_manager/src/controller_manager.cpp Show resolved Hide resolved
bijoua29
bijoua29 approved these changes Feb 11, 2022
View reviewed changes
Copy link
Contributor

@bijoua29 bijoua29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me

controller_manager/doc/userdoc.rst Show resolved Hide resolved
ShawnSchaerer
ShawnSchaerer reviewed Feb 14, 2022
View reviewed changes
Copy link

@ShawnSchaerer ShawnSchaerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a basic review. Not a lot to comment on until I understand ROS Control better.

bmagyar
bmagyar reviewed Feb 17, 2022
View reviewed changes
controller_manager/src/controller_manager.cpp Show resolved Hide resolved
controller_manager/src/controller_manager.cpp Outdated

response->component.reserve(hw_components_info.size());

for (auto it = hw_components_info.begin(); it != hw_components_info.end(); ++it)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use range-based for with maps too

option 3 https://stackoverflow.com/a/6963910

controller_manager/test/test_hardware_management_srvs.cpp Outdated Show resolved Hide resolved
@destogl destogl requested a review from bmagyar February 22, 2022 21:44
@destogl destogl merged commit bb131f8 into ros-controls:master Feb 23, 2022
@destogl destogl deleted the cm-services-for-hw-lifecycle branch February 23, 2022 19:19
@destogl destogl linked an issue Feb 24, 2022 that may be closed by this pull request
Create lifecycle for components/hardware #107
Closed
masf7g added a commit to quasi-robotics/ros2_control that referenced this pull request Feb 24, 2022
@masf7g
Revert "Enable controller manager services to control hardware lifecy…
588c0ef 
…cle #abi-breaking (ros-controls#637)"

This reverts commit bb131f8.
@2b-t
Copy link

2b-t commented May 3, 2022 •
edited
Loading

@destogl What is the current state of this feature (in particular in galactic)? The commit for foxy has been reverted the day after the pull request was merged. What were the problems with it and what steps are required for resolving them? I'd be glad to contribute.
From what I understand there is currently no way to recover from an emergency stop on the hardware component level: After an error emerges in the read or write routines and it gets handled in on_error, I have no way of bringing the state of the hardware from UNCONFIGURED back to ACTIVE again. Is that correct? Clearly a recovery procedure like outlined by you in UR ROS2 Driver would be ideal.

pac48 pushed a commit to pac48/ros2_control that referenced this pull request Jan 26, 2024
@mergify @petkovich
@ARK3r @destogl
Steering odometry library and controllers (backport ros-controls#484) (…
ad30ca2 
…ros-controls#624)

* Steering odometry library and controllers (ros-controls#484)

(cherry picked from commit 45d008321fe593db0e7f68921baa326b25c38daa)

* Revert Rolling mean accumulator (ros-controls#637)
* Move reference timeout management into `update_and_write_commands` to have access to the proper time variable.

---------

Co-authored-by: Tomislav Petković <51706321+petkovich@users.noreply.github.com>
Co-authored-by: Reza Kermani <kermani.areza@gmail.com>
Co-authored-by: Denis Štogl <denis@stoglrobotics.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmagyar bmagyar bmagyar approved these changes

@harderthan harderthan harderthan approved these changes

@ShawnSchaerer ShawnSchaerer ShawnSchaerer approved these changes

@bijoua29 bijoua29 bijoua29 approved these changes

Assignees

@destogl destogl

Labels
request-reviews This label should be used by maintaines only! Label triggers adding of reviewers to PR.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create lifecycle for components/hardware
6 participants
@destogl @2b-t @bmagyar @harderthan @ShawnSchaerer @bijoua29