[deepakpopli]

What changed and why?

When the WebRTC connection drops, the camera often doesn't come back.

• start(), which camera.start_streaming() uses, never starts the reconnect monitor.
• When the monitor does run, one failed retry leaves it stuck, because it skips while pc is None.

Now start() starts the monitor, stop() cancels it, and a failed retry is tried again. After the first connect, run_with_auto_reconnect() leaves drops to the monitor, so the two don't retry at the same time.

Note: with auto_reconnect=True (the default), start() now reconnects on its own. I kept the 10 try limit. Should it keep retrying until the network is back instead? Happy to change it.

How did you test it?

• poetry install -E camera, then poetry run pytest tests/test_base_video_reconnect_monitor.py -v: 10 passed. They need aiortc from the camera extra, so they skip in CI.
• Stopped a local WebRTC receiver for 5 s and 70 s. Before this change start_streaming() stayed down. With it, the stream came back every time.

Contributor checklist

• I added or updated tests for behavior changes, or explained why they are unnecessary.
• I updated relevant documentation or examples.
• I noted any public API, optional dependency, or simulation/live behavior changes.

Recognition

Merged human contributions receive a thank-you in the Cyberwave Discord using
the PR title, link, and GitHub username. Discord membership is not required.

• Please skip the Discord celebration for this PR.

[github-actions]

Thanks for contributing to the Cyberwave Python SDK, @deepak61296!

Maintainer contacts: @KhushiSharma22. When this PR is ready, we request a maintainer review and run an automated advisory review. CI and human review remain the source of truth.

Please describe what changed and how you tested it. See the contribution guide or ask for help on Discord. Drafts are welcome.

Merged human contributions receive a Discord thank-you. To opt out, check the PR template's Discord opt-out box or add <!-- cyberwave:no-celebration --> to the PR description.

[github-actions]

Automated advisory review · Claude

Commit: 32bc5a3111b3fd15e1dccd7cbe2b329bf3d9b616. Human review and CI are still required.

The patch refactors BaseVideoStreamer to split start/stop into inner helpers (_start_webrtc, _close_peer_connection) and adds a monitor-task lifecycle. Three concrete defects are visible in the supplied excerpts. 1. The _monitor_connection loop condition for pc==None was changed in a way that creates an infinite busy-spin when pc is None and reconnect_attempt > 0 after a failed reconnect attempt. 2. stop() no longer guards against being called with no running event loop or outside an async context when the monitor task is None, but more critically the busy-poll cancel loop can run forever if the monitor task never transitions to done (e.g., when it is blocked in a non-cancellable await). 3. _handle_start_command now calls _start_webrtc() instead of start(), so when auto_reconnect is True and _handle_start_command is invoked (e.g., via MQTT command), no monitor task is started, leaving drops undetected for the rest of the session.

• High · cyberwave/sensor/base_video.py: Trigger: _handle_start_command() was changed to call self._start_webrtc() instead of self.start(). self.start() is the only place that creates the monitor task when auto_reconnect is True. _start_webrtc() does not create any monitor task. Consequence: When a start_video MQTT command arrives (the normal remote-start path), the WebRTC connection is established but the reconnect monitor is never launched. Any subsequent connection drop is silently ignored for the remainder of the session; the stream never recovers without an explicit reconnect command. Fix: Either call self.start() from _handle_start_command() (as before), or duplicate the monitor-task creation logic that start() contains immediately after the _start_webrtc() call inside _handle_start_command(). The simpler fix is to restore the call to self.start() and handle the 'already running' guard inside start() rather than at the call sites.
• High · cyberwave/sensor/base_video.py: Trigger: _monitor_connection's guard was changed to 'if not self._should_reconnect or (self.pc is None and reconnect_attempt == 0)'. Once a reconnect attempt fails (reconnect_attempt increments above 0) and pc is still None, the new condition evaluates to False, so the loop falls through to 'if self.pc is None or self._is_connection_lost()' and immediately calls _attempt_reconnect again without waiting for the 1-second sleep at the bottom — except the sleep IS still at the bottom. However, if _attempt_reconnect returns quickly (e.g., because _should_reconnect is False or stop_event is set), the loop immediately re-evaluates. More critically, the removal of the 'if reconnect_attempt < 0: break' guard means the monitor can no longer exit the reconnect loop when max_attempts is reached; _attempt_reconnect now returns 0 and sets _should_reconnect=False, but on the next iteration the guard condition 'self.pc is None and reconnect_attempt == 0' is True (reconnect_attempt was reset to 0), so the loop sleeps instead of breaking — meaning a later start_video that sets _should_reconnect=True will immediately re-enter reconnect attempts even though the peer connection was never cleanly establ
• Medium · cyberwave/sensor/base_video.py: Trigger: stop() cancels the monitor task in a tight loop: 'while monitor is not None and not monitor.done(): monitor.cancel(); await asyncio.wait({monitor}, timeout=0.1)'. The comment says a cancel can be swallowed while the monitor drains the sync task. However, the sync task (_wait_and_publish_camera_sync_frame) is itself awaited inside the monitor without shielding, and the new code in _close_peer_connection does not cancel the sync task first. If the sync task is long-lived or ignores CancelledError internally (e.g., it catches it for cleanup and re-awaits), the monitor's CancelledError is re-raised out of the inner await only after the sync task finishes, and each 0.1 s window can fire a fresh cancel into the monitor during its cleanup phase, corrupting the cleanup sequence. Consequence: Depending on the sync-task implementation, stop() may return before the peer connection is fully closed (because _close_peer_connection is not awaited again after the monitor exits), or it may take an unexpectedly long time if the sync task is stuck. The test test_stop_during_sync_drain_does_not_hang explicitly documents this risk but works around it by forcibly setting _is_running rather than

Coverage: 3 of 3 changed files; patch excerpts only. Generated REST code, lockfiles, notebooks, SVGs, missing patches and files beyond the 40-file / 60,000-character budget are excluded. GitHub may truncate individual patches.

[deepakpopli]

Rebased on 0.7.3.

• stop() now repeats the monitor cancel until the monitor is done. The new sync-task drain can swallow a cancel, which could leave stop() waiting forever. Added a test for it.
• test_stop_drains_sync_waiter_before_reconnect now checks the sync task directly instead of counting tasks, since start() also starts the reconnect monitor.

Tested with poetry install -E camera: poetry run pytest tests/test_base_video_reconnect_monitor.py tests/test_base_video_shutdown.py gives 15 passed, and poetry run pytest tests/ -n auto passes (4066). These tests skip in CI because aiortc is only in the camera extra.

[KhushiSharma22]

@deepak61296 Hi! Some tests are failing here, let me know if you want me to fix these or if you wanna take a quick run through? Happy to proceed with whatever is the most convenient for you.

[deepakpopli]

hi @KhushiSharma22 i'll take a look and fix it.