From 546b678d50a385dbab736dcfaa171a7531b59c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Thu, 10 Sep 2026 11:50:37 +0000 Subject: [PATCH] fix(motion): drive screen wake from the accelerometer interrupt (#11758) * fix(motion): drive screen wake from the accelerometer interrupt The BHI260AP ISR body was empty and BHI_IRQ was never set or read, so the attach only consumed a GPIO slot. ICM20948 could not reach its interrupt path at all: the ICM_20948_INT_PIN fallback in the header is guarded on ICM_20948_WOM_THRESHOLD, which the block above it always defines, so the pin was never defined and the config, attach and interrupt-driven runOnce() were dropped by the preprocessor on every board. BHI260AP now configures the FIFO interrupt, attaches an ISR that sets a flag, and enables the wrist tilt gesture so runOnce() can call wakeScreen(). BMA423 arms INT1 push-pull active-high, which the BMA4 reset default leaves disabled, and drains on the interrupt instead of every 50 ms. Both keep a slow keepalive drain so a pin that never asserts degrades to polling rather than losing tilt and tap wake. MOTION_WAKE_INT_PIN resolves whichever motion interrupt a variant declares. doLightSleep() arms it as a GPIO wake source and lsIdle() attributes the resulting wake to motion, which it previously charged to BUTTON_PIN and dropped. Both are gated on config.display.wake_on_tap_or_motion, matching MotionSensor::wakeScreen(). Closes #11755 * fix(motion): use the ICM20948 interrupt without dropping the compass The ICM_20948_INT_PIN build of runOnce() was a full replacement for the polled one and kept only wake-on-motion, so defining the pin would have dropped the magnetometer fusion that feeds screen->setHeading(), the calibration flow and the IMU sleep handling. providesHeading() returns true for this part, so that is the compass. Merge the two: the pin now selects the wake-on-motion mechanism only. The status register poll stays compiled in behind a keepalive, since no shipped firmware has exercised this line, so a pin that never asserts costs latency rather than wake-on-motion. Declare the pin on t-echo-card. Sensor_INT is P1.13, open drain with a 10K pullup to VDD3V3, matching the driver's active-low config and FALLING attach. The schematic's SCL P1.02 / SDA P1.04 match PIN_WIRE_SCL and PIN_WIRE_SDA. * Revert the t-echo-card ICM20948 interrupt pin Sensor_INT is not the IMU. In both T-Echo-Lite_V1.0 and T-Echo-Lite-Card_V1.0 it appears only on the unannotated 5-pin expansion header (P?, 5PIN_PA1.0) carrying SDA_P1.04, SCL_P1.02, VDD3V3, GND and Sensor_INT with its 10K pullup, and it leaves the sheet as an off-sheet port. Neither schematic contains an ICM20948 symbol at all, and the vendor pin map declares only ICM20948_SDA, ICM20948_SCL and ICM20948_ADDRESS for the part. The interrupt belongs to whatever plugs into that header, so the onboard IMU has no reason to drive it. The driver keeps polling. * Poll until an ICM20948 interrupt pin proves itself A variant that declares ICM_20948_INT_PIN is asserting routing no vendor firmware has ever exercised, so treat the line as unproven: keep polling the wake-on-motion status register at full rate, and only back off to the keepalive once the pin has actually fired. A wrong pin then behaves exactly as before rather than trading wake latency for the guess. * feat(t-impulse-plus): drive ICM20948 wake-on-motion from its INT pin The LilyGO pinmap documents the IMU's INT on P0.07, and variant.cpp already maps and names it as D27, but the pin was never handed to the driver, so wake-on-motion polled the status register every 50 ms. Use the D number: pinMode() and attachInterrupt() index g_ADigitalPinMap, where a raw 7 selects P1.13, the LoRa RF_VC1 TXEN line. The driver polls until the pin proves itself, so an ICM20948 that turns out not to drive it keeps working as before. * Derive MOTION_WAKE_INT_PIN after the build exclusions MESHTASTIC_MINIMIZE_BUILD defines MESHTASTIC_EXCLUDE_I2C further down the file, so the guard read as unset and a minimized build defined the pin anyway. doLightSleep() would then arm a GPIO no motion driver configures, since every driver is compiled out with I2C. Latent rather than live: nothing sets MESHTASTIC_MINIMIZE_BUILD today, and the variants that pass -DMESHTASTIC_EXCLUDE_I2C were already correct because a build flag is defined before this file is parsed. * fix(motion): keep the BMA423 INT1 config failure non-fatal Restores the resolution made when feature/sensorlib-0.4.1 was merged into this branch. That merge is gone after the rebase, and neither parent carried this: the interrupt path is an optimisation over the existing poll, so a pin-config failure should log and fall back rather than be ignored outright. --- src/PowerFSM.cpp | 7 +++ src/configuration.h | 27 +++++++++++ src/motion/BHI260APSensor.cpp | 56 ++++++++++++++++------ src/motion/BHI260APSensor.h | 8 +++- src/motion/BMA423Sensor.cpp | 27 +++++++++++ src/motion/BMA423Sensor.h | 3 ++ src/motion/ICM20948Sensor.cpp | 34 +++++++------ src/motion/ICM20948Sensor.h | 10 ++-- src/motion/MotionSensor.h | 2 + src/sleep.cpp | 10 ++++ variants/nrf52840/t-impulse-plus/variant.h | 2 + 11 files changed, 149 insertions(+), 37 deletions(-) diff --git a/src/PowerFSM.cpp b/src/PowerFSM.cpp index 400aabc676..7f35bf41e3 100644 --- a/src/PowerFSM.cpp +++ b/src/PowerFSM.cpp @@ -167,6 +167,13 @@ static void lsIdle() if (pressed) { powerFSM.trigger(EVENT_PRESS); } +#ifdef MOTION_WAKE_INT_PIN + // Not the button: the accelerometer can have raised the line instead. + else if (config.display.wake_on_tap_or_motion && + digitalRead(MOTION_WAKE_INT_PIN) == (MOTION_WAKE_INT_ACTIVE_HIGH ? HIGH : LOW)) { + powerFSM.trigger(EVENT_INPUT); + } +#endif break; } default: diff --git a/src/configuration.h b/src/configuration.h index 2018fe678c..45e18bf6f0 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -642,6 +642,33 @@ along with this program. If not, see . #define HAS_SCREEN 0 #endif +// ----------------------------------------------------------------------------- +// Motion sensor wake +// ----------------------------------------------------------------------------- + +/* The motion driver that owns this pin attaches the ISR. sleep.cpp reuses it as a + light-sleep wake source and PowerFSM attributes the resulting GPIO wake to motion. + Must stay below the exclusion cascade: MESHTASTIC_MINIMIZE_BUILD derives + MESHTASTIC_EXCLUDE_I2C above, and no motion driver is built when it is set. */ +#if !MESHTASTIC_EXCLUDE_I2C +#if defined(BMA4XX_INT) && defined(HAS_BMA423) +#define MOTION_WAKE_INT_PIN BMA4XX_INT +#define MOTION_WAKE_INT_ACTIVE_HIGH 1 +#elif defined(BHI260AP_INT) && defined(HAS_BHI260AP) +#define MOTION_WAKE_INT_PIN BHI260AP_INT +#define MOTION_WAKE_INT_ACTIVE_HIGH 1 +#elif defined(STK8XXX_INT) && defined(HAS_STK8XXX) +#define MOTION_WAKE_INT_PIN STK8XXX_INT +#define MOTION_WAKE_INT_ACTIVE_HIGH 1 +#elif defined(ICM_20948_INT_PIN) && defined(HAS_ICM20948) +#define MOTION_WAKE_INT_PIN ICM_20948_INT_PIN +#define MOTION_WAKE_INT_ACTIVE_HIGH 0 +#elif defined(QMA_6100P_INT_PIN) && defined(HAS_QMA6100P) +#define MOTION_WAKE_INT_PIN QMA_6100P_INT_PIN +#define MOTION_WAKE_INT_ACTIVE_HIGH 0 +#endif +#endif + #ifndef USE_ETHERNET_DEFAULT #define USE_ETHERNET_DEFAULT 0 #endif diff --git a/src/motion/BHI260APSensor.cpp b/src/motion/BHI260APSensor.cpp index c9efda96c6..bf5290dce3 100644 --- a/src/motion/BHI260APSensor.cpp +++ b/src/motion/BHI260APSensor.cpp @@ -3,8 +3,19 @@ #if !defined(ARCH_STM32WL) && !MESHTASTIC_EXCLUDE_I2C && defined(HAS_BHI260AP) && __has_include() #define BOSCH_BHI260_KLIO +#include "mesh/Throttle.h" #include + +#ifdef BHI260AP_INT +static volatile bool BHI_IRQ = false; +#endif + BHI260APSensor::BHI260APSensor(ScanI2C::FoundDevice foundDevice) : MotionSensor::MotionSensor(foundDevice) {} + +void BHI260APSensor::onWristTilt(uint8_t, const uint8_t *, uint32_t, uint64_t *, void *user_data) +{ + static_cast(user_data)->wakeRequested = true; +} // https://github.com/lewisxhe/SensorLib/blob/master/examples/Sensors/IMU/BHI260AP_InterruptSettings/BHI260AP_InterruptSettings.ino bool BHI260APSensor::init() @@ -29,18 +40,25 @@ bool BHI260APSensor::init() // sensor.configAccelerometer(sensor.RANGE_2G, sensor.ODR_100HZ, sensor.BW_NORMAL_AVG4, sensor.PERF_CONTINUOUS_MODE); // sensor.enableAccelerometer(); - // sensor.configInterrupt(); #ifdef BHI260AP_INT + // Defaults: active-high, level-triggered, push-pull, FIFO sources unmasked. + InterruptConfig intConfig; + sensor.configureInterrupt(intConfig); pinMode(BHI260AP_INT, INPUT); attachInterrupt( - BHI260AP_INT, - [] { - // Set interrupt to set irq value to true - }, - RISING); // Select the interrupt mode according to the actual circuit + BHI260AP_INT, [] { BHI_IRQ = true; }, RISING); #endif + // Wrist tilt wakes the screen. Not every firmware image ships it and SensorAnyMotion + // is BHI360-only, so fall back to step counting alone. + constexpr uint8_t wristTilt = static_cast(BoschSensorID::WRIST_TILT_GESTURE); + if (sensor.onResultEvent(wristTilt, onWristTilt, this) && sensor.configure(wristTilt, 1.0f, 0)) { + LOG_DEBUG("BHI260AP wrist tilt wake enabled"); + } else { + LOG_WARN("BHI260AP firmware has no wrist tilt gesture, motion wake unavailable"); + } + // stepDetector->enable(1.0, 0); stepCounter->enable(1.0, 0); LOG_DEBUG("BHI260AP init ok"); @@ -52,6 +70,14 @@ bool BHI260APSensor::init() int32_t BHI260APSensor::runOnce() { +#ifdef BHI260AP_INT + // The INT line is the fast path; the keepalive keeps the step counter alive without it. + if (!BHI_IRQ && !Throttle::hasElapsed(lastPollMs, MOTION_SENSOR_IRQ_KEEPALIVE_MS)) + return MOTION_SENSOR_CHECK_INTERVAL_MS; + BHI_IRQ = false; + lastPollMs = millis(); +#endif + sensor.update(); if (stepCounter->hasUpdated()) { steps = stepCounter->getStepCount(); @@ -59,14 +85,16 @@ int32_t BHI260APSensor::runOnce() if (screen) screen->steps = steps; } - // LOG_WARN("Step count: %u", stepCounter->getStepCount()); - // if (sensor.readIrqStatus()) { - // if (sensor.isTilt() || sensor.isDoubleTap()) { - // wakeScreen(); - // return 500; - // } - //} + if (wakeRequested) { + wakeRequested = false; + wakeScreen(); + } +#ifdef BHI260AP_INT + // Tick fast for tilt latency; without the INT line every tick would be an I2C drain. + return MOTION_SENSOR_CHECK_INTERVAL_MS; +#else return 1000; +#endif } -#endif \ No newline at end of file +#endif diff --git a/src/motion/BHI260APSensor.h b/src/motion/BHI260APSensor.h index b0a4064872..ec8163c45e 100644 --- a/src/motion/BHI260APSensor.h +++ b/src/motion/BHI260APSensor.h @@ -15,10 +15,16 @@ class BHI260APSensor : public MotionSensor { private: SensorBHI260AP sensor; - volatile bool BHI_IRQ = false; SensorStepCounter *stepCounter; SensorStepDetector *stepDetector; uint32_t steps = 0; + bool wakeRequested = false; +#ifdef BHI260AP_INT + uint32_t lastPollMs = 0; +#endif + + // Fires from sensor.update() when the fusion hub reports a wrist tilt. + static void onWristTilt(uint8_t sensor_id, const uint8_t *data, uint32_t size, uint64_t *timestamp, void *user_data); public: explicit BHI260APSensor(ScanI2C::FoundDevice foundDevice); diff --git a/src/motion/BMA423Sensor.cpp b/src/motion/BMA423Sensor.cpp index 98491311b9..6271d70caa 100755 --- a/src/motion/BMA423Sensor.cpp +++ b/src/motion/BMA423Sensor.cpp @@ -2,6 +2,12 @@ #if !defined(ARCH_STM32WL) && !MESHTASTIC_EXCLUDE_I2C && defined(HAS_BMA423) && __has_include() +#include "mesh/Throttle.h" + +#ifdef BMA4XX_INT +static volatile bool BMA_IRQ = false; +#endif + BMA423Sensor::BMA423Sensor(ScanI2C::FoundDevice foundDevice) : MotionSensor::MotionSensor(foundDevice) {} bool BMA423Sensor::init() @@ -24,6 +30,13 @@ bool BMA423Sensor::init() sensor.setRemapAxes(SensorRemap::BOTTOM_LAYER_BOTTOM_LEFT_CORNER); #endif +#ifdef BMA4XX_INT + // enableTiltDetector()/enableTapDetector() only map the feature onto INT1, and the BMA4 + // reset default leaves that pin's output driver off. Arm it push-pull active-high. + if (!sensor.setInterruptPinConfig(InterruptPinMap::PIN1, false, false, true, false)) + LOG_DEBUG("BMA423 INT1 pin config failed, keeping the polled path"); // not fatal +#endif + // The tap detector defaults to double tap; tilt and double tap both wake the screen. sensor.setOnTiltDetectedCallback([this] { wakeRequested = true; }); sensor.setOnTapCallback([this](TapType) { wakeRequested = true; }); @@ -32,12 +45,26 @@ bool BMA423Sensor::init() return false; } +#ifdef BMA4XX_INT + pinMode(BMA4XX_INT, INPUT); + attachInterrupt( + BMA4XX_INT, [] { BMA_IRQ = true; }, RISING); +#endif + LOG_DEBUG("BMA423 init ok"); return true; } int32_t BMA423Sensor::runOnce() { +#ifdef BMA4XX_INT + // INT1 is the fast path; update() reads and clears the status register and fires the callbacks. + if (!BMA_IRQ && !Throttle::hasElapsed(lastPollMs, MOTION_SENSOR_IRQ_KEEPALIVE_MS)) + return MOTION_SENSOR_CHECK_INTERVAL_MS; + BMA_IRQ = false; + lastPollMs = millis(); +#endif + wakeRequested = false; sensor.update(); if (wakeRequested) { diff --git a/src/motion/BMA423Sensor.h b/src/motion/BMA423Sensor.h index 7ce5525c66..512457daf9 100755 --- a/src/motion/BMA423Sensor.h +++ b/src/motion/BMA423Sensor.h @@ -14,6 +14,9 @@ class BMA423Sensor : public MotionSensor private: SensorBMA423 sensor; bool wakeRequested = false; +#ifdef BMA4XX_INT + uint32_t lastPollMs = 0; +#endif public: explicit BMA423Sensor(ScanI2C::FoundDevice foundDevice); diff --git a/src/motion/ICM20948Sensor.cpp b/src/motion/ICM20948Sensor.cpp index 8238b3dbc9..a05f9aeca7 100644 --- a/src/motion/ICM20948Sensor.cpp +++ b/src/motion/ICM20948Sensor.cpp @@ -2,6 +2,7 @@ #if !defined(ARCH_STM32WL) && !MESHTASTIC_EXCLUDE_I2C && __has_include() #include "detect/ScanI2CTwoWire.h" +#include "mesh/Throttle.h" #if !defined(MESHTASTIC_EXCLUDE_SCREEN) // screen is defined in main.cpp @@ -34,21 +35,6 @@ bool ICM20948Sensor::init() return wakeOnMotionOk; } -#ifdef ICM_20948_INT_PIN - -int32_t ICM20948Sensor::runOnce() -{ - // Wake on motion using hardware interrupts - this is the most efficient way to check for motion - if (ICM20948_IRQ) { - ICM20948_IRQ = false; - sensor->clearInterrupts(); - wakeScreen(); - } - return MOTION_SENSOR_CHECK_INTERVAL_MS; -} - -#else - int32_t ICM20948Sensor::runOnce() { #if !defined(MESHTASTIC_EXCLUDE_SCREEN) && HAS_SCREEN @@ -105,7 +91,21 @@ int32_t ICM20948Sensor::runOnce() screen->setHeading(heading); #endif - // Wake on motion using polling - this is not as efficient as using hardware interrupt pin (see above) +#ifdef ICM_20948_INT_PIN + if (ICM20948_IRQ) { + ICM20948_IRQ = false; + intPinProven = true; + sensor->clearInterrupts(); + wakeScreen(); + return MOTION_SENSOR_CHECK_INTERVAL_MS; + } + // Back off to the keepalive only once the pin has actually fired. No vendor firmware + // uses this line, so an unproven one keeps full-rate polling instead of costing latency. + if (intPinProven && !Throttle::hasElapsed(lastWomPollMs, MOTION_SENSOR_IRQ_KEEPALIVE_MS)) + return MOTION_SENSOR_CHECK_INTERVAL_MS; + lastWomPollMs = millis(); +#endif + auto status = sensor->setBank(0); if (sensor->status != ICM_20948_Stat_Ok) { LOG_DEBUG("ICM20948 isWakeOnMotion failed to set bank - %s", sensor->statusString()); @@ -126,8 +126,6 @@ int32_t ICM20948Sensor::runOnce() return MOTION_SENSOR_CHECK_INTERVAL_MS; } -#endif - void ICM20948Sensor::calibrate(uint16_t forSeconds) { #if !defined(MESHTASTIC_EXCLUDE_SCREEN) && HAS_SCREEN diff --git a/src/motion/ICM20948Sensor.h b/src/motion/ICM20948Sensor.h index e84c7ea1bd..152ea9c95c 100755 --- a/src/motion/ICM20948Sensor.h +++ b/src/motion/ICM20948Sensor.h @@ -24,10 +24,8 @@ #define ICM_20948_WOM_THRESHOLD 16U #endif -// Define a pin in variant.h to use interrupts to read the ICM-20948 -#ifndef ICM_20948_WOM_THRESHOLD -#define ICM_20948_INT_PIN 255 -#endif +// Define ICM_20948_INT_PIN in variant.h to drive wake-on-motion from the INT pin +// instead of polling. The driver configures it active-low. // Uncomment this line to enable helpful debug messages on Serial // #define ICM_20948_DEBUG 1 @@ -83,6 +81,10 @@ class ICM20948Sensor : public MotionSensor ICM20948Singleton *sensor = nullptr; bool showingScreen = false; bool isAsleep = false; +#ifdef ICM_20948_INT_PIN + uint32_t lastWomPollMs = 0; + bool intPinProven = false; +#endif static constexpr const char *compassCalibrationFileName = "/prefs/compass_icm20948.dat"; #ifdef MUZI_BASE float highestX = 449.000000, lowestX = -140.000000, highestY = 422.000000, lowestY = -232.000000, highestZ = 749.000000, diff --git a/src/motion/MotionSensor.h b/src/motion/MotionSensor.h index ef84e1b19b..5a7111c52b 100755 --- a/src/motion/MotionSensor.h +++ b/src/motion/MotionSensor.h @@ -3,6 +3,8 @@ #define _MOTION_SENSOR_H_ #define MOTION_SENSOR_CHECK_INTERVAL_MS 50 +// Safety-net drain for the interrupt-driven drivers: a dead INT pin degrades to polling. +#define MOTION_SENSOR_IRQ_KEEPALIVE_MS 1000 #define MOTION_SENSOR_CLICK_THRESHOLD 40 #include "../configuration.h" diff --git a/src/sleep.cpp b/src/sleep.cpp index 31c91e80d0..2b6f58843c 100644 --- a/src/sleep.cpp +++ b/src/sleep.cpp @@ -477,6 +477,12 @@ esp_sleep_wakeup_cause_t doLightSleep(uint64_t sleepMsec) // FIXME, use a more r #endif #if defined(WAKE_ON_TOUCH) gpio_wakeup_enable((gpio_num_t)SCREEN_TOUCH_INT, GPIO_INTR_LOW_LEVEL); +#endif +#ifdef MOTION_WAKE_INT_PIN + // Only arm motion wake when the user asked for it, otherwise every tilt costs a wakeup. + if (config.display.wake_on_tap_or_motion) + gpio_wakeup_enable((gpio_num_t)MOTION_WAKE_INT_PIN, + MOTION_WAKE_INT_ACTIVE_HIGH ? GPIO_INTR_HIGH_LEVEL : GPIO_INTR_LOW_LEVEL); #endif enableLoraInterrupt(); #ifdef PMU_IRQ @@ -524,6 +530,10 @@ esp_sleep_wakeup_cause_t doLightSleep(uint64_t sleepMsec) // FIXME, use a more r #if defined(WAKE_ON_TOUCH) gpio_wakeup_disable((gpio_num_t)SCREEN_TOUCH_INT); #endif +#ifdef MOTION_WAKE_INT_PIN + // Unconditional: the config can have changed while we were asleep. + gpio_wakeup_disable((gpio_num_t)MOTION_WAKE_INT_PIN); +#endif #if !defined(SOC_PM_SUPPORT_EXT_WAKEUP) && defined(LORA_DIO1) && (LORA_DIO1 != RADIOLIB_NC) if (radioType != RF95_RADIO) { gpio_wakeup_disable((gpio_num_t)LORA_DIO1); diff --git a/variants/nrf52840/t-impulse-plus/variant.h b/variants/nrf52840/t-impulse-plus/variant.h index e3a6c7ee63..33cfaff247 100644 --- a/variants/nrf52840/t-impulse-plus/variant.h +++ b/variants/nrf52840/t-impulse-plus/variant.h @@ -157,6 +157,8 @@ static const uint8_t SCL = PIN_WIRE_SCL; // IMU (ICM20948 on Wire1) // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ #define HAS_ICM20948 +// D27, not (0 + 7): pinMode/attachInterrupt index g_ADigitalPinMap, where 7 is P1.13 (RF_VC1). +#define ICM_20948_INT_PIN D27 // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ // Charger (SGM41562 on Wire1 @ 0x03)