From ec7b2be37745451b1a5541dff48a4df345bbcdba Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Tue, 19 May 2026 15:55:39 -0400 Subject: [PATCH 1/3] Remove Reconfigurable to unblock SDK upgrade The Reconfigurable base class was removed from viam-cpp-sdk (commit 571407760), so we need to drop our use of it before we can bump the SDK. With Reconfigurable gone, a config change now tears down and reconstructs the component instance instead of transitioning in place. While here, simplify the now-redundant context-change handling in Microphone::get_audio: - Drop the `is_reconfigure` parameter on `setup_stream_params` (the only caller now always passes `false`). - Drop the inner `setup_stream_params` re-run on context change. The watchdog restarts a stream with the same format, so chunk sizes and MP3 encoder state are still valid. - Drop `StreamGuard` / `active_streams_` (only used to warn during reconfigure). --- README.md | 28 -------- src/discovery.cpp | 1 - src/discovery.hpp | 1 - src/microphone.cpp | 111 +++----------------------------- src/microphone.hpp | 9 +-- src/mp3_encoder.hpp | 1 - src/speaker.cpp | 62 ++---------------- src/speaker.hpp | 8 +-- test/microphone_test.cpp | 135 --------------------------------------- 9 files changed, 19 insertions(+), 337 deletions(-) diff --git a/README.md b/README.md index 9411969..653efb1 100644 --- a/README.md +++ b/README.md @@ -34,34 +34,6 @@ The following attributes are available for the `viam:audio:microphone` model: | `latency` | int | **Optional** | Suggested input latency in milliseconds. This controls how much audio PortAudio buffers before making it available. Lower values (5-20ms) provide more responsive audio capture but use more CPU time. Higher values (50-100ms) are more stable but less responsive. If not specified, uses the device's default low latency setting (typically 10-20ms). | | `historical_throttle_ms` | int | **Optional** | Delay in milliseconds between chunks when streaming historical audio data using the previous_timestamp parameter (default: 50ms). Gives clients adequate time to process buffered audio data. | -### Reconfiguration Behavior - -The microphone component supports reconfiguration - you can change stream attributes without restarting the audio stream RPC calls. When you reconfigure: - -- Active `get_audio()` calls will automatically transition to the new configuration -- There may be a brief gap in audio during the transition - -#### Important Considerations - -1. **Writing to fixed-format files (WAV, MP3, etc.)** - - WAV files have a fixed header with sample rate and channel count - - Changing `sample_rate` or `num_channels` mid-stream will corrupt the file - - **Solution:** Stop recording, save the file, then reconfigure and start a new file - -2. **During active audio encoding** - - If you're encoding the streamed audio (e.g., to OPUS, AAC), changing `sample_rate` or `num_channels` will break the initialized encoder - - **Solution:** Reinitialize the encoder when reconfigurations occur - -**No client-side handling required:** -- When streaming audio chunks that are processed independently -- Changing `device_name` to switch microphones -- Adjusting `latency` for performance tuning -- Between `get_audio` RPC calls - -**Clients should:** -- Monitor the `audio_info` field in each audio chunk -- Detect when `sample_rate` or `num_channels` changes -- Handle the transition appropriately ## Model viam:audio:speaker ### Configuration diff --git a/src/discovery.cpp b/src/discovery.cpp index ae95a3e..93c287b 100644 --- a/src/discovery.cpp +++ b/src/discovery.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include "microphone.hpp" #include "portaudio.hpp" diff --git a/src/discovery.hpp b/src/discovery.hpp index 6db4685..df3c114 100644 --- a/src/discovery.hpp +++ b/src/discovery.hpp @@ -2,7 +2,6 @@ #include #include -#include #include #include "device_id.hpp" #include "portaudio.hpp" diff --git a/src/microphone.cpp b/src/microphone.cpp index f77ac69..dae3a33 100644 --- a/src/microphone.cpp +++ b/src/microphone.cpp @@ -68,23 +68,6 @@ static int calculate_chunk_size(const audio::codec::AudioCodec codec, } } -// RAII guard to automatically increment and decrement the stream counter -// during get_audio calls -class StreamGuard { - std::mutex& mutex_; - int& counter_; - - public: - StreamGuard(std::mutex& m, int& c) : mutex_(m), counter_(c) { - std::lock_guard lock(mutex_); - counter_++; - } - ~StreamGuard() { - std::lock_guard lock(mutex_); - counter_--; - } -}; - // === Microphone Class Implementation === void Microphone::restart_stalled_stream(const std::shared_ptr& stream_context) { @@ -137,7 +120,6 @@ void Microphone::restart_stalled_stream(const std::shared_ptr Microphone::validate(viam::sdk::ResourceConfig cfg) { return {}; } -void Microphone::reconfigure(const viam::sdk::Dependencies& deps, const viam::sdk::ResourceConfig& cfg) { - VIAM_SDK_LOG(info) << "[reconfigure] Microphone reconfigure start"; - - try { - // Warn if reconfiguring with active streams - // Changing the sample rate or number of channels mid stream - // might cause issues client side, clients need to be actively - // checking the audioinfo for changes. Changing these parameters - // may also cause a small gap in audio. - { - std::lock_guard lock(stream_ctx_mu_); - if (active_streams_ > 0) { - VIAM_SDK_LOG(info) << "[reconfigure] Reconfiguring with " << active_streams_ - << " active stream(s). See README for reconfiguration considerations."; - } - } - - // Close the existing stream before setting up audio device — Pa_IsFormatSupported internally tries to - // open the device, which fails if it's already in use. - { - std::lock_guard lock(stream_ctx_mu_); - if (stream_) { - audio::utils::shutdown_stream(stream_, pa_); - stream_ = nullptr; - } - } - - auto setup = - audio::utils::setup_audio_device(cfg, audio::utils::StreamDirection::Input, AudioCallback, pa_); - - // Set new configuration and restart stream under lock - { - std::lock_guard lock(stream_ctx_mu_); - - stream_params_ = setup.stream_params; - stream_params_.user_data = setup.audio_context.get(); - device_id_ = setup.config_params.device_id; - audio::utils::restart_stream(stream_, stream_params_, pa_); - audio_context_ = setup.audio_context; - restart_attempts_ = 0; - requested_sample_rate_ = setup.config_params.sample_rate.value_or( - setup.stream_params.sample_rate); // User's requested rate, defaults to device rate - historical_throttle_ms_ = setup.config_params.historical_throttle_ms.value_or(DEFAULT_HISTORICAL_THROTTLE_MS); - } - VIAM_SDK_LOG(info) << "[reconfigure] Reconfigure completed successfully"; - } catch (const std::exception& e) { - VIAM_SDK_LOG(error) << "[reconfigure] Reconfigure failed: " << e.what(); - throw; - } -} - viam::sdk::ProtoStruct Microphone::do_command(const viam::sdk::ProtoStruct& command) { VIAM_SDK_LOG(error) << "do_command not implemented"; return viam::sdk::ProtoStruct(); @@ -377,9 +304,6 @@ void Microphone::get_audio(std::string const& codec, // Parse codec string to enum const AudioCodec codec_enum = audio::codec::parse_codec(codec); - // guard to increment and decrement the active stream count - StreamGuard stream_guard(stream_ctx_mu_, active_streams_); - // Track audio duration using timestamps int64_t first_chunk_start_timestamp_ns = 0; bool duration_limit_set = false; @@ -419,7 +343,6 @@ void Microphone::get_audio(std::string const& codec, // Setup initial stream parameters and initialize encoder setup_stream_params(codec_enum, mp3_ctx, - false, stream_sample_rate, requested_sample_rate, stream_num_channels, @@ -428,43 +351,25 @@ void Microphone::get_audio(std::string const& codec, device_samples_per_chunk); while (true) { - // Check if audio_context_ changed - bool context_changed = false; PaStream* current_stream = nullptr; { std::lock_guard lock(stream_ctx_mu_); - // Detect context change (device reconfigured or stream restarted) + // The watchdog may have restarted a stalled stream, which swaps in a + // fresh audio_context_ (same format, fresh circular buffer). Skip any + // stale data from the old buffer and reset diagnostic counters. if (audio_context_ != stream_context) { - if (stream_context != nullptr) { - VIAM_SDK_LOG(info) << "Detected stream change"; - context_changed = true; - } - // Switch to new context and reset read position + VIAM_SDK_LOG(info) << "Detected stream change"; stream_context = audio_context_; read_position = stream_context->get_write_position(); restart_attempts_ = 0; - // Brief gap in audio, but stream continues + last_logged_overflow_count = 0; + last_logged_underflow_count = 0; + last_staleness_log_ns = 0; } current_stream = stream_; } - // Reconfigure stream parameters if context changed - if (context_changed) { - setup_stream_params(codec_enum, - mp3_ctx, - true, - stream_sample_rate, - requested_sample_rate, - stream_num_channels, - stream_historical_throttle_ms, - samples_per_chunk, - device_samples_per_chunk); - last_logged_overflow_count = 0; - last_logged_underflow_count = 0; - last_staleness_log_ns = 0; - } - // Check if we have enough samples for a full chunk const uint64_t write_pos = stream_context->get_write_position(); const uint64_t available_samples = write_pos - read_position; diff --git a/src/microphone.hpp b/src/microphone.hpp index 81e48bb..c679485 100644 --- a/src/microphone.hpp +++ b/src/microphone.hpp @@ -9,7 +9,6 @@ #include #include #include -#include #include "audio_codec.hpp" #include "audio_stream.hpp" #include "audio_utils.hpp" @@ -33,7 +32,7 @@ PaDeviceIndex findDeviceByName(const std::string& name, const audio::portaudio:: // Returns the write position if previous_timestamp == 0 (default: most recent audio) uint64_t get_initial_read_position(const std::shared_ptr& stream_context, int64_t previous_timestamp); -class Microphone final : public viam::sdk::AudioIn, public viam::sdk::Reconfigurable { +class Microphone final : public viam::sdk::AudioIn { public: Microphone(viam::sdk::Dependencies deps, viam::sdk::ResourceConfig cfg, audio::portaudio::PortAudioInterface* pa = nullptr); @@ -51,7 +50,6 @@ class Microphone final : public viam::sdk::AudioIn, public viam::sdk::Reconfigur viam::sdk::audio_properties get_properties(const viam::sdk::ProtoStruct& extra); std::vector get_geometries(const viam::sdk::ProtoStruct& extra); - void reconfigure(const viam::sdk::Dependencies& deps, const viam::sdk::ResourceConfig& cfg); // Restarts the stream. // Must NOT be called while holding stream_ctx_mu_. @@ -59,7 +57,6 @@ class Microphone final : public viam::sdk::AudioIn, public viam::sdk::Reconfigur void setup_stream_params(audio::codec::AudioCodec codec_enum, MP3EncoderContext& mp3_ctx, - bool is_reconfigure, int& stream_sample_rate, int& requested_sample_rate, int& stream_num_channels, @@ -72,14 +69,12 @@ class Microphone final : public viam::sdk::AudioIn, public viam::sdk::Reconfigur int historical_throttle_ms_; // Throttle time for historical data stream static vsdk::Model model; - // The mutex protects the stream, context, and the active streams counter + // The mutex protects the stream and context std::mutex stream_ctx_mu_; PaStream* stream_; std::shared_ptr audio_context_; // This is null in production and used for testing to inject the mock portaudio functions const audio::portaudio::PortAudioInterface* pa_; - // Count of active get_audio calls - int active_streams_; int restart_attempts_; audio::utils::StreamParams stream_params_; diff --git a/src/mp3_encoder.hpp b/src/mp3_encoder.hpp index b963483..d1c5d6e 100644 --- a/src/mp3_encoder.hpp +++ b/src/mp3_encoder.hpp @@ -5,7 +5,6 @@ #include #include #include -#include #include "audio_utils.hpp" namespace microphone { diff --git a/src/speaker.cpp b/src/speaker.cpp index a1bb854..4f2df73 100644 --- a/src/speaker.cpp +++ b/src/speaker.cpp @@ -79,12 +79,11 @@ vsdk::Model Speaker::model = {"viam", "system-audio", "speaker"}; * Tear down the existing stream and bring up a fresh one with the saved params. * * Bails out if `playback_context` is no longer the active audio_context_ — that means - * either reconfigure() or another stall recovery already replaced it, so the in-flight - * play() call (if any) is about to exit on its own and we shouldn't race. + * another stall recovery already replaced it, so the in-flight play() call (if any) + * is about to exit on its own and we shouldn't race. * - * On a successful restart, any unplayed audio in the old buffer is discarded — same - * behavior as reconfigure(). The in-flight play() loop will see audio_context_ change - * and return early. + * On a successful restart, any unplayed audio in the old buffer is discarded. The + * in-flight play() loop will see audio_context_ change and return early. */ void Speaker::restart_stalled_stream(const std::shared_ptr& playback_context) { std::lock_guard lock(stream_mu_); @@ -447,12 +446,12 @@ void Speaker::play(std::vector const& audio_data, VIAM_SDK_LOG(debug) << "Playback stopped by stop command"; return; } - // Check if context changed (reconfigure happened) + // Check if context changed (stream restarted by watchdog) PaStream* current_stream = nullptr; { std::lock_guard lock(stream_mu_); if (audio_context_ != playback_context) { - VIAM_SDK_LOG(debug) << "Audio playback interrupted by reconfigure, exiting"; + VIAM_SDK_LOG(debug) << "Audio playback interrupted by stream restart, exiting"; return; } current_stream = stream_; @@ -504,53 +503,4 @@ std::vector Speaker::get_geometries(const viam::sdk:: throw std::runtime_error("get_geometries is unimplemented"); } -void Speaker::reconfigure(const vsdk::Dependencies& deps, const vsdk::ResourceConfig& cfg) { - VIAM_SDK_LOG(info) << "[reconfigure] Speaker reconfigure start"; - - try { - // Check if there's unplayed audio before reconfiguring - { - std::lock_guard lock(stream_mu_); - if (audio_context_) { - const uint64_t write_pos = audio_context_->get_write_position(); - const uint64_t playback_pos = audio_context_->playback_position.load(); - - if (write_pos > playback_pos) { - const uint64_t unplayed_samples = write_pos - playback_pos; - const double unplayed_seconds = - static_cast(unplayed_samples) / (audio_context_->info.sample_rate_hz * audio_context_->info.num_channels); - VIAM_SDK_LOG(warn) << "[reconfigure] Discarding " << unplayed_seconds << " seconds of unplayed audio"; - } - } - } - - auto setup = audio::utils::setup_audio_device( - cfg, audio::utils::StreamDirection::Output, speakerCallback, pa_, audio::BUFFER_DURATION_SECONDS); - - // Set new configuration and restart stream under lock - { - std::lock_guard lock(stream_mu_); - - // Stop the stream first efore replacing audio_context_ - // Otherwise the callback thread may still be accessing the old context - // after we destroy it (heap-use-after-free) - setup.stream_params.user_data = setup.audio_context.get(); - stream_params_ = setup.stream_params; - audio::utils::restart_stream(stream_, stream_params_, pa_); - latency_ = audio::utils::get_stream_latency(stream_, stream_params_, pa_); - audio_context_ = setup.audio_context; - device_id_ = setup.config_params.device_id; - restart_attempts_ = 0; - volume_ = setup.config_params.volume; - if (volume_) { - audio::volume::set_volume(stream_params_.device_name, *volume_); - } - } - VIAM_SDK_LOG(info) << "[reconfigure] Reconfigure completed successfully"; - } catch (const std::exception& e) { - VIAM_SDK_LOG(error) << "[reconfigure] Reconfigure failed: " << e.what(); - throw; - } -} - } // namespace speaker diff --git a/src/speaker.hpp b/src/speaker.hpp index f810ec5..7c9c5ff 100644 --- a/src/speaker.hpp +++ b/src/speaker.hpp @@ -10,7 +10,6 @@ #include #include #include -#include #include "audio_stream.hpp" #include "audio_utils.hpp" #include "portaudio.h" @@ -43,7 +42,7 @@ int speakerCallback(const void* inputBuffer, PaStreamCallbackFlags statusFlags, void* userData); -class Speaker final : public viam::sdk::AudioOut, public viam::sdk::Reconfigurable { +class Speaker final : public viam::sdk::AudioOut { public: Speaker(viam::sdk::Dependencies deps, viam::sdk::ResourceConfig cfg, audio::portaudio::PortAudioInterface* pa = nullptr); @@ -57,7 +56,6 @@ class Speaker final : public viam::sdk::AudioOut, public viam::sdk::Reconfigurab viam::sdk::audio_properties get_properties(const viam::sdk::ProtoStruct& extra); std::vector get_geometries(const viam::sdk::ProtoStruct& extra); - void reconfigure(const viam::sdk::Dependencies& deps, const viam::sdk::ResourceConfig& cfg); // Member variables double latency_; @@ -87,8 +85,8 @@ class Speaker final : public viam::sdk::AudioOut, public viam::sdk::Reconfigurab // PortAudio index, so we recover from kernel re-enumeration (e.g. USB unplug/replug). std::string device_id_; - // Counts consecutive failed restart attempts; reset to 0 after a successful restart - // or a reconfigure(). Once it reaches audio::utils::MAX_RESTART_ATTEMPTS, the watchdog + // Counts consecutive failed restart attempts; reset to 0 after a successful restart. + // Once it reaches audio::utils::MAX_RESTART_ATTEMPTS, the watchdog // backs off to slow retries (audio::utils::BACKOFF_INTERVAL) instead of polling // every audio::utils::POLL_INTERVAL — supports hot-replug recovery without spamming // the kernel. The counter is capped at MAX so it doesn't grow unbounded. diff --git a/test/microphone_test.cpp b/test/microphone_test.cpp index 3016d0e..33e9a4d 100644 --- a/test/microphone_test.cpp +++ b/test/microphone_test.cpp @@ -467,141 +467,6 @@ TEST_F(MicrophoneTest, DefaultDeviceNotFoundThrows) { EXPECT_THROW(microphone::Microphone(test_deps_, config, mock_pa_.get()), std::runtime_error); } -TEST_F(MicrophoneTest, ReconfigureDifferentDeviceName) { - auto config = createConfig(testDeviceName, 44100, 2); - expectSuccessfulStreamCreation(); - microphone::Microphone mic(test_deps_, config, mock_pa_.get()); - const char* new_device_name = "New Device"; - PaStream* dummy_stream = reinterpret_cast(0x1234); - - // Reconfigure with different device name - auto new_config = createConfig(new_device_name, 22000, 2); - - PaDeviceInfo new_device; - new_device.name = new_device_name; - new_device.maxInputChannels = 2; - new_device.defaultLowInputLatency = 0.01; - new_device.defaultSampleRate = test_utils::DEFAULT_DEVICE_SAMPLE_RATE; - - EXPECT_CALL(*mock_pa_, stopStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - EXPECT_CALL(*mock_pa_, closeStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - EXPECT_CALL(*mock_pa_, getDeviceCount()).WillOnce(::testing::Return(2)); - EXPECT_CALL(*mock_pa_, getDeviceInfo(1)).WillRepeatedly(::testing::Return(&new_device)); - EXPECT_CALL(*mock_pa_, openStream(::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_)) - .WillOnce(::testing::DoAll(::testing::SetArgPointee<0>(dummy_stream), ::testing::Return(paNoError))); - EXPECT_CALL(*mock_pa_, startStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - - EXPECT_NO_THROW(mic.reconfigure(test_deps_, new_config)); - - EXPECT_EQ(mic.stream_params_.device_name, new_device_name); - EXPECT_EQ(mic.stream_params_.sample_rate, 22000); - EXPECT_EQ(mic.requested_sample_rate_, 22000); - EXPECT_EQ(mic.stream_params_.num_channels, 2); -} - - -TEST_F(MicrophoneTest, ReconfigureDifferentSampleRate) { - auto config = createConfig(testDeviceName, 44100, 2); - expectSuccessfulStreamCreation(); - microphone::Microphone mic(test_deps_, config, mock_pa_.get()); - - // reconfigure with different sample rate - auto new_config = createConfig(testDeviceName, 2000, 2); - PaStream* dummy_stream = reinterpret_cast(0x1234); - - - EXPECT_CALL(*mock_pa_, stopStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - EXPECT_CALL(*mock_pa_, closeStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - EXPECT_CALL(*mock_pa_, getDeviceCount()).WillOnce(::testing::Return(1)); - EXPECT_CALL(*mock_pa_, getDeviceInfo(0)).WillRepeatedly(::testing::Return(&mock_device_info_)); - EXPECT_CALL(*mock_pa_, openStream(::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_)) - .WillOnce(::testing::DoAll(::testing::SetArgPointee<0>(dummy_stream), ::testing::Return(paNoError))); - EXPECT_CALL(*mock_pa_, startStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - - - EXPECT_NO_THROW(mic.reconfigure(test_deps_, new_config)); - - EXPECT_EQ(mic.stream_params_.device_name, testDeviceName); - EXPECT_EQ(mic.requested_sample_rate_, 2000); - EXPECT_EQ(mic.stream_params_.num_channels, 2); -} - - -TEST_F(MicrophoneTest, ReconfigureDifferentNumChannels) { - auto config = createConfig(testDeviceName, 44100, 2); - expectSuccessfulStreamCreation(); - microphone::Microphone mic(test_deps_, config, mock_pa_.get()); - PaStream* dummy_stream = reinterpret_cast(0x1234); - - // Reconfigure with different num channels - auto new_config = createConfig(testDeviceName, 44100, 1); - - - EXPECT_CALL(*mock_pa_, stopStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - EXPECT_CALL(*mock_pa_, closeStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - EXPECT_CALL(*mock_pa_, getDeviceCount()).WillOnce(::testing::Return(2)); - EXPECT_CALL(*mock_pa_, getDeviceInfo(1)).WillRepeatedly(::testing::Return(&mock_device_info_)); - EXPECT_CALL(*mock_pa_, openStream(::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_)) - .WillOnce(::testing::DoAll(::testing::SetArgPointee<0>(dummy_stream), ::testing::Return(paNoError))); - EXPECT_CALL(*mock_pa_, startStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - - EXPECT_NO_THROW(mic.reconfigure(test_deps_, new_config)); - - EXPECT_EQ(mic.stream_params_.device_name, testDeviceName); - EXPECT_EQ(mic.stream_params_.sample_rate, test_utils::DEFAULT_DEVICE_SAMPLE_RATE); - EXPECT_EQ(mic.stream_params_.num_channels, 1); -} - -TEST_F(MicrophoneTest, ReconfigureChangesAudioContext) { - auto config = createConfig(testDeviceName, 44100, 1); - expectSuccessfulStreamCreation(); - microphone::Microphone mic(test_deps_, config, mock_pa_.get()); - - // Get the initial audio_context_ pointer and verify its properties - auto initial_context = mic.audio_context_; - ASSERT_NE(initial_context, nullptr); - EXPECT_EQ(initial_context->info.sample_rate_hz, test_utils::DEFAULT_DEVICE_SAMPLE_RATE); - EXPECT_EQ(initial_context->info.num_channels, 1); - EXPECT_EQ(initial_context->info.codec, viam::sdk::audio_codecs::PCM_16); - - // Write some samples to the initial context - for (int i = 0; i < 100; i++) { - initial_context->write_sample(static_cast(i)); - } - EXPECT_EQ(initial_context->get_write_position(), 100); - - // Reconfigure with different sample rate and channels - auto new_config = createConfig(testDeviceName, 48000, 2); - PaStream* dummy_stream = reinterpret_cast(0x1234); - - // Setup expectations for reconfigure (shutdown old stream, open new stream) - EXPECT_CALL(*mock_pa_, stopStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - EXPECT_CALL(*mock_pa_, closeStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - EXPECT_CALL(*mock_pa_, getDeviceCount()).WillOnce(::testing::Return(1)); - EXPECT_CALL(*mock_pa_, getDeviceInfo(0)).WillRepeatedly(::testing::Return(&mock_device_info_)); - EXPECT_CALL(*mock_pa_, openStream(::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_)) - .WillOnce(::testing::DoAll(::testing::SetArgPointee<0>(dummy_stream), ::testing::Return(paNoError))); - EXPECT_CALL(*mock_pa_, startStream(::testing::_)).WillOnce(::testing::Return(paNoError)); - - EXPECT_NO_THROW(mic.reconfigure(test_deps_, new_config)); - - // Verify that audio_context_ was replaced with a new instance - auto new_context = mic.audio_context_; - ASSERT_NE(new_context, nullptr); - EXPECT_NE(new_context, initial_context); - - // audio context will have the natively supported requested sample rate - EXPECT_EQ(new_context->info.sample_rate_hz, 48000); - EXPECT_EQ(new_context->info.num_channels, 2); - EXPECT_EQ(new_context->info.codec, viam::sdk::audio_codecs::PCM_16); - - // Verify new context starts fresh (no samples written yet) - EXPECT_EQ(new_context->get_write_position(), 0); - - // Verify old context still exists with its data (kept alive by shared_ptr) - EXPECT_EQ(initial_context->get_write_position(), 100); -} - TEST_F(MicrophoneTest, MultipleConcurrentGetAudioCalls) { auto config = createConfig(testDeviceName, 44100, 2); microphone::Microphone mic(test_deps_, config, mock_pa_.get()); From 9f1f53447a0b406a1934a4c036f82e6c4f5144ee Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 20 May 2026 15:08:43 -0400 Subject: [PATCH 2/3] pr comments --- src/watchdog.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/watchdog.hpp b/src/watchdog.hpp index fd852cc..ef8f1c7 100644 --- a/src/watchdog.hpp +++ b/src/watchdog.hpp @@ -20,7 +20,7 @@ constexpr std::chrono::milliseconds STALL_THRESHOLD{2000}; // Once the attempts budget is exhausted, the watchdog stops attempting fast restarts and // 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. +// without requiring a config change. constexpr std::chrono::milliseconds BACKOFF_INTERVAL{2000}; // Background watchdog that polls an audio component's `last_callback_time_ns` and @@ -38,12 +38,12 @@ class StallWatchdog { // Returns the current consecutive-failed-restart count. Watchdog stops calling // restart_fn once this reaches MAX_RESTART_ATTEMPTS; the latch clears automatically - // once the count drops back below max (e.g. after a reconfigure). + // once the count drops back below max (e.g. after a successful restart). using GetAttemptsFn = std::function; // Performs the restart. Argument is the same context get_context returned a moment - // earlier — the component should bail if it has since been swapped (reconfigure / - // peer restart). + // earlier — the component should bail if it has since been swapped by a prior + // restart. using RestartFn = std::function&)>; StallWatchdog(GetContextFn get_context, GetAttemptsFn get_attempts, RestartFn restart_fn, std::string log_prefix) @@ -101,7 +101,7 @@ class StallWatchdog { if (attempts >= MAX_RESTART_ATTEMPTS) { // Budget exhausted — back off to slow retries instead of giving up // permanently, so hot-replug (device unplugged then plugged back in) - // recovers automatically without a reconfigure. + // recovers automatically without requiring a config change. const auto since_last = std::chrono::milliseconds((now_ns - last_attempt_ns_.load()) / audio::NS_PER_MS); if (since_last < BACKOFF_INTERVAL) { if (!backoff_logged_.exchange(true)) { From 441b7a39b7a10c3c494be8fc2036a34a0497344c Mon Sep 17 00:00:00 2001 From: oliviamiller <106617921+oliviamiller@users.noreply.github.com> Date: Wed, 20 May 2026 15:10:06 -0400 Subject: [PATCH 3/3] readme --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 653efb1..616846e 100644 --- a/README.md +++ b/README.md @@ -129,6 +129,10 @@ All audio data uses **little-endian** byte order. The specific format depends on - **Microphone (`get_audio`)**: Returns audio data in interleaved format - **Speaker (`play`)**: Expects audio data in interleaved format +## Reconfigure Behavior + +Any config change terminates in-flight streams. Callers must handle the error +and resubmit the request to resume. ## Setup ```bash