From 8b2d197af78f26663c78b26b2b5080d228df20d1 Mon Sep 17 00:00:00 2001 From: Ixitxachitl Date: Tue, 25 Aug 2026 18:10:35 +0000 Subject: [PATCH 1/5] fix(nrf52): drive the PWM RTTTL sequencer from a FreeRTOS timer The NonBlockingRTTTL sequencer was polled from ExternalNotificationModule's cooperative thread every 25 ms, so the start of each note waited behind whatever the main loop was doing. On boards whose display path stalls the loop for tens of milliseconds per frame (TFT_eSPI on nRF52 pushes each pixel as two blocking single-byte SPI transactions) that lands as audible gaps between notes exactly while the notification banner is being drawn. tone() is hardware timed on nRF52, so only note starts need servicing. Run rtttl::play() from a 2 ms FreeRTOS software timer; begin()/stop() stay on the main thread under a mutex, and the timer stops itself when the song ends. Falls back to main-loop polling if the timer cannot be created. Other architectures are unchanged. --- src/modules/ExternalNotificationModule.cpp | 42 +++++++++-- src/platform/nrf52/NRF52RtttlTicker.cpp | 82 ++++++++++++++++++++++ src/platform/nrf52/NRF52RtttlTicker.h | 21 ++++++ 3 files changed, 141 insertions(+), 4 deletions(-) create mode 100644 src/platform/nrf52/NRF52RtttlTicker.cpp create mode 100644 src/platform/nrf52/NRF52RtttlTicker.h diff --git a/src/modules/ExternalNotificationModule.cpp b/src/modules/ExternalNotificationModule.cpp index 420697689f4..e680cc37c19 100644 --- a/src/modules/ExternalNotificationModule.cpp +++ b/src/modules/ExternalNotificationModule.cpp @@ -44,6 +44,9 @@ bool ascending = true; #if defined(HAS_I2S_SPEAKER_NRF52) #include "platform/nrf52/NRF52RtttlPlayer.h" #endif +#ifdef ARCH_NRF52 +#include "platform/nrf52/NRF52RtttlTicker.h" +#endif /* Documentation: @@ -60,6 +63,35 @@ bool ascending = true; #define EXT_NOTIFICATION_FAST_THREAD_MS 25 +// The PWM buzzer sequencer is normally polled from this cooperative thread, so a slow display refresh +// delays the next note. nRF52 runs it from a FreeRTOS timer instead (NRF52RtttlTicker). +static void pwmRtttlBegin(uint8_t pin, const char *song) +{ +#ifdef ARCH_NRF52 + NRF52RtttlTicker::begin(pin, song); +#else + rtttl::begin(pin, song); +#endif +} + +static void pwmRtttlPump() +{ +#ifdef ARCH_NRF52 + NRF52RtttlTicker::pump(); +#else + rtttl::play(); +#endif +} + +static void pwmRtttlStop() +{ +#ifdef ARCH_NRF52 + NRF52RtttlTicker::stop(); +#else + rtttl::stop(); +#endif +} + #define ASCII_BELL 0x07 meshtastic_RTTTLConfig rtttlConfig; @@ -78,6 +110,8 @@ int32_t ExternalNotificationModule::runOnce() return INT32_MAX; // we don't need this thread here... } else { uint32_t delay = EXT_NOTIFICATION_MODULE_OUTPUT_MS; + // Racy by design: the sequencer's flag is one byte, stale only for a cycle at song end, which + // just defers stopNow(). Locking it would block this loop on the timer task it hands work to. bool isRtttlPlaying = rtttl::isPlaying(); #ifdef HAS_I2S // audioThread->isPlaying() also handles actually playing the RTTTL, needs to be called in loop @@ -172,10 +206,10 @@ int32_t ExternalNotificationModule::runOnce() // now let the PWM buzzer play if (moduleConfig.external_notification.use_pwm && config.device.buzzer_gpio && canBuzz() && buzzerShouldAlert) { if (rtttl::isPlaying()) { - rtttl::play(); + pwmRtttlPump(); } else if (isNagging && !Throttle::deadlinePassed(nagCycleCutoff)) { // start the song again if we have time left - rtttl::begin(config.device.buzzer_gpio, rtttlConfig.ringtone); + pwmRtttlBegin(config.device.buzzer_gpio, rtttlConfig.ringtone); } // we need fast updates to play the RTTTL delay = EXT_NOTIFICATION_FAST_THREAD_MS; @@ -283,7 +317,7 @@ void ExternalNotificationModule::stopNow() { LOG_INFO("Turning off external notification: "); LOG_INFO("Stop RTTTL playback"); - rtttl::stop(); + pwmRtttlStop(); #ifdef HAS_I2S LOG_INFO("Stop audioThread playback"); audioThread->stop(); @@ -489,7 +523,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const meshtastic_MeshP audioThread->beginRttl(rtttlConfig.ringtone, strlen_P(rtttlConfig.ringtone)); #endif } else if (moduleConfig.external_notification.use_pwm) { - rtttl::begin(config.device.buzzer_gpio, rtttlConfig.ringtone); + pwmRtttlBegin(config.device.buzzer_gpio, rtttlConfig.ringtone); } else { setExternalState(2, true); } diff --git a/src/platform/nrf52/NRF52RtttlTicker.cpp b/src/platform/nrf52/NRF52RtttlTicker.cpp new file mode 100644 index 00000000000..cae75486965 --- /dev/null +++ b/src/platform/nrf52/NRF52RtttlTicker.cpp @@ -0,0 +1,82 @@ +#include "NRF52RtttlTicker.h" + +#ifdef ARCH_NRF52 + +#include "DebugConfiguration.h" +#include "freertosinc.h" +#include +#include + +namespace NRF52RtttlTicker +{ +namespace +{ +constexpr uint32_t kTickMs = 2; + +TimerHandle_t timer = nullptr; +SemaphoreHandle_t lock = nullptr; +// True only while the timer is servicing a song; pump() polls from the main loop whenever it is not. +bool timerRunning = false; + +void onTick(TimerHandle_t) +{ + // The main thread holds the lock only across begin()/stop(); skip this tick rather than block the timer task. + if (xSemaphoreTake(lock, 0) != pdTRUE) + return; + if (rtttl::isPlaying()) + rtttl::play(); + else + xTimerStop(timer, 0); // song finished on its own + xSemaphoreGive(lock); +} + +bool ensureInit() +{ + if (timer) + return true; + if (!lock) + lock = xSemaphoreCreateMutex(); + if (lock) + timer = xTimerCreate("rtttl", pdMS_TO_TICKS(kTickMs), pdTRUE, nullptr, onTick); + if (!timer) + LOG_ERROR("RTTTL timer unavailable, falling back to main-loop playback"); + return timer != nullptr; +} +} // namespace + +void begin(uint8_t pin, const char *song) +{ + if (!ensureInit()) { + rtttl::begin(pin, song); + return; + } + xSemaphoreTake(lock, portMAX_DELAY); + rtttl::begin(pin, song); + xSemaphoreGive(lock); + // A start rejected by a full timer command queue must fall back to polling, or the song never advances. + timerRunning = xTimerStart(timer, pdMS_TO_TICKS(10)) == pdPASS; + if (!timerRunning) + LOG_WARN("RTTTL timer start rejected, falling back to main-loop playback"); +} + +void pump() +{ + if (!timerRunning && rtttl::isPlaying()) + rtttl::play(); +} + +void stop() +{ + if (!timer) { + rtttl::stop(); + return; + } + xTimerStop(timer, pdMS_TO_TICKS(10)); + timerRunning = false; + xSemaphoreTake(lock, portMAX_DELAY); + rtttl::stop(); + xSemaphoreGive(lock); +} +} // namespace NRF52RtttlTicker + +#endif diff --git a/src/platform/nrf52/NRF52RtttlTicker.h b/src/platform/nrf52/NRF52RtttlTicker.h new file mode 100644 index 00000000000..d0b7f7902b5 --- /dev/null +++ b/src/platform/nrf52/NRF52RtttlTicker.h @@ -0,0 +1,21 @@ +#pragma once + +#include "configuration.h" + +#ifdef ARCH_NRF52 + +#include + +// Advances the NonBlockingRTTTL sequencer from a FreeRTOS timer, so the next note does not wait +// behind the cooperative main loop. tone() is hardware timed, so only note starts need servicing. +namespace NRF52RtttlTicker +{ +void begin(uint8_t pin, const char *song); + +// Only advances the song if the timer could not be created or started; otherwise a no-op. +void pump(); + +void stop(); +} // namespace NRF52RtttlTicker + +#endif From 38d845bd3789ae5cd3a1f0cb0b5bdf045583ccb5 Mon Sep 17 00:00:00 2001 From: Ixitxachitl Date: Thu, 27 Aug 2026 12:42:26 +0000 Subject: [PATCH 2/5] fix(nrf52): keep system tones off the buzzer while a ringtone plays Review follow-ups for the RTTTL FreeRTOS timer. tone() on nRF52 mutates a file-static _pwm_config and drives PWM2 through a non-atomic stopPlayback -> initializeFromPulseCountAndTimePeriod -> applyConfiguration -> startPlayback sequence. The ticker's mutex covers NonBlockingRTTTL's state, not that, so a main-thread playTones() caller that is not routed through InputBroker's nagging() early return (GPS lock/sleep beeps, InkHUD chirps) can now interleave with the timer callback and trigger a SEQSTART for a config the registers no longer hold. Bail out of playTones() while a ringtone is playing; that also fixes the pre-existing same-thread case where a system beep stomped a ringtone note. Also: 5 ms instead of 2 ms (the stall being fixed is tens of milliseconds, and this cuts the daemon wake rate by 2.5x), with a static_assert so the period can never round down to zero ticks and degrade to polling unnoticed; one-shot the timer-creation error so nag restarts do not repeat it; report the timer daemon's stack high water mark once per boot, since onTick -> tone is now the deepest chain on its 1 KB stack; and cast away the xTimerStop() return with a note on why a rejected stop is harmless. --- src/buzz/buzz.cpp | 9 ++++++++ src/platform/nrf52/NRF52RtttlTicker.cpp | 29 ++++++++++++++++++++----- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/buzz/buzz.cpp b/src/buzz/buzz.cpp index 63f4e9b13b2..ed6195f06d2 100644 --- a/src/buzz/buzz.cpp +++ b/src/buzz/buzz.cpp @@ -2,6 +2,11 @@ #include "NodeDB.h" #include "configuration.h" +#if !defined(ARCH_PORTDUINO) && !defined(ARCH_STM32WL) +#include +#define HAS_PWM_RTTTL 1 +#endif + #if !defined(ARCH_ESP32) && !defined(ARCH_RP2040) && !defined(ARCH_PORTDUINO) #include "Tone.h" #endif @@ -113,6 +118,10 @@ void playTones(const ToneDuration *tone_durations, int size) // Buzzer is disabled or not set to system tones return; } +#ifdef HAS_PWM_RTTTL + if (rtttl::isPlaying()) + return; // a notification ringtone owns the buzzer, don't reprogram PWM under it +#endif #ifdef HAS_I2S if (moduleConfig.external_notification.use_i2s_as_buzzer && audioThread) { playTonesRTTTL(tone_durations, size); diff --git a/src/platform/nrf52/NRF52RtttlTicker.cpp b/src/platform/nrf52/NRF52RtttlTicker.cpp index cae75486965..bdcbe75043a 100644 --- a/src/platform/nrf52/NRF52RtttlTicker.cpp +++ b/src/platform/nrf52/NRF52RtttlTicker.cpp @@ -11,22 +11,36 @@ namespace NRF52RtttlTicker { namespace { -constexpr uint32_t kTickMs = 2; +// The stall this works around is tens of milliseconds, so 5 ms is inaudible and keeps the timer +// daemon at 200 wakeups/s. Must stay above one tick (configTICK_RATE_HZ is 1024 here) or +// pdMS_TO_TICKS() rounds to zero, xTimerCreate() fails, and we silently drop back to polling. +constexpr uint32_t kTickMs = 5; +static_assert(kTickMs * configTICK_RATE_HZ >= 1000, "kTickMs rounds to zero ticks"); TimerHandle_t timer = nullptr; SemaphoreHandle_t lock = nullptr; // True only while the timer is servicing a song; pump() polls from the main loop whenever it is not. bool timerRunning = false; +bool reportedInitFailure = false; +bool reportedStackMargin = false; void onTick(TimerHandle_t) { // The main thread holds the lock only across begin()/stop(); skip this tick rather than block the timer task. if (xSemaphoreTake(lock, 0) != pdTRUE) return; - if (rtttl::isPlaying()) + if (rtttl::isPlaying()) { rtttl::play(); - else + } else { + // onTick -> rtttl::play -> tone -> applyConfiguration is the deepest chain on the timer + // daemon's 1 KB stack, which is shared with Bluefruit. Report the margin once per boot. + if (!reportedStackMargin) { + reportedStackMargin = true; + LOG_DEBUG("RTTTL timer daemon stack free: %u words", + (unsigned)uxTaskGetStackHighWaterMark(xTimerGetTimerDaemonTaskHandle())); + } xTimerStop(timer, 0); // song finished on its own + } xSemaphoreGive(lock); } @@ -38,8 +52,11 @@ bool ensureInit() lock = xSemaphoreCreateMutex(); if (lock) timer = xTimerCreate("rtttl", pdMS_TO_TICKS(kTickMs), pdTRUE, nullptr, onTick); - if (!timer) + if (!timer && !reportedInitFailure) { + // begin() runs again on every nag restart, so only complain the first time. + reportedInitFailure = true; LOG_ERROR("RTTTL timer unavailable, falling back to main-loop playback"); + } return timer != nullptr; } } // namespace @@ -71,7 +88,9 @@ void stop() rtttl::stop(); return; } - xTimerStop(timer, pdMS_TO_TICKS(10)); + // Unlike the start, a rejected stop is harmless: rtttl::stop() below clears the playing flag, + // so the next onTick() stops the timer itself. + (void)xTimerStop(timer, pdMS_TO_TICKS(10)); timerRunning = false; xSemaphoreTake(lock, portMAX_DELAY); rtttl::stop(); From 6408bcd1242cebf05ec9c185a6772421e3b92c11 Mon Sep 17 00:00:00 2001 From: Ixitxachitl Date: Thu, 27 Aug 2026 12:50:14 +0000 Subject: [PATCH 3/5] fix(nrf52): record the measured RTTTL timer daemon stack margin uxTaskGetStackHighWaterMark(xTimerGetTimerDaemonTaskHandle()) on a T-Echo Plus reports 195 of 256 words free after a ringtone, so onTick -> tone peaks at 61 words. Drop the one-shot probe and keep the figure as a comment. --- src/platform/nrf52/NRF52RtttlTicker.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/platform/nrf52/NRF52RtttlTicker.cpp b/src/platform/nrf52/NRF52RtttlTicker.cpp index bdcbe75043a..006ab2c5f60 100644 --- a/src/platform/nrf52/NRF52RtttlTicker.cpp +++ b/src/platform/nrf52/NRF52RtttlTicker.cpp @@ -22,25 +22,18 @@ SemaphoreHandle_t lock = nullptr; // True only while the timer is servicing a song; pump() polls from the main loop whenever it is not. bool timerRunning = false; bool reportedInitFailure = false; -bool reportedStackMargin = false; void onTick(TimerHandle_t) { // The main thread holds the lock only across begin()/stop(); skip this tick rather than block the timer task. if (xSemaphoreTake(lock, 0) != pdTRUE) return; - if (rtttl::isPlaying()) { + // onTick -> rtttl::play -> tone -> applyConfiguration is the deepest chain on the timer daemon's + // 256 word stack, shared with Bluefruit; measured 61 words peak on a T-Echo Plus. + if (rtttl::isPlaying()) rtttl::play(); - } else { - // onTick -> rtttl::play -> tone -> applyConfiguration is the deepest chain on the timer - // daemon's 1 KB stack, which is shared with Bluefruit. Report the margin once per boot. - if (!reportedStackMargin) { - reportedStackMargin = true; - LOG_DEBUG("RTTTL timer daemon stack free: %u words", - (unsigned)uxTaskGetStackHighWaterMark(xTimerGetTimerDaemonTaskHandle())); - } + else xTimerStop(timer, 0); // song finished on its own - } xSemaphoreGive(lock); } From a72ae0bff8b06ee18fc99ce3d488dce02aa4934e Mon Sep 17 00:00:00 2001 From: Ixitxachitl Date: Thu, 27 Aug 2026 13:07:02 +0000 Subject: [PATCH 4/5] fix(buzz): scope the ringtone guard to the PWM tone path The early return sat above the I2S branches, but rtttl::isPlaying() only tracks the PWM sequencer. runOnce() starts the I2S and PWM ringtone paths from separate conditions - only triggerBuzzerOutput() treats them as exclusive - so enabling use_i2s_as_buzzer alongside use_pwm, or any HAS_I2S_SPEAKER_NRF52 board, could leave the flag set while I2S owns system tones and silence them for no reason. Move it next to the tone() loop whose registers it protects. --- src/buzz/buzz.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/buzz/buzz.cpp b/src/buzz/buzz.cpp index ed6195f06d2..f79ea08625c 100644 --- a/src/buzz/buzz.cpp +++ b/src/buzz/buzz.cpp @@ -118,10 +118,6 @@ void playTones(const ToneDuration *tone_durations, int size) // Buzzer is disabled or not set to system tones return; } -#ifdef HAS_PWM_RTTTL - if (rtttl::isPlaying()) - return; // a notification ringtone owns the buzzer, don't reprogram PWM under it -#endif #ifdef HAS_I2S if (moduleConfig.external_notification.use_i2s_as_buzzer && audioThread) { playTonesRTTTL(tone_durations, size); @@ -159,6 +155,10 @@ void playTones(const ToneDuration *tone_durations, int size) config.device.buzzer_gpio = PIN_BUZZER; #endif if (config.device.buzzer_gpio) { +#ifdef HAS_PWM_RTTTL + if (rtttl::isPlaying()) + return; // a notification ringtone owns the PWM, don't reprogram it mid-note +#endif for (int i = 0; i < size; i++) { const auto &tone_duration = tone_durations[i]; tone(config.device.buzzer_gpio, tone_duration.frequency_khz, tone_duration.duration_ms); From f6a54c65089367f0e46ff5c6212b46b4c2e0572c Mon Sep 17 00:00:00 2001 From: Ixitxachitl Date: Thu, 27 Aug 2026 13:11:28 +0000 Subject: [PATCH 5/5] fix(nrf52): serialize the RTTTL polling fallback against the timer callback timerRunning going false does not prove the timer stopped: a stop rejected by a full command queue, in stop() or in onTick() itself, leaves the auto-reload timer live. If the next begin() then has its start rejected too, pump() polled from the main thread while onTick() was still firing, and two threads drove rtttl::play() and tone() with nothing between them. Take the same mutex in pump(), non blocking, so whichever path is live has the sequencer to itself. play() is time gated on noteDelay, so a tick skipped for a contended lock is a no-op rather than a dropped note. --- src/platform/nrf52/NRF52RtttlTicker.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/platform/nrf52/NRF52RtttlTicker.cpp b/src/platform/nrf52/NRF52RtttlTicker.cpp index 006ab2c5f60..e2efd5ab5d5 100644 --- a/src/platform/nrf52/NRF52RtttlTicker.cpp +++ b/src/platform/nrf52/NRF52RtttlTicker.cpp @@ -11,9 +11,8 @@ namespace NRF52RtttlTicker { namespace { -// The stall this works around is tens of milliseconds, so 5 ms is inaudible and keeps the timer -// daemon at 200 wakeups/s. Must stay above one tick (configTICK_RATE_HZ is 1024 here) or -// pdMS_TO_TICKS() rounds to zero, xTimerCreate() fails, and we silently drop back to polling. +// 5 ms is inaudible against the tens-of-ms stall being fixed, and holds the daemon to 200 wakeups/s. +// Must exceed one tick, or pdMS_TO_TICKS() rounds to zero and xTimerCreate() fails back into polling. constexpr uint32_t kTickMs = 5; static_assert(kTickMs * configTICK_RATE_HZ >= 1000, "kTickMs rounds to zero ticks"); @@ -71,8 +70,18 @@ void begin(uint8_t pin, const char *song) void pump() { - if (!timerRunning && rtttl::isPlaying()) + if (timerRunning || !rtttl::isPlaying()) + return; + if (!lock) { // no mutex means ensureInit() never made a timer, so nothing can race us rtttl::play(); + return; + } + // A rejected start or stop leaves the auto-reload timer live even with timerRunning clear, so take + // the lock rather than assume onTick() is idle. play() is time gated, so a skipped tick costs nothing. + if (xSemaphoreTake(lock, 0) != pdTRUE) + return; + rtttl::play(); + xSemaphoreGive(lock); } void stop()