make rcl_lifecycle_com_interface optional in lifecycle nodes

[Karsten1987]

connects to ros2/rclcpp#1506

[Karsten1987]

@fujitatomoya I've opted for introducing two more sets of init/fini, namely ..publisher_init, ..publisher_fini, ..services_init, ..services_fini. I've incorporated these appropriately in the goto fail: sections in order to deduplicate code.

As discussed in the rclcpp ticket, that allows us to explicitly initialize the transition event publisher without initializing the services. Let me know what you think.

[fujitatomoya]

As discussed in the rclcpp ticket, that allows us to explicitly initialize the transition event publisher without initializing the services.

it looks okay. i am okay to go with this approach.

but I'd like to get back to discussion if lifecycle_com_interface includes /<node_name>/transition_event. as far as i see from the current implementation, it does. in that case, publication should be disabled along with services? this can be done, disabling all communication path, and we can check state_machine_ see if publisher is initialized and then update publish_update.

CC: @ivanpauno @clalancette @wjwwood

[Karsten1987]

My/Our use case here is actually pretty straight forward. We'd like to disable external access to state changes. That is, we'd like to block all incoming communication which modifies the state machine. That however does not prevent any outgoing traffic, such as a publication on the transition event topics. In fact, I believe it should stay active in order to let other nodes in the network know about the triggered transitions.

Now, granted, with this definition we could also split the services further, where each individual service could be activated or not. That is, we only disable the change_state service, but leave the get_state et. al. services alive.

[clalancette]

@fujitatomoya Since you had some initial comments, I assigned this one to you to take a look at. If you don't have time for this, please feel free to unassign yourself.

[fujitatomoya]

@Karsten1987

overall, i am good to go with this. there are three unused_warning https://build.ros2.org/job/Rpr__rcl__ubuntu_focal_amd64/471/gcc/new/, probably we could change where to log the error messages? what do you think?

[Karsten1987]

@fujitatomoya I think I've addressed the unused warnings. I tried to mention it in the line comments, however I do believe we don't have to log twice and can ignore the return value of the fini calls in the fail section. The called function itself logs the reason of failure.

I did however have to add another flag to the com_interface for bookkeeping whether the com interface was initialized or not. That was necessary to make the call to is_initialized successful in both cases.

I'll link CI to the overall issue ticket once more: ros2/rclcpp#1506 (comment)

[fujitatomoya]

looks good to me.

[fujitatomoya]

@clalancette just in case, could you also take a look when you got time?

[Karsten1987]

This is good for another review. CI can be found on the top level issue: ros2/rclcpp#1506 (comment)

[clalancette]

One nit inline, but otherwise looks pretty good to me.

[ivanpauno]

lgtm

[Karsten1987]

CI is green (ros2/rclcpp#1506 (comment)), merging.
Thanks for the review.