Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix param base name which starts with '.' #87

Closed
wants to merge 2 commits into from

Conversation

wep21
Copy link

@wep21 wep21 commented Feb 21, 2022

param base name which starts with '.' causes the problem for parameter as below.

❯ ros2 param dump /sensing/camera/camera1/rectify_node
/sensing/camera/camera1/rectify_node:
  ros__parameters:
    ? ''
    : image_rect:
        format: jpeg
        jpeg_quality: 95
        png_level: 3
    interpolation: 1
    qos_overrides:
      /parameter_events:
        publisher:
          depth: 1000
          durability: volatile
          history: keep_last
          reliability: reliable
    queue_size: 5
    use_sim_time: false
    use_system_default_qos: false

@wep21
Copy link
Author

wep21 commented Feb 21, 2022

FYI @mjcarroll @gbiggs

@gbiggs
Copy link

gbiggs commented Feb 21, 2022

Please post a clearer example and explanation of what the error is?

Co-authored-by: Geoffrey Biggs <[email protected]>
@wep21
Copy link
Author

wep21 commented Feb 22, 2022

@gbiggs I found out the output of ros2 param dump seems wrong, but I cannot reproduce it again, so I close this PR once.

  • wrong
  ros__parameters:
    ? ''
    : image_rect:
        format: jpeg
        jpeg_quality: 95
        png_level: 3
  • correct
/sensing/camera/camera0/rectify_node:
  ros__parameters:
    image_rect:
      format: jpeg
      jpeg_quality: 80
      png_level: 3

If I come across this problem again, I will reopen this PR.

@wep21 wep21 closed this Feb 22, 2022
@wep21 wep21 deleted the foxy-devel branch February 22, 2022 02:38
@wep21
Copy link
Author

wep21 commented Feb 22, 2022

@gbiggs I confirmed that a parameter starts with '.' causes the problem. I have tested with parameter blackboard.

  • set .aaa parameter
    ros2 param set /parameter_blackboard .aaa 200
  • .aaa parameter dumped on the screen seems wrong. Also I can't get parameter from .aaa.
❯ ros2 param dump --print /parameter_blackboard
/parameter_blackboard:
  ros__parameters:
    ? ''
    : aaa: 200
    qos_overrides:
      /parameter_events:
        publisher:
          depth: 1000
          durability: volatile
          history: keep_last
          reliability: reliable
    use_sim_time: false

Do you think this PR should be reopened?

@gbiggs
Copy link

gbiggs commented Feb 22, 2022

So it turns out that dots in parameter names are used for namespace separation in a number of places. This means that we can't consider a dot to be a valid character in a parameter name.

There is an old proposal to change this, but it didn't get implemented. I suggest you resurrect that if you want move forward with this idea.
ros2/design#241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants