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

Use lifecycle nodes in controllers again #538

Merged
merged 16 commits into from
Mar 14, 2022

Conversation

vatanaksoytezer
Copy link
Contributor

Adds the lifecycle nodes back, that are removed in #261.

@vatanaksoytezer vatanaksoytezer marked this pull request as draft September 21, 2021 00:49
@vatanaksoytezer
Copy link
Contributor Author

The only thing left in this should be a test failure where we try to reconfigure and start an unconfigured loaded controller, the cli exits. I suspect that due to a cleanup error.

@vatanaksoytezer vatanaksoytezer marked this pull request as ready for review September 21, 2021 13:49
@vatanaksoytezer
Copy link
Contributor Author

vatanaksoytezer commented Sep 21, 2021

@bmagyar I think I addressed all your review, but still I have one failing test in spawner_unspawner and controller_manager_srvs that seems to be stemming from cleanup.

See https://github.com/ros-controls/ros2_control/runs/3663696172?check_suite_focus=true#step:3:3170 and https://github.com/ros-controls/ros2_control/runs/3663696172?check_suite_focus=true#step:3:3076 for context.

@bmagyar bmagyar added the rolling label Nov 3, 2021
@destogl destogl self-assigned this Nov 12, 2021
@vatanaksoytezer
Copy link
Contributor Author

vatanaksoytezer commented Feb 9, 2022

I guess this is not gonna get merged soon, and I am more likely to not commit a lot of time into this PR, so closing it. Let me know if you need this or have any intentions to merge.

@mergify
Copy link
Contributor

mergify bot commented Feb 24, 2022

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

@destogl
Copy link
Member

destogl commented Feb 24, 2022

@vatanaksoytezer I will take this over. I hope I have access to push to your fork.

Thanks for the initial work. It looks good.

@vatanaksoytezer
Copy link
Contributor Author

@vatanaksoytezer I will take this over. I hope I have access to push to your fork.

You should have access, let me know if you need anything.

@destogl destogl self-requested a review February 24, 2022 22:19
@destogl destogl changed the title WIP: Add lifecycle nodes Use lifecycle nodes in controllers again Mar 3, 2022
@destogl destogl force-pushed the pr-lifecycle_nodes branch 2 times, most recently from 57a63ba to 858b210 Compare March 6, 2022 13:22
Co-authored-by: Denis Štogl <denis@stogl.de>
@destogl destogl merged commit 9f13b60 into ros-controls:master Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Galactic+] Use LifecycleNode once transitions cannot be triggered by externals.
4 participants