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

Cleanup the lifecycle implementation #2027

Merged
merged 7 commits into from
Oct 24, 2022
Merged

Cleanup the lifecycle implementation #2027

merged 7 commits into from
Oct 24, 2022

Conversation

clalancette
Copy link
Contributor

This series of patches cleans up the code that implements LifecycleNodeInterfaceImpl, the PIMPL interface for the LifecycleNode class. In particular, it does the following things:

  1. Moves the implementation from completely in a header file into a split header file/source file. This class is not templated, so there is no reason the implementation has to be in the header file.
  2. Marks the LifecycleNodeInterfaceImpl class as final (nothing can possibly derive from it).
  3. Updates some documentation.
  4. Marks a number of methods inside of LifecycleNodeInterfaceImpl as const.
  5. Makes quite a bit of the internals of LifecycleNodeInterfaceImpl private.
  6. Disables copies of LifecycleNodeInterfaceImpl.
  7. Marks some methods from LifecycleNode as const.

The main goal here is cleanup, but this should also make PRs like #1756 more clear.

@fujitatomoya please take a look when you get a chance.

@clalancette
Split lifecycle_node_interface_impl into header and implementation.
ff8d389 
There is no reason it should all be in the header file.  No
functional change.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Mark LifecycleNodeInterfaceImpl as final.
bbcc1e2 
Just a minor optimization.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Update documentation about return codes.
fd4a2b3 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Mark a bunch of LifecycleNodeInterfaceImpl methods as const.
5626b3a 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Make most of LifecycleNodeInterfaceImpl private.
2412f2a 
There is no reason to expose the guts of it.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Mark some LifecycleNode methods as const.
9569476 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Disable copies on LifecycleNodeInterfaceImpl.
caea626 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette mentioned this pull request Oct 17, 2022
Merged
@clalancette
Copy link
Contributor Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor Author

Thanks for the review! I'm going to go ahead with this one, even though it will definitely cause merge conflicts with #1756 . With this in, we'll be able to further review the use of get_current_state and hopefully unblock #1756 .

@clalancette clalancette merged commit 28890bf into rolling Oct 24, 2022
@clalancette clalancette deleted the clalancette/cleanup-lifecycle-impl branch October 24, 2022 12:43
@fujitatomoya fujitatomoya mentioned this pull request Oct 27, 2022
Merged
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Mar 24, 2023
@clalancette
Cleanup the lifecycle implementation (ros2#2027)
50c33a5 
* Split lifecycle_node_interface_impl into header and implementation.

There is no reason it should all be in the header file.  No
functional change.

* Mark LifecycleNodeInterfaceImpl as final.

* Update documentation about return codes.

* Mark a bunch of LifecycleNodeInterfaceImpl methods as const.

* Make most of LifecycleNodeInterfaceImpl private.

* Mark some LifecycleNode methods as const.

* Disable copies on LifecycleNodeInterfaceImpl.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Mar 24, 2023
@clalancette
Cleanup the lifecycle implementation (ros2#2027)
3f82d12 
* Split lifecycle_node_interface_impl into header and implementation.

There is no reason it should all be in the header file.  No
functional change.

* Mark LifecycleNodeInterfaceImpl as final.

* Update documentation about return codes.

* Mark a bunch of LifecycleNodeInterfaceImpl methods as const.

* Make most of LifecycleNodeInterfaceImpl private.

* Mark some LifecycleNode methods as const.

* Disable copies on LifecycleNodeInterfaceImpl.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette mentioned this pull request Apr 27, 2023
Closed
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Apr 29, 2023
@clalancette
Cleanup the lifecycle implementation (ros2#2027)
b22b860 
* Split lifecycle_node_interface_impl into header and implementation.

There is no reason it should all be in the header file.  No
functional change.

* Mark LifecycleNodeInterfaceImpl as final.

* Update documentation about return codes.

* Mark a bunch of LifecycleNodeInterfaceImpl methods as const.

* Make most of LifecycleNodeInterfaceImpl private.

* Mark some LifecycleNode methods as const.

* Disable copies on LifecycleNodeInterfaceImpl.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Apr 29, 2023
@clalancette
Cleanup the lifecycle implementation (ros2#2027)
3147c81 
* Split lifecycle_node_interface_impl into header and implementation.

There is no reason it should all be in the header file.  No
functional change.

* Mark LifecycleNodeInterfaceImpl as final.

* Update documentation about return codes.

* Mark a bunch of LifecycleNodeInterfaceImpl methods as const.

* Make most of LifecycleNodeInterfaceImpl private.

* Mark some LifecycleNode methods as const.

* Disable copies on LifecycleNodeInterfaceImpl.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request May 3, 2023
@clalancette
Cleanup the lifecycle implementation (ros2#2027)
4db1add 
* Split lifecycle_node_interface_impl into header and implementation.

There is no reason it should all be in the header file.  No
functional change.

* Mark LifecycleNodeInterfaceImpl as final.

* Update documentation about return codes.

* Mark a bunch of LifecycleNodeInterfaceImpl methods as const.

* Make most of LifecycleNodeInterfaceImpl private.

* Mark some LifecycleNode methods as const.

* Disable copies on LifecycleNodeInterfaceImpl.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alsora alsora alsora approved these changes

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@clalancette @alsora