Always trigger guard condition waitset

[alsora]

Ttrigger guard condition waitset regardless of whether a trigger callback is present
This should address #1917

Also minor cleanup at the class (remove unneded lock and better use of if-else)

Signed-off-by: Alberto Soragna alberto.soragna@gmail.com

[alsora]

I restored the mutex to prevent race condition that could cause problems.

[fujitatomoya]

lgtm with minor nit

[wjwwood]

Looks reasonable.

It seems like a test to avoid this regressing in the future would be good to have.

Also, if it's not already clear from the docstring what should happen in this corner case, it should be clarified there.

[alsora]

Added unit-tests.

Running full CI

• Linux
• Linux-aarch64
• Windows

[wjwwood]

This doc block should mention how it interacts with the "on trigger callback":

rclcpp/rclcpp/include/rclcpp/guard_condition.hpp

Lines 75 to 85 in 02802bc

	/// Notify the wait set waiting on this condition, if any, that the condition had been met.
	/**
	* This function is thread-safe, and may be called concurrently with waiting
	* on this guard condition in a wait set.
	*
	* \throws rclcpp::exceptions::RCLError based exceptions when underlying
	* rcl functions fail.
	*/
	RCLCPP_PUBLIC
	void
	trigger();

And, I guess we missed it, but we should have some documentation on the "on trigger callback" method as well (it currently has none):

rclcpp/rclcpp/include/rclcpp/guard_condition.hpp

Lines 110 to 112 in 02802bc

	RCLCPP_PUBLIC
	void
	set_on_trigger_callback(std::function<void(size_t)> callback);

[alsora]

@wjwwood @fujitatomoya anything else to do here?

[fujitatomoya]

@alsora i am good to go! @wjwwood could you check your comments on #1923 (comment)?

[wjwwood]

lgtm

Sorry for not re-reviewing it. I don't get notifications for pushed commits, so a comment is helpful to bump a re-review.