From b20d89974a053d9d82ed7808f8736cd65f718a0c Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Sat, 18 Jul 2026 06:26:20 -0500 Subject: [PATCH] Stop accelerometer thread when double-tap/wake-on-motion disabled at runtime (#11025) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Stop accelerometer thread when double-tap/wake-on-motion disabled at runtime double_tap_as_button_press and wake_on_tap_or_motion are applied live: the OFF->ON edge calls accelerometerThread->start(), but there was no ON->OFF branch, so turning either flag off left the sensor thread running (polling I2C, drawing power) until reboot. Worse, because enabled stayed true, a later OFF->ON edge was a no-op (the enabled==false guard blocked re-start), leaving the feature un-restartable without a reboot. Add the symmetric ON->OFF branch in both handlers. When a flag goes true->off and the other consumer of the shared thread is also off, call accelerometerThread->disable() (stops runOnce polling and clears enabled so a later re-enable can start() again). Each branch checks the other flag first so disabling one feature never stops the sensor while the other still needs it. * Keep accelerometer thread running when its sensor drives the compass * Guard accelerometer thread config toggles against a null thread pointer * AdminModule: factor shared accelerometer start/stop into a helper The device and display config handlers had mirror-image blocks reconciling the shared accelerometer thread. Extract reconcileAccelerometerThread(wasOn, nowOn, otherFeatureOn) so the null guard, edge logic, compass (providesHeading) guard, and rationale live in one place; each call site is now a single call. Behavior is unchanged. Also drops the redundant per-field assignment that the whole-struct `config.device = ...` / `config.display = ...` overwrites anyway. --------- Co-authored-by: Thomas Göttgens --- src/modules/AdminModule.cpp | 37 +++++++++++++++++++++----------- src/motion/AccelerometerThread.h | 4 ++++ src/motion/BMM150Sensor.h | 1 + src/motion/BMX160Sensor.h | 1 + src/motion/ICM20948Sensor.h | 1 + src/motion/MotionSensor.h | 6 ++++++ 6 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index b9cf4b03b..e7990a5a5 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -786,6 +786,27 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) } } +#if !defined(ARCH_PORTDUINO) && !defined(ARCH_STM32WL) && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR && \ + !MESHTASTIC_EXCLUDE_ACCELEROMETER +// Shared by double-tap-as-button (device) and wake-on-motion (display). Start on either flag's +// off->on edge; stop on the on->off edge once neither needs it (disable() clears `enabled` so a +// later re-enable restarts). Skip the stop if the sensor also drives the compass, else the heading +// freezes until reboot. wasOn/nowOn = old/new of the changed flag; otherFeatureOn = the other flag. +static void reconcileAccelerometerThread(bool wasOn, bool nowOn, bool otherFeatureOn) +{ + if (!accelerometerThread) // null unless a sensor was detected at boot + return; + + if (!wasOn && nowOn && accelerometerThread->enabled == false) { + accelerometerThread->enabled = true; + accelerometerThread->start(); + } else if (wasOn && !nowOn && !otherFeatureOn && accelerometerThread->enabled == true && + !accelerometerThread->providesHeading()) { + accelerometerThread->disable(); + } +} +#endif + void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) { auto changes = SEGMENT_CONFIG; @@ -801,12 +822,8 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) config.has_device = true; #if !defined(ARCH_PORTDUINO) && !defined(ARCH_STM32WL) && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR && \ !MESHTASTIC_EXCLUDE_ACCELEROMETER - if (config.device.double_tap_as_button_press == false && c.payload_variant.device.double_tap_as_button_press == true && - accelerometerThread->enabled == false) { - config.device.double_tap_as_button_press = c.payload_variant.device.double_tap_as_button_press; - accelerometerThread->enabled = true; - accelerometerThread->start(); - } + reconcileAccelerometerThread(config.device.double_tap_as_button_press, + c.payload_variant.device.double_tap_as_button_press, config.display.wake_on_tap_or_motion); #endif if (config.device.button_gpio == c.payload_variant.device.button_gpio && config.device.buzzer_gpio == c.payload_variant.device.buzzer_gpio && @@ -905,12 +922,8 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) } #if !defined(ARCH_PORTDUINO) && !defined(ARCH_STM32WL) && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR && \ !MESHTASTIC_EXCLUDE_ACCELEROMETER - if (config.display.wake_on_tap_or_motion == false && c.payload_variant.display.wake_on_tap_or_motion == true && - accelerometerThread->enabled == false) { - config.display.wake_on_tap_or_motion = c.payload_variant.display.wake_on_tap_or_motion; - accelerometerThread->enabled = true; - accelerometerThread->start(); - } + reconcileAccelerometerThread(config.display.wake_on_tap_or_motion, c.payload_variant.display.wake_on_tap_or_motion, + config.device.double_tap_as_button_press); #endif config.display = c.payload_variant.display; break; diff --git a/src/motion/AccelerometerThread.h b/src/motion/AccelerometerThread.h index 8482dafe4..6847b9aa4 100755 --- a/src/motion/AccelerometerThread.h +++ b/src/motion/AccelerometerThread.h @@ -60,6 +60,10 @@ class AccelerometerThread : public concurrency::OSThread } } + // True if the active sensor drives the compass heading in runOnce(). Callers must not + // disable() the thread in this case, or the compass would freeze until the next reboot. + bool providesHeading() const { return isInitialised && sensor && sensor->providesHeading(); } + protected: int32_t runOnce() override { diff --git a/src/motion/BMM150Sensor.h b/src/motion/BMM150Sensor.h index 879045400..8c5b1244e 100644 --- a/src/motion/BMM150Sensor.h +++ b/src/motion/BMM150Sensor.h @@ -50,6 +50,7 @@ class BMM150Sensor : public MotionSensor // Called each time our sensor gets a chance to run virtual int32_t runOnce() override; + virtual bool providesHeading() const override { return true; } }; #endif diff --git a/src/motion/BMX160Sensor.h b/src/motion/BMX160Sensor.h index d60477521..d0d21d3dd 100755 --- a/src/motion/BMX160Sensor.h +++ b/src/motion/BMX160Sensor.h @@ -25,6 +25,7 @@ class BMX160Sensor : public MotionSensor virtual bool init() override; virtual int32_t runOnce() override; virtual void calibrate(uint16_t forSeconds) override; + virtual bool providesHeading() const override { return true; } }; #else diff --git a/src/motion/ICM20948Sensor.h b/src/motion/ICM20948Sensor.h index d8369b3ca..e84c7ea1b 100755 --- a/src/motion/ICM20948Sensor.h +++ b/src/motion/ICM20948Sensor.h @@ -100,6 +100,7 @@ class ICM20948Sensor : public MotionSensor // Called each time our sensor gets a chance to run virtual int32_t runOnce() override; virtual void calibrate(uint16_t forSeconds) override; + virtual bool providesHeading() const override { return true; } }; #endif diff --git a/src/motion/MotionSensor.h b/src/motion/MotionSensor.h index 717c89a9b..38876fb77 100755 --- a/src/motion/MotionSensor.h +++ b/src/motion/MotionSensor.h @@ -42,6 +42,12 @@ class MotionSensor virtual void calibrate(uint16_t forSeconds){}; + // True if this sensor produces the compass heading (screen->setHeading()) in runOnce(). + // Combined accel+magnetometer parts (e.g. BMX160, ICM20948) and standalone magnetometers + // handled by the accelerometer thread (e.g. BMM150) override this. Used to avoid halting + // the thread - and freezing the compass - when motion-only features are disabled at runtime. + inline virtual bool providesHeading() const { return false; }; + protected: // Turn on the screen when a tap or motion is detected virtual void wakeScreen();