From d6caa4c5dfcfb8bb7e461ac86668778a8f5003c4 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Thu, 16 Jul 2026 12:41:37 -0500 Subject: [PATCH 1/4] 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. --- src/modules/AdminModule.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 278fe6179af..b54bf745277 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -794,6 +794,13 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) accelerometerThread->enabled = true; accelerometerThread->start(); } + // Turning double-tap off: stop the accelerometer thread, but only if wake-on-motion doesn't still need it. + // disable() clears enabled so a later re-enable can restart it. + else if (config.device.double_tap_as_button_press == true && + c.payload_variant.device.double_tap_as_button_press == false && config.display.wake_on_tap_or_motion == false && + accelerometerThread->enabled == true) { + accelerometerThread->disable(); + } #endif if (config.device.button_gpio == c.payload_variant.device.button_gpio && config.device.buzzer_gpio == c.payload_variant.device.buzzer_gpio && @@ -898,6 +905,12 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) accelerometerThread->enabled = true; accelerometerThread->start(); } + // Turning wake-on-motion off: stop the accelerometer thread, but only if double-tap doesn't still need it. + // disable() clears enabled so a later re-enable can restart it. + else if (config.display.wake_on_tap_or_motion == true && c.payload_variant.display.wake_on_tap_or_motion == false && + config.device.double_tap_as_button_press == false && accelerometerThread->enabled == true) { + accelerometerThread->disable(); + } #endif config.display = c.payload_variant.display; break; From 16f78becc847ffbf83287897c620fa71cb13563c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Fri, 17 Jul 2026 15:10:54 +0200 Subject: [PATCH 2/4] Keep accelerometer thread running when its sensor drives the compass --- src/modules/AdminModule.cpp | 9 +++++++-- 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, 20 insertions(+), 2 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index b54bf745277..5629e12903e 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -796,9 +796,11 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) } // Turning double-tap off: stop the accelerometer thread, but only if wake-on-motion doesn't still need it. // disable() clears enabled so a later re-enable can restart it. + // Skip the disable when the sensor also drives the compass (e.g. BMX160, ICM20948, BMM150), + // otherwise halting the thread would freeze the heading until the next reboot. else if (config.device.double_tap_as_button_press == true && c.payload_variant.device.double_tap_as_button_press == false && config.display.wake_on_tap_or_motion == false && - accelerometerThread->enabled == true) { + accelerometerThread->enabled == true && !accelerometerThread->providesHeading()) { accelerometerThread->disable(); } #endif @@ -907,8 +909,11 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) } // Turning wake-on-motion off: stop the accelerometer thread, but only if double-tap doesn't still need it. // disable() clears enabled so a later re-enable can restart it. + // Skip the disable when the sensor also drives the compass (e.g. BMX160, ICM20948, BMM150), + // otherwise halting the thread would freeze the heading until the next reboot. else if (config.display.wake_on_tap_or_motion == true && c.payload_variant.display.wake_on_tap_or_motion == false && - config.device.double_tap_as_button_press == false && accelerometerThread->enabled == true) { + config.device.double_tap_as_button_press == false && accelerometerThread->enabled == true && + !accelerometerThread->providesHeading()) { accelerometerThread->disable(); } #endif diff --git a/src/motion/AccelerometerThread.h b/src/motion/AccelerometerThread.h index 8482dafe4b0..6847b9aa405 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 8790454008e..8c5b1244e42 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 d60477521c9..d0d21d3dda5 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 d8369b3ca16..e84c7ea1bd4 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 717c89a9bf7..38876fb7767 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(); From 5421922e9a5df1af44df01fd60cd34aeb0c1453b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Fri, 17 Jul 2026 15:15:06 +0200 Subject: [PATCH 3/4] Guard accelerometer thread config toggles against a null thread pointer --- src/modules/AdminModule.cpp | 63 ++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 5629e12903e..8a80be8a5e5 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -788,20 +788,24 @@ 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(); - } - // Turning double-tap off: stop the accelerometer thread, but only if wake-on-motion doesn't still need it. - // disable() clears enabled so a later re-enable can restart it. - // Skip the disable when the sensor also drives the compass (e.g. BMX160, ICM20948, BMM150), - // otherwise halting the thread would freeze the heading until the next reboot. - else if (config.device.double_tap_as_button_press == true && - c.payload_variant.device.double_tap_as_button_press == false && config.display.wake_on_tap_or_motion == false && - accelerometerThread->enabled == true && !accelerometerThread->providesHeading()) { - accelerometerThread->disable(); + // Only allocated when an accelerometer was actually detected at boot, so it may be null here. + if (accelerometerThread) { + 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(); + } + // Turning double-tap off: stop the accelerometer thread, but only if wake-on-motion doesn't still need it. + // disable() clears enabled so a later re-enable can restart it. + // Skip the disable when the sensor also drives the compass (e.g. BMX160, ICM20948, BMM150), + // otherwise halting the thread would freeze the heading until the next reboot. + else if (config.device.double_tap_as_button_press == true && + c.payload_variant.device.double_tap_as_button_press == false && + config.display.wake_on_tap_or_motion == false && accelerometerThread->enabled == true && + !accelerometerThread->providesHeading()) { + accelerometerThread->disable(); + } } #endif if (config.device.button_gpio == c.payload_variant.device.button_gpio && @@ -901,20 +905,23 @@ 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(); - } - // Turning wake-on-motion off: stop the accelerometer thread, but only if double-tap doesn't still need it. - // disable() clears enabled so a later re-enable can restart it. - // Skip the disable when the sensor also drives the compass (e.g. BMX160, ICM20948, BMM150), - // otherwise halting the thread would freeze the heading until the next reboot. - else if (config.display.wake_on_tap_or_motion == true && c.payload_variant.display.wake_on_tap_or_motion == false && - config.device.double_tap_as_button_press == false && accelerometerThread->enabled == true && - !accelerometerThread->providesHeading()) { - accelerometerThread->disable(); + // Only allocated when an accelerometer was actually detected at boot, so it may be null here. + if (accelerometerThread) { + 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(); + } + // Turning wake-on-motion off: stop the accelerometer thread, but only if double-tap doesn't still need it. + // disable() clears enabled so a later re-enable can restart it. + // Skip the disable when the sensor also drives the compass (e.g. BMX160, ICM20948, BMM150), + // otherwise halting the thread would freeze the heading until the next reboot. + else if (config.display.wake_on_tap_or_motion == true && c.payload_variant.display.wake_on_tap_or_motion == false && + config.device.double_tap_as_button_press == false && accelerometerThread->enabled == true && + !accelerometerThread->providesHeading()) { + accelerometerThread->disable(); + } } #endif config.display = c.payload_variant.display; From 44a9247b9ca5cf4a6b7cca3ace7fef0d96f19445 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Fri, 17 Jul 2026 18:44:12 -0500 Subject: [PATCH 4/4] 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. --- src/modules/AdminModule.cpp | 62 +++++++++++++++---------------------- 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 6c430782c5e..e7990a5a5ed 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,25 +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 - // Only allocated when an accelerometer was actually detected at boot, so it may be null here. - if (accelerometerThread) { - 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(); - } - // Turning double-tap off: stop the accelerometer thread, but only if wake-on-motion doesn't still need it. - // disable() clears enabled so a later re-enable can restart it. - // Skip the disable when the sensor also drives the compass (e.g. BMX160, ICM20948, BMM150), - // otherwise halting the thread would freeze the heading until the next reboot. - else if (config.device.double_tap_as_button_press == true && - c.payload_variant.device.double_tap_as_button_press == false && - config.display.wake_on_tap_or_motion == false && accelerometerThread->enabled == true && - !accelerometerThread->providesHeading()) { - accelerometerThread->disable(); - } - } + 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 && @@ -918,24 +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 - // Only allocated when an accelerometer was actually detected at boot, so it may be null here. - if (accelerometerThread) { - 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(); - } - // Turning wake-on-motion off: stop the accelerometer thread, but only if double-tap doesn't still need it. - // disable() clears enabled so a later re-enable can restart it. - // Skip the disable when the sensor also drives the compass (e.g. BMX160, ICM20948, BMM150), - // otherwise halting the thread would freeze the heading until the next reboot. - else if (config.display.wake_on_tap_or_motion == true && c.payload_variant.display.wake_on_tap_or_motion == false && - config.device.double_tap_as_button_press == false && accelerometerThread->enabled == true && - !accelerometerThread->providesHeading()) { - accelerometerThread->disable(); - } - } + 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;