From 72ad5d75dfafa14beaea58866777931cf664570f Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Tue, 21 May 2019 15:48:57 -0700 Subject: [PATCH 1/6] Initialize QoSProfile with values from rmw_qos_profile_default, to match behavior from rclcpp Signed-off-by: Emerson Knapp --- rclpy/rclpy/qos.py | 52 ++++++++++++++++++++++++++---------------- rclpy/test/test_qos.py | 9 +++++++- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/rclpy/rclpy/qos.py b/rclpy/rclpy/qos.py index 6f3d13caf..0e2b4a41e 100644 --- a/rclpy/rclpy/qos.py +++ b/rclpy/rclpy/qos.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from argparse import Namespace from enum import Enum from enum import IntEnum import warnings @@ -20,6 +21,10 @@ from rclpy.impl.implementation_singleton import rclpy_action_implementation as _rclpy_action from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy +# "Forward-declare" this value so that it can be used in the QoSProfile initializer. +# It will have a value by the end of definitions, before user code runs. +_qos_default_value_for_init = [] + class QoSProfile: """Define Quality of Service policies.""" @@ -39,6 +44,17 @@ class QoSProfile: def __init__(self, **kwargs): assert all('_' + key in self.__slots__ for key in kwargs.keys()), \ 'Invalid arguments passed to constructor: %r' % kwargs.keys() + + if not len(_qos_default_value_for_init): + # Any of the setters, upon receiving these None values, would assert. + # This can't happen though, because this is only used at definition time, when + # we are initializing fully-defined preset profiles. + from_profile = Namespace(**{ + slot[1:]: None for slot in self.__slots__ + }) + else: + from_profile = _qos_default_value_for_init[0] + if 'history' not in kwargs: if 'depth' in kwargs: kwargs['history'] = QoSHistoryPolicy.RMW_QOS_POLICY_HISTORY_KEEP_LAST @@ -46,9 +62,7 @@ def __init__(self, **kwargs): warnings.warn( "QoSProfile needs a 'history' and/or 'depth' setting when constructed", stacklevel=2) - self.history = kwargs.get( - 'history', - QoSHistoryPolicy.RMW_QOS_POLICY_HISTORY_SYSTEM_DEFAULT) + self.history = kwargs.get('history', from_profile.history) if ( QoSHistoryPolicy.RMW_QOS_POLICY_HISTORY_KEEP_LAST == self.history and 'depth' not in kwargs @@ -56,22 +70,17 @@ def __init__(self, **kwargs): warnings.warn( 'A QoSProfile with history set to KEEP_LAST needs a depth specified', stacklevel=2) - self.depth = kwargs.get('depth', int()) - self.reliability = kwargs.get( - 'reliability', - QoSReliabilityPolicy.RMW_QOS_POLICY_RELIABILITY_SYSTEM_DEFAULT) - self.durability = kwargs.get( - 'durability', - QoSDurabilityPolicy.RMW_QOS_POLICY_DURABILITY_SYSTEM_DEFAULT) - self.lifespan = kwargs.get('lifespan', Duration()) - self.deadline = kwargs.get('deadline', Duration()) - self.liveliness = kwargs.get( - 'liveliness', - QoSLivelinessPolicy.RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT) - self.liveliness_lease_duration = kwargs.get('liveliness_lease_duration', Duration()) + + self.depth = kwargs.get('depth', from_profile.depth) + self.reliability = kwargs.get('reliability', from_profile.reliability) + self.durability = kwargs.get('durability', from_profile.durability) + self.lifespan = kwargs.get('lifespan', from_profile.lifespan) + self.deadline = kwargs.get('deadline', from_profile.deadline) + self.liveliness = kwargs.get('liveliness', from_profile.liveliness) + self.liveliness_lease_duration = kwargs.get( + 'liveliness_lease_duration', from_profile.liveliness_lease_duration) self.avoid_ros_namespace_conventions = kwargs.get( - 'avoid_ros_namespace_conventions', - False) + 'avoid_ros_namespace_conventions', from_profile.avoid_ros_namespace_conventions) @property def history(self): @@ -325,9 +334,11 @@ def __init__(self, qos_profile: QoSProfile, profile_name: str): self.name = profile_name -__qos_profile_default = _rclpy.rclpy_get_rmw_qos_profile( +_qos_profile_default = _rclpy.rclpy_get_rmw_qos_profile( 'qos_profile_default') -qos_profile_default = DeprecatedQoSProfile(__qos_profile_default, 'qos_profile_default') +_qos_default_value_for_init.append(_qos_profile_default) + +qos_profile_default = DeprecatedQoSProfile(_qos_profile_default, 'qos_profile_default') qos_profile_system_default = _rclpy.rclpy_get_rmw_qos_profile( 'qos_profile_system_default') qos_profile_sensor_data = _rclpy.rclpy_get_rmw_qos_profile( @@ -343,6 +354,7 @@ def __init__(self, qos_profile: QoSProfile, profile_name: str): class QoSPresetProfiles(Enum): + DEFAULT = _qos_profile_default SYSTEM_DEFAULT = qos_profile_system_default SENSOR_DATA = qos_profile_sensor_data SERVICES_DEFAULT = qos_profile_services_default diff --git a/rclpy/test/test_qos.py b/rclpy/test/test_qos.py index 4693f78f4..f17dec4d8 100644 --- a/rclpy/test/test_qos.py +++ b/rclpy/test/test_qos.py @@ -99,8 +99,9 @@ def test_deprecation_warnings(self): with warnings.catch_warnings(record=True) as w: warnings.simplefilter('always') QoSProfile() - assert len(w) == 1 + assert len(w) == 2 # must supply depth or history, _and_ KEEP_LAST needs depth assert issubclass(w[0].category, UserWarning) + with warnings.catch_warnings(record=True) as w: warnings.simplefilter('always') # No deprecation if history is supplied @@ -142,3 +143,9 @@ def test_preset_profiles(self): assert ( QoSPresetProfiles.SYSTEM_DEFAULT.value == QoSPresetProfiles.get_from_short_key('system_default')) + + def test_default_profile(self): + profile = QoSProfile(depth=10) + assert all( + profile.__getattribute__(k) == QoSPresetProfiles.DEFAULT.value.__getattribute__(k) + for k in profile.__slots__) From febc403ede68905e2bc72d4379365a8ec2da26bc Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Wed, 22 May 2019 10:42:41 -0700 Subject: [PATCH 2/6] Remove the global list variable i thought i needed Signed-off-by: Emerson Knapp --- rclpy/rclpy/qos.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/rclpy/rclpy/qos.py b/rclpy/rclpy/qos.py index 0e2b4a41e..878c6fc7e 100644 --- a/rclpy/rclpy/qos.py +++ b/rclpy/rclpy/qos.py @@ -23,7 +23,7 @@ # "Forward-declare" this value so that it can be used in the QoSProfile initializer. # It will have a value by the end of definitions, before user code runs. -_qos_default_value_for_init = [] +_qos_profile_default = None class QoSProfile: @@ -45,7 +45,7 @@ def __init__(self, **kwargs): assert all('_' + key in self.__slots__ for key in kwargs.keys()), \ 'Invalid arguments passed to constructor: %r' % kwargs.keys() - if not len(_qos_default_value_for_init): + if not _qos_profile_default: # Any of the setters, upon receiving these None values, would assert. # This can't happen though, because this is only used at definition time, when # we are initializing fully-defined preset profiles. @@ -53,7 +53,7 @@ def __init__(self, **kwargs): slot[1:]: None for slot in self.__slots__ }) else: - from_profile = _qos_default_value_for_init[0] + from_profile = _qos_profile_default if 'history' not in kwargs: if 'depth' in kwargs: @@ -336,8 +336,6 @@ def __init__(self, qos_profile: QoSProfile, profile_name: str): _qos_profile_default = _rclpy.rclpy_get_rmw_qos_profile( 'qos_profile_default') -_qos_default_value_for_init.append(_qos_profile_default) - qos_profile_default = DeprecatedQoSProfile(_qos_profile_default, 'qos_profile_default') qos_profile_system_default = _rclpy.rclpy_get_rmw_qos_profile( 'qos_profile_system_default') From 3545abc067e32c06a93f7c015e7abbca5bdd9354 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Wed, 22 May 2019 10:49:36 -0700 Subject: [PATCH 3/6] Add assertion to make assumptions even more clear Signed-off-by: Emerson Knapp --- rclpy/rclpy/qos.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rclpy/rclpy/qos.py b/rclpy/rclpy/qos.py index 878c6fc7e..c8b0f46bb 100644 --- a/rclpy/rclpy/qos.py +++ b/rclpy/rclpy/qos.py @@ -46,12 +46,12 @@ def __init__(self, **kwargs): 'Invalid arguments passed to constructor: %r' % kwargs.keys() if not _qos_profile_default: - # Any of the setters, upon receiving these None values, would assert. - # This can't happen though, because this is only used at definition time, when - # we are initializing fully-defined preset profiles. - from_profile = Namespace(**{ - slot[1:]: None for slot in self.__slots__ - }) + # It is still definition time, and all calls to this initializer are expected to be + # fully-defined preset profiles from the C side. + assert all(kwargs[slot[1:]] is not None for slot in self.__slots__) + # Any of the setters, upon receiving these None values, would assert + # if the above assertion failed. + from_profile = Namespace(**{slot[1:]: None for slot in self.__slots__}) else: from_profile = _qos_profile_default From 1c9f44abb90f058785c421c1dcf5ba708cb9d5d7 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Thu, 23 May 2019 11:50:04 -0700 Subject: [PATCH 4/6] Don't expose default profile explicitly to users Signed-off-by: Emerson Knapp --- rclpy/rclpy/qos.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rclpy/rclpy/qos.py b/rclpy/rclpy/qos.py index c8b0f46bb..c404ee31d 100644 --- a/rclpy/rclpy/qos.py +++ b/rclpy/rclpy/qos.py @@ -352,7 +352,6 @@ def __init__(self, qos_profile: QoSProfile, profile_name: str): class QoSPresetProfiles(Enum): - DEFAULT = _qos_profile_default SYSTEM_DEFAULT = qos_profile_system_default SENSOR_DATA = qos_profile_sensor_data SERVICES_DEFAULT = qos_profile_services_default From 19f8cac5b5a3490593c22bbd8fafecba622111b6 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Thu, 23 May 2019 12:02:13 -0700 Subject: [PATCH 5/6] Fix test after re-concealing default profile Signed-off-by: Emerson Knapp --- rclpy/test/test_qos.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rclpy/test/test_qos.py b/rclpy/test/test_qos.py index f17dec4d8..3220d9dde 100644 --- a/rclpy/test/test_qos.py +++ b/rclpy/test/test_qos.py @@ -17,6 +17,7 @@ from rclpy.duration import Duration from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy +from rclpy.qos import _qos_profile_default from rclpy.qos import qos_profile_system_default from rclpy.qos import QoSDurabilityPolicy from rclpy.qos import QoSHistoryPolicy @@ -145,7 +146,9 @@ def test_preset_profiles(self): QoSPresetProfiles.get_from_short_key('system_default')) def test_default_profile(self): - profile = QoSProfile(depth=10) + with warnings.catch_warnings(record=True): + warnings.simplefilter('always') + profile = QoSProfile() assert all( - profile.__getattribute__(k) == QoSPresetProfiles.DEFAULT.value.__getattribute__(k) + profile.__getattribute__(k) == _qos_profile_default.__getattribute__(k) for k in profile.__slots__) From 0e155455075247e1f89629c762d3b456be35dbda Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Thu, 23 May 2019 13:23:40 -0700 Subject: [PATCH 6/6] Remove whitespace change Signed-off-by: Emerson Knapp --- rclpy/test/test_qos.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rclpy/test/test_qos.py b/rclpy/test/test_qos.py index 3220d9dde..d74516706 100644 --- a/rclpy/test/test_qos.py +++ b/rclpy/test/test_qos.py @@ -102,7 +102,6 @@ def test_deprecation_warnings(self): QoSProfile() assert len(w) == 2 # must supply depth or history, _and_ KEEP_LAST needs depth assert issubclass(w[0].category, UserWarning) - with warnings.catch_warnings(record=True) as w: warnings.simplefilter('always') # No deprecation if history is supplied