RSDK-13862 Add watchdogs to speaker and microphone #39
Open
oliviamiller wants to merge 3 commits intoviam-modules:mainfrom
Open
RSDK-13862 Add watchdogs to speaker and microphone #39oliviamiller wants to merge 3 commits intoviam-modules:mainfrom
oliviamiller wants to merge 3 commits intoviam-modules:mainfrom
Conversation
seanavery
reviewed
May 6, 2026
seanavery
left a comment
seanavery
left a comment
There was a problem hiding this comment.
Looking good! Just got done with my first pass here
| // instead retries every BACKOFF_INTERVAL. This supports hot-replug scenarios: the device | ||
| // may come back later (USB plug-in, driver recovery) and we want to resume recovery | ||
| // without forcing the user to reconfigure. | ||
| constexpr std::chrono::milliseconds BACKOFF_INTERVAL{2000}; |
There was a problem hiding this comment.
Do we want 2 second or 5 second backoff interval?
|
|
||
| // 1. Restart fires when the callback is stale and attempts are below the budget. | ||
| TEST(StallWatchdog, RestartFiresWhenStale) { | ||
| const auto ctx = make_context_stale_by(5000); // 5s past threshold |
There was a problem hiding this comment.
Should this be 2000 to match watchdog implementation?
| // Member variables | ||
| std::string device_name_; | ||
| double latency_; | ||
| int sample_rate_; |
There was a problem hiding this comment.
[maybe nit] should we also remove sample rate now that we have StreamParams attached to the class?
| std::string device_name_; | ||
| double latency_; | ||
| int sample_rate_; | ||
| int num_channels_; |
| } | ||
|
|
||
| private: | ||
| void loop() { |
There was a problem hiding this comment.
Any race condition considerations between reconfigure and this loop that calls restart_stalled_stream?
Basically, do we need to reconsider locking strategy because of restart path moving from get_audio to poll?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a background
StallWatchdogtoSpeakerandMicrophonethat detectswhen the PortAudio callback stops firing and recovers automatically.
Audio callbacks can stop firing for a few reasons — USB device unplugged,
kernel re-enumeration, ALSA xrun escalation, driver hiccup. Before this PR
the only recovery path for the speaker was an inline staleness log (warn-only,
no restart), and the mic's recovery only ran while a
get_audiocall wasactive.
the audio callback last fired; if the gap exceeds the staleness threshold,
it triggers a stream restart.
audio device state, so a USB device that came back at a different card
index is found automatically.
retry cadence instead of giving up permanently — when the device comes
back, recovery happens on its own.
outright to avoid noisy driver-level errors during the wait.
Manual Testing
USB hot-replug: unplug the device, wait, plug it back in. Logs show 3
fast retry attempts, then 5s backoff retries until the replug — at
which point the next retry resolves the new card index and succeeds.