From f4dba8588aaab52581d32f8d7c0a7fe9cef943ec Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Sat, 5 Sep 2026 16:36:18 +0100 Subject: [PATCH 1/2] sound: a switch that gives the memory back, and a level for the board that has one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sounder was a build-time fact. On the board whose sounder is a speaker that made it an awkward one: its task and DMA ring are about 5.4 KB of internal RAM, which on that board is enough to stop the portal serving, so choosing between a chime and a web interface meant reflashing. sound.enabled is a setting now, and switching it off releases the hardware rather than merely silencing it — which is the only version of the switch worth having. Measured on the T-Deck, live, without a restart: 18972 bytes free with it off, 11672 with it on, 18940 when switched off again. The audio task stands down rather than being killed. A sentinel note reaches it only between notes, and it deletes itself there; deleting it by handle could land inside an I2S write and leave the driver's state to chance. If it does not answer within a second the sounder is left up and says so, because a wedged task holding a queue that has just been freed is a worse outcome than a kilobyte unreclaimed. sound.volume scales the speaker's amplitude. On a piezo it is accepted and ignored, deliberately: a PWM element has one loudness, duty cycle changes the timbre rather than the level, and a fleet should be able to carry one configuration across boards that answer it differently. Defaults follow the cost. A piezo is a channel and a timer, so those boards ship on and behave as they always did; the speaker ships off, and turning it on is a choice made knowing what it spends. Both controls appear on the device's settings screen without that file being touched, which is the point of its being generated from the key table — the volume as a slider, beside the display's brightness. Verified on both boards: the T-Deck for the memory going and coming back, the M9 for the piezo path and for the defaults being what they claim. The device screen was verified by somebody moving the slider on the glass while this was being written, which is a better test than the one that was planned. --- src/Config.h | 19 +++++++ src/sys/Buzzer.cpp | 111 ++++++++++++++++++++++++++++++++++++- src/sys/Buzzer.h | 15 ++++- src/sys/Settings.cpp | 10 ++++ src/sys/Settings.h | 10 ++++ src/sys/SettingsFields.cpp | 23 ++++++++ src/ui/lvgl/UiSettings.cpp | 2 + 7 files changed, 187 insertions(+), 3 deletions(-) diff --git a/src/Config.h b/src/Config.h index a3ef61f..ad8daca 100644 --- a/src/Config.h +++ b/src/Config.h @@ -592,6 +592,25 @@ #ifndef BUZZER_KIND #define BUZZER_KIND BUZZER_KIND_PWM #endif +// Whether the sounder starts switched on, which is decided by what it costs. +// A piezo is a PWM channel and a timer — free, so it ships on and the node +// keeps chirping as it always did. A speaker behind an I2S amplifier is a task +// and a DMA ring, about 5.4 KB of internal RAM, which on the board that has one +// is a quarter of what it has spare and enough to stop the portal serving. That +// ships off, and turning it on is a choice made knowing the trade. +#ifndef SOUND_ENABLED_DEFAULT + #if BUZZER_KIND == BUZZER_KIND_I2S + #define SOUND_ENABLED_DEFAULT 0 + #else + #define SOUND_ENABLED_DEFAULT 1 + #endif +#endif +// Percent. Only the speaker can act on it: a piezo is driven at the one +// amplitude its PWM channel produces, so the setting is offered where it means +// something and refused where it does not. +#ifndef SOUND_VOLUME_DEFAULT + #define SOUND_VOLUME_DEFAULT 40 +#endif // The I2S speaker's pins, meaningless on a PWM board. #ifndef PIN_I2S_BCLK #define PIN_I2S_BCLK -1 diff --git a/src/sys/Buzzer.cpp b/src/sys/Buzzer.cpp index dc55503..6a9f8d9 100644 --- a/src/sys/Buzzer.cpp +++ b/src/sys/Buzzer.cpp @@ -38,6 +38,7 @@ #include #include +#include "Settings.h" #if BUZZER_KIND == BUZZER_KIND_I2S #include @@ -60,7 +61,12 @@ constexpr uint32_t kRateHz = 8000; // A quarter of full scale. The MAX98357A on the board this was written for // straps its gain pin to ground, which is 12 dB, and full scale through that // into a small speaker is startling rather than informative. -constexpr int16_t kAmplitude = 8000; +constexpr int16_t kAmplitudeFull = 8000; +// The amplitude actually used, from the volume setting. A quarter of full scale +// is the ceiling rather than the level: the MAX98357A straps its gain to ground, +// which is 12 dB, and full scale through that into a small speaker is startling +// rather than informative. +int16_t sAmplitude = kAmplitudeFull; // Square rather than sine, deliberately: it is what the piezo this stands in // for produces, it needs no floating point in the task that generates it, and // the difference is inaudible in a 120 ms beep. @@ -91,7 +97,7 @@ void play(uint32_t hz, uint32_t ms) { while (left) { const size_t n = left < kChunk ? left : kChunk; for (size_t i = 0; i < n; i++) { - buf[i] = (halfPeriod && (phase / halfPeriod) % 2) ? kAmplitude : (int16_t)-kAmplitude; + buf[i] = (halfPeriod && (phase / halfPeriod) % 2) ? sAmplitude : (int16_t)-sAmplitude; phase++; } sI2s.write((const uint8_t*)buf, n * sizeof(int16_t)); @@ -103,10 +109,22 @@ void play(uint32_t hz, uint32_t ms) { sI2s.write((const uint8_t*)quiet, sizeof(quiet)); } +// Asked to stand down, and the acknowledgement. The task deletes itself rather +// than being deleted: it can be inside an I2S write when the switch is thrown, +// and killing a task mid-driver leaves the driver's state to chance. A sentinel +// note reaches it only between notes, which is exactly when it is safe to go. +volatile bool sStopping = false; +volatile bool sStopped = false; + void audioTask(void*) { Note n; for (;;) { if (xQueueReceive(sQueue, &n, portMAX_DELAY) != pdTRUE) continue; + if (!n.hz && !n.ms) { // the sentinel: nothing to play, time to go + sStopped = true; + vTaskDelete(nullptr); + return; + } play(n.hz, n.ms); if (n.nextHz) play(n.nextHz, n.ms); // Only now. Clearing it when the note was taken off the queue would open @@ -194,7 +212,50 @@ bool start(uint32_t hz, uint32_t ms, uint32_t nextHz) { namespace Buzzer { +namespace { + +// Volume as an amplitude, clamped. Zero is silence rather than a click: a note +// of zero amplitude still costs the time it takes to play. +void setVolume(uint8_t percent) { +#if BUZZER_KIND == BUZZER_KIND_I2S + if (percent > 100) percent = 100; + sAmplitude = (int16_t)((int32_t)kAmplitudeFull * percent / 100); +#else + // A piezo on a PWM channel has one loudness: the pin swings rail to rail and + // the element is as loud as it is. Duty cycle changes the timbre rather than + // the level, and driving it quieter would mean driving it wrong. The setting + // is accepted and ignored here rather than hidden, so a fleet can carry one + // configuration across boards that answer it differently. + (void)percent; +#endif +} + +bool sUp = false; // the hardware is taken up + +void takeUp(); +void release(); + +} // namespace + void begin() { + setVolume(settings.sound().volume); + apply(); +} + +// Match the hardware to the setting. Idempotent, so the settings layer can call +// it after every save without knowing whether anything changed. +void apply() { + setVolume(settings.sound().volume); + const bool want = settings.sound().enabled; + if (want == sUp) return; + if (want) takeUp(); else release(); +} + +bool present() { return sUp; } + +namespace { + +void takeUp() { #if BUZZER_KIND == BUZZER_KIND_I2S sI2s.setPins(PIN_I2S_BCLK, PIN_I2S_LRCLK, PIN_I2S_DOUT); if (!sI2s.begin(I2S_MODE_STD, kRateHz, I2S_DATA_BIT_WIDTH_16BIT, I2S_SLOT_MODE_MONO)) { @@ -231,8 +292,54 @@ void begin() { }; if (esp_timer_create(&args, &sTimer) != ESP_OK) sTimer = nullptr; #endif + sUp = true; } +// Give the hardware back. On the speaker this is the whole reason the switch +// exists — the task's stack and the DMA ring are the cost, and a switch that +// only silenced them would save nothing. +void release() { +#if BUZZER_KIND == BUZZER_KIND_I2S + sReady = false; // refuse new notes before anything goes + if (sQueue) { + sStopping = true; + sStopped = false; + const Note bye{0, 0, 0}; + // Room for it: a note in flight is at most a couple of hundred + // milliseconds, and the queue is one deep. + if (xQueueSend(sQueue, &bye, pdMS_TO_TICKS(500)) == pdTRUE) { + for (int i = 0; i < 100 && !sStopped; i++) vTaskDelay(pdMS_TO_TICKS(10)); + } + if (!sStopped) { + // It did not go. Leave the queue and the driver alone rather than pull + // them out from under a task that is still running in them: the sounder + // stays up and the memory stays spent, which is the safe half of a bad + // pair. + log_w("buzzer: the audio task did not stand down; leaving the speaker up"); + sStopping = false; + sReady = true; + return; + } + vQueueDelete(sQueue); + sQueue = nullptr; + } + sStopping = false; + sI2s.end(); + portENTER_CRITICAL(&sBusyMux); + sBusy = false; // whatever was playing is not any more + portEXIT_CRITICAL(&sBusyMux); + log_i("buzzer: speaker released; its task and DMA are back"); +#else + if (sTimer) { esp_timer_stop(sTimer); esp_timer_delete(sTimer); sTimer = nullptr; } + ledcWriteTone(PIN_BUZZER, 0); + ledcDetach(PIN_BUZZER); + sBusy = false; +#endif + sUp = false; +} + +} // namespace + // Near a small piezo's resonance, short enough to be polite. Worth saying: // the first bench V4 produced no sound at any drive or frequency while every // other function worked, so its sounder is likely simply not fitted — the pin diff --git a/src/sys/Buzzer.h b/src/sys/Buzzer.h index d8e7d08..5c1b6ea 100644 --- a/src/sys/Buzzer.h +++ b/src/sys/Buzzer.h @@ -42,10 +42,21 @@ namespace Buzzer { -void begin(); // claims the pin; silent until asked +void begin(); // claims what the board needs, if the sounder is switched on void boot(); // two short notes up: running void message(); // one note: something arrived for you +// Take up or release the hardware to match the setting, without a restart. +// That is the point of the switch on the speaker boards: the task and the DMA +// ring are about 5.4 KB of internal RAM, a quarter of what one of these boards +// has spare and enough to stop the portal serving — so turning the sounder off +// has to give that back rather than merely stay quiet. +void apply(); + +// Whether the sounder is up and can be heard. False when the board has none, +// when it is switched off, and when it would not start. +bool present(); + } // namespace Buzzer #else @@ -54,6 +65,8 @@ namespace Buzzer { inline void begin() {} inline void boot() {} inline void message() {} +inline void apply() {} +inline bool present() { return false; } } // namespace Buzzer #endif // HAS_BUZZER diff --git a/src/sys/Settings.cpp b/src/sys/Settings.cpp index 09c38b0..7684ae5 100644 --- a/src/sys/Settings.cpp +++ b/src/sys/Settings.cpp @@ -93,6 +93,8 @@ void Settings::load() { if (_prefs.isKey("w_stap")) _prefs.getString("w_stap", _wifi.staPassword, sizeof(_wifi.staPassword)); if (_prefs.isKey("a_pass")) _prefs.getString("a_pass", _admin.password, sizeof(_admin.password)); + LOAD(_sound.enabled, "snd_en", getBool ("snd_en")); + LOAD(_sound.volume, "snd_vol", getUChar("snd_vol")); LOAD(_display.touchWake, "d_twake", getBool ("d_twake")); LOAD(_display.brightness, "d_bri", getUChar("d_bri")); LOAD(_display.daylight, "d_day", getBool ("d_day")); @@ -192,6 +194,14 @@ bool Settings::saveAdminPassword(const char* password) { return ok; } +bool Settings::saveSound(const SoundSettings& s) { + _sound = s; + bool ok = _prefs.putBool ("snd_en", s.enabled) > 0 + && _prefs.putUChar("snd_vol", s.volume) > 0; + if (!ok) log_e("NVS write failed (sound)"); + return ok; +} + bool Settings::saveDisplay(const DisplaySettings& d) { _display = d; bool ok = _prefs.putBool("d_twake", d.touchWake) > 0; diff --git a/src/sys/Settings.h b/src/sys/Settings.h index 07cea27..b73a795 100644 --- a/src/sys/Settings.h +++ b/src/sys/Settings.h @@ -147,6 +147,13 @@ struct LinkSettings { // What the glass does, on boards that have one worth configuring. Its own // section because it will grow — brightness, sleep timing — and because a // UI behaviour is neither radio nor maintenance. +// The sounder, which on one board is the difference between a portal that +// serves and one that cannot allocate (Buzzer.h). +struct SoundSettings { + bool enabled = SOUND_ENABLED_DEFAULT != 0; + uint8_t volume = SOUND_VOLUME_DEFAULT; // percent; the piezo cannot act on it +}; + struct DisplaySettings { // Whether a tap wakes a blanked panel. On by the phone convention; off for // a pocketed device whose every accidental touch would light the glass. @@ -205,6 +212,7 @@ class Settings { const LinkSettings& links() const { return _links; } const MaintenanceSettings& maintenance() const { return _maintenance; } const DisplaySettings& display() const { return _display; } + const SoundSettings& sound() const { return _sound; } bool saveRadio(const RadioSettings& r); bool saveLinks(const LinkSettings& l); @@ -213,6 +221,7 @@ class Settings { bool saveAdminPassword(const char* password); bool saveTransport(const TransportSettings& t); bool saveDisplay(const DisplaySettings& d); + bool saveSound(const SoundSettings& s); void factoryReset(); // wipes the namespace, restores defaults // Valid LoRa bandwidths shared by SX126x and SX127x, in kHz. @@ -229,6 +238,7 @@ class Settings { LinkSettings _links; MaintenanceSettings _maintenance; DisplaySettings _display; + SoundSettings _sound; }; extern Settings settings; diff --git a/src/sys/SettingsFields.cpp b/src/sys/SettingsFields.cpp index 412e9d3..86872c6 100644 --- a/src/sys/SettingsFields.cpp +++ b/src/sys/SettingsFields.cpp @@ -21,6 +21,7 @@ // section the same way its HTTP handler does. #include "SettingsFields.h" #include "RnsTransport.h" +#include "Buzzer.h" #include #include @@ -590,6 +591,28 @@ const Entry kFields[] = { if (!Power::profileFromName(v, pp)) { snprintf(e, n, "power_profile must be performance|balanced|battery"); return Result::BadValue; } TransportSettings t = settings.transport(); t.powerProfile = (uint8_t)pp; return commitTransport(t, e, n); } }, + // --- sound --------------------------------------------------------------- + // The switch gives the hardware back rather than merely silencing it, which + // is why it is worth having at runtime: on a board whose sounder is a speaker + // the task and its DMA are about 5.4 KB of internal RAM, and that is the + // difference between a portal that serves and one that cannot allocate. + { "sound.enabled", + [](char* o, size_t n) { snprintf(o, n, "%s", settings.sound().enabled ? "on" : "off"); }, + [](const char* v, char* e, size_t n) { bool b; + if (!parseBool(v, b)) { snprintf(e, n, "expected on or off"); return Result::BadValue; } + SoundSettings s = settings.sound(); s.enabled = b; + if (!settings.saveSound(s)) { snprintf(e, n, "could not be saved"); return Result::NvsFailed; } + Buzzer::apply(); // takes the hardware up or gives it back, now + return Result::Ok; } }, + { "sound.volume", + [](char* o, size_t n) { snprintf(o, n, "%u", (unsigned)settings.sound().volume); }, + [](const char* v, char* e, size_t n) { uint32_t u; + if (!parseU32Max(v, 100, u, e, n)) return Result::BadValue; + SoundSettings s = settings.sound(); s.volume = (uint8_t)u; + if (!settings.saveSound(s)) { snprintf(e, n, "could not be saved"); return Result::NvsFailed; } + Buzzer::apply(); + return Result::Ok; } }, + // --- admin ------------------------------------------------------------- { "admin.password", [](char* o, size_t n) { renderSecret(o, n, settings.admin().password); }, diff --git a/src/ui/lvgl/UiSettings.cpp b/src/ui/lvgl/UiSettings.cpp index 44a73f1..8af5a0b 100644 --- a/src/ui/lvgl/UiSettings.cpp +++ b/src/ui/lvgl/UiSettings.cpp @@ -54,6 +54,8 @@ enum class Kind : uint8_t { Text, Secret, Number, Switch, Region, Words, Slider Kind kindFor(const char* key, const char* value, bool quoted) { if (strcmp(key, "display.brightness") == 0) return Kind::Slider; + // A level, not a figure to type: the useful gesture is "a bit louder". + if (strcmp(key, "sound.volume") == 0) return Kind::Slider; if (strcmp(key, "radio.region") == 0) return Kind::Region; if (strcmp(key, "wifi.security") == 0) return Kind::Words; if (strcmp(key, "display.theme") == 0) return Kind::Words; From 32f449fae795c5e4e071dadd896c68d588c623d0 Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Sat, 5 Sep 2026 17:09:15 +0100 Subject: [PATCH 2/2] buzzer: the piezo path called itself up when its timer had not been created MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every failure in taking up the hardware returned early except one. Without the timer a note starts and never ends, so start() refuses and the sounder is silent — but the flag saying it was up got set anyway, three lines further down and past the #endif that hid it from the branch it belonged to. Two things follow from that flag, and both are wrong in this state. present() reports a sounder that cannot make a sound. And apply() compares the setting against it to decide whether anything needs doing, so a sounder that failed to start would never be tried again — a transient failure made permanent until the next restart. It gives the pin back and stays down now, and says so. The I2S path was already right: each of its failures returns before the flag. This was the one branch where the early return had been left out and the shared line at the end covered for it. From Copilot's review of #118. Its second comment quotes the volume comment back rather than finding anything, so there is nothing to answer there. Verified on the M9, which is the board with the piezo: the sounder comes up, goes off and comes back with no refusal on either transition. --- src/sys/Buzzer.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/sys/Buzzer.cpp b/src/sys/Buzzer.cpp index 6a9f8d9..9dcd95d 100644 --- a/src/sys/Buzzer.cpp +++ b/src/sys/Buzzer.cpp @@ -290,7 +290,17 @@ void takeUp() { .callback = step, .arg = nullptr, .dispatch_method = ESP_TIMER_TASK, .name = "buzzer", .skip_unhandled_events = true, }; - if (esp_timer_create(&args, &sTimer) != ESP_OK) sTimer = nullptr; + if (esp_timer_create(&args, &sTimer) != ESP_OK) { + // Without the timer a note starts and never ends, so start() refuses and + // the sounder is silent. Give the pin back and stay down rather than + // report a sounder that cannot make a sound: sUp is what apply() compares + // the setting against, so claiming success here would also mean it never + // tried again — a transient failure would become a permanent one. + sTimer = nullptr; + ledcDetach(PIN_BUZZER); + log_w("buzzer: no timer for the sounder; it stays off and will be retried"); + return; + } #endif sUp = true; }