Cannot load parameters declared with namespace from yaml or command line

[guru-florida]

When using the following syntax to create parameters in a namespace, the parameters will not be loaded with values from either the command line or config yaml params file. Specifically the namespace= argument seems to be the issue.

        self.declare_parameters(
            namespace="ns1",
            parameters=[("bar", "default_bar1")],
        )

If we instead use this syntax, simply adding the ns1 prefix to the parameter itself then both params file and command line works. According to documentation this should be equivalent to the first mode.

        self.declare_parameters(
            namespace="",
            parameters=[("ns1.bar", "default_bar1")],
        )

Required Info:

• Operating System:
  • Ubuntu 20.04 on x86_64 and arm_64 (Rpi4)
• Installation type:
  • binaries from apt repo
• Version or commit hash:
  • latest galactic on amd64 (ros-galactic-rclpy/now 1.9.0-1focal.20211222.234056)
  • latest galactic on arm64 RPi4 (ros-galactic-rclpy/focal,now 1.9.0-1focal.20220203.003739)
• DDS implementation:
  • Cyclone
• Client library (if applicable):
  • rclpy

Steps to reproduce issue

Retrieve the sample test from this github gist:
https://gist.github.com/guru-florida/37c9d89adb082a2329dec709e58a045d

Expected behavior

Both declaration modes should have equivalent behavior. Both modes do in fact result in the nodes _parameters
collection having the expected namespace prefix.
This example program should set params {foo, ns1.bar, ns2.bar} to {p1, p2, p3} respectively.

Actual behavior

Parameters using the namespace argument of declare_parameters() cannot be loaded from sysargs (using -p) or
from yaml config files.
This example program outputs params {foo, ns1.bar, ns2.bar} as {p1, default_bar1, p3} respectively.

Additional information

Inspecting at runtime, it seems that both modes add the parameter to the node._parameters dict with the proper prefix (so same behavior) but when the declare_parameters() eventually calls set_parameters() it is unable to find the ns1.bar parameter even though it is supplied. Furthermore, if you supply the parameter on the command line or in the params file without the namespace then it will find and use this parameter and place into the namespaced parameter. (I.e. In the example command line, change "-p ns1.bar:=p2" to "-p bar:=wrong" and it "ns1.bar" will be set to "wrong" on program output)

Also, there is a ROS Answers question alluding to the same issue:
https://answers.ros.org/question/395375/ros2-how-to-specify-parameter-with-namespace-in-python-launch-file/

I am working on diagnosing further but figured I had enough to trigger an issue.

[guru-florida]

Found the issue. In declare_parameters(...) it gets down to Line 468 where it attempts to load the user-supplied value from node._parameter_overrides[name] but at this point the namespace hasn't been prefixed to the name. This prefixing occurs on the code immediately following on line 471.

    def declare_parameters(...):
            # .... snip ...

            # Get value from parameter overrides, of from tuple if it doesn't exist.
            if not ignore_override and name in self._parameter_overrides:
                # (oops) name has not been prefixed with namespace yet
                value = self._parameter_overrides[name].value

            # I think this line should be moved up ^^^
            if namespace:
                name = f'{namespace}.{name}'

Can we move the prefixing code higher? Why not put it at the top where name variable is first set to be sure it behaves the same as the other mode?
(edit: to test, I moved the prefixing line to just after line 421 and it now works as expected)

If someone can review this I can submit a PR. @ivanpauno @jacobperron @clalancette ?

[jacobperron]

The change you suggest sounds reasonable to me. I'm guessing it is simply a bug and not intended logic.
I'm happy to review a PR; please target the default branch and we can later consider backporting to Galactic and Foxy. 🙂

[devrite]

We did not find the issue yesterday and manually found the same issue and resolution. I provided a prover and will also provide a PR for that case later.