`EventHandlerBase::get_number_of_ready_events()` does not compute number of ready events

[Yadunund]

Feature request

Feature description

The current implementation of EventHandlerBase::get_number_of_ready_events() always returns 1. This function was originally introduced in https://github.com/ros2/rclcpp/pull/695 and is only used in rclcpp/test_qos_event.

However, having an accurate implementation of this function could benefit other tests like test_publisher:run_event_handlers which is calling take_data() without checking if the event is ready (ie, not calling is_ready()) or if get_number_of_ready_events > 0.

Implementation considerations

To correctly compute the number of ready events, we would likely need to

• Add a method to rmw/event.h to retrieve the count of events for a given rmw_event_type_t.
• Add a methods to rcl/event.h to retrieve number of ready events of publishers and subscriptions resp. given an rcl_event_t handle. This method will in turn invoke the newly introduced rmw method above.

Then the implementation for EventHandlerBase::get_number_of_ready_events() can be updated to invoke the appropriate rcl methods added above.

If this approach sounds ok, I can work on the implementations.

[wjwwood]

Sorry it took me so long to come back around to this.

I think there's a misunderstanding on the purpose of this API, it comes from rclcpp::Waitable as the base class:

https://github.com/ros2/rclcpp/blob/48a4761faa2ac6339aea45ccd00cd23fd5456d4d/rclcpp/include/rclcpp/waitable.hpp#L74-L83

And the number returned is the number of distinct "event sources", or "event types", that have at least one event ready to be taken from them. It doesn't imply anything about the number of events available from a single event source.

For example, if you had a waitable that contained two different events in it, and one had two events occur and the second had none, the number this method would return is 1, not 2.

And since the EventHandlerBase only ever deals with a single event type at a time, it could only ever return 0 or 1. I believe it always returns 1 because it assumes (perhaps incorrectly) that if this method (get_number_of_ready_events()) is being called, then it is because the event triggered a wait set to mark it as ready, which means the event must be ready. However, if this method (get_number_of_ready_events()) is called without a wait set marking the event ready, or it is called after already taking the event that was ready, then it could incorrectly indicate the event is ready to be taken from when it is in fact not ready. Which could lead to a call to take_data() when there's nothing to take (which should be allowed, but ideally would be avoided to avoid wasted time).

The other reason this function likely always returns 1 is that it is not possible or not easy to check if the event is actually ready without calling take_data(), in which case you might as well return 1 and see what take_data() does. We could add an API that allows this to be checked, but I would expect it might be that some implementations choose to return 1 unconditionally themselves for performance reasons.