QoS - API and implementation for Liveliness and Deadline event callbacks

[emersonknapp]

Depends on ros2/rmw#171
Depends on ros2/rmw_fastrtps#276
Depends on ros2/rcl#432

• Present the Python-side API for receiving QoS event callbacks for Liveliness and Deadline QoS policies.
• Provide the C-side translation layer to rcl to support that API.

Unit tests are still in progress - but the code is working according to manual testing and is ready for review.

[sloretz]

@emersonknapp Looks like there are merge conflicts

[emersonknapp]

Added new commit to refactor to use the new Subscription+Handle stuff properly

[emersonknapp]

@sloretz when you get a chance, would love feedback on this PR

[sloretz]

Mostly complete review with mostly nitpick comments

[emersonknapp]

@sloretz thanks for the feeback, I am trying to address these as quickly as possible today because we want to get this through for the API freeze today. The part that will take the most time is using the new Handle type, it is a good change but definitely caught me unprepared.

1. Are you planning on merging any more changes that would affect this feature?
2. Will you be available to review this later this afternoon so that we can push it through once the dependency is merged?

[sloretz]

Are you planning on merging any more changes that would affect this feature?

No changes are planned. I would like to add API to the Waitable interface to make destroying them thread safe too, but I doubt I will be able to work on that before Dashing is released.

Will you be available to review this later this afternoon so that we can push it through once the dependency is merged?

Yes

[emersonknapp]

@sloretz I've updated, tests are passing locally. I will have to do a rebase when https://waffle.io/ros2/ros2/cards/5c9e5639f2c40c0066440aba is merged (because of #305) but it should affect very little of what's here - a few lines.

Will I need to do any further restructuring of this?

[sloretz]

LGTM. Comments below are just nitpicks that don't affect API. If you already have green CI or are running CI, mind addressing these in a follow up PR after API freeze?

[emersonknapp]

@sloretz thanks - it seems like it'd be hard to run CI on this change until ros2/rmw#171 (comment) this batch passes and is merged first, since these features depend on those new APIs from rcl and rmw, and I'm not sure I'd trust a CI run against the pre-merged PR branches.

Got my fingers crossedon that build.

If you need to get some other things under this that are small (saw the comment from the other one), it should be fine - I will be doing the one final rebase for this, and that conflict looks trivial to resolve.

[emersonknapp]

While wringing my hands over the other ci - addressed the nitpicks

[emersonknapp]

@thomas-moulard - please run the following CI job:
NOTE - it contains the changes for this PR plus its two new dependencies
ros2/rcl#432
ros2/rmw_fastrtps#276

• Gist file: https://gist.githubusercontent.com/emersonknapp/67d93f9e90183987832715b5979640e8/raw/e75240e9745803f9f8fa09632f997cd07ce95f83/qos-4-callbacks.repos
• Additional BUILD flags: --packages-up-to rclpy
• Additional TEST flags: --packages-select rclpy
• CI Job: ci_launcher

[emersonknapp]

That's right, it just adds optional kwargs to the create_ functions

[emersonknapp]

@wjwwood ok to merge now? Looks like the API freeze board has been closed out

[wjwwood]

lgtm, but a lot has been merged since the last CI, so I'm running Linux again:

It's possible this needs to be rebased or merged with master.

[emersonknapp]

@wjwwood Ok - rebased - built and tested locally on linux, looks good, can you run that again?

[wjwwood]

Sure thing, thanks for the rebase:

[suab321321]

@emersonknapp could you please brief me what did you do on the backend side(.c files) to achieve?
I m trying to implement this rclnodejs issue RobotWebTools/rclnodejs#492.
:)

[emersonknapp]

@suab321321 do you have a specific question about the code? Most of it is just data type conversions between rcl and the Python layer. The functionality part of it is exposing the new Waitable type rcl_event_t and adding the polling for events.

You can see this PR ros2/rclcpp#695 for how this feature was added to rclcpp, for more context on the pattern.

[suab321321]

@emersonknapp thanks for helping me :).If you could tell me about the hiarchey of this implementation from its core to python side user side api,example how _rclpy.c calls _rclpy_qos_event.c like this a brief word about you did, will be very much helpful :)

[emersonknapp]

https://github.com/ros2/rclpy/pull/316/files?file-filters%5B%5D=.c#diff-80dda03110b5be606b823d6b0558b5c2R40 you can see on this line that the source file is included directly, they are only split for readability.

[suab321321]

@emersonknapp why qos events are waitables type

[suab321321]

class QoSEventHandler(Waitable):
    """Waitable type to handle QoS events."""

    def __init__(
        self,
        *,
        callback_group: CallbackGroup,
        callback: Callable,
        event_type: IntEnum,
        parent_handle: Handle,
    ):
        # Waitable init adds self to callback_group
        super().__init__(callback_group)
        self.event_type = event_type
        self.callback = callback

        self._parent_handle = parent_handle
        with parent_handle as parent_capsule:
            event_capsule = _rclpy.rclpy_create_event(event_type, parent_capsule)
        self._event_handle = Handle(event_capsule)
        self._event_handle.requires(self._parent_handle)
        self._ready_to_take_data = False
        self._event_index = None

file : qos_event.py
@emersonknapp can you please explain what the third last line self._event_handle.requires(self._parent_handle) is doing.

[emersonknapp]

Waitables are used for asynchronous communication. You "wait" for things to happen on it. If you familiarize yourself with the rclpy code, you will see that Action Servers and Clients are also Waitables. You add them to a "wait set" that is checked during "spin" to see if the event has occurred.

For your second question, please look at https://github.com/ros2/rclpy/blob/master/rclpy/rclpy/handle.py#L103 - requires on a Python Capsule Handle means that the Handle does not work without the other entity. In this case, the parent is the subscription or publisher that the QoSEventHandler is attached to. If that parent entity is deleted, this event handler is no longer valid.

[suab321321]

@emersonknapp I just want to know that in _rclpy.c for a timer callback to get executed there is method rcl_timer_call taking rcl_timer_t* as a paramter,so is there any method like this which is used for calling the qos callback? :)

[emersonknapp]

Timers, Subscriptions, Guards, Clients, and Services use an old way of asynchronous communication, they all have special lists that they are held in (https://github.com/ros2/rclpy/blob/master/rclpy/rclpy/executors.py#L589)

QoS Events use the generic Waitable pattern which can hold many types of objects, not just QoS Events. It is checked right before that special cases loop - (https://github.com/ros2/rclpy/blob/master/rclpy/rclpy/executors.py#L579) -
you can see that it checks on every spin loop, if an event is_ready (has an event happened?), then it uses take_data to grab the event data if one is ready. Python side and C side