Skip to content

SimpleManagedEntity is very specific #1677

Description

@EGAlberts

The SimpleManagedEntity provided as a part of rclpy_lifecycle e.g. https://github.com/ros2/rclpy/blob/humble/rclpy/rclpy/lifecycle/managed_entity.py

is quite specific to its use as for the lifecycle publisher. In that it does not return the value of its wrapped function. If someone were to wrap an entity that would return something, e.g. a client, and decides to reuse the SimpleManagedEntity they may not realize this and face errors as a result.

It should not affect the use in Lifecycle publisher to simply add return values, like this:

    def when_enabled(wrapped=None, *, when_not_enabled=None):
        def decorator(wrapped):
            @wraps(wrapped)
            def only_when_enabled_wrapper(self: SimpleManagedEntity, *args, **kwargs):
                if not self._enabled:
                    if when_not_enabled is not None:
                        **return** when_not_enabled()
                    return
                **return** wrapped(self, *args, **kwargs)
            return only_when_enabled_wrapper
        if wrapped is None:
            return decorator
        return decorator(wrapped)

At the same time, the current implementation does not result in any issues as is, otherwise I would have provided a PR, but if someone agrees I can provide a simple PR to add the return statements.

P.S. It is quite odd that only create_publisher has an override for the publisher class (seemingly due to lifecycle publishers), I would imagine this is either something every create method of a node has or doesn't. This could also be addressed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions