From fdad6e3b0e968fd53c39454d3300979016c05a1d Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Wed, 28 Oct 2020 13:51:26 +0000
Subject: [PATCH 01/29] Add QoSOverridingOptions

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/qos_overriding_options.py     | 170 ++++++++++++++++++++++
 rclpy/test/test_qos_overriding_options.py |  29 ++++
 2 files changed, 199 insertions(+)
 create mode 100644 rclpy/rclpy/qos_overriding_options.py
 create mode 100644 rclpy/test/test_qos_overriding_options.py

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
new file mode 100644
index 000000000..a2d96f64d
--- /dev/null
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -0,0 +1,170 @@
+# Copyright 2020 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from typing import Callable
+from typing import Iterable
+from typing import Optional
+from typing import Text
+from typing import Type
+from typing import Union
+
+from rcl_interfaces.msg import ParameterDescriptor
+
+import rclpy
+from rclpy.duration import Duration
+from rclpy.node import Node
+from rclpy.parameter import Parameter
+from rclpy.publisher import Publisher
+from rclpy.qos import QoSProfile
+from rclpy.qos import QoSPolicyKind
+from rclpy.subscription import Subscription
+
+
+class InvalidQosOverridesError(Exception):
+    pass
+
+
+class QoSOverridingOptions:
+    """
+    Options to customize qos parameter overrides.
+    """
+
+    def __init__(
+        self,
+        *,
+        policy_kinds: Iterable[QoSPolicyKind],
+        callback: Optional[Callable[[QoSProfile], bool]] = None,
+        id: Optional[Text] = None
+    ):
+        """
+        Construct a QoSOverridingOptions object.
+
+        :param policy_kinds: Qos kinds that will have a declared parameter.
+        :param callback: Callback that will be used to validate the qos profile
+            after the paramter overrides get applied.
+        :param id: Optional identifier, to disambiguate in the case that different qos
+            policies for the same topic are desired.
+        """
+        self._policy_kinds = policy_kinds
+        self._callback = callback
+        self._id = id
+
+    @property
+    def policy_kinds(self) -> Iterable[QoSPolicyKind]:
+        """Get qos policy kinds that will have a parameter override."""
+        return self._policy_kinds
+
+    @property
+    def callback(self) -> Optional[Callable[[QoSProfile], bool]]:
+        """Get the validation callback."""
+        return self._callback
+
+    @property
+    def id(self) -> Optional[Text]:
+        """Get the optional entity id."""
+        return self._id
+
+
+def _declare_qos_parameteres(
+    entity_type: Union[Type[Publisher], Type[Subscription]],
+    node: Node,
+    topic_name: Text,
+    qos: QoSProfile,
+    options: QoSOverridingOptions) -> QoSProfile:
+    """
+    Internal.
+    Declares qos parameters for a Publisher or a Subscription.
+
+    :param entity_type: Either `rclpy.node.Publisher` or `rclpy.node.Subscription`.
+    :param node: Node used to declare the parameters.
+    :param topic_name: Topic name of the entity being created.
+    :param qos: Default qos settings of the entity being created, that will be overriden
+        with the user provided qos parameter overrides.
+    :param options: Options that indicates which parameters are going to be declared.
+    """
+    if not issubclass(entity_type, (Publisher, Subscription)):
+        raise TypeError("Argument `entity_type` should be a subclass of Publisher or Subscription")
+    entity_type_str = 'publisher' if issubclass(entity_type, Publisher) else Subscription
+    id_suffix = '' if options.id is None else f'_{options.id}'
+    name = f'qos_overrides.{topic_name}.{entity_type_str}{id_suffix}.' '{}'
+    description = '{}' f' for {entity_type_str} `{topic_name}` with id `{id}`'
+    allowed_policies = _get_allowed_policies(entity_type)
+    for policy in options.policy_kinds:
+        if policy not in allowed_policies:
+            continue
+        policy_name = policy.name.lower()
+        descriptor = ParameterDescriptor()
+        descriptor.description = description.format(policy_name)
+        descriptor.read_only = True
+        param = node.declare_parameter(
+            name.format(policy_name),
+            _get_qos_policy_parameter(qos, policy),
+            descriptor)
+        _override_qos_policy_with_param(qos, policy, param)
+    if options.callback is not None and not options.callback(qos):
+        raise InvalidQosOverridesError(
+            description.format('Provided qos overrides') + ', are not valid')
+
+
+def _get_allowed_policies(entity_type: Union[Type[Publisher], Type[Subscription]]):
+    allowed_policies = list(QoSPolicyKind.__members__.values())
+    if issubclass(entity_type, Subscription):
+        allowed_policies.remove(QoSPolicyKind.LIFESPAN)
+    return allowed_policies
+
+
+def _get_qos_policy_parameter(qos: QoSProfile, policy: QoSPolicyKind) -> Parameter:
+    value = getattr(qos, policy.name.lower())
+    if policy in (
+        QoSPolicyKind.LIVELINESS, QoSPolicyKind.RELIABILITY,
+        QoSPolicyKind.HISTORY, QoSPolicyKind.DURABILITY
+    ):
+        value = value.name.lower()
+        if value == 'unknown':
+            raise ValueError('User provided qos profile is invalid')
+    if policy in (
+        QoSPolicyKind.LIFESPAN, QoSPolicyKind.DEADLINE, QoSPolicyKind.LIVELINESS_LEASE_DURATION
+    ):
+        value = value.nanoseconds()
+    return value
+
+
+def _override_qos_policy_with_param(qos: QoSProfile, policy: QoSPolicyKind, param: Parameter):
+    # policy_enum_map = {
+    #     QoSPolicyKind.LIVELINESS: rclpy.qos.LivelinessPolicy,
+    #     QoSPolicyKind.RELIABILITY: rclpy.qos.LivelinessPolicy,
+    #     QoSPolicyKind.LIVELINESS: rclpy.qos.LivelinessPolicy,
+    #     QoSPolicyKind.LIVELINESS: rclpy.qos.LivelinessPolicy,
+    # }
+    value = param.value
+    policy_name = policy.name.lower()
+    if policy in (
+        QoSPolicyKind.LIVELINESS, QoSPolicyKind.RELIABILITY,
+        QoSPolicyKind.HISTORY, QoSPolicyKind.DURABILITY
+    ):
+        def capitalize_first_letter(x):
+            return x[0].upper() + x[1:]
+        # e.g. `policy=QosPolicyKind.LIVELINESS` -> `policy_enum_class=rclpy.qos.LivelinessPolicy`
+        policy_enum_class = getattr(
+            rclpy.qos, f"{capitalize_first_letter(policy_name)}Policy")
+        try:
+            value = policy_enum_class[value.upper()]
+        except KeyError:
+            raise RuntimeError(
+                f'Unexpected qos override for policy `{policy.name.lower()}`: `{value}`')
+    if policy in (
+        QoSPolicyKind.LIFESPAN, QoSPolicyKind.DEADLINE, QoSPolicyKind.LIVELINESS_LEASE_DURATION
+    ):
+        value = Duration(nanoseconds=value)
+    setattr(qos, policy.name.lower(), value)
diff --git a/rclpy/test/test_qos_overriding_options.py b/rclpy/test/test_qos_overriding_options.py
new file mode 100644
index 000000000..cc149a534
--- /dev/null
+++ b/rclpy/test/test_qos_overriding_options.py
@@ -0,0 +1,29 @@
+# Copyright 2020 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import rclpy
+from rclpy.node import Node
+from rclpy.publisher import Publisher
+from rclpy.qos import QoSPolicyKind, QoSProfile
+from rclpy.qos_overriding_options import _declare_qos_parameteres, QoSOverridingOptions
+
+
+def test_declare_qos_parameters():
+    rclpy.init()
+    node = Node("my_node")
+    _declare_qos_parameteres(
+        Publisher, node, '/my_topic', QoSProfile(depth=10),
+        QoSOverridingOptions(policy_kinds=(QoSPolicyKind.RELIABILITY, QoSPolicyKind.HISTORY))
+    )
+    rclpy.shutdown()

From 3300d10eb6593f44d7cfde9193b2846c6f82d339 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Wed, 28 Oct 2020 13:53:36 -0300
Subject: [PATCH 02/29] Fix Node.get_parameters_by_prefix() type annotation

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/node.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py
index 2c28053d0..b010e2a1d 100644
--- a/rclpy/rclpy/node.py
+++ b/rclpy/rclpy/node.py
@@ -20,6 +20,7 @@
 from typing import Iterator
 from typing import List
 from typing import Optional
+from typing import Sequence
 from typing import Tuple
 from typing import TypeVar
 from typing import Union
@@ -530,7 +531,10 @@ def get_parameter_or(
 
         return self._parameters.get(name, alternative_value)
 
-    def get_parameters_by_prefix(self, prefix: str) -> List[Parameter]:
+    def get_parameters_by_prefix(self, prefix: str) -> Dict[str, Optional[Union[
+        bool, int, float, str, bytes,
+        Sequence[bool], Sequence[int], Sequence[float], Sequence[str]
+    ]]]:
         """
         Get parameters that have a given prefix in their names as a dictionary.
 
@@ -547,16 +551,14 @@ def get_parameters_by_prefix(self, prefix: str) -> List[Parameter]:
         :param prefix: The prefix of the parameters to get.
         :return: Dict of parameters with the given prefix.
         """
-        parameters_with_prefix = {}
         if prefix:
             prefix = prefix + PARAMETER_SEPARATOR_STRING
         prefix_len = len(prefix)
-        for parameter_name in self._parameters:
-            if parameter_name.startswith(prefix):
-                parameters_with_prefix.update(
-                    {parameter_name[prefix_len:]: self._parameters.get(parameter_name)})
-
-        return parameters_with_prefix
+        return {
+            param_name[prefix_len:]: param_value
+            for param_name, param_value in self._parameters.items()
+            if param_name.startswith(prefix)
+        }
 
     def set_parameters(self, parameter_list: List[Parameter]) -> List[SetParametersResult]:
         """

From 67ac124d5c5b50eb0ff8c308c0c893ca28c49740 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Wed, 28 Oct 2020 13:54:12 -0300
Subject: [PATCH 03/29] Add helper classmethod for using default policies

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/qos_overriding_options.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index a2d96f64d..3bc14fe95 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -75,6 +75,18 @@ def id(self) -> Optional[Text]:
         """Get the optional entity id."""
         return self._id
 
+    @classmethod
+    def with_default_policies(
+        cls, *,
+        callback: Optional[Callable[[QoSProfile], bool]] = None,
+        id: Optional[Text] = None
+    ) -> 'QoSOverridingOptions':
+        return cls(
+            policy_kinds=(QoSPolicyKind.HISTORY, QoSPolicyKind.DEPTH, QoSPolicyKind.RELIABILITY),
+            callback=callback,
+            id=id,
+        )
+
 
 def _declare_qos_parameteres(
     entity_type: Union[Type[Publisher], Type[Subscription]],

From 80781c97e1365aaa221b23e2d5fba75f7341a3f6 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Wed, 28 Oct 2020 13:54:25 -0300
Subject: [PATCH 04/29] Add test cases

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/test/test_qos_overriding_options.py | 100 +++++++++++++++++++++-
 1 file changed, 98 insertions(+), 2 deletions(-)

diff --git a/rclpy/test/test_qos_overriding_options.py b/rclpy/test/test_qos_overriding_options.py
index cc149a534..4557cb2de 100644
--- a/rclpy/test/test_qos_overriding_options.py
+++ b/rclpy/test/test_qos_overriding_options.py
@@ -12,11 +12,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import pytest
+
 import rclpy
 from rclpy.node import Node
+from rclpy.parameter import Parameter
 from rclpy.publisher import Publisher
 from rclpy.qos import QoSPolicyKind, QoSProfile
-from rclpy.qos_overriding_options import _declare_qos_parameteres, QoSOverridingOptions
+from rclpy.qos_overriding_options import _declare_qos_parameteres
+from rclpy.qos_overriding_options import InvalidQosOverridesError
+from rclpy.qos_overriding_options import QoSOverridingOptions
 
 
 def test_declare_qos_parameters():
@@ -24,6 +29,97 @@ def test_declare_qos_parameters():
     node = Node("my_node")
     _declare_qos_parameteres(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
-        QoSOverridingOptions(policy_kinds=(QoSPolicyKind.RELIABILITY, QoSPolicyKind.HISTORY))
+        QoSOverridingOptions.with_default_policies()
+    )
+    qos_overrides = node.get_parameters_by_prefix('qos_overrides')
+    assert len(qos_overrides) == 3
+    expected_params = (
+        ('/my_topic.publisher.depth', 10),
+        ('/my_topic.publisher.history', 'keep_last'),
+        ('/my_topic.publisher.reliability', 'reliable'),
+    )
+    for actual, expected in zip (
+        sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
+    ):
+        assert actual[0] == expected[0]  # same param name
+        assert actual[1].value == expected[1]  # same param value
+
+    rclpy.shutdown()
+
+
+def test_declare_qos_parameters_with_overrides():
+    rclpy.init()
+    node = Node("my_node", parameter_overrides=[
+        Parameter('qos_overrides./my_topic.publisher.depth', value=100),
+        Parameter('qos_overrides./my_topic.publisher.reliability', value='best_effort'),
+    ])
+    _declare_qos_parameteres(
+        Publisher, node, '/my_topic', QoSProfile(depth=10),
+        QoSOverridingOptions.with_default_policies()
     )
+    qos_overrides = node.get_parameters_by_prefix('qos_overrides')
+    assert len(qos_overrides) == 3
+    expected_params = (
+        ('/my_topic.publisher.depth', 100),
+        ('/my_topic.publisher.history', 'keep_last'),
+        ('/my_topic.publisher.reliability', 'best_effort'),
+    )
+    for actual, expected in zip (
+        sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
+    ):
+        assert actual[0] == expected[0]  # same param name
+        assert actual[1].value == expected[1]  # same param value
+
+    rclpy.shutdown()
+
+
+def test_declare_qos_parameters_with_happy_callback():
+    rclpy.init()
+    node = Node("my_node")
+    _declare_qos_parameteres(
+        Publisher, node, '/my_topic', QoSProfile(depth=10),
+        QoSOverridingOptions.with_default_policies(callback=lambda x: True)
+    )
+    qos_overrides = node.get_parameters_by_prefix('qos_overrides')
+    assert len(qos_overrides) == 3
+    expected_params = (
+        ('/my_topic.publisher.depth', 10),
+        ('/my_topic.publisher.history', 'keep_last'),
+        ('/my_topic.publisher.reliability', 'reliable'),
+    )
+    rclpy.shutdown()
+
+
+def test_declare_qos_parameters_with_unhappy_callback():
+    rclpy.init()
+    node = Node("my_node")
+
+    with pytest.raises(InvalidQosOverridesError):
+        _declare_qos_parameteres(
+            Publisher, node, '/my_topic', QoSProfile(depth=10),
+            QoSOverridingOptions.with_default_policies(callback=lambda x: False)
+        )
+    rclpy.shutdown()
+
+
+def test_declare_qos_parameters_with_id():
+    rclpy.init()
+    node = Node("my_node")
+    _declare_qos_parameteres(
+        Publisher, node, '/my_topic', QoSProfile(depth=10),
+        QoSOverridingOptions.with_default_policies(id='i_have_an_id')
+    )
+    qos_overrides = node.get_parameters_by_prefix('qos_overrides')
+    assert len(qos_overrides) == 3
+    expected_params = (
+        ('/my_topic.publisher_i_have_an_id.depth', 10),
+        ('/my_topic.publisher_i_have_an_id.history', 'keep_last'),
+        ('/my_topic.publisher_i_have_an_id.reliability', 'reliable'),
+    )
+    for actual, expected in zip (
+        sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
+    ):
+        assert actual[0] == expected[0]  # same param name
+        assert actual[1].value == expected[1]  # same param value
+
     rclpy.shutdown()

From 5fbc30c73d14c7581d8b5ac31cf7f5de80339489 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Wed, 28 Oct 2020 14:21:32 -0300
Subject: [PATCH 05/29] Please linters + improve test setup and teardown

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/qos_overriding_options.py     | 33 +++++++----------
 rclpy/test/test_qos_overriding_options.py | 44 +++++++++++------------
 2 files changed, 34 insertions(+), 43 deletions(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 3bc14fe95..d7e26c170 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -26,8 +26,8 @@
 from rclpy.node import Node
 from rclpy.parameter import Parameter
 from rclpy.publisher import Publisher
-from rclpy.qos import QoSProfile
 from rclpy.qos import QoSPolicyKind
+from rclpy.qos import QoSProfile
 from rclpy.subscription import Subscription
 
 
@@ -36,16 +36,14 @@ class InvalidQosOverridesError(Exception):
 
 
 class QoSOverridingOptions:
-    """
-    Options to customize qos parameter overrides.
-    """
+    """Options to customize qos parameter overrides."""
 
     def __init__(
         self,
         *,
         policy_kinds: Iterable[QoSPolicyKind],
         callback: Optional[Callable[[QoSProfile], bool]] = None,
-        id: Optional[Text] = None
+        entity_id: Optional[Text] = None
     ):
         """
         Construct a QoSOverridingOptions object.
@@ -53,12 +51,12 @@ def __init__(
         :param policy_kinds: Qos kinds that will have a declared parameter.
         :param callback: Callback that will be used to validate the qos profile
             after the paramter overrides get applied.
-        :param id: Optional identifier, to disambiguate in the case that different qos
+        :param entity_id: Optional identifier, to disambiguate in the case that different qos
             policies for the same topic are desired.
         """
         self._policy_kinds = policy_kinds
         self._callback = callback
-        self._id = id
+        self._entity_id = entity_id
 
     @property
     def policy_kinds(self) -> Iterable[QoSPolicyKind]:
@@ -71,20 +69,20 @@ def callback(self) -> Optional[Callable[[QoSProfile], bool]]:
         return self._callback
 
     @property
-    def id(self) -> Optional[Text]:
+    def entity_id(self) -> Optional[Text]:
         """Get the optional entity id."""
-        return self._id
+        return self._entity_id
 
     @classmethod
     def with_default_policies(
         cls, *,
         callback: Optional[Callable[[QoSProfile], bool]] = None,
-        id: Optional[Text] = None
+        entity_id: Optional[Text] = None
     ) -> 'QoSOverridingOptions':
         return cls(
             policy_kinds=(QoSPolicyKind.HISTORY, QoSPolicyKind.DEPTH, QoSPolicyKind.RELIABILITY),
             callback=callback,
-            id=id,
+            entity_id=entity_id,
         )
 
 
@@ -95,8 +93,7 @@ def _declare_qos_parameteres(
     qos: QoSProfile,
     options: QoSOverridingOptions) -> QoSProfile:
     """
-    Internal.
-    Declares qos parameters for a Publisher or a Subscription.
+    Internal, declare qos parameters for a Publisher or a Subscription.
 
     :param entity_type: Either `rclpy.node.Publisher` or `rclpy.node.Subscription`.
     :param node: Node used to declare the parameters.
@@ -108,9 +105,9 @@ def _declare_qos_parameteres(
     if not issubclass(entity_type, (Publisher, Subscription)):
         raise TypeError("Argument `entity_type` should be a subclass of Publisher or Subscription")
     entity_type_str = 'publisher' if issubclass(entity_type, Publisher) else Subscription
-    id_suffix = '' if options.id is None else f'_{options.id}'
+    id_suffix = '' if options.entity_id is None else f'_{options.entity_id}'
     name = f'qos_overrides.{topic_name}.{entity_type_str}{id_suffix}.' '{}'
-    description = '{}' f' for {entity_type_str} `{topic_name}` with id `{id}`'
+    description = '{}' f' for {entity_type_str} `{topic_name}` with id `{options.entity_id}`'
     allowed_policies = _get_allowed_policies(entity_type)
     for policy in options.policy_kinds:
         if policy not in allowed_policies:
@@ -153,12 +150,6 @@ def _get_qos_policy_parameter(qos: QoSProfile, policy: QoSPolicyKind) -> Paramet
 
 
 def _override_qos_policy_with_param(qos: QoSProfile, policy: QoSPolicyKind, param: Parameter):
-    # policy_enum_map = {
-    #     QoSPolicyKind.LIVELINESS: rclpy.qos.LivelinessPolicy,
-    #     QoSPolicyKind.RELIABILITY: rclpy.qos.LivelinessPolicy,
-    #     QoSPolicyKind.LIVELINESS: rclpy.qos.LivelinessPolicy,
-    #     QoSPolicyKind.LIVELINESS: rclpy.qos.LivelinessPolicy,
-    # }
     value = param.value
     policy_name = policy.name.lower()
     if policy in (
diff --git a/rclpy/test/test_qos_overriding_options.py b/rclpy/test/test_qos_overriding_options.py
index 4557cb2de..1b0b139f1 100644
--- a/rclpy/test/test_qos_overriding_options.py
+++ b/rclpy/test/test_qos_overriding_options.py
@@ -18,15 +18,22 @@
 from rclpy.node import Node
 from rclpy.parameter import Parameter
 from rclpy.publisher import Publisher
-from rclpy.qos import QoSPolicyKind, QoSProfile
+from rclpy.qos import QoSPolicyKind
+from rclpy.qos import QoSProfile
 from rclpy.qos_overriding_options import _declare_qos_parameteres
 from rclpy.qos_overriding_options import InvalidQosOverridesError
 from rclpy.qos_overriding_options import QoSOverridingOptions
 
 
-def test_declare_qos_parameters():
+@pytest.fixture(autouse=True)
+def init_shutdown():
     rclpy.init()
-    node = Node("my_node")
+    yield
+    rclpy.shutdown()
+
+
+def test_declare_qos_parameters():
+    node = Node('my_node')
     _declare_qos_parameteres(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
         QoSOverridingOptions.with_default_policies()
@@ -38,18 +45,15 @@ def test_declare_qos_parameters():
         ('/my_topic.publisher.history', 'keep_last'),
         ('/my_topic.publisher.reliability', 'reliable'),
     )
-    for actual, expected in zip (
+    for actual, expected in zip(
         sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
     ):
         assert actual[0] == expected[0]  # same param name
         assert actual[1].value == expected[1]  # same param value
 
-    rclpy.shutdown()
-
 
 def test_declare_qos_parameters_with_overrides():
-    rclpy.init()
-    node = Node("my_node", parameter_overrides=[
+    node = Node('my_node', parameter_overrides=[
         Parameter('qos_overrides./my_topic.publisher.depth', value=100),
         Parameter('qos_overrides./my_topic.publisher.reliability', value='best_effort'),
     ])
@@ -64,17 +68,14 @@ def test_declare_qos_parameters_with_overrides():
         ('/my_topic.publisher.history', 'keep_last'),
         ('/my_topic.publisher.reliability', 'best_effort'),
     )
-    for actual, expected in zip (
+    for actual, expected in zip(
         sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
     ):
         assert actual[0] == expected[0]  # same param name
         assert actual[1].value == expected[1]  # same param value
 
-    rclpy.shutdown()
-
 
 def test_declare_qos_parameters_with_happy_callback():
-    rclpy.init()
     node = Node("my_node")
     _declare_qos_parameteres(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
@@ -87,27 +88,28 @@ def test_declare_qos_parameters_with_happy_callback():
         ('/my_topic.publisher.history', 'keep_last'),
         ('/my_topic.publisher.reliability', 'reliable'),
     )
-    rclpy.shutdown()
+    for actual, expected in zip(
+        sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
+    ):
+        assert actual[0] == expected[0]  # same param name
+        assert actual[1].value == expected[1]  # same param value
 
 
 def test_declare_qos_parameters_with_unhappy_callback():
-    rclpy.init()
-    node = Node("my_node")
+    node = Node('my_node')
 
     with pytest.raises(InvalidQosOverridesError):
         _declare_qos_parameteres(
             Publisher, node, '/my_topic', QoSProfile(depth=10),
             QoSOverridingOptions.with_default_policies(callback=lambda x: False)
         )
-    rclpy.shutdown()
 
 
 def test_declare_qos_parameters_with_id():
-    rclpy.init()
-    node = Node("my_node")
+    node = Node('my_node')
     _declare_qos_parameteres(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
-        QoSOverridingOptions.with_default_policies(id='i_have_an_id')
+        QoSOverridingOptions.with_default_policies(entity_id='i_have_an_id')
     )
     qos_overrides = node.get_parameters_by_prefix('qos_overrides')
     assert len(qos_overrides) == 3
@@ -116,10 +118,8 @@ def test_declare_qos_parameters_with_id():
         ('/my_topic.publisher_i_have_an_id.history', 'keep_last'),
         ('/my_topic.publisher_i_have_an_id.reliability', 'reliable'),
     )
-    for actual, expected in zip (
+    for actual, expected in zip(
         sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
     ):
         assert actual[0] == expected[0]  # same param name
         assert actual[1].value == expected[1]  # same param value
-
-    rclpy.shutdown()

From 5e6235bf7a63b818af4ca4b1faf87c265408054e Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Wed, 28 Oct 2020 17:35:20 -0300
Subject: [PATCH 06/29] Declare qos overrides parameters in
 create_publisher/create_subscription + fixes

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/node.py                       | 32 +++++++++++++++++++++--
 rclpy/rclpy/qos_overriding_options.py     | 18 ++++++++-----
 rclpy/test/test_qos_overriding_options.py |  3 +--
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py
index b010e2a1d..3a551fe89 100644
--- a/rclpy/rclpy/node.py
+++ b/rclpy/rclpy/node.py
@@ -63,6 +63,8 @@
 from rclpy.qos import QoSProfile
 from rclpy.qos_event import PublisherEventCallbacks
 from rclpy.qos_event import SubscriptionEventCallbacks
+from rclpy.qos_overriding_options import _declare_qos_parameteres
+from rclpy.qos_overriding_options import QoSOverridingOptions
 from rclpy.service import Service
 from rclpy.subscription import Subscription
 from rclpy.time_source import TimeSource
@@ -1127,6 +1129,7 @@ def create_publisher(
         *,
         callback_group: Optional[CallbackGroup] = None,
         event_callbacks: Optional[PublisherEventCallbacks] = None,
+        qos_overriding_options: Optional[QoSOverridingOptions] = None,
     ) -> Publisher:
         """
         Create a new publisher.
@@ -1146,9 +1149,21 @@ def create_publisher(
 
         callback_group = callback_group or self.default_callback_group
 
+        failed = False
+        try:
+            final_topic = self.resolve_topic_or_service_name(topic)
+        except RuntimeError:
+            failed = True
+        if failed:
+            self._validate_topic_or_service_name(topic)
+
+        if qos_overriding_options is None:
+            qos_overriding_options = QoSOverridingOptions([])
+        _declare_qos_parameteres(
+            Publisher, self, final_topic, qos_profile, qos_overriding_options)
+
         # this line imports the typesupport for the message module if not already done
         check_for_type_support(msg_type)
-        failed = False
         try:
             with self.handle as node_capsule:
                 publisher_capsule = _rclpy.rclpy_create_publisher(
@@ -1184,6 +1199,7 @@ def create_subscription(
         *,
         callback_group: Optional[CallbackGroup] = None,
         event_callbacks: Optional[SubscriptionEventCallbacks] = None,
+        qos_overriding_options: Optional[QoSOverridingOptions] = None,
         raw: bool = False
     ) -> Subscription:
         """
@@ -1207,9 +1223,21 @@ def create_subscription(
 
         callback_group = callback_group or self.default_callback_group
 
+        failed = False
+        try:
+            final_topic = self.resolve_topic_or_service_name(topic)
+        except RuntimeError:
+            failed = True
+        if failed:
+            self._validate_topic_or_service_name(topic)
+
+        if qos_overriding_options is None:
+            qos_overriding_options = QoSOverridingOptions([])
+        _declare_qos_parameteres(
+            Subscription, self, final_topic, qos_profile, qos_overriding_options)
+
         # this line imports the typesupport for the message module if not already done
         check_for_type_support(msg_type)
-        failed = False
         try:
             with self.handle as capsule:
                 subscription_capsule = _rclpy.rclpy_create_subscription(
diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index d7e26c170..6f6adfe45 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -17,19 +17,22 @@
 from typing import Optional
 from typing import Text
 from typing import Type
+from typing import TYPE_CHECKING
 from typing import Union
 
 from rcl_interfaces.msg import ParameterDescriptor
 
 import rclpy
 from rclpy.duration import Duration
-from rclpy.node import Node
 from rclpy.parameter import Parameter
 from rclpy.publisher import Publisher
 from rclpy.qos import QoSPolicyKind
 from rclpy.qos import QoSProfile
 from rclpy.subscription import Subscription
 
+if TYPE_CHECKING:
+    from rclpy.node import Node
+
 
 class InvalidQosOverridesError(Exception):
     pass
@@ -40,8 +43,8 @@ class QoSOverridingOptions:
 
     def __init__(
         self,
-        *,
         policy_kinds: Iterable[QoSPolicyKind],
+        *,
         callback: Optional[Callable[[QoSProfile], bool]] = None,
         entity_id: Optional[Text] = None
     ):
@@ -88,12 +91,13 @@ def with_default_policies(
 
 def _declare_qos_parameteres(
     entity_type: Union[Type[Publisher], Type[Subscription]],
-    node: Node,
+    node: 'Node',
     topic_name: Text,
     qos: QoSProfile,
-    options: QoSOverridingOptions) -> QoSProfile:
+    options: QoSOverridingOptions
+) -> QoSProfile:
     """
-    Internal, declare qos parameters for a Publisher or a Subscription.
+    Declare qos parameters for a Publisher or a Subscription.
 
     :param entity_type: Either `rclpy.node.Publisher` or `rclpy.node.Subscription`.
     :param node: Node used to declare the parameters.
@@ -103,7 +107,7 @@ def _declare_qos_parameteres(
     :param options: Options that indicates which parameters are going to be declared.
     """
     if not issubclass(entity_type, (Publisher, Subscription)):
-        raise TypeError("Argument `entity_type` should be a subclass of Publisher or Subscription")
+        raise TypeError('Argument `entity_type` should be a subclass of Publisher or Subscription')
     entity_type_str = 'publisher' if issubclass(entity_type, Publisher) else Subscription
     id_suffix = '' if options.entity_id is None else f'_{options.entity_id}'
     name = f'qos_overrides.{topic_name}.{entity_type_str}{id_suffix}.' '{}'
@@ -160,7 +164,7 @@ def capitalize_first_letter(x):
             return x[0].upper() + x[1:]
         # e.g. `policy=QosPolicyKind.LIVELINESS` -> `policy_enum_class=rclpy.qos.LivelinessPolicy`
         policy_enum_class = getattr(
-            rclpy.qos, f"{capitalize_first_letter(policy_name)}Policy")
+            rclpy.qos, f'{capitalize_first_letter(policy_name)}Policy')
         try:
             value = policy_enum_class[value.upper()]
         except KeyError:
diff --git a/rclpy/test/test_qos_overriding_options.py b/rclpy/test/test_qos_overriding_options.py
index 1b0b139f1..cdd741363 100644
--- a/rclpy/test/test_qos_overriding_options.py
+++ b/rclpy/test/test_qos_overriding_options.py
@@ -18,7 +18,6 @@
 from rclpy.node import Node
 from rclpy.parameter import Parameter
 from rclpy.publisher import Publisher
-from rclpy.qos import QoSPolicyKind
 from rclpy.qos import QoSProfile
 from rclpy.qos_overriding_options import _declare_qos_parameteres
 from rclpy.qos_overriding_options import InvalidQosOverridesError
@@ -76,7 +75,7 @@ def test_declare_qos_parameters_with_overrides():
 
 
 def test_declare_qos_parameters_with_happy_callback():
-    node = Node("my_node")
+    node = Node('my_node')
     _declare_qos_parameteres(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
         QoSOverridingOptions.with_default_policies(callback=lambda x: True)

From b18d0ba5bcbb5cbb47a242eb0a0d571c2263fdd5 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:38:04 -0300
Subject: [PATCH 07/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 6f6adfe45..89ffe168a 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -39,7 +39,7 @@ class InvalidQosOverridesError(Exception):
 
 
 class QoSOverridingOptions:
-    """Options to customize qos parameter overrides."""
+    """Options to customize QoS parameter overrides."""
 
     def __init__(
         self,

From ef1e7bbb297c51b80bd9e081550795574900f281 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:38:19 -0300
Subject: [PATCH 08/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 89ffe168a..fc8ed0d5d 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -51,7 +51,7 @@ def __init__(
         """
         Construct a QoSOverridingOptions object.
 
-        :param policy_kinds: Qos kinds that will have a declared parameter.
+        :param policy_kinds: QoS kinds that will have a declared parameter.
         :param callback: Callback that will be used to validate the qos profile
             after the paramter overrides get applied.
         :param entity_id: Optional identifier, to disambiguate in the case that different qos

From fa434f9862f4ba0e0153fc98b4530c0a92ad61d1 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:38:31 -0300
Subject: [PATCH 09/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index fc8ed0d5d..653d84e47 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -52,7 +52,7 @@ def __init__(
         Construct a QoSOverridingOptions object.
 
         :param policy_kinds: QoS kinds that will have a declared parameter.
-        :param callback: Callback that will be used to validate the qos profile
+        :param callback: Callback that will be used to validate the QoS profile
             after the paramter overrides get applied.
         :param entity_id: Optional identifier, to disambiguate in the case that different qos
             policies for the same topic are desired.

From 2e5297f6b06974973c97a07ca7a85e01ea0cedf5 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:38:46 -0300
Subject: [PATCH 10/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 653d84e47..657ef0185 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -54,7 +54,7 @@ def __init__(
         :param policy_kinds: QoS kinds that will have a declared parameter.
         :param callback: Callback that will be used to validate the QoS profile
             after the paramter overrides get applied.
-        :param entity_id: Optional identifier, to disambiguate in the case that different qos
+        :param entity_id: Optional identifier, to disambiguate in the case that different QoS
             policies for the same topic are desired.
         """
         self._policy_kinds = policy_kinds

From 9b06ffa21574fa1b5d34ecaa1bb0d6b2a1aa4db8 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:39:02 -0300
Subject: [PATCH 11/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 657ef0185..36f1a850c 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -63,7 +63,7 @@ def __init__(
 
     @property
     def policy_kinds(self) -> Iterable[QoSPolicyKind]:
-        """Get qos policy kinds that will have a parameter override."""
+        """Get QoS policy kinds that will have a parameter override."""
         return self._policy_kinds
 
     @property

From b34a04c022ebd4983559d779469fe1efb2587149 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:40:09 -0300
Subject: [PATCH 12/29] style

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 36f1a850c..6d615a58d 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -73,7 +73,7 @@ def callback(self) -> Optional[Callable[[QoSProfile], bool]]:
 
     @property
     def entity_id(self) -> Optional[Text]:
-        """Get the optional entity id."""
+        """Get the optional entity ID."""
         return self._entity_id
 
     @classmethod

From 51bddf4c1a874a74be0ceb6174fc2b47103af503 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:40:19 -0300
Subject: [PATCH 13/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 6d615a58d..20b538113 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -127,7 +127,7 @@ def _declare_qos_parameteres(
         _override_qos_policy_with_param(qos, policy, param)
     if options.callback is not None and not options.callback(qos):
         raise InvalidQosOverridesError(
-            description.format('Provided qos overrides') + ', are not valid')
+            description.format('Provided QoS overrides') + ', are not valid')
 
 
 def _get_allowed_policies(entity_type: Union[Type[Publisher], Type[Subscription]]):

From fad4467e8577d81a32e9a2f7ab369c63c526a560 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:41:39 -0300
Subject: [PATCH 14/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 20b538113..22f82ad24 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -97,7 +97,7 @@ def _declare_qos_parameteres(
     options: QoSOverridingOptions
 ) -> QoSProfile:
     """
-    Declare qos parameters for a Publisher or a Subscription.
+    Declare QoS parameters for a Publisher or a Subscription.
 
     :param entity_type: Either `rclpy.node.Publisher` or `rclpy.node.Subscription`.
     :param node: Node used to declare the parameters.

From 85a8cb771be1b89857be457c50f62b8992efbd22 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:41:55 -0300
Subject: [PATCH 15/29] grammar

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 22f82ad24..88d0f4542 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -102,7 +102,7 @@ def _declare_qos_parameteres(
     :param entity_type: Either `rclpy.node.Publisher` or `rclpy.node.Subscription`.
     :param node: Node used to declare the parameters.
     :param topic_name: Topic name of the entity being created.
-    :param qos: Default qos settings of the entity being created, that will be overriden
+    :param qos: Default QoS settings of the entity being created, that will be overridden
         with the user provided qos parameter overrides.
     :param options: Options that indicates which parameters are going to be declared.
     """

From 589d2a28cc680466cb7e9f64aea2d65bcd612698 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:42:12 -0300
Subject: [PATCH 16/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 88d0f4542..32b14b99a 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -103,7 +103,7 @@ def _declare_qos_parameteres(
     :param node: Node used to declare the parameters.
     :param topic_name: Topic name of the entity being created.
     :param qos: Default QoS settings of the entity being created, that will be overridden
-        with the user provided qos parameter overrides.
+        with the user provided QoS parameter overrides.
     :param options: Options that indicates which parameters are going to be declared.
     """
     if not issubclass(entity_type, (Publisher, Subscription)):

From 0c5e7ce3c90256d52d4774248eea00bad3b24310 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:42:50 -0300
Subject: [PATCH 17/29] bug

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 32b14b99a..2c1a188ba 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -108,7 +108,7 @@ def _declare_qos_parameteres(
     """
     if not issubclass(entity_type, (Publisher, Subscription)):
         raise TypeError('Argument `entity_type` should be a subclass of Publisher or Subscription')
-    entity_type_str = 'publisher' if issubclass(entity_type, Publisher) else Subscription
+    entity_type_str = 'publisher' if issubclass(entity_type, Publisher) else 'subscription'
     id_suffix = '' if options.entity_id is None else f'_{options.entity_id}'
     name = f'qos_overrides.{topic_name}.{entity_type_str}{id_suffix}.' '{}'
     description = '{}' f' for {entity_type_str} `{topic_name}` with id `{options.entity_id}`'

From 17c834a3f468c8fd08ce6854db651490f432294a Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:43:12 -0300
Subject: [PATCH 18/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 2c1a188ba..2af457a36 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -145,7 +145,7 @@ def _get_qos_policy_parameter(qos: QoSProfile, policy: QoSPolicyKind) -> Paramet
     ):
         value = value.name.lower()
         if value == 'unknown':
-            raise ValueError('User provided qos profile is invalid')
+            raise ValueError('User provided QoS profile is invalid')
     if policy in (
         QoSPolicyKind.LIFESPAN, QoSPolicyKind.DEADLINE, QoSPolicyKind.LIVELINESS_LEASE_DURATION
     ):

From 50700ac84ae33790e57f0f78b11aa6617f2b3202 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:47:22 -0300
Subject: [PATCH 19/29] Fix _get_qos_policy_parameter type annotation

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 2af457a36..f87b5b112 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -137,7 +137,7 @@ def _get_allowed_policies(entity_type: Union[Type[Publisher], Type[Subscription]
     return allowed_policies
 
 
-def _get_qos_policy_parameter(qos: QoSProfile, policy: QoSPolicyKind) -> Parameter:
+def _get_qos_policy_parameter(qos: QoSProfile, policy: QoSPolicyKind) -> Union[str, int, bool]:
     value = getattr(qos, policy.name.lower())
     if policy in (
         QoSPolicyKind.LIVELINESS, QoSPolicyKind.RELIABILITY,

From 523dd593529781017bd550ee06ea6a1c56924f79 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:48:10 -0300
Subject: [PATCH 20/29] qos -> QoS

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
---
 rclpy/rclpy/qos_overriding_options.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index f87b5b112..583ab8312 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -169,7 +169,7 @@ def capitalize_first_letter(x):
             value = policy_enum_class[value.upper()]
         except KeyError:
             raise RuntimeError(
-                f'Unexpected qos override for policy `{policy.name.lower()}`: `{value}`')
+                f'Unexpected QoS override for policy `{policy.name.lower()}`: `{value}`')
     if policy in (
         QoSPolicyKind.LIFESPAN, QoSPolicyKind.DEADLINE, QoSPolicyKind.LIVELINESS_LEASE_DURATION
     ):

From 812d44f701fa5f71ba6dc2d02a4d2ea52fdf1049 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 14:50:13 -0300
Subject: [PATCH 21/29] _declare_qos_parameteres -> _declare_qos_parameters

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/node.py                       |  6 +++---
 rclpy/rclpy/qos_overriding_options.py     |  2 +-
 rclpy/test/test_qos_overriding_options.py | 12 ++++++------
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py
index 3a551fe89..0bf725dd6 100644
--- a/rclpy/rclpy/node.py
+++ b/rclpy/rclpy/node.py
@@ -63,7 +63,7 @@
 from rclpy.qos import QoSProfile
 from rclpy.qos_event import PublisherEventCallbacks
 from rclpy.qos_event import SubscriptionEventCallbacks
-from rclpy.qos_overriding_options import _declare_qos_parameteres
+from rclpy.qos_overriding_options import _declare_qos_parameters
 from rclpy.qos_overriding_options import QoSOverridingOptions
 from rclpy.service import Service
 from rclpy.subscription import Subscription
@@ -1159,7 +1159,7 @@ def create_publisher(
 
         if qos_overriding_options is None:
             qos_overriding_options = QoSOverridingOptions([])
-        _declare_qos_parameteres(
+        _declare_qos_parameters(
             Publisher, self, final_topic, qos_profile, qos_overriding_options)
 
         # this line imports the typesupport for the message module if not already done
@@ -1233,7 +1233,7 @@ def create_subscription(
 
         if qos_overriding_options is None:
             qos_overriding_options = QoSOverridingOptions([])
-        _declare_qos_parameteres(
+        _declare_qos_parameters(
             Subscription, self, final_topic, qos_profile, qos_overriding_options)
 
         # this line imports the typesupport for the message module if not already done
diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 583ab8312..d824b21a8 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -89,7 +89,7 @@ def with_default_policies(
         )
 
 
-def _declare_qos_parameteres(
+def _declare_qos_parameters(
     entity_type: Union[Type[Publisher], Type[Subscription]],
     node: 'Node',
     topic_name: Text,
diff --git a/rclpy/test/test_qos_overriding_options.py b/rclpy/test/test_qos_overriding_options.py
index cdd741363..c0c89cee6 100644
--- a/rclpy/test/test_qos_overriding_options.py
+++ b/rclpy/test/test_qos_overriding_options.py
@@ -19,7 +19,7 @@
 from rclpy.parameter import Parameter
 from rclpy.publisher import Publisher
 from rclpy.qos import QoSProfile
-from rclpy.qos_overriding_options import _declare_qos_parameteres
+from rclpy.qos_overriding_options import _declare_qos_parameters
 from rclpy.qos_overriding_options import InvalidQosOverridesError
 from rclpy.qos_overriding_options import QoSOverridingOptions
 
@@ -33,7 +33,7 @@ def init_shutdown():
 
 def test_declare_qos_parameters():
     node = Node('my_node')
-    _declare_qos_parameteres(
+    _declare_qos_parameters(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
         QoSOverridingOptions.with_default_policies()
     )
@@ -56,7 +56,7 @@ def test_declare_qos_parameters_with_overrides():
         Parameter('qos_overrides./my_topic.publisher.depth', value=100),
         Parameter('qos_overrides./my_topic.publisher.reliability', value='best_effort'),
     ])
-    _declare_qos_parameteres(
+    _declare_qos_parameters(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
         QoSOverridingOptions.with_default_policies()
     )
@@ -76,7 +76,7 @@ def test_declare_qos_parameters_with_overrides():
 
 def test_declare_qos_parameters_with_happy_callback():
     node = Node('my_node')
-    _declare_qos_parameteres(
+    _declare_qos_parameters(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
         QoSOverridingOptions.with_default_policies(callback=lambda x: True)
     )
@@ -98,7 +98,7 @@ def test_declare_qos_parameters_with_unhappy_callback():
     node = Node('my_node')
 
     with pytest.raises(InvalidQosOverridesError):
-        _declare_qos_parameteres(
+        _declare_qos_parameters(
             Publisher, node, '/my_topic', QoSProfile(depth=10),
             QoSOverridingOptions.with_default_policies(callback=lambda x: False)
         )
@@ -106,7 +106,7 @@ def test_declare_qos_parameters_with_unhappy_callback():
 
 def test_declare_qos_parameters_with_id():
     node = Node('my_node')
-    _declare_qos_parameteres(
+    _declare_qos_parameters(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
         QoSOverridingOptions.with_default_policies(entity_id='i_have_an_id')
     )

From c4f109b9eeec7c4f951a1dee184352541daa6a35 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Fri, 30 Oct 2020 17:03:18 -0300
Subject: [PATCH 22/29] Fix after rebasing

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/node.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py
index 0bf725dd6..1fd8863fc 100644
--- a/rclpy/rclpy/node.py
+++ b/rclpy/rclpy/node.py
@@ -1151,7 +1151,7 @@ def create_publisher(
 
         failed = False
         try:
-            final_topic = self.resolve_topic_or_service_name(topic)
+            final_topic = self.resolve_topic_name(topic)
         except RuntimeError:
             failed = True
         if failed:
@@ -1225,7 +1225,7 @@ def create_subscription(
 
         failed = False
         try:
-            final_topic = self.resolve_topic_or_service_name(topic)
+            final_topic = self.resolve_topic_name(topic)
         except RuntimeError:
             failed = True
         if failed:

From 751a6010aafd3613dac856bee18c7263414c9c19 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Mon, 2 Nov 2020 17:48:56 -0300
Subject: [PATCH 23/29] Handle exception gracefully

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/node.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py
index 1fd8863fc..462d7af3b 100644
--- a/rclpy/rclpy/node.py
+++ b/rclpy/rclpy/node.py
@@ -1153,9 +1153,10 @@ def create_publisher(
         try:
             final_topic = self.resolve_topic_name(topic)
         except RuntimeError:
-            failed = True
-        if failed:
+            # if it's name validation error, raise a more appropriate exception.
             self._validate_topic_or_service_name(topic)
+            # else reraise the previous exception
+            raise
 
         if qos_overriding_options is None:
             qos_overriding_options = QoSOverridingOptions([])
@@ -1163,6 +1164,7 @@ def create_publisher(
             Publisher, self, final_topic, qos_profile, qos_overriding_options)
 
         # this line imports the typesupport for the message module if not already done
+        failed = False
         check_for_type_support(msg_type)
         try:
             with self.handle as node_capsule:
@@ -1223,13 +1225,13 @@ def create_subscription(
 
         callback_group = callback_group or self.default_callback_group
 
-        failed = False
         try:
             final_topic = self.resolve_topic_name(topic)
         except RuntimeError:
-            failed = True
-        if failed:
+            # if it's name validation error, raise a more appropriate exception.
             self._validate_topic_or_service_name(topic)
+            # else reraise the previous exception
+            raise
 
         if qos_overriding_options is None:
             qos_overriding_options = QoSOverridingOptions([])
@@ -1237,6 +1239,7 @@ def create_subscription(
             Subscription, self, final_topic, qos_profile, qos_overriding_options)
 
         # this line imports the typesupport for the message module if not already done
+        failed = False
         check_for_type_support(msg_type)
         try:
             with self.handle as capsule:

From 59951a392dc23461d3c0504a7179a3bfe1ddd88c Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Tue, 3 Nov 2020 10:36:29 -0300
Subject: [PATCH 24/29] Suppress exception context

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/node.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py
index 462d7af3b..206f07696 100644
--- a/rclpy/rclpy/node.py
+++ b/rclpy/rclpy/node.py
@@ -43,6 +43,7 @@
 from rclpy.clock import ROSClock
 from rclpy.constants import S_TO_NS
 from rclpy.context import Context
+from rclpy.exceptions import InvalidTopicNameException
 from rclpy.exceptions import InvalidParameterValueException
 from rclpy.exceptions import NotInitializedException
 from rclpy.exceptions import ParameterAlreadyDeclaredException
@@ -1154,7 +1155,10 @@ def create_publisher(
             final_topic = self.resolve_topic_name(topic)
         except RuntimeError:
             # if it's name validation error, raise a more appropriate exception.
-            self._validate_topic_or_service_name(topic)
+            try:
+                self._validate_topic_or_service_name(topic)
+            except InvalidTopicNameException as ex:
+                raise ex from None
             # else reraise the previous exception
             raise
 
@@ -1229,7 +1233,10 @@ def create_subscription(
             final_topic = self.resolve_topic_name(topic)
         except RuntimeError:
             # if it's name validation error, raise a more appropriate exception.
-            self._validate_topic_or_service_name(topic)
+            try:
+                self._validate_topic_or_service_name(topic)
+            except InvalidTopicNameException as ex:
+                raise ex from None
             # else reraise the previous exception
             raise
 

From 9fce94be574f11a658731c2d4ad264f6799f522f Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Tue, 3 Nov 2020 10:43:52 -0300
Subject: [PATCH 25/29] flake8

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/node.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py
index 206f07696..9535d9e71 100644
--- a/rclpy/rclpy/node.py
+++ b/rclpy/rclpy/node.py
@@ -43,8 +43,8 @@
 from rclpy.clock import ROSClock
 from rclpy.constants import S_TO_NS
 from rclpy.context import Context
-from rclpy.exceptions import InvalidTopicNameException
 from rclpy.exceptions import InvalidParameterValueException
+from rclpy.exceptions import InvalidTopicNameException
 from rclpy.exceptions import NotInitializedException
 from rclpy.exceptions import ParameterAlreadyDeclaredException
 from rclpy.exceptions import ParameterImmutableException

From f8ba331a3ad0d09a76b2d57bf17534428ce20d04 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Thu, 5 Nov 2020 17:20:40 -0300
Subject: [PATCH 26/29] Add test to CMakeLists

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/CMakeLists.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rclpy/CMakeLists.txt b/rclpy/CMakeLists.txt
index 1c7d396b7..c9339adb7 100644
--- a/rclpy/CMakeLists.txt
+++ b/rclpy/CMakeLists.txt
@@ -233,6 +233,7 @@ if(BUILD_TESTING)
       test/test_publisher.py
       test/test_qos.py
       test/test_qos_event.py
+      test/test_qos_overriding_options.py
       test/test_rate.py
       test/test_serialization.py
       test/test_subscription.py

From 0446d248c2ce99736fef0b2dd44f40941dbec3f9 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Thu, 5 Nov 2020 17:20:53 -0300
Subject: [PATCH 27/29] Use declare_or_get logic

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/qos_overriding_options.py | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index d824b21a8..044357483 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -24,6 +24,7 @@
 
 import rclpy
 from rclpy.duration import Duration
+from rclpy.exceptions import ParameterAlreadyDeclaredException
 from rclpy.parameter import Parameter
 from rclpy.publisher import Publisher
 from rclpy.qos import QoSPolicyKind
@@ -120,10 +121,13 @@ def _declare_qos_parameters(
         descriptor = ParameterDescriptor()
         descriptor.description = description.format(policy_name)
         descriptor.read_only = True
-        param = node.declare_parameter(
-            name.format(policy_name),
-            _get_qos_policy_parameter(qos, policy),
-            descriptor)
+        try:
+            param = node.declare_parameter(
+                name.format(policy_name),
+                _get_qos_policy_parameter(qos, policy),
+                descriptor)
+        except ParameterAlreadyDeclaredException:
+            param = node.get_parameter(name.format(policy_name))
         _override_qos_policy_with_param(qos, policy, param)
     if options.callback is not None and not options.callback(qos):
         raise InvalidQosOverridesError(

From a9fc10f019398ae47a75bf8ac3416f8d29467c35 Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Thu, 5 Nov 2020 18:05:10 -0300
Subject: [PATCH 28/29] Improve testing

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/test/test_qos_overriding_options.py | 33 ++++++++++++-----------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/rclpy/test/test_qos_overriding_options.py b/rclpy/test/test_qos_overriding_options.py
index c0c89cee6..08e119359 100644
--- a/rclpy/test/test_qos_overriding_options.py
+++ b/rclpy/test/test_qos_overriding_options.py
@@ -56,22 +56,23 @@ def test_declare_qos_parameters_with_overrides():
         Parameter('qos_overrides./my_topic.publisher.depth', value=100),
         Parameter('qos_overrides./my_topic.publisher.reliability', value='best_effort'),
     ])
-    _declare_qos_parameters(
-        Publisher, node, '/my_topic', QoSProfile(depth=10),
-        QoSOverridingOptions.with_default_policies()
-    )
-    qos_overrides = node.get_parameters_by_prefix('qos_overrides')
-    assert len(qos_overrides) == 3
-    expected_params = (
-        ('/my_topic.publisher.depth', 100),
-        ('/my_topic.publisher.history', 'keep_last'),
-        ('/my_topic.publisher.reliability', 'best_effort'),
-    )
-    for actual, expected in zip(
-        sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
-    ):
-        assert actual[0] == expected[0]  # same param name
-        assert actual[1].value == expected[1]  # same param value
+    for i in range(2):  # try twice, the second time the parameters will be get and not declared
+        _declare_qos_parameters(
+            Publisher, node, '/my_topic', QoSProfile(depth=10),
+            QoSOverridingOptions.with_default_policies()
+        )
+        qos_overrides = node.get_parameters_by_prefix('qos_overrides')
+        assert len(qos_overrides) == 3
+        expected_params = (
+            ('/my_topic.publisher.depth', 100),
+            ('/my_topic.publisher.history', 'keep_last'),
+            ('/my_topic.publisher.reliability', 'best_effort'),
+        )
+        for actual, expected in zip(
+            sorted(qos_overrides.items(), key=lambda x: x[0]), expected_params
+        ):
+            assert actual[0] == expected[0]  # same param name
+            assert actual[1].value == expected[1]  # same param value
 
 
 def test_declare_qos_parameters_with_happy_callback():

From 6b6f2c2a2736a2e45419e51fd51004ece93bb2db Mon Sep 17 00:00:00 2001
From: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Date: Mon, 9 Nov 2020 16:00:23 -0300
Subject: [PATCH 29/29] Add error message in validation callback result

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
---
 rclpy/rclpy/qos_overriding_options.py     | 21 +++++++++++++++------
 rclpy/test/test_qos_overriding_options.py | 19 ++++++++++++++++---
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/rclpy/rclpy/qos_overriding_options.py b/rclpy/rclpy/qos_overriding_options.py
index 044357483..1db48cfd9 100644
--- a/rclpy/rclpy/qos_overriding_options.py
+++ b/rclpy/rclpy/qos_overriding_options.py
@@ -21,6 +21,7 @@
 from typing import Union
 
 from rcl_interfaces.msg import ParameterDescriptor
+from rcl_interfaces.msg import SetParametersResult
 
 import rclpy
 from rclpy.duration import Duration
@@ -39,6 +40,12 @@ class InvalidQosOverridesError(Exception):
     pass
 
 
+# Return type of qos validation callbacks
+QosCallbackResult = SetParametersResult
+# Qos callback type annotation
+QosCallbackType = Callable[[QoSProfile], QosCallbackResult]
+
+
 class QoSOverridingOptions:
     """Options to customize QoS parameter overrides."""
 
@@ -46,7 +53,7 @@ def __init__(
         self,
         policy_kinds: Iterable[QoSPolicyKind],
         *,
-        callback: Optional[Callable[[QoSProfile], bool]] = None,
+        callback: Optional[QosCallbackType] = None,
         entity_id: Optional[Text] = None
     ):
         """
@@ -68,7 +75,7 @@ def policy_kinds(self) -> Iterable[QoSPolicyKind]:
         return self._policy_kinds
 
     @property
-    def callback(self) -> Optional[Callable[[QoSProfile], bool]]:
+    def callback(self) -> Optional[QosCallbackType]:
         """Get the validation callback."""
         return self._callback
 
@@ -80,7 +87,7 @@ def entity_id(self) -> Optional[Text]:
     @classmethod
     def with_default_policies(
         cls, *,
-        callback: Optional[Callable[[QoSProfile], bool]] = None,
+        callback: Optional[QosCallbackType] = None,
         entity_id: Optional[Text] = None
     ) -> 'QoSOverridingOptions':
         return cls(
@@ -129,9 +136,11 @@ def _declare_qos_parameters(
         except ParameterAlreadyDeclaredException:
             param = node.get_parameter(name.format(policy_name))
         _override_qos_policy_with_param(qos, policy, param)
-    if options.callback is not None and not options.callback(qos):
-        raise InvalidQosOverridesError(
-            description.format('Provided QoS overrides') + ', are not valid')
+    if options.callback is not None:
+        result = options.callback(qos)
+        if not result.successful:
+            raise InvalidQosOverridesError(
+                f"{description.format('Provided QoS overrides')}, are not valid: {result.reason}")
 
 
 def _get_allowed_policies(entity_type: Union[Type[Publisher], Type[Subscription]]):
diff --git a/rclpy/test/test_qos_overriding_options.py b/rclpy/test/test_qos_overriding_options.py
index 08e119359..933dc6be7 100644
--- a/rclpy/test/test_qos_overriding_options.py
+++ b/rclpy/test/test_qos_overriding_options.py
@@ -21,6 +21,7 @@
 from rclpy.qos import QoSProfile
 from rclpy.qos_overriding_options import _declare_qos_parameters
 from rclpy.qos_overriding_options import InvalidQosOverridesError
+from rclpy.qos_overriding_options import QosCallbackResult
 from rclpy.qos_overriding_options import QoSOverridingOptions
 
 
@@ -76,10 +77,15 @@ def test_declare_qos_parameters_with_overrides():
 
 
 def test_declare_qos_parameters_with_happy_callback():
+    def qos_validation_callback(qos):
+        result = QosCallbackResult()
+        result.successful = True
+        return result
+
     node = Node('my_node')
     _declare_qos_parameters(
         Publisher, node, '/my_topic', QoSProfile(depth=10),
-        QoSOverridingOptions.with_default_policies(callback=lambda x: True)
+        QoSOverridingOptions.with_default_policies(callback=qos_validation_callback)
     )
     qos_overrides = node.get_parameters_by_prefix('qos_overrides')
     assert len(qos_overrides) == 3
@@ -96,13 +102,20 @@ def test_declare_qos_parameters_with_happy_callback():
 
 
 def test_declare_qos_parameters_with_unhappy_callback():
+    def qos_validation_callback(qos):
+        result = QosCallbackResult()
+        result.successful = False
+        result.reason = 'my_custom_error_message'
+        return result
+
     node = Node('my_node')
 
-    with pytest.raises(InvalidQosOverridesError):
+    with pytest.raises(InvalidQosOverridesError) as err:
         _declare_qos_parameters(
             Publisher, node, '/my_topic', QoSProfile(depth=10),
-            QoSOverridingOptions.with_default_policies(callback=lambda x: False)
+            QoSOverridingOptions.with_default_policies(callback=qos_validation_callback)
         )
+    assert 'my_custom_error_message' in str(err.value)
 
 
 def test_declare_qos_parameters_with_id():