Add convenience name translations for use by commandline utilities etc.

[emersonknapp]

Unblocks ros2/ros2cli#240

Signed-off-by: Emerson Knapp eknapp@amazon.com

[emersonknapp]

@mjcarroll see this and ros2/ros2cli#240, is this along the lines of what you were thinking?

[sloretz]

I would rather see these names become part of the enum class. I think if the name is closer to the enum it will be more likely to be updated if/when new QoS features are added. Also, personally I want shorter names in code.

#!/usr/bin/env python3

from enum import IntEnum


class QoSLivelinessPolicy(IntEnum):
    """
    Enum for QoS Liveliness settings.
    This enum matches the one defined in rmw/types.h
    """

    RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT = 0
    system_default = RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT 
    RMW_QOS_POLICY_LIVELINESS_AUTOMATIC = 1
    automatic = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC 
    RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE = 2
    manual_by_node = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE 
    RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC = 3
    manual_by_topic = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC 


import code
code.interact(local=locals())

Enum classes are generated with a __getitem__ that allows getting the member with a string.

>>> QoSLivelinessPolicy['system_default']
<QoSLivelinessPolicy.RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT: 0>
>>> QoSLivelinessPolicy['automatic']
<QoSLivelinessPolicy.RMW_QOS_POLICY_LIVELINESS_AUTOMATIC: 1>

The keys can be gotten off the class via __members__. One drawback for a CLI tool is the values would be a bit noisy as it includes the long all capitals name.

>>> QoSLivelinessPolicy.__members__.keys()
odict_keys(['RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT', 'system_default', 'RMW_QOS_POLICY_LIVELINESS_AUTOMATIC', 'automatic', 'RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE', 'manual_by_node', 'RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC', 'manual_by_topic'])

But that could be cleaned up with a little filtering

>>> [k for k in QoSLivelinessPolicy.__members__.keys() if not k.startswith('RMW')]
['system_default', 'automatic', 'manual_by_node', 'manual_by_topic']

[emersonknapp]

I like this suggestion, thanks! I have pushed a new commit to use this pattern, what do you think?

[sloretz]

Would these still be useful for the CLI if the names were SYSTEM_DEFAULT instead of system_default? Since PEP 8 wants all-caps, it seems likely a linter will complain about these being lower case now or in the future.

The enum documentation also argues in favor of all-caps.

[emersonknapp]

I think it would be a reasonable use case to have users call PolicyEnum[arg.upper()], or perhaps provide another convenience function QoSPolicyEnum.get_short_name(cls, name) that uppercases the name

[sloretz]

or perhaps provide another convenience function QoSPolicyEnum.get_short_name(cls, name) that uppercases the name

Did you mean a convenience function that provides lower case names? I suspect the linter tests in this package will fail in the future if system_default etc is lowercase instead of uppercase, meaning this might need to be

class QoSLivelinessPolicy(IntEnum):
    """
    Enum for QoS Liveliness settings.
    This enum matches the one defined in rmw/types.h
    """

    RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT = 0
    SYSTEM_DEFAULT = RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT 
    RMW_QOS_POLICY_LIVELINESS_AUTOMATIC = 1
    AUTOMATIC = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC 
    RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE = 2
    MANUAL_BY_NODE = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE 
    RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC = 3
    MANUAL_BY_TOPIC = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC 

[emersonknapp]

New commit should clarify what I was trying to say : yes use all uppercase names for enum, but provide case-insensitive getter function for users.

[sloretz]

LGTM with green CI

Dismissing for now with lower-case/upper-case question

[emersonknapp]

@thomas-moulard can you please run CI for this change
Gist: https://gist.githubusercontent.com/emersonknapp/9116731124636553c3e0a03a1bdf1509/raw/628a181f31d6ee43b70c179a592c395b867d6921/ros2.repos
Build args: --packages-up-to rclpy
Test args: --packages-select rclpy
Job: ci_launcher

[jacobperron]

• Linux
• Linux-aarch64
• macOS
• Windows

[emersonknapp]

I'm pretty confused about the osx and windows build failures there. Both show 100% tests pass and fail with

17:08:06 [95.826s] colcon.colcon_core.executor ERROR Exception in executor extension 'sequential': no running event loop
17:08:06 Traceback (most recent call last):
17:08:06   File "c:\python37\lib\site-packages\colcon_core\executor\__init__.py", line 327, in execute_jobs
17:08:06     rc = func(context.args, jobs, **kwargs)
17:08:06   File "c:\python37\lib\site-packages\colcon_core\executor\sequential.py", line 99, in execute
17:08:06     for task in all_tasks():
17:08:06   File "c:\python37\lib\asyncio\tasks.py", line 37, in all_tasks
17:08:06     loop = events.get_running_loop()
17:08:06 RuntimeError: no running event loop

[dirk-thomas]

This smells like a problem in this recent change: colcon/colcon-core#187

[emersonknapp]

Same issue on these builds ros2/demos#338 (comment) - so that seems likely to me since these two have no common changes

[jacobperron]

Problem fixed in ros2/launch#245.

Re-triggering builds:

• macOS:
• Windows: