Revert "Revert "Decouple rosout publisher init from node init. (#351)…

[fujitatomoya]

…" (#352)"

This reverts commit 2648503.

[fujitatomoya]

depends on ros2/rcl#1065

[fujitatomoya]

/assign @fujitatomoya

[fujitatomoya]

@JanStaschulat if you have time take a look at this? thanks in advance.

[fujitatomoya]

CI:

• Linux
• Linux-aarch64
• Windows

[codecov-commenter]

Codecov Report

Patch coverage: 12.50% and project coverage change: -0.24% ⚠️

Comparison is base (d263be2) 69.65% compared to head (5cf0040) 69.42%.

Additional details and impacted files

@@             Coverage Diff             @@
##           rolling     #392      +/-   ##
===========================================
- Coverage    69.65%   69.42%   -0.24%     
===========================================
  Files           16       16              
  Lines         2762     2770       +8     
  Branches       766      771       +5     
===========================================
- Hits          1924     1923       -1     
- Misses         453      459       +6     
- Partials       385      388       +3     

Files Changed	Coverage Δ
rclc/src/rclc/node.c	65.62% <12.50%> (-17.71%)	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fujitatomoya]

CI:

• Linux
• Linux-aarch64

see ros2/rcl#1065 (comment) why windows is excluded.

[fujitatomoya]

@JanStaschulat this is good to go, requesting review on this. thanks in advance.

[JanStaschulat]

In my opinion, the init and fini method are unsymmetric regarding RCLC library:

• The rcl_logging_rosout_init_publisher_for_node is called inside rclc library, but
• the rcl_logging_rosout_fini_publisher_for_node needs to be called by the user.

All the unit test code and the example code had to be updated. All existing rclc applications would have to be updated as well. That's not ideal.

I expect, this could lead to in-complete applications. Users will simply forget to call rcl_logging_rosout_fini_publisher_for_node.

I would suggest, to either call the rcl_logging_rosout_fini_publisher_for_node in RCL's rcl_node_fini method or we have to create a new rclc_node_fini method in RCLC library, to hide this function call from the user.

@fujitatomoya @iuhilnehc-ynos What do you think?

[fujitatomoya]

I would suggest, to either call the rcl_logging_rosout_fini_publisher_for_node in RCL's rcl_node_fini method

this has been separated to address the issue ros2/rclcpp#2147, rclcpp and rclpy can keep the consistent behavior for user application since it conceals this change.

or we have to create a new rclc_node_fini method in RCLC library, to hide this function call from the user.

i would take this path so that we can have the same design aligned with rclcpp and rclpy. besides goes with rclc_node_init_with_options?

[JanStaschulat]

@fujitatomoya What is the status of this pull request? Can it be merged?

[fujitatomoya]

@JanStaschulat i will check and rebase.

[fujitatomoya]

I need to create rclc_node_fini new API for this PR, please hold this PR.