Fix nested mapped parameters map#306
Fix nested mapped parameters map#306NickLaurenson-Visionick wants to merge 6 commits intoPickNikRobotics:mainfrom
Conversation
NickLaurenson-Visionick
force-pushed
the
fix/278
branch
3 times, most recently
from
1664af6 to
7fef7cf
Compare
NickLaurenson-Visionick
changed the title
christophfroehlich
left a comment
christophfroehlich
left a comment
There was a problem hiding this comment.
Can you please have a look at the failing CI jobs?
|
Yes sure I will take a look in the next day |
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
force-pushed
the
fix/278
branch
from
7fef7cf to
0a39b4d
Compare
|
I update the branch, and fix the compilation locally (via ci_industrial tool). @christophfroehlich can you see it it fix the ci too? |
|
Fix the style issue, everything should be fine now. |
|
@christophfroehlich would it be possible to have a review? |
christophfroehlich
left a comment
christophfroehlich
left a comment
There was a problem hiding this comment.
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?
| set_up(yaml_test_file) | ||
| except Exception as e: | ||
| assert False, f'failed to parse valid file, reason:{e}' | ||
| @pytest.mark.parametrize( |
There was a problem hiding this comment.
should we move this in a standalone unit test file?
There was a problem hiding this comment.
I wonder if we really want to test such code implementation detail.
Let me know what you prefer and I will do the change:
- new
YAML_parse_test.pytest file that testget_dynamic_mapped_parameter - remove this test
- keep as it is
I don't have a real opinion on it, so personally as default I would keep it as it is now
2 participants
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 returnfinal_fieldcausing error in the generated code.Regarding the test, I wrote some basic one, let me know if need to do something else