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

Support LifecycleNode and EventHandler in frontend #236

Open
kenji-miyake opened this issue May 7, 2021 · 7 comments · May be fixed by #327
Open

Support LifecycleNode and EventHandler in frontend #236

kenji-miyake opened this issue May 7, 2021 · 7 comments · May be fixed by #327
Labels
help wanted Extra attention is needed

Comments

@kenji-miyake
Copy link
Contributor

Feature request

Feature description

It's useful to support LifecycleNode in frontend(xml) as well.

As for LifecycleNode itself, if we only use simple launch files as below, it can be done by just exposing like this kenji-miyake@8daecd8.

<launch>
  <lifecycle_node pkg="lifecycle" exec="lifecycle_talker" name="lc_talker" namespace=""/>
  <node pkg="lifecycle" exec="lifecycle_listener" />
  <node pkg="lifecycle" exec="lifecycle_service_client" />
</launch>

However, I believe it's not enough useful because we usually would like to write state transitions as well like this.

So I think it's better to support RegisterEventHandler and EventHandlers.

Implementation considerations

I think there are at least two problems.

  1. How to expose EventHandlers?
    Is it okay to add parse_event_handler and instantiate_event_handler as Action class does?

  2. How to refer Python Object from frontend?
    For example, OnStateTransition requires LifecycleNode object but I think it can't be given in frontend easily.
    Probably this is what @ivanpauno mentioned here https://answers.ros.org/question/333404/launch-composed-nodes-using-the-launch-xml-frontend/, right?

The topic of having references (i.e.: being able to identify) another action in the XML/YAML frontend is an interesting feature currently not supported. It would be useful, for example, to support event handlers.

So I guess some mechanism for finding Object from all entities.

Does anybody have any idea about this?
If somebody guides me, I'll try to work on this task!

@clalancette clalancette added the help wanted Extra attention is needed label May 27, 2021
@clalancette
Copy link
Contributor

@jacobperron To provide some advice on how to move forward with this.

@jacobperron
Copy link
Member

@kenji-miyake Certainly, what you have is a good start (kenji-miyake/launch_ros@8daecd8). I would accept that in a pull request on its own.

Regarding exposing events and event handlers (etc) in the front-end, I suggest we open up a separate issue in launch. I imagine it should be easy to expose the RegisterEventHandler action. Then we need a way to parse EventHandlers, which I would guess should be fairly easy except when we need to reference other launch actions (e.g. referring to the Python objects as you mention). In these cases, maybe we could limit functionality such that launch actions must be referenced by name (ie. a string). I haven't thought about it much, but if you'd like to continue the discussion it would be great if you opened up a ticket over in launch.

@kenji-miyake
Copy link
Contributor Author

Thank you!
I can't make time so much recently, but I'll try to work on this task, so please wait for that.
Or somebody else would like to proceed more quickly, then I'd appreciate it if they could open issues and implement a prototype.

@jacobperron jacobperron removed their assignment Oct 26, 2021
@esteve
Copy link
Member

esteve commented Jun 28, 2022

@kenji-miyake I can work on this. What is missing in your branch? How do you think it should continue? Thanks.

@kenji-miyake
Copy link
Contributor Author

@esteve What I've done is just exposing LifecycleAction to frontend.
So missing things are (even though it's the same as the description of this issue):

  • To expose EventHandler (that is not an action) as well.
  • To improve class designs so that they can refer to Python objects by name.
    • It's because we can't use raw Python objects in XML. We have to search objects by name.

@esteve esteve linked a pull request Oct 4, 2022 that will close this issue
@esteve
Copy link
Member

esteve commented Oct 4, 2022

@kenji-miyake Certainly, what you have is a good start (kenji-miyake/launch_ros@8daecd8). I would accept that in a pull request on its own.

@jacobperron I've created a PR based on @kenji-miyake 's work at #327, I'll work on the remaining tasks in separate PRs

@esteve
Copy link
Member

esteve commented Oct 4, 2022

@jacobperron I've created ros2/launch#658 to continue the discussion there about the RegisterEventHandler action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants