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.

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

[rclcpp_lifecycle] introduce new service for valid transitions to call #550

Closed
Karsten1987 opened this issue Sep 5, 2018 · 10 comments · Fixed by #555
Closed

[rclcpp_lifecycle] introduce new service for valid transitions to call #550

Karsten1987 opened this issue Sep 5, 2018 · 10 comments · Fixed by #555
Assignees
Labels
bug Something isn't working

Comments

@Karsten1987
Copy link
Contributor

Karsten1987 commented Sep 5, 2018

see discourse discussion here: https://answers.ros.org/question/302402/ros2cli-lifecycle-shutdown-transition/

@Karsten1987 Karsten1987 added the bug Something isn't working label Sep 5, 2018
@Karsten1987 Karsten1987 self-assigned this Sep 5, 2018
@Karsten1987 Karsten1987 added the in progress Actively being worked on (Kanban column) label Sep 12, 2018
@dirk-thomas
Copy link
Member

Beside the three PRs updating ros2lifecycle (list -> nodes and a new list showing the available transitions from the current state) and this looks 🔝

@Karsten1987
Copy link
Contributor Author

CI:

Build Status
Build Status
Build Status

@Karsten1987 Karsten1987 added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Oct 10, 2018
@Karsten1987
Copy link
Contributor Author

@Myzhar Do you mind having a look at this and let me know if this meets your expectations? I believe you have quite some experiences with the lifecycle implementation by now (given that you have been vocal about it on github and discourse).

The set of patches (branch lifecycle_refactor) implements a new service to get all available transitions for the current state as well as a service to introspect the state machine completely.

@Myzhar
Copy link

Myzhar commented Oct 10, 2018

I will look at it with very pleasure and will let you know

@Myzhar
Copy link

Myzhar commented Oct 11, 2018

@Karsten1987 I need a few suggestion on how to update my workspace with the patches introduced with the lifecycle_refactor branch

@Karsten1987
Copy link
Contributor Author

so essentially you want to checkout the branches, right?

So in your workspace you can run the following command to do so:

vcs custom --git --args checkout lifecycle_refactor

If not, you should be able to go into each referenced repository and change the git branch manually.
These are:

  • demos
  • rclcpp
  • rcl
  • rcl_interfaces
  • ros2lifecycle

@Myzhar
Copy link

Myzhar commented Oct 11, 2018

Ok, from Waffle I found the list of repositories that have been patched:

  • ros2/ros2cli
  • ros2/rcl_interfaces
  • ros2/demos
  • ros2/rcl
  • ros2/rclcpp

I launched a git checkout manually (I'm sure that a better method exists) for all of them and I was able to compile.

[Update: I received your previous comment while I was writing mine]

I modified the code of my node to follow the new returns type and the other modifications and I finally noticed that the command:
$ ros2 lifecycle set /zed/zed_node shutdown
works 👍

This is also a very useful command:

$ ros2 lifecycle list /zed/zed_node 
- cleanup [2]
	Start: inactive
	Goal: cleaningup
- activate [3]
	Start: inactive
	Goal: activating
- shutdown [51]
	Start: inactive
	Goal: shuttingdown

I tried also to understand what kind of other modifications you have introduced, to see if I can get enhancements at code level, but it's not easy.

@Karsten1987
Copy link
Contributor Author

We do have the code review, so don't worry too much about that. It was more like to ask whether that gives you a solution to the issue you posted on discourse.
But then I'll take it, that it indeed works for you as well.

@Myzhar
Copy link

Myzhar commented Oct 11, 2018

What I have not understood is if now it is possible to change state internally calling a transition.
I mainly think about going to "error processing" from, for example, a running thread... this is a possible situation reading the design documents, but not yet available until now (as far as I know).

To be more clear I explain my use case:
I'm writing the ROS2 driver for the Stereolabs ZED and ZED mini.
I configure everything and I open the camera in "on_configuring".
I start the grab thread in "on_activating".
But I have not a "lifecycle way" to handle a disconnection. I'd like to deactivate and cleanup to bring the node in a valid state waiting for a new configuration and activation.

This is how I understood the way of working with the Lifecycle node... if I'm right.

@Karsten1987
Copy link
Contributor Author

Karsten1987 commented Oct 11, 2018

So from what I understand is that you want to shutdown the camera either when deactivate or cleanup is called? In both cases you have a callback for this.
Everything which is happening in other threads has to be manually managed. The callbacks are solely a trigger for it if you want.
You can trigger this callbacks programmatically on your lifecycle node by calling deactivate() or cleanup() on it.

Nevertheless, I am going to merge this then as it seems to fulfill the requirements described in your discourse post.

@Karsten1987 Karsten1987 removed the in review Waiting for review (Kanban column) label Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants