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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ParamVec and bridge from Ignition #261

Merged
merged 10 commits into from
Jun 22, 2022
Merged

Add ParamVec and bridge from Ignition #261

merged 10 commits into from
Jun 22, 2022

Conversation

mjcarroll
Copy link
Collaborator

@mjcarroll mjcarroll commented Jun 21, 2022

馃帀 New feature

Summary

  • Introduces ros_ign_interfaces::msg::ParamVec for storing a list of Parameters that are int, bool, double, or string.
  • Introduces bridge for ignition::msgs::param to ros_ign_interfaces::msg::ParamVec
  • Introduces bridge for ignition::msgs::param_v to ros_ign_interfaces::msg::ParamVec
  • Adds specific unit tests for the conversion process, as there are some caveats.

Caveats: Ignition allows for parameter messages to be infinitely nested. ROS stores it's parameters in a flat list. When there are nested messages in Ignition, they will be prefixed with child_<N>/ in their parameter name to represent where in the structure they came from. This process is not applied in reverse.

If the source is an ignition::msgs::param_v, it will be flattened to be stored in the ros structure. This flattening happens by prefixing each of the entries with param_<N>/

Test it

  • Unit tests covering the conversion have been added.
  • Integrations tests have been added.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll requested a review from iche033 June 21, 2022 19:55
@mjcarroll mjcarroll self-assigned this Jun 21, 2022
@osrf-triage osrf-triage added this to Inbox in Core development Jun 21, 2022
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@iche033
Copy link
Contributor

iche033 commented Jun 22, 2022

Changes look good to me. As for the todo on converting ROS ParamVec back to IGN Param_V, I think maybe we should do this soon, e.g. before the next Galactic release, so that people won't depend on the current behavior.

Core development automation moved this from Inbox to In review Jun 22, 2022
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll enabled auto-merge (squash) June 22, 2022 22:40
@mjcarroll mjcarroll merged commit 7dbbdf1 into galactic Jun 22, 2022
@mjcarroll mjcarroll deleted the bridge_param_v branch June 22, 2022 22:53
Core development automation moved this from In review to Done Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants