Skip to content

Log port list once, then only log changes#1339

Merged
amilcarlucas merged 1 commit intomasterfrom
log_less
Mar 2, 2026
Merged

Log port list once, then only log changes#1339
amilcarlucas merged 1 commit intomasterfrom
log_less

Conversation

@amilcarlucas
Copy link
Copy Markdown
Collaborator

Replace repeated "Available connection ports are:" dumps on every 3-second refresh with a diff-based approach: the full port list is logged once on first discovery; subsequent discoveries only log "Connection port added:" or "Connection port removed:" lines when the list actually changes, and are silent otherwise.

Replace repeated "Available connection ports are:" dumps on every
3-second refresh with a diff-based approach: the full port list is
logged once on first discovery; subsequent discoveries only log
"Connection port added:" or "Connection port removed:" lines when
the list actually changes, and are silent otherwise.
Copilot AI review requested due to automatic review settings March 1, 2026 23:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces log noise during periodic connection port discovery by logging the full port list only once per FlightControllerConnection instance, and subsequently logging only added/removed ports when the discovered set changes.

Changes:

  • Track the last logged set of discovered connection tuples.
  • Replace repeated full port dumps with _log_connection_changes() that logs only diffs after the first discovery.

Comment on lines +182 to +200
def _log_connection_changes(self) -> None:
"""Log port list on first discovery; log only added/removed ports on subsequent discoveries."""
current = self._connection_tuples
previous = self._logged_connection_tuples

if previous is None:
# First discovery - log everything
logging_info(_("Available connection ports are:"))
for port in current:
logging_info("%s - %s", port[0], port[1])
else:
current_set = set(current)
previous_set = set(previous)
for port in sorted(current_set - previous_set):
logging_info(_("Connection port added: %s - %s"), port[0], port[1])
for port in sorted(previous_set - current_set):
logging_info(_("Connection port removed: %s - %s"), port[0], port[1])

self._logged_connection_tuples = list(current)
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new diff-based logging behavior in _log_connection_changes isn’t covered by tests. Since this repo already uses pytest for FlightControllerConnection, please add a test that captures logs (e.g., via caplog) to assert: (1) first discover_connections() logs the full list once, (2) a second call with the same ports produces no new port-list logs, and (3) adding/removing a port results in exactly the expected "added"/"removed" log lines.

Copilot generated this review using guidance from repository custom instructions.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 2, 2026

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
11551 10518 91% 89% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
ardupilot_methodic_configurator/backend_flightcontroller_connection.py 92% 🟢
TOTAL 92% 🟢

updated for commit: fcadae8 by action🐍

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 2, 2026

Test Results

    2 files  ±0      2 suites  ±0   28m 26s ⏱️ -13s
3 028 tests ±0  3 023 ✅ ±0   5 💤 ±0  0 ❌ ±0 
6 056 runs  ±0  6 046 ✅ ±0  10 💤 ±0  0 ❌ ±0 

Results for commit fcadae8. ± Comparison against base commit e4dc4d7.

@amilcarlucas amilcarlucas merged commit 2257974 into master Mar 2, 2026
34 of 36 checks passed
@amilcarlucas amilcarlucas deleted the log_less branch March 2, 2026 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants