Skip to content

Commit 532013a

Browse files
committed
fix(connection renames): do not perform connection renames when it would create duplicates
Let the users handle it manually however they want. fixes #950
1 parent 2708ea1 commit 532013a

2 files changed

Lines changed: 23 additions & 17 deletions

File tree

ardupilot_methodic_configurator/data_model_configuration_step.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,13 +300,14 @@ def _calculate_connection_rename_operations(
300300
renames = ConfigurationStepProcessor._generate_connection_renames(list(parameters.keys()), new_connection_prefix)
301301

302302
# Track unique new names and actual renames to perform
303-
new_names = set()
304-
duplicates = set()
303+
# Initialize with all existing parameter names to prevent conflicts
304+
new_names = set(parameters.keys())
305+
duplicates: set[str] = set()
305306
renamed_pairs = []
306307
for old_name, new_name in renames.items():
307308
if new_name in new_names:
308-
# This would be a duplicate - mark for removal
309-
duplicates.add(old_name)
309+
# Do not perform rename due to conflict, let the user handle it
310+
pass
310311
else:
311312
new_names.add(new_name)
312313
if new_name != old_name:

tests/test_data_model_configuration_step.py

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -547,12 +547,12 @@ def test_apply_renames_without_duplicates(self, connection_params) -> None:
547547

548548
def test_apply_renames_with_duplicates(self, connection_params) -> None:
549549
"""
550-
Test calculating renames identifies duplicate conflicts correctly.
550+
Test calculating renames skips individual operations when they would create conflicts.
551551
552-
GIVEN: A user has parameters that would create naming conflicts after renaming
552+
GIVEN: A user has parameters that would create naming conflicts for some renames
553553
WHEN: They calculate connection renaming operations
554-
THEN: Duplicate parameters should be identified for removal
555-
AND: The system should track which parameters would conflict
554+
THEN: Conflicting renames are skipped but non-conflicting renames proceed
555+
AND: No parameters are marked as duplicates for removal
556556
AND: Original parameters dict remains unchanged (immutable)
557557
"""
558558
# Arrange: Create params with potential duplicates
@@ -562,19 +562,24 @@ def test_apply_renames_with_duplicates(self, connection_params) -> None:
562562
original_keys = set(params.keys())
563563

564564
# Act: Calculate renames for CAN1 to CAN2
565-
duplicated_params, _renamed_pairs = ConfigurationStepProcessor._calculate_connection_rename_operations(params, "CAN2")
565+
duplicated_params, renamed_pairs = ConfigurationStepProcessor._calculate_connection_rename_operations(params, "CAN2")
566566

567-
# Assert: The ALREADY-EXISTING parameter is marked as duplicate (not the one being renamed)
568-
# This is because CAN_P2_DRIVER already exists, so it would conflict with the rename
569-
assert "CAN_P2_DRIVER" in duplicated_params
567+
# Assert: No duplicates are marked when individual conflicts occur
568+
assert len(duplicated_params) == 0
569+
570+
# Assert: Non-conflicting renames proceed (CAN_P1_DRIVER conflicts, others don't)
571+
expected_renames = [
572+
("CAN_P1_BITRATE", "CAN_P2_BITRATE"),
573+
("CAN_D1_PROTOCOL", "CAN_D2_PROTOCOL"),
574+
("CAN_D1_UC_NODE", "CAN_D2_UC_NODE"),
575+
("CAN_D1_UC_OPTION", "CAN_D2_UC_OPTION"),
576+
]
577+
assert renamed_pairs == expected_renames
570578

571579
# Assert: Original params dict unchanged (immutable)
572580
assert set(params.keys()) == original_keys
573-
assert "CAN_P1_DRIVER" in params # Original rename source still present
574-
assert "CAN_P2_DRIVER" in params # Pre-existing conflict still there
575-
576-
# Check duplicated parameters are tracked (the pre-existing target that was removed)
577-
assert "CAN_P2_DRIVER" in duplicated_params
581+
assert "CAN_P1_DRIVER" in params # Original parameter still present
582+
assert "CAN_P2_DRIVER" in params # Conflicting parameter still there
578583

579584
def test_apply_renames_with_variables(self, connection_params) -> None:
580585
"""

0 commit comments

Comments
 (0)