Skip to content

Fix nested mapped parameters map#306

Merged
christophfroehlich merged 13 commits intoPickNikRobotics:mainfrom
NickLaurenson-Visionick:fix/278
Mar 14, 2026
Merged

Fix nested mapped parameters map#306
christophfroehlich merged 13 commits intoPickNikRobotics:mainfrom
NickLaurenson-Visionick:fix/278

Conversation

@NickLaurenson-Visionick
Copy link
Contributor

@NickLaurenson-Visionick NickLaurenson-Visionick commented Jan 16, 2026

Hello, this PR aim to fix #278.

As mention in the issue, the root caused came from get_dynamic_mapped_parameter_names, not return the full nested path:
if the the field to be mapped was .nested_field.final_field, the function return final_field causing error in the generated code.

Regarding the test, I wrote some basic one, let me know if need to do something else

@NickLaurenson-Visionick NickLaurenson-Visionick force-pushed the fix/278 branch 3 times, most recently from 1664af6 to 7fef7cf Compare January 16, 2026 10:28
@NickLaurenson-Visionick NickLaurenson-Visionick changed the title Add tests for nested mapped parameters Fix nested mapped parameters map Jan 19, 2026
Copy link
Collaborator

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please have a look at the failing CI jobs?

@NickLaurenson-Visionick
Copy link
Contributor Author

Yes sure I will take a look in the next day

Nick Laurenson added 3 commits February 23, 2026 15:06
Previously, the generator assumed that mapped parameter keys (the arrays
used for __map_ segments) were either at the root level or provided as
bare names. This caused the generator to fail or produce incorrect paths
 when dynamic parameters were nested inside deep struct hierarchies where
the "key" array was defined in a parent scope.

fix PickNikRobotics#278
- group test that test setup_up function via parametrization
- add some config file file dor nested parameter map
- add a test about get_dynamic_mapped_parameter
@NickLaurenson-Visionick
Copy link
Contributor Author

I update the branch, and fix the compilation locally (via ci_industrial tool).

@christophfroehlich can you see it it fix the ci too?

@NickLaurenson-Visionick
Copy link
Contributor Author

Fix the style issue, everything should be fine now.

@NickLaurenson-Visionick
Copy link
Contributor Author

@christophfroehlich would it be possible to have a review?

Copy link
Collaborator

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is happy now, the changes look good as far as I can tell.

Can you please also update https://github.com/PickNikRobotics/generate_parameter_library?tab=readme-ov-file#mapped-parameters with the possible variants?

@christophfroehlich christophfroehlich added the backport-humble Triggers PR backport to ROS 2 humble. label Mar 13, 2026
…st/mapped_parameter_test.py

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
Copy link
Collaborator

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the iterations. I tried it out on my own now, and added some test output to the examples.

@christophfroehlich christophfroehlich merged commit 9b93579 into PickNikRobotics:main Mar 14, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-humble Triggers PR backport to ROS 2 humble.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No way to reference non top-level keys in parameters map

2 participants