From 3eae8540719616d87c02a335ff445519efaa97a6 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Sat, 1 Aug 2020 00:03:40 -0700 Subject: [PATCH 01/15] add negative check to init py Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index d4d4f7363..ce7963eb8 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -61,6 +61,8 @@ def interpret_dict_as_qos_profile(qos_profile_dict: Dict) -> QoSProfile: elif policy_key in _QOS_POLICY_FROM_SHORT_NAME: new_profile_dict[policy_key] = _QOS_POLICY_FROM_SHORT_NAME[policy_key](policy_value) elif policy_key in _VALUE_KEYS: + if policy_value < 0: + raise ValueError('`{}` may not be a negative value.'.format(policy_key)) new_profile_dict[policy_key] = policy_value else: raise ValueError('Unexpected key `{}` for QoS profile.'.format(policy_key)) From 67955dd7ded9c90615d7bac953cd6de58af16d23 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Sat, 1 Aug 2020 00:31:08 -0700 Subject: [PATCH 02/15] add unit test Signed-off-by: Jesse Ikawa --- ros2bag/test/resources/negative_qos_profile.yaml | 11 +++++++++++ ros2bag/test/test_play_qos_profiles.py | 12 ++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 ros2bag/test/resources/negative_qos_profile.yaml diff --git a/ros2bag/test/resources/negative_qos_profile.yaml b/ros2bag/test/resources/negative_qos_profile.yaml new file mode 100644 index 000000000..9e4439f2c --- /dev/null +++ b/ros2bag/test/resources/negative_qos_profile.yaml @@ -0,0 +1,11 @@ +/test_topic: + depth: -1 + deadline: + sec: -1 + nsec: -1 + lifespan: + sec: -1 + nsec: -1 + liveliness_lease_duration: + sec: -1 + nsec: -1 diff --git a/ros2bag/test/test_play_qos_profiles.py b/ros2bag/test/test_play_qos_profiles.py index b6f719d48..ce3e2e352 100644 --- a/ros2bag/test/test_play_qos_profiles.py +++ b/ros2bag/test/test_play_qos_profiles.py @@ -82,3 +82,15 @@ def test_qos_incomplete(self): expected_string_regex = re.compile(ERROR_STRING) matches = expected_string_regex.search(bag_command.output) assert not matches, print('ros2bag CLI did not produce the expected output') + + def test_qos_negative(self): + """Test a QoS profile with negative integer values for a single topic.""" + profile_path = RESOURCES_PATH / 'negative_qos_profile.yaml' + bag_path = RESOURCES_PATH / 'empty_bag' + arguments = ['play', '--qos-profile-overrides-path', profile_path.as_posix(), + bag_path.as_posix()] + with self.launch_bag_command(arguments=arguments) as bag_command: + bag_command.wait_for_shutdown(timeout=5) + expected_string_regex = re.compile(ERROR_STRING) + matches = expected_string_regex.search(bag_command.output) + assert not matches, print('ros2bag CLI did not produce the expected output') From b702cacdc2ca7489d699ef649607a338bc5f6aea Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Mon, 3 Aug 2020 14:33:43 -0700 Subject: [PATCH 03/15] modify unit test Signed-off-by: Jesse Ikawa --- ros2bag/test/resources/negative_qos_profile.yaml | 11 ----------- ros2bag/test/test_api.py | 14 ++++++++++++++ ros2bag/test/test_play_qos_profiles.py | 13 +------------ 3 files changed, 15 insertions(+), 23 deletions(-) delete mode 100644 ros2bag/test/resources/negative_qos_profile.yaml diff --git a/ros2bag/test/resources/negative_qos_profile.yaml b/ros2bag/test/resources/negative_qos_profile.yaml deleted file mode 100644 index 9e4439f2c..000000000 --- a/ros2bag/test/resources/negative_qos_profile.yaml +++ /dev/null @@ -1,11 +0,0 @@ -/test_topic: - depth: -1 - deadline: - sec: -1 - nsec: -1 - lifespan: - sec: -1 - nsec: -1 - liveliness_lease_duration: - sec: -1 - nsec: -1 diff --git a/ros2bag/test/test_api.py b/ros2bag/test/test_api.py index e01992110..a75ee0ffa 100644 --- a/ros2bag/test/test_api.py +++ b/ros2bag/test/test_api.py @@ -74,3 +74,17 @@ def test_convert_yaml_to_qos_profile(self): assert qos_profiles[topic_name_2].avoid_ros_namespace_conventions == expected_convention assert qos_profiles[topic_name_2].history == \ QoSHistoryPolicy.RMW_QOS_POLICY_HISTORY_KEEP_ALL + + def test_interpret_dict_as_qos_profile_negative(self): + qos_dict = {'depth': '-1'} + with self.assertRaises(ValueError): + interpret_dict_as_qos_profile(qos_dict) + qos_dict = {'deadline': {'sec': -1, 'nsec': -1}} + with self.assertRaises(ValueError): + interpret_dict_as_qos_profile(qos_dict) + qos_dict = {'lifespan': {'sec': -1, 'nsec': -1}} + with self.assertRaises(ValueError): + interpret_dict_as_qos_profile(qos_dict) + qos_dict = {'liveliness_lease_duration': {'sec': -1, 'nsec': -1}} + with self.assertRaises(ValueError): + interpret_dict_as_qos_profile(qos_dict) diff --git a/ros2bag/test/test_play_qos_profiles.py b/ros2bag/test/test_play_qos_profiles.py index ce3e2e352..d44110a0e 100644 --- a/ros2bag/test/test_play_qos_profiles.py +++ b/ros2bag/test/test_play_qos_profiles.py @@ -82,15 +82,4 @@ def test_qos_incomplete(self): expected_string_regex = re.compile(ERROR_STRING) matches = expected_string_regex.search(bag_command.output) assert not matches, print('ros2bag CLI did not produce the expected output') - - def test_qos_negative(self): - """Test a QoS profile with negative integer values for a single topic.""" - profile_path = RESOURCES_PATH / 'negative_qos_profile.yaml' - bag_path = RESOURCES_PATH / 'empty_bag' - arguments = ['play', '--qos-profile-overrides-path', profile_path.as_posix(), - bag_path.as_posix()] - with self.launch_bag_command(arguments=arguments) as bag_command: - bag_command.wait_for_shutdown(timeout=5) - expected_string_regex = re.compile(ERROR_STRING) - matches = expected_string_regex.search(bag_command.output) - assert not matches, print('ros2bag CLI did not produce the expected output') + \ No newline at end of file From 25ec40708a9db6cf1997cdcc69c9c86ff13f016c Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Mon, 3 Aug 2020 15:25:21 -0700 Subject: [PATCH 04/15] revert test_play Signed-off-by: Jesse Ikawa --- ros2bag/test/test_play_qos_profiles.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ros2bag/test/test_play_qos_profiles.py b/ros2bag/test/test_play_qos_profiles.py index d44110a0e..b6f719d48 100644 --- a/ros2bag/test/test_play_qos_profiles.py +++ b/ros2bag/test/test_play_qos_profiles.py @@ -82,4 +82,3 @@ def test_qos_incomplete(self): expected_string_regex = re.compile(ERROR_STRING) matches = expected_string_regex.search(bag_command.output) assert not matches, print('ros2bag CLI did not produce the expected output') - \ No newline at end of file From 2e8583e25a454f8f86f86923002793f56f210dd0 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Mon, 3 Aug 2020 21:30:11 -0700 Subject: [PATCH 05/15] typo string to int Signed-off-by: Jesse Ikawa --- ros2bag/test/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ros2bag/test/test_api.py b/ros2bag/test/test_api.py index a75ee0ffa..2739085c4 100644 --- a/ros2bag/test/test_api.py +++ b/ros2bag/test/test_api.py @@ -76,7 +76,7 @@ def test_convert_yaml_to_qos_profile(self): QoSHistoryPolicy.RMW_QOS_POLICY_HISTORY_KEEP_ALL def test_interpret_dict_as_qos_profile_negative(self): - qos_dict = {'depth': '-1'} + qos_dict = {'depth': -1} with self.assertRaises(ValueError): interpret_dict_as_qos_profile(qos_dict) qos_dict = {'deadline': {'sec': -1, 'nsec': -1}} From 336abcb617e487bf7df61af6c114e998f5243d61 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Mon, 3 Aug 2020 21:55:40 -0700 Subject: [PATCH 06/15] add history value to unit tests Signed-off-by: Jesse Ikawa --- ros2bag/test/test_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ros2bag/test/test_api.py b/ros2bag/test/test_api.py index 2739085c4..f90a9dbe4 100644 --- a/ros2bag/test/test_api.py +++ b/ros2bag/test/test_api.py @@ -76,15 +76,15 @@ def test_convert_yaml_to_qos_profile(self): QoSHistoryPolicy.RMW_QOS_POLICY_HISTORY_KEEP_ALL def test_interpret_dict_as_qos_profile_negative(self): - qos_dict = {'depth': -1} + qos_dict = {'history': 'keep_all', 'depth': -1} with self.assertRaises(ValueError): interpret_dict_as_qos_profile(qos_dict) - qos_dict = {'deadline': {'sec': -1, 'nsec': -1}} + qos_dict = {'history': 'keep_all', 'deadline': {'sec': -1, 'nsec': -1}} with self.assertRaises(ValueError): interpret_dict_as_qos_profile(qos_dict) - qos_dict = {'lifespan': {'sec': -1, 'nsec': -1}} + qos_dict = {'history': 'keep_all', 'lifespan': {'sec': -1, 'nsec': -1}} with self.assertRaises(ValueError): interpret_dict_as_qos_profile(qos_dict) - qos_dict = {'liveliness_lease_duration': {'sec': -1, 'nsec': -1}} + qos_dict = {'history': 'keep_all', 'liveliness_lease_duration': {'sec': -1, 'nsec': -1}} with self.assertRaises(ValueError): interpret_dict_as_qos_profile(qos_dict) From ceb344ea31f4fea7738b810f305268a72c53b150 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Mon, 3 Aug 2020 22:05:55 -0700 Subject: [PATCH 07/15] add validation to duration keys Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index ce7963eb8..c44a29bbc 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -57,6 +57,8 @@ def interpret_dict_as_qos_profile(qos_profile_dict: Dict) -> QoSProfile: new_profile_dict = {} for policy_key, policy_value in qos_profile_dict.items(): if policy_key in _DURATION_KEYS: + if policy_value < 0: + raise ValueError('`{}` may not be a negative value.'.format(policy_key)) new_profile_dict[policy_key] = dict_to_duration(policy_value) elif policy_key in _QOS_POLICY_FROM_SHORT_NAME: new_profile_dict[policy_key] = _QOS_POLICY_FROM_SHORT_NAME[policy_key](policy_value) From 12884e12c812d212ae03825c4c8f335e0dbff365 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Mon, 3 Aug 2020 22:14:25 -0700 Subject: [PATCH 08/15] modify duration validation Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index c44a29bbc..1160db169 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -44,6 +44,12 @@ def dict_to_duration(time_dict: Optional[Dict[str, int]]) -> Duration: """Convert a QoS duration profile from YAML into an rclpy Duration.""" if time_dict: try: + if(time_dict['sec'] < 0) + raise ValueError('`sec` may not be a negative value.') + ) + if(time_dict['nsec'] < 0) + raise ValueError('`nsec` may not be a negative value.') + ) return Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) except KeyError: raise ValueError( @@ -57,8 +63,6 @@ def interpret_dict_as_qos_profile(qos_profile_dict: Dict) -> QoSProfile: new_profile_dict = {} for policy_key, policy_value in qos_profile_dict.items(): if policy_key in _DURATION_KEYS: - if policy_value < 0: - raise ValueError('`{}` may not be a negative value.'.format(policy_key)) new_profile_dict[policy_key] = dict_to_duration(policy_value) elif policy_key in _QOS_POLICY_FROM_SHORT_NAME: new_profile_dict[policy_key] = _QOS_POLICY_FROM_SHORT_NAME[policy_key](policy_value) From 833019381f250a0ddf1f48e386f09632a85f25e2 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Mon, 3 Aug 2020 22:20:03 -0700 Subject: [PATCH 09/15] fix syntax Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index 1160db169..a9fbfe632 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -44,12 +44,10 @@ def dict_to_duration(time_dict: Optional[Dict[str, int]]) -> Duration: """Convert a QoS duration profile from YAML into an rclpy Duration.""" if time_dict: try: - if(time_dict['sec'] < 0) + if time_dict['sec'] < 0: raise ValueError('`sec` may not be a negative value.') - ) - if(time_dict['nsec'] < 0) + if time_dict['nsec'] < 0: raise ValueError('`nsec` may not be a negative value.') - ) return Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) except KeyError: raise ValueError( From 4e3631e6ac7dff93f79f2cee30b1fd46365cf794 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Tue, 4 Aug 2020 09:51:56 -0700 Subject: [PATCH 10/15] modify duration validation Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index a9fbfe632..8858c9921 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -44,10 +44,8 @@ def dict_to_duration(time_dict: Optional[Dict[str, int]]) -> Duration: """Convert a QoS duration profile from YAML into an rclpy Duration.""" if time_dict: try: - if time_dict['sec'] < 0: + if Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec'] < 0: raise ValueError('`sec` may not be a negative value.') - if time_dict['nsec'] < 0: - raise ValueError('`nsec` may not be a negative value.') return Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) except KeyError: raise ValueError( From c2773c6c37cb9a861073754a63adce9052f71cf6 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Tue, 4 Aug 2020 10:01:10 -0700 Subject: [PATCH 11/15] fix syntax Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index 8858c9921..b115ba40e 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -44,7 +44,7 @@ def dict_to_duration(time_dict: Optional[Dict[str, int]]) -> Duration: """Convert a QoS duration profile from YAML into an rclpy Duration.""" if time_dict: try: - if Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec'] < 0: + if Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < 0: raise ValueError('`sec` may not be a negative value.') return Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) except KeyError: From 6c25bcc1a300434789840e4494701720bb6be7f2 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Tue, 4 Aug 2020 10:24:50 -0700 Subject: [PATCH 12/15] modify comparison Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index b115ba40e..7e903c88f 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -44,8 +44,8 @@ def dict_to_duration(time_dict: Optional[Dict[str, int]]) -> Duration: """Convert a QoS duration profile from YAML into an rclpy Duration.""" if time_dict: try: - if Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < 0: - raise ValueError('`sec` may not be a negative value.') + if Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < Duration(second=0): + raise ValueError('Time duration may not be a negative value.') return Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) except KeyError: raise ValueError( From a15ed88723b466b97b6099912aa4765dc5cd7039 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Tue, 4 Aug 2020 10:29:42 -0700 Subject: [PATCH 13/15] fix syntax Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index 7e903c88f..2246f0713 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -44,7 +44,7 @@ def dict_to_duration(time_dict: Optional[Dict[str, int]]) -> Duration: """Convert a QoS duration profile from YAML into an rclpy Duration.""" if time_dict: try: - if Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < Duration(second=0): + if Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < Duration(seconds=0): raise ValueError('Time duration may not be a negative value.') return Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) except KeyError: From 9fe17cb12e9d12cabbf60a8f49fc599dab901617 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Tue, 4 Aug 2020 11:07:29 -0700 Subject: [PATCH 14/15] modify style Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index 2246f0713..d14f40b82 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -44,7 +44,8 @@ def dict_to_duration(time_dict: Optional[Dict[str, int]]) -> Duration: """Convert a QoS duration profile from YAML into an rclpy Duration.""" if time_dict: try: - if Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < Duration(seconds=0): + if (Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < + Duration(seconds=0)): raise ValueError('Time duration may not be a negative value.') return Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) except KeyError: From b648c7e84f6b84508adb55f7ea8d305c706422b9 Mon Sep 17 00:00:00 2001 From: Jesse Ikawa Date: Tue, 4 Aug 2020 11:11:40 -0700 Subject: [PATCH 15/15] modify style Signed-off-by: Jesse Ikawa --- ros2bag/ros2bag/api/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ros2bag/ros2bag/api/__init__.py b/ros2bag/ros2bag/api/__init__.py index d14f40b82..b96e81f8b 100644 --- a/ros2bag/ros2bag/api/__init__.py +++ b/ros2bag/ros2bag/api/__init__.py @@ -44,7 +44,7 @@ def dict_to_duration(time_dict: Optional[Dict[str, int]]) -> Duration: """Convert a QoS duration profile from YAML into an rclpy Duration.""" if time_dict: try: - if (Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < + if (Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec']) < Duration(seconds=0)): raise ValueError('Time duration may not be a negative value.') return Duration(seconds=time_dict['sec'], nanoseconds=time_dict['nsec'])