From 755f883d66cb3aaefb256d2a79d6e2902c5a932d Mon Sep 17 00:00:00 2001 From: meck Date: Fri, 10 Jul 2026 10:28:14 +0000 Subject: [PATCH] Watches: set the BMA423 step counter watermark, raise ODR to 100 Hz Both watches undercount steps. Two divergences from LilyGo's own configuration for this hardware, applied to both boards: 1. Watermark. LilyGoLib calls setStepCounterWatermark(1) and SensorLib's BMA423_StepDetector example passes step_counter_wm = 1. Meck's raw-I2C enable never touched those bits, leaving whatever the config-file blob defaults to. The watermark is BMA423_STEP_CNTR_WM_MSK (0x03FF), spanning feature_config [0x36] as LSB and the low two bits of [0x37] as MSB; it does not collide with the enable bit, which is bit 4 of [0x37]. Both are now written in one read-modify-write. The bit arithmetic was checked exhaustively against bma423_step_counter_set_watermark() + feature_enable() over all 65536 starting states of cfg[0x36]/cfg[0x37]: identical. 2. ODR. SensorLib's step detector example runs the accelerometer at 100 Hz. Meck used 50 Hz. Everything else already matched: NORMAL, FS_2G, OSR2_AVG2, CIC_AVG_MODE. Reverting the single 100.0f literal to 50.0f A/Bs this against the watermark change. Ruled out: the axis remap is BOTTOM_LAYER_TOP_RIGHT_CORNER on both, identical to LilyGoLib, and the enable bit itself was already register-for-register what bma423_feature_enable(BMA423_STEP_CNTR, TRUE) writes. Not attempted: the 25-value pedometer parameter block at feature_config[0x04..0x35], reachable via bma423_stepcounter_set_parameter() but with no reference values. --- variants/lilygo_twatch_s3/TWatchS3Board.cpp | 21 ++++++++++++++++++- .../TWatchS3PlusBoard.cpp | 21 ++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/variants/lilygo_twatch_s3/TWatchS3Board.cpp b/variants/lilygo_twatch_s3/TWatchS3Board.cpp index 0e60aca4..6037d6f8 100644 --- a/variants/lilygo_twatch_s3/TWatchS3Board.cpp +++ b/variants/lilygo_twatch_s3/TWatchS3Board.cpp @@ -16,6 +16,16 @@ void IRAM_ATTR TWatchS3Board::onTiltISR() { _tilt_flag = true; } #define BMA423_FEATURE_LEN 64 #define BMA423_STEP_EN_BYTE 0x37 // BMA423_STEP_CNTR_OFFSET(0x36) + 1 #define BMA423_STEP_EN_BIT 0x10 // BMA423_STEP_CNTR_EN_MSK +// Step counter watermark: a 10-bit field (BMA423_STEP_CNTR_WM_MSK = 0x03FF) +// spanning feature_config[0x36] as LSB and the low two bits of [0x37] as MSB. +// It does not collide with the enable bit, which is bit 4 of [0x37] (bit 12 of +// the 16-bit word). Bosch's bma423_step_counter_set_watermark() writes exactly +// these bits; SensorLib's enableStepCounter() calls it with 1, and LilyGo's own +// firmware calls setStepCounterWatermark(1). Meck never set it, which is the +// prime suspect for the undercounting. +#define BMA423_STEP_WM_LSB_BYTE 0x36 // BMA423_STEP_CNTR_OFFSET +#define BMA423_STEP_WM_MSK 0x03FF // BMA423_STEP_CNTR_WM_MSK +#define BMA423_STEP_WM_LEVEL 1 // matches LilyGoLib and the SensorLib example #define BMA423_REG_POWER_CONF 0x7C // BMA4_POWER_CONF_ADDR #define BMA423_ADV_PWR_SAVE_BIT 0x01 // BMA4_ADVANCE_POWER_SAVE_MSK @@ -49,6 +59,11 @@ static void bma423EnableStepCounter() { uint8_t cfg[BMA423_FEATURE_LEN]; if (bma423ReadRegs(BMA423_REG_FEATURE_CONFIG, cfg, BMA423_FEATURE_LEN)) { + // Watermark first, then the enable bit, in a single read-modify-write. + uint16_t wm = ((uint16_t)cfg[BMA423_STEP_EN_BYTE] << 8) | cfg[BMA423_STEP_WM_LSB_BYTE]; + wm = (wm & ~BMA423_STEP_WM_MSK) | (BMA423_STEP_WM_LEVEL & BMA423_STEP_WM_MSK); + cfg[BMA423_STEP_WM_LSB_BYTE] = (uint8_t)(wm & 0xFF); + cfg[BMA423_STEP_EN_BYTE] = (uint8_t)((wm >> 8) & 0xFF); cfg[BMA423_STEP_EN_BYTE] |= BMA423_STEP_EN_BIT; bma423WriteRegs(BMA423_REG_FEATURE_CONFIG, cfg, BMA423_FEATURE_LEN); delay(1); // write settle @@ -66,8 +81,12 @@ void TWatchS3Board::begin() { _accel = new SensorBMA423(); if (_accel->begin(Wire, I2C_ADDR_ACCEL, PIN_BOARD_SDA, PIN_BOARD_SCL)) { _accel->setRemapAxes(SensorRemap::BOTTOM_LAYER_TOP_RIGHT_CORNER); + // 100 Hz ODR: SensorLib's BMA423_StepDetector example runs the pedometer at + // 100 Hz, not the 50 Hz used here previously. Everything else already matched + // (NORMAL, FS_2G, OSR2_AVG2, CIC_AVG_MODE). Revert this one literal to 50.0f + // to A/B it against the watermark change above. _accel->configAccelerometer(OperationMode::NORMAL, AccelFullScaleRange::FS_2G, - 50.0f, AccelBandwidth::OSR2_AVG2, AccelPerfMode::CIC_AVG_MODE); + 100.0f, AccelBandwidth::OSR2_AVG2, AccelPerfMode::CIC_AVG_MODE); // INT1 pin electrical config: level trigger, active high, push-pull, // output enabled. INT1_IO_CTRL resets to output-disabled, so without // this the pin never drives and INPUT_PULLDOWN reads low forever. diff --git a/variants/lilygo_twatch_s3_plus/TWatchS3PlusBoard.cpp b/variants/lilygo_twatch_s3_plus/TWatchS3PlusBoard.cpp index 9a7fe685..2957900c 100644 --- a/variants/lilygo_twatch_s3_plus/TWatchS3PlusBoard.cpp +++ b/variants/lilygo_twatch_s3_plus/TWatchS3PlusBoard.cpp @@ -17,6 +17,16 @@ void IRAM_ATTR TWatchS3PlusBoard::onTiltISR() { _tilt_flag = true; _tilt_isr_cou #define BMA423_FEATURE_LEN 64 #define BMA423_STEP_EN_BYTE 0x37 // BMA423_STEP_CNTR_OFFSET(0x36) + 1 #define BMA423_STEP_EN_BIT 0x10 // BMA423_STEP_CNTR_EN_MSK +// Step counter watermark: a 10-bit field (BMA423_STEP_CNTR_WM_MSK = 0x03FF) +// spanning feature_config[0x36] as LSB and the low two bits of [0x37] as MSB. +// It does not collide with the enable bit, which is bit 4 of [0x37] (bit 12 of +// the 16-bit word). Bosch's bma423_step_counter_set_watermark() writes exactly +// these bits; SensorLib's enableStepCounter() calls it with 1, and LilyGo's own +// firmware calls setStepCounterWatermark(1). Meck never set it, which is the +// prime suspect for the undercounting. +#define BMA423_STEP_WM_LSB_BYTE 0x36 // BMA423_STEP_CNTR_OFFSET +#define BMA423_STEP_WM_MSK 0x03FF // BMA423_STEP_CNTR_WM_MSK +#define BMA423_STEP_WM_LEVEL 1 // matches LilyGoLib and the SensorLib example #define BMA423_REG_POWER_CONF 0x7C // BMA4_POWER_CONF_ADDR #define BMA423_ADV_PWR_SAVE_BIT 0x01 // BMA4_ADVANCE_POWER_SAVE_MSK @@ -50,6 +60,11 @@ static void bma423EnableStepCounter() { uint8_t cfg[BMA423_FEATURE_LEN]; if (bma423ReadRegs(BMA423_REG_FEATURE_CONFIG, cfg, BMA423_FEATURE_LEN)) { + // Watermark first, then the enable bit, in a single read-modify-write. + uint16_t wm = ((uint16_t)cfg[BMA423_STEP_EN_BYTE] << 8) | cfg[BMA423_STEP_WM_LSB_BYTE]; + wm = (wm & ~BMA423_STEP_WM_MSK) | (BMA423_STEP_WM_LEVEL & BMA423_STEP_WM_MSK); + cfg[BMA423_STEP_WM_LSB_BYTE] = (uint8_t)(wm & 0xFF); + cfg[BMA423_STEP_EN_BYTE] = (uint8_t)((wm >> 8) & 0xFF); cfg[BMA423_STEP_EN_BYTE] |= BMA423_STEP_EN_BIT; bma423WriteRegs(BMA423_REG_FEATURE_CONFIG, cfg, BMA423_FEATURE_LEN); delay(1); // write settle @@ -67,8 +82,12 @@ void TWatchS3PlusBoard::begin() { _accel = new SensorBMA423(); if (_accel->begin(Wire, I2C_ADDR_ACCEL, PIN_BOARD_SDA, PIN_BOARD_SCL)) { _accel->setRemapAxes(SensorRemap::BOTTOM_LAYER_TOP_RIGHT_CORNER); + // 100 Hz ODR: SensorLib's BMA423_StepDetector example runs the pedometer at + // 100 Hz, not the 50 Hz used here previously. Everything else already matched + // (NORMAL, FS_2G, OSR2_AVG2, CIC_AVG_MODE). Revert this one literal to 50.0f + // to A/B it against the watermark change above. _accel->configAccelerometer(OperationMode::NORMAL, AccelFullScaleRange::FS_2G, - 50.0f, AccelBandwidth::OSR2_AVG2, AccelPerfMode::CIC_AVG_MODE); + 100.0f, AccelBandwidth::OSR2_AVG2, AccelPerfMode::CIC_AVG_MODE); // INT1 pin electrical config: level trigger, active high, push-pull, // output enabled. INT1_IO_CTRL resets to output-disabled, so without // this the pin never drives and INPUT_PULLDOWN reads low forever.