[nflyernz]

Summary

• Reconcile conditional derived rows in place after inline component edits, including new rows and retirement of obsolete automatic rows.
• Preserve saved parameters, manual overrides, user additions, deleted rows, and connection rename destinations.
• Add model regression tests for port changes, None selection, idempotent refresh, FC availability, export, and parameter provenance.

Validation

• pytest -q tests/test_data_model_parameter_editor.py tests/test_data_model_configuration_step.py (430 passed)
• Ruff check and format, targeted ty, focused Pylint, and git diff --check passed. Pylint's existing argument-count findings were excluded from the focused run.

Related inline-edit reports: #2284, #2287, #2288. GNSS forced/derived behavior from #2288 remains deferred.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-10-30)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: COMMENT

Reviewed at head c3da9d8371.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/methodicconfigurator/2289/1.html#prMethodicConfigurator-2289

The reconciliation fix works for port moves, saved renames and FC availability, but it can bring back a derived row the user has just deleted.

Should fix

• data_model_parameter_editor.py:2834: an automatically added derived row that the user deletes (e.g. SERIAL2_PROTOCOL) comes back on the next inline component edit and gets saved or uploaded, because delete_parameter_from_current_file only tombstones rows that exist in the saved file. Record user deletions of any row and honour them in the applicability filter, and test this through the delete API instead of seeding _deleted_parameters.

Minor

• data_model_parameter_editor.py:2794: the filter also excludes rows that the step's delete_parameters rule removed at load, so switching the battery monitor inline from Analog to I2C never restores BATT_I2C_BUS (a fresh load does). Not a regression; consider tracking rule-driven deletions separately from user deletions.

Minor test coverage (1)

• data_model_parameter_editor.py:2822: the is_dirty guard, the export-time _inline_derived_parameters.clear() (line 2622) and the GNSS rename-target skip (line 2812) can each be removed without failing any test; the first two change behaviour, so focused tests would help.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-10-30)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: COMMENT

Reviewed at head ad15fb41ca.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/methodicconfigurator/2289/2.html#prMethodicConfigurator-2289

The two follow-up commits fix the main problem, and what is left is non-blocking: a pre-existing loss of saved rows, plus minor edge cases and test gaps.

Since the last review at c3da9d8, a derived row that the user deletes now stays deleted through inline component edits, port changes and save, until it is re-added or the step is reloaded.

Should fix

• data_model_parameter_editor.py:2631: rows lost after save (pre-existing, base is worse). _export_current_file leaves _connection_renames keyed by the old SERIAL1_* names. So after "Upload selected and stay", the next inline component edit, even an unrelated one, deletes SERIAL2_BAUD and BRD_SER2_RTSCTS, and the next save writes the file without them. Clear or rebase _connection_renames on export, here or in a follow-up.

Minor

• data_model_parameter_editor.py:2790: deleted row returns through a rename (pre-existing). If SERIAL2_PROTOCOL came from a saved SERIAL1 rename and is deleted, moving the port to SERIAL3 and back recreates and exports it, because the rename refresh does not check _suppressed_derived_parameters.
• data_model_parameter_editor.py:2803: BATT_I2C_BUS not restored inline (still open).

Minor test coverage (2)

• data_model_parameter_editor.py:2832: untested guards (still open).
• data_model_parameter_editor.py:2099: nothing tests the reload clear of _suppressed_derived_parameters or the doc_dict re-add unsuppress (data_model_parameter_editor.py:2414); each can be removed with all tests still passing. Add a refresh after the reload at the end of the new test, plus a doc-only re-add case.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-10-31)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: COMMENT

Reviewed at head 7685d356b3.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/methodicconfigurator/2289/3.html#prMethodicConfigurator-2289

The latest commit fixes the saved-row loss, but it adds a narrow regression when a renamed serial row is deleted and then re-added. Not blocking, but worth fixing before merge.

Since the review at ad15fb4, saving now rebases connection renames, so serial rows survive upload-and-stay plus a later edit. A deleted renamed row also stays deleted through a port round trip.

Should fix

• data_model_parameter_editor.py:2406: re-add leaves the row on the old port. Deleting SERIAL2_BAUD (or _PROTOCOL) that came from a saved SERIAL1 rename suppresses SERIAL1_BAUD, but re-adding it discards only SERIAL2_BAUD. After a later change to SERIAL3, the save has SERIAL2_BAUD=57 and no SERIAL3_BAUD. The docs branch at 2424 has the same problem. On re-add, also discard the sources whose current _connection_renames target is param_name.

Minor (2)

• data_model_parameter_editor.py:2816: BATT_I2C_BUS not restored inline (still open, pre-existing).
• test_data_model_parameter_editor.py:5909: test gaps (still open). The dirty-value test never reaches the is_dirty guard because the fixture FC lacks OTHER; add OTHER to fc_parameters or clear it. The export clear of _suppressed_connection_renames, both re-add discards, the reload clear of _suppressed_derived_parameters and the docs-path derived unsuppress can each still be removed with all tests passing.

Head moved during review: observed c5af74165ab6a9b85bdebe0e5c57704c28d74c61. This review covers only 7685d356b3a3e04df239af58567b38968e813995; followup eligible.

[nflyernz]

Follow-up validation — latest ParameterEditor changes

I've pushed the latest correction in c5af7416, addressing automatic derived-parameter ownership across export and connection renames.

The changes ensure that automatically derived serial assignments can be retired when their derivation no longer applies, while preserving existing saved settings, user additions and manual overrides.

Validation:

• 568 affected ParameterEditor and ConfigurationStep tests passed.
• Combined the current fix: reconcile inline derived parameters after component edits #2289 head (c5af7416) with my local consolidated ArduPlane serial-port integration (77edfa67) in an isolated temporary worktree.
• 807 tests passed, 0 failed, including all four previously failing transitions where a saved serial selection was subsequently changed back to None or RCIN.
• RC receiver, telemetry, ESC telemetry, GNSS, same-UART driver changes, repeated export and OSD template checks passed.
• Ruff, targeted ty and focused Pylint checks passed.

The consolidated ArduPlane work combines the corrections currently in draft PRs #2284, #2285, #2287 and #2288. It remains local and unpushed, pending #2289.

The ownership tracking is deliberately session-only; no persistent project metadata or flight-controller reset values were introduced.

The separate, pre-existing BATT_I2C_BUS inline-restoration issue is noted for a possible follow-up after the Basic configuration work.

[coveralls]

Coverage Report for CI Build 41163415068

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+1.2%) to 94.789%

Details

• Coverage increased (+1.2%) from the base build.
• Patch coverage: No coverable lines changed in this PR.
• 61 coverage regressions across 5 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

61 previously-covered lines in 5 files lost coverage.

File	Lines Losing Coverage	Coverage
data_model_parameter_editor.py	28	97.61%
main.py	14	90.57%
frontend_tkinter_parameter_editor.py	10	96.0%
plugins/frontend_tkinter_motor_test.py	8	98.44%
frontend_tkinter_component_editor.py	1	94.57%

---

Coverage Stats

Relevant Lines:	29919
Covered Lines:	28360
Line Coverage:	94.79%
Relevant Branches:	9032
Covered Branches:	8059
Branch Coverage:	89.23%
Branches in Coverage %:	No
Coverage Strength:	2.83 hits per line

---

💛 - Coveralls

[AP-Review]

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: COMMENT

Reviewed at head ad0b3b4499.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/methodicconfigurator/2289/4.html#prMethodicConfigurator-2289

The two new commits fix most of the earlier re-add problem, but one adds a regression where a saved serial protocol does not come back when its port is re-selected. Nothing is blocking.

Since the review at 7685d35, re-adding a deleted renamed row straight away now follows later port changes, edited values move with the port, and manual overrides on renamed rows are kept.

Should fix

• data_model_parameter_editor.py:2836: saved derived row lost on re-select. After a save, deselecting a port (e.g. RC receiver SERIAL5 to RCin/SBUS) retires SERIAL5_PROTOCOL into _deleted_parameters, and the filter at 2822 treats that as a user deletion. Re-selecting SERIAL5 shows no SERIAL5_PROTOCOL row and the next save writes the file without it until the step is reloaded. Regression from c5af741. Track retirements separately from explicit deletions.
• data_model_parameter_editor.py:2933: re-add after an intermediate edit (still open, narrowed). If any component edit runs while SERIAL2_BAUD is deleted, this branch drops the SERIAL1_BAUD→SERIAL2_BAUD mapping, so after re-add and a move to SERIAL3 the file has SERIAL2_BAUD=230 and no SERIAL3_BAUD (FC and docs paths). Keep the mapping while the source is suppressed.

Minor (3)

• data_model_parameter_editor.py:2944: return to the original port drops the edit (pre-existing). A re-added SERIAL2_BAUD edited to 230 and moved back to SERIAL1 saves SERIAL1_BAUD=57, while a move to SERIAL3 keeps 230; carry the current value here too.
• data_model_parameter_editor.py:2822: BATT_I2C_BUS not restored inline (still open, pre-existing).
• test_data_model_parameter_editor.py:5909: test gaps (still open). Also, no test re-selects a port after a saved retirement or edits while a renamed row is deleted, which is why both should-fix items pass CI.