From cb62db76ebe73397a1acaa17cc668a52f043e1a1 Mon Sep 17 00:00:00 2001 From: Leon McEniery Date: Tue, 11 Nov 2025 10:15:09 +0800 Subject: [PATCH 1/3] fix: fix nested parameters within a __map --- .../parse_yaml.py | 4 +- .../test/test_nested_map_fix.py | 126 ++++++++++++++++++ 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/generate_parameter_library_py/generate_parameter_library_py/parse_yaml.py b/generate_parameter_library_py/generate_parameter_library_py/parse_yaml.py index aa90f3b..1f49cfc 100644 --- a/generate_parameter_library_py/generate_parameter_library_py/parse_yaml.py +++ b/generate_parameter_library_py/generate_parameter_library_py/parse_yaml.py @@ -805,8 +805,8 @@ def parse_params(self, name, value, nested_name_list): # add variable to struct var = VariableDeclaration(code_gen_variable) - # check if runtime parameter - is_runtime_parameter = is_mapped_parameter(self.struct_tree.struct_name) + # check if runtime parameter - check any ancestor in the path is a mapped parameter + is_runtime_parameter = any(is_mapped_parameter(x) for x in nested_name_list) if is_runtime_parameter: declare_parameter_set = SetRuntimeParameter(param_name, code_gen_variable) diff --git a/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py b/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py index 15ec26a..524a460 100644 --- a/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py +++ b/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py @@ -156,3 +156,129 @@ def test_single_map_parameter_names(): # Clean up temporary files os.unlink(yaml_file.name) os.unlink(output_file.name) + + +def test_control_modes_nested_structures(): + """Test nested structures within mapped parameters. + + This tests the specific issue where parameters nested within mapped structures + (like control_modes.__map_control_mode_ids.fixed_heading.enabled) were not + generating loop iteration code correctly. + """ + + control_modes_yaml_content = """autopilot_params: + active_control_mode: + type: string + default_value: "dynamic_heading" + description: "The current active control mode" + + control_mode_ids: + type: string_array + default_value: ["dynamic_heading", "fixed_heading"] + description: "List of available control modes" + + control_modes: + __map_control_mode_ids: + use_trajectory: + type: bool + default_value: true + description: "Use generated trajectory as control reference" + + fixed_heading: + enabled: + type: bool + default_value: false + description: "Enable fixed heading control" + angle: + type: double + default_value: 0.0 + description: "Fixed heading angle in degrees" + validation: + bounds<>: [0.0, 360.0] +""" + + with tempfile.NamedTemporaryFile( + mode='w', suffix='.yaml', delete=False + ) as yaml_file: + yaml_file.write(control_modes_yaml_content) + yaml_file.flush() + + with tempfile.NamedTemporaryFile( + mode='w', suffix='.py', delete=False + ) as output_file: + try: + run_python(output_file.name, yaml_file.name, 'test_validate.hpp') + + with open(output_file.name, 'r') as f: + generated_code = f.read() + + # Check for proper loop generation + assert ( + 'for value_1 in updated_params.control_mode_ids:' in generated_code + ), 'Should generate loop over control_mode_ids' + + # Check that we're using get_entry correctly + assert ( + 'updated_params.control_modes.get_entry(value_1)' in generated_code + or 'entry = updated_params.control_modes.get_entry(value_1)' in generated_code + ), 'Should use get_entry to access mapped parameters' + + # Get all parameter name lines + lines = generated_code.split('\n') + param_name_lines = [ + line + for line in lines + if 'param_name' in line and 'control_modes' in line + ] + + # Assert there are parameter name lines + assert ( + len(param_name_lines) > 0 + ), 'Should have found parameter name lines for control_modes' + + # Check for expected parameter patterns (using {value_1} for dynamic substitution) + expected_patterns = [ + 'control_modes.{value_1}.use_trajectory', + 'control_modes.{value_1}.fixed_heading.enabled', + 'control_modes.{value_1}.fixed_heading.angle', + ] + + for pattern in expected_patterns: + pattern_found = any(pattern in line for line in param_name_lines) + assert ( + pattern_found + ), f"Expected pattern '{pattern}' not found in generated code.\nParam lines:\n" + '\n'.join(param_name_lines[:5]) + + # Ensure no double dots in parameter names + for line in param_name_lines: + assert ( + '..' not in line + ), f'Found double dots in parameter name: {line.strip()}' + + # Check that we're accessing nested parameters via entry, not via __map_ + entry_access_lines = [ + line for line in lines + if 'entry.' in line and ('fixed_heading' in line) + ] + assert ( + len(entry_access_lines) > 0 + ), 'Should access nested parameters via entry variable' + + # Verify no direct access to __map_control_mode_ids + wrong_access_lines = [ + line for line in lines + if 'control_modes.__map_control_mode_ids' in line + ] + # Filter out comments + wrong_access_lines = [ + line for line in wrong_access_lines + if not line.strip().startswith('#') + ] + assert ( + len(wrong_access_lines) == 0 + ), f'Should not directly access __map_control_mode_ids. Found:\n' + '\n'.join(wrong_access_lines) + + finally: + # Clean up temporary files + os.unlink(yaml_file.name) + os.unlink(output_file.name) From b6f91831971c1b9da776f30d5e1204de93abc7dd Mon Sep 17 00:00:00 2001 From: Leon McEniery Date: Tue, 11 Nov 2025 10:22:20 +0800 Subject: [PATCH 2/3] chore: lint --- .../test/test_nested_map_fix.py | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py b/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py index 524a460..1662c45 100644 --- a/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py +++ b/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py @@ -220,7 +220,8 @@ def test_control_modes_nested_structures(): # Check that we're using get_entry correctly assert ( 'updated_params.control_modes.get_entry(value_1)' in generated_code - or 'entry = updated_params.control_modes.get_entry(value_1)' in generated_code + or 'entry = updated_params.control_modes.get_entry(value_1)' + in generated_code ), 'Should use get_entry to access mapped parameters' # Get all parameter name lines @@ -245,9 +246,10 @@ def test_control_modes_nested_structures(): for pattern in expected_patterns: pattern_found = any(pattern in line for line in param_name_lines) - assert ( - pattern_found - ), f"Expected pattern '{pattern}' not found in generated code.\nParam lines:\n" + '\n'.join(param_name_lines[:5]) + assert pattern_found, ( + f"Expected pattern '{pattern}' not found in generated code.\nParam lines:\n" + + '\n'.join(param_name_lines[:5]) + ) # Ensure no double dots in parameter names for line in param_name_lines: @@ -257,7 +259,8 @@ def test_control_modes_nested_structures(): # Check that we're accessing nested parameters via entry, not via __map_ entry_access_lines = [ - line for line in lines + line + for line in lines if 'entry.' in line and ('fixed_heading' in line) ] assert ( @@ -266,17 +269,20 @@ def test_control_modes_nested_structures(): # Verify no direct access to __map_control_mode_ids wrong_access_lines = [ - line for line in lines + line + for line in lines if 'control_modes.__map_control_mode_ids' in line ] # Filter out comments wrong_access_lines = [ - line for line in wrong_access_lines + line + for line in wrong_access_lines if not line.strip().startswith('#') ] - assert ( - len(wrong_access_lines) == 0 - ), f'Should not directly access __map_control_mode_ids. Found:\n' + '\n'.join(wrong_access_lines) + assert len(wrong_access_lines) == 0, ( + f'Should not directly access __map_control_mode_ids. Found:\n' + + '\n'.join(wrong_access_lines) + ) finally: # Clean up temporary files From 02c8b5aeead80326c6895c9fe4fb3a397ecd634a Mon Sep 17 00:00:00 2001 From: Leon McEniery Date: Tue, 11 Nov 2025 11:01:00 +0800 Subject: [PATCH 3/3] fix: revert to existing solution --- .../generate_parameter_library_py/parse_yaml.py | 13 +++++++------ .../test/test_nested_map_fix.py | 14 ++++++-------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/generate_parameter_library_py/generate_parameter_library_py/parse_yaml.py b/generate_parameter_library_py/generate_parameter_library_py/parse_yaml.py index 1f49cfc..7fe98d2 100644 --- a/generate_parameter_library_py/generate_parameter_library_py/parse_yaml.py +++ b/generate_parameter_library_py/generate_parameter_library_py/parse_yaml.py @@ -101,8 +101,9 @@ def int_to_integer_str(value: str): def get_dynamic_parameter_field(yaml_parameter_name: str): tmp = yaml_parameter_name.split('.') - parameter_field = tmp[-1] - return parameter_field + num_nested = [i for i, val in enumerate(tmp) if is_mapped_parameter(val)] + field = tmp[(max(num_nested) + 1) :] if len(num_nested) else [tmp[-1]] + return '.'.join(field) def get_dynamic_mapped_parameter(yaml_parameter_name: str): @@ -115,8 +116,8 @@ def get_dynamic_mapped_parameter(yaml_parameter_name: str): def get_dynamic_struct_name(yaml_parameter_name: str): tmp = yaml_parameter_name.split('.') - num_nested = sum([is_mapped_parameter(val) for val in tmp]) - struct_name = tmp[: -(num_nested + 1)] + num_nested = [i for i, val in enumerate(tmp) if is_mapped_parameter(val)] + struct_name = tmp[: (min(num_nested))] if len(num_nested) else [] return '.'.join(struct_name) @@ -805,8 +806,8 @@ def parse_params(self, name, value, nested_name_list): # add variable to struct var = VariableDeclaration(code_gen_variable) - # check if runtime parameter - check any ancestor in the path is a mapped parameter - is_runtime_parameter = any(is_mapped_parameter(x) for x in nested_name_list) + # check if runtime parameter + is_runtime_parameter = is_mapped_parameter(param_name) if is_runtime_parameter: declare_parameter_set = SetRuntimeParameter(param_name, code_gen_variable) diff --git a/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py b/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py index 1662c45..d433fd2 100644 --- a/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py +++ b/generate_parameter_library_py/generate_parameter_library_py/test/test_nested_map_fix.py @@ -267,20 +267,18 @@ def test_control_modes_nested_structures(): len(entry_access_lines) > 0 ), 'Should access nested parameters via entry variable' - # Verify no direct access to __map_control_mode_ids + # Verify no direct access to __map_control_mode_ids in actual code + # (error messages can reference the YAML path, but code should not access __map_ attributes) wrong_access_lines = [ line for line in lines if 'control_modes.__map_control_mode_ids' in line - ] - # Filter out comments - wrong_access_lines = [ - line - for line in wrong_access_lines - if not line.strip().startswith('#') + and not line.strip().startswith('#') # Ignore comments + and 'InvalidParameterValueException' + not in line # Ignore error messages ] assert len(wrong_access_lines) == 0, ( - f'Should not directly access __map_control_mode_ids. Found:\n' + f'Should not directly access __map_control_mode_ids in code. Found:\n' + '\n'.join(wrong_access_lines) )