Skip to content

[backport/1.4] Keep Ping port watcher running after unexpected errors - #4052

Open
joaoantoniocardoso wants to merge 1 commit into
bluerobotics:1.4-devfrom
joaoantoniocardoso:backports/1.4/ping-port-watcher-resilience
Open

[backport/1.4] Keep Ping port watcher running after unexpected errors#4052
joaoantoniocardoso wants to merge 1 commit into
bluerobotics:1.4-devfrom
joaoantoniocardoso:backports/1.4/ping-port-watcher-resilience

Conversation

@joaoantoniocardoso

@joaoantoniocardoso joaoantoniocardoso commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

This is a backport of #4051 into 1.4.

Supersedes #4042 (recreated from fork joaoantoniocardoso/BlueOS-docker instead of same-repo head).

@joaoantoniocardoso joaoantoniocardoso mentioned this pull request Jul 27, 2026
Closed
@github-actions

github-actions Bot commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Automated PR Review

0. Summary

  • Verdict: LGTM :shipit:

Straightforward backport of #4051 to 1.4-dev: wraps the PortWatcher.start_watching polling body in a try/except Exception so a transient failure (e.g. KeyError from a network interface disappearing between psutil.net_if_stats/net_if_addrs snapshots in add_ping360) no longer kills the watcher task. Adds a targeted pytest-asyncio regression test and pulls the psutil == 5.7.2 dependency into core/services/ping/setup.py and the root pyproject.toml dev-deps (already used transitively by ping360_ethernet_prober.py and pingutils.py). asyncio.CancelledError is a BaseException in 3.11 so it still propagates and cancellation semantics are preserved — the test correctly asserts this.

No further comments, nice job 👍

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.

@joaoantoniocardoso
joaoantoniocardoso marked this pull request as draft July 27, 2026 21:34
@joaoantoniocardoso
joaoantoniocardoso force-pushed the backports/1.4/ping-port-watcher-resilience branch 2 times, most recently from e2258ac to 6cde1de Compare July 28, 2026 15:02
@joaoantoniocardoso
joaoantoniocardoso force-pushed the backports/1.4/ping-port-watcher-resilience branch from 6cde1de to d7e7d26 Compare July 28, 2026 20:02
@joaoantoniocardoso
joaoantoniocardoso marked this pull request as ready for review August 3, 2026 19:03
patrickelectric
patrickelectric reviewed Aug 3, 2026
View reviewed changes
Comment thread core/services/ping/test_portwatcher.py
@pytest.mark.asyncio
async def test_start_watching_continues_after_add_ping360_error() -> None:
watcher = PortWatcher(probe_callback=AsyncMock(), found_callback=AsyncMock())
calls = {"n": 0}

@patrickelectric patrickelectric Aug 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there a reason for this to be a dictionary ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@patrickelectric patrickelectric patrickelectric left review comments
@Williangalvani Williangalvani Awaiting requested review from Williangalvani
@nicoschmdt nicoschmdt Awaiting requested review from nicoschmdt
@joaomariolago joaomariolago Awaiting requested review from joaomariolago

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@joaoantoniocardoso @patrickelectric