Preserve history connections across periodic port rediscovery by amilcarlucas · Pull Request #1374 · ArduPilot/MethodicConfigurator

amilcarlucas · 2026-03-12T21:22:08Z

The 3-second periodic port refresh rebuilt _connection_tuples entirely from auto-discovered ports, silently dropping any connections that were added from persistent history or via the UI but were not physically present on the bus at that moment.

Add preserved_connections parameter to discover_connections() so the caller (frontend) can supply history entries to re-merge after each auto-discovery scan, keeping the backend free of settings/filesystem dependencies
Pass ProgramSettings.get_connection_history() as preserved_connections on every periodic refresh in ConnectionSelectionWidgets._refresh_ports()
Store successfully connected device to ProgramSettings and register it via add_connection() in reconnect(), not only when the connection was typed via the "Add another" dialog but also when an existing combobox entry is selected
Update protocol, facade, and tests accordingly

Copilot

Pull request overview

This PR ensures connection entries originating from user history/UI aren’t silently dropped during the periodic (3s) port rediscovery cycle by allowing the frontend to re-merge preserved connections after each scan.

Changes:

Extend discover_connections() (protocol + facade + connection manager) to accept preserved_connections and merge them into the discovered list.
Update the Tkinter connection-selection widget to pass ProgramSettings.get_connection_history() on each periodic refresh and to persist successful connections during reconnect().
Update/extend unit tests to cover preserved-connection behavior and the new method signature forwarding.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py`	Pass preserved history on refresh; persist connected device and register it via `add_connection()` on successful reconnect.
`ardupilot_methodic_configurator/backend_flightcontroller_protocols.py`	Add `preserved_connections` to the connection discovery protocol signature.
`ardupilot_methodic_configurator/backend_flightcontroller_connection.py`	Merge caller-supplied preserved connections into the discovered connection tuples.
`ardupilot_methodic_configurator/backend_flightcontroller.py`	Forward `preserved_connections` through the FlightController facade to the connection manager.
`tests/test_frontend_tkinter_connection_selection.py`	Update periodic-refresh expectations for the new `discover_connections()` signature and patch history access.
`tests/test_backend_flightcontroller_connection.py`	Add tests verifying preserved connections survive rediscovery and that non-preserved ports disappear.
`tests/test_backend_flightcontroller.py`	Update mock assertion for forwarding `preserved_connections=None`.

You can also share your feedback on Copilot code review. Take the survey.

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

tests/test_frontend_tkinter_connection_selection.py

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

ardupilot_methodic_configurator/backend_flightcontroller_connection.py

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

github-actions · 2026-03-12T21:37:58Z

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
11732	10907	93%	89%	🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: b3efc9d by action🐍

github-actions · 2026-03-12T21:38:12Z

Test Results

4 files ±0 4 suites ±0 41m 22s ⏱️ +37s
3 202 tests +2 3 200 ✅ +2 2 💤 ±0 0 ❌ ±0
12 600 runs +8 12 580 ✅ +8 20 💤 ±0 0 ❌ ±0

Results for commit e932e47. ± Comparison against base commit 1757ef6.

♻️ This comment has been updated with latest results.

…rmetic tests Address three unresolved code-review issues from PR #1374: - Cache ProgramSettings.get_connection_history() once at widget init as _connection_history_cache instead of calling it (a disk read) on every 3-second periodic port-refresh cycle, preventing potential UI stalls on slow disks or remote home directories. - Update _connection_history_cache in-memory (prepend + deduplicate) in both reconnect() and add_connection() after calling store_connection(), keeping the cache consistent without further disk reads. - Fix reconnect() to persist `selected_connection or device` instead of always persisting self.flight_controller.comport.device. This avoids storing resolved, unstable device paths (e.g. /dev/ttyACM0) when the user explicitly selected a /dev/serial/by-id/* symlink, preventing duplicates in the connection history. - Patch ProgramSettings.store_connection in test_periodic_refresh_stops_on_connection to prevent writes to the real settings.json during tests, and assert the correct connection string was persisted to verify the fix above.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

You can also share your feedback on Copilot code review. Take the survey.

ardupilot_methodic_configurator/backend_flightcontroller_connection.py

ardupilot_methodic_configurator/backend_flightcontroller_protocols.py

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

…discovery The 3-second periodic port refresh rebuilt _connection_tuples entirely from auto-discovered ports, silently dropping any connections that were added from persistent history or via the UI but were not physically present on the bus at that moment. - Add `preserved_connections` parameter to `discover_connections()` so the caller (frontend) can supply history entries to re-merge after each auto-discovery scan, keeping the backend free of settings/filesystem dependencies - Pass `ProgramSettings.get_connection_history()` as `preserved_connections` on every periodic refresh in `ConnectionSelectionWidgets._refresh_ports()` - Store successfully connected device to `ProgramSettings` and register it via `add_connection()` in `reconnect()`, not only when the connection was typed via the "Add another" dialog but also when an existing combobox entry is selected - Update protocol, facade, and tests accordingly

…rmetic tests Address three unresolved code-review issues from PR #1374: - Cache ProgramSettings.get_connection_history() once at widget init as _connection_history_cache instead of calling it (a disk read) on every 3-second periodic port-refresh cycle, preventing potential UI stalls on slow disks or remote home directories. - Update _connection_history_cache in-memory (prepend + deduplicate) in both reconnect() and add_connection() after calling store_connection(), keeping the cache consistent without further disk reads. - Fix reconnect() to persist `selected_connection or device` instead of always persisting self.flight_controller.comport.device. This avoids storing resolved, unstable device paths (e.g. /dev/ttyACM0) when the user explicitly selected a /dev/serial/by-id/* symlink, preventing duplicates in the connection history. - Patch ProgramSettings.store_connection in test_periodic_refresh_stops_on_connection to prevent writes to the real settings.json during tests, and assert the correct connection string was persisted to verify the fix above.

… duplication - Guard against empty strings and duplicate entries in preserved_connections by skipping falsy values and updating auto_discovered after each append, preventing ("", "") tuples and redundant entries in the connection list - Widen preserved_connections type from list[str] to Sequence[str] across protocol, facade and connection manager so callers can pass any iterable without forcing list materialisation - Extract duplicated store_connection + cache-update logic from add_connection() and reconnect() into _persist_and_cache_connection(), centralising edge-case handling in a single place

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

You can also share your feedback on Copilot code review. Take the survey.

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

ardupilot_methodic_configurator/backend_flightcontroller_connection.py

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

…g it would yield individual characters

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

… in _persist_and_cache_connection Strip whitespace and validate length before updating _connection_history_cache, ensuring the in-memory cache always holds the same normalized value that was persisted. Prevents blank/duplicate entries from surfacing in the combobox and being re-sent as preserved_connections on periodic port refresh.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py:232

_persist_and_cache_connection() normalizes via ProgramSettings.store_connection() but add_connection() continues to register/select the raw selected_connection in the backend (add_connection) and the combobox (set_entries_tuple). If the user enters leading/trailing whitespace (allowed by validator; stored stripped), the subsequent refresh will contain the normalized key while the current selection is the raw key, and PairTupleCombobox will log a critical error when it can't find the selected key. Consider having _persist_and_cache_connection() return the normalized string (or otherwise expose it) and consistently use that normalized value for flight_controller.add_connection(...) and combobox selection updates.

        if selected_connection:
            self._persist_and_cache_connection(selected_connection)
            error_msg = _("Will add new connection: {selected_connection} if not duplicated")
            logging_debug(error_msg.format(**locals()))
            self.flight_controller.add_connection(selected_connection)
            connection_tuples = self.flight_controller.get_connection_tuples()
            error_msg = _("Updated connection tuples: {connection_tuples} with selected connection: {selected_connection}")
            logging_debug(error_msg.format(**locals()))
            self.conn_selection_combobox.set_entries_tuple(connection_tuples, selected_connection)
            self.reconnect(selected_connection)

You can also share your feedback on Copilot code review. Take the survey.

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

ardupilot_methodic_configurator/backend_flightcontroller_connection.py

… desync _persist_and_cache_connection() now returns the normalized value (str.strip applied by ProgramSettings.store_connection) instead of None. reconnect() and add_connection() use the returned value for previous_selection and flight_controller.add_connection() so that the in-memory cache, the combobox keys, and the flight controller connection registry all hold the same string. Previously, a user-supplied string with surrounding whitespace (e.g. " COM3 ") was stored as "COM3" but the original padded value was kept as the combobox selection key, causing selection-not-found errors during periodic port rediscovery.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

You can also share your feedback on Copilot code review. Take the survey.

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

tests/test_backend_flightcontroller_connection.py

…rmetic tests Address three unresolved code-review issues from PR #1374: - Cache ProgramSettings.get_connection_history() once at widget init as _connection_history_cache instead of calling it (a disk read) on every 3-second periodic port-refresh cycle, preventing potential UI stalls on slow disks or remote home directories. - Update _connection_history_cache in-memory (prepend + deduplicate) in both reconnect() and add_connection() after calling store_connection(), keeping the cache consistent without further disk reads. - Fix reconnect() to persist `selected_connection or device` instead of always persisting self.flight_controller.comport.device. This avoids storing resolved, unstable device paths (e.g. /dev/ttyACM0) when the user explicitly selected a /dev/serial/by-id/* symlink, preventing duplicates in the connection history. - Patch ProgramSettings.store_connection in test_periodic_refresh_stops_on_connection to prevent writes to the real settings.json during tests, and assert the correct connection string was persisted to verify the fix above.

Copilot AI review requested due to automatic review settings March 12, 2026 21:22

Copilot started reviewing on behalf of amilcarlucas March 12, 2026 21:22 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

amilcarlucas force-pushed the conn_history_fix branch from a9a33d3 to abd1d29 Compare March 12, 2026 23:00

amilcarlucas requested a review from Copilot March 12, 2026 23:16

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Copilot started reviewing on behalf of amilcarlucas March 12, 2026 23:30 View session

amilcarlucas added 3 commits March 14, 2026 11:17

amilcarlucas force-pushed the conn_history_fix branch from c5d1766 to bd7328e Compare March 14, 2026 10:17

amilcarlucas requested a review from Copilot March 14, 2026 10:41

Copilot AI reviewed Mar 14, 2026

View reviewed changes

Copilot started reviewing on behalf of amilcarlucas March 14, 2026 10:56 View session

amilcarlucas added 2 commits March 14, 2026 13:01

docs(connection): Update the documentation

602ea8d

fix(connection-selection): str is itself a Sequence[str], so iteratin…

9a2f72a

…g it would yield individual characters

amilcarlucas force-pushed the conn_history_fix branch from 2755652 to 9a2f72a Compare March 14, 2026 12:14

amilcarlucas requested a review from Copilot March 14, 2026 12:23

Copilot started reviewing on behalf of amilcarlucas March 14, 2026 12:23 View session

Copilot AI reviewed Mar 14, 2026

View reviewed changes

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py Outdated Show resolved Hide resolved

amilcarlucas requested a review from Copilot March 14, 2026 18:32

Copilot started reviewing on behalf of amilcarlucas March 14, 2026 18:33 View session

Copilot AI reviewed Mar 14, 2026

View reviewed changes

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py Outdated Show resolved Hide resolved

ardupilot_methodic_configurator/backend_flightcontroller_connection.py Show resolved Hide resolved

amilcarlucas requested a review from Copilot March 15, 2026 19:08

Copilot AI reviewed Mar 15, 2026

View reviewed changes

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py Outdated Show resolved Hide resolved

tests/test_backend_flightcontroller_connection.py Outdated Show resolved Hide resolved

Copilot started reviewing on behalf of amilcarlucas March 15, 2026 19:26 View session

amilcarlucas added 3 commits March 16, 2026 05:47

test(connection-selection): add more tests

c1f33c2

test(coverage): Improve test coverage

561791b

fix(connection-selection): consistency fixes

b3efc9d

amilcarlucas force-pushed the conn_history_fix branch from 3d3f27a to b3efc9d Compare March 16, 2026 04:50

amilcarlucas merged commit dedf801 into master Mar 16, 2026
30 checks passed

amilcarlucas deleted the conn_history_fix branch March 16, 2026 08:36

Conversation

amilcarlucas commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☂️ Python Coverage

Overall Coverage

New Files

Modified Files

Uh oh!

github-actions bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Mar 12, 2026 •

edited

Loading

github-actions bot commented Mar 12, 2026 •

edited

Loading