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

Handle HW errors on read and write in CM by stopping controllers #742

Merged
merged 2 commits into from
Sep 28, 2022
Merged

Handle HW errors on read and write in CM by stopping controllers #742

merged 2 commits into from
Sep 28, 2022

Conversation

destogl
Copy link
Member

@destogl destogl commented Jun 29, 2022

In short: when a hardware fails, i.e., error happens on read and write and hardware goes to "UNCONFIGURED" or "FINALIZED" state, controllers should be stopped automatically.
this addresses few recent issues.

  • Add code for deactivating controller when hardware gets an error on read and write.

  • Remove all interface from available list for hardware when an error happens.

  • To add in follow up PR: automatically restart Broadcasters (and trigger publishing before stopping)

@mergify
Copy link
Contributor

mergify bot commented Jun 30, 2022

This pull request is in conflict. Could you fix it @destogl?

@ejalaa12
Copy link
Contributor

ejalaa12 commented Jul 4, 2022

If a hardware stopped working you would stop the controller.
But in any case if that happens the controller manager won't be able to recover from it.
We might need a way to control the hardware interface lifecycle as well no? Otherwise, this hardware is not controllable anymore until we restart the whole controller manager.
Or did I miss something?

@destogl
Copy link
Member Author

destogl commented Jul 5, 2022

If a hardware stopped working you would stop the controller. But in any case if that happens the controller manager won't be able to recover from it. We might need a way to control the hardware interface lifecycle as well no? Otherwise, this hardware is not controllable anymore until we restart the whole controller manager. Or did I miss something?

I think (and hope) you missed something 😄

There is a service set_hardware_state where one can control hardware state from an external component, terminal. Would this be sufficient for now?

And yes, unfortunately, this is manual process for now. I hope to get some external components soon that would be a state-machines for ros2_control. Adding this into framework would it make even more cluttered and complex.

@destogl
Copy link
Member Author

destogl commented Jul 15, 2022

I'll split this into two PRs

bmagyar
bmagyar reviewed Aug 15, 2022
View reviewed changes
controller_manager/src/controller_manager.cpp
@@ -1496,6 +1541,20 @@ void ControllerManager::list_hardware_components_srv_cb(
hwi.name = interface;
hwi.is_available = resource_manager_->command_interface_is_available(interface);
hwi.is_claimed = resource_manager_->command_interface_is_claimed(interface);
// TODO(destogl): Add here mapping to controller that has claimed or
Copy link
Member

Choose a reason for hiding this comment

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

what shall we do with this?

@destogl destogl force-pushed the handle-hardware-errors-in-cm branch from c6a0e1e to 0970d01 Compare August 25, 2022 17:38
@destogl destogl mentioned this pull request Aug 25, 2022
Merged
@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2022

This pull request is in conflict. Could you fix it @destogl?

@destogl
Add code for deactivating controller when hardware gets an error on r…
64ec9e2 
…ead and write.

Fix misleading variable name in the tests.

Error handling in RM of read/write is tested.

Remove all interface from available list for hardware when an error happens.

Working verison of stopping controller on read error. Write error also works but the tests are missing.

Finalized tests for deactivating controllers when write failes.

Do some more variable renaming to the new nomenclature.
@destogl destogl mentioned this pull request Sep 26, 2022
Closed
@bmagyar bmagyar merged commit ecf6c69 into ros-controls:master Sep 28, 2022
@destogl destogl deleted the handle-hardware-errors-in-cm branch September 28, 2022 18:40
@destogl
Copy link
Member Author

destogl commented Oct 15, 2022

@Mergifyio backport humble

mergify bot pushed a commit that referenced this pull request Oct 15, 2022
@destogl @mergify
Handle HW errors on read and write in CM by stopping controllers (#742)
69e0d38 
Add code for deactivating controller when hardware gets an error on read and write.
Fix misleading variable name in the tests.
Remove all interface from available list for hardware when an error happens.
Do some more variable renaming to the new nomenclature.

(cherry picked from commit ecf6c69)
@mergify mergify bot mentioned this pull request Oct 15, 2022
Closed
@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2022

backport humble

✅ Backports have been created

destogl added a commit that referenced this pull request Mar 13, 2023
@destogl
Handle HW errors on read and write in CM by stopping controllers (#742)
884ebff 
Add code for deactivating controller when hardware gets an error on read and write.
Fix misleading variable name in the tests.
Remove all interface from available list for hardware when an error happens.
Do some more variable renaming to the new nomenclature.

(cherry picked from commit ecf6c69)
destogl added a commit that referenced this pull request Apr 11, 2023
@destogl
Handle HW errors on read and write in CM by stopping controllers (#742)
9da9a05 
Add code for deactivating controller when hardware gets an error on read and write.
Fix misleading variable name in the tests.
Remove all interface from available list for hardware when an error happens.
Do some more variable renaming to the new nomenclature.

(cherry picked from commit ecf6c69)
destogl added a commit that referenced this pull request Aug 9, 2023
@destogl
Handle HW errors on read and write in CM by stopping controllers (#742)
db81a20 
Add code for deactivating controller when hardware gets an error on read and write.
Fix misleading variable name in the tests.
Remove all interface from available list for hardware when an error happens.
Do some more variable renaming to the new nomenclature.

(cherry picked from commit ecf6c69)
pac48 pushed a commit to pac48/ros2_control that referenced this pull request Jan 26, 2024
@mergify @christophfroehlich
Remove wrong description (ros-controls#742) (ros-controls#747)
76de69d 
(cherry picked from commit 5178503f1044cc988663c196c32a0039c72418b0)

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
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

@DasRoteSkelett DasRoteSkelett Awaiting requested review from DasRoteSkelett

@harderthan harderthan Awaiting requested review from harderthan

@jackcenter jackcenter Awaiting requested review from jackcenter

@mhubii mhubii Awaiting requested review from mhubii

@UsamaHamayun1 UsamaHamayun1 Awaiting requested review from UsamaHamayun1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@destogl @ejalaa12 @bmagyar