Upgrade ESP32 Arduino to 3.3.12 and NimBLE-Arduino to 2.5.1

.github/workflows/compile.yml

@@ -21,8 +21,11 @@ jobs:
       - name: Install ESP32 core and NimBLE
         run: |
           arduino-cli core update-index --additional-urls https://espressif.github.io/arduino-esp32/package_esp32_index.json
-          arduino-cli core install esp32:esp32@3.3.10 --additional-urls https://espressif.github.io/arduino-esp32/package_esp32_index.json
-          arduino-cli lib install NimBLE-Arduino@2.5.0
+          arduino-cli core install esp32:esp32@3.3.12 --additional-urls https://espressif.github.io/arduino-esp32/package_esp32_index.json
+          arduino-cli lib install NimBLE-Arduino@2.5.1
+
+      - name: Run host regression tests
+        run: bash tests/run.sh
 
       - name: Compile TrainerBridge
         run: arduino-cli compile --warnings all --fqbn esp32:esp32:esp32 TrainerBridge

README.md

@@ -66,8 +66,8 @@ $$P_{\text{wheel}} = \left(C_{\text{rr}} \cdot m_{\text{total}} \cdot g\right) v
 
 ## Requirements
 
-- ESP32 Arduino core **3.3.10**
-- [NimBLE-Arduino](https://github.com/h2zero/NimBLE-Arduino) **2.5.0**
+- ESP32 Arduino core **3.3.12**
+- [NimBLE-Arduino](https://github.com/h2zero/NimBLE-Arduino) **2.5.1**
 - Arduino CLI or Arduino IDE
 
 ---
@@ -103,14 +103,14 @@ When operating headless without a serial monitor, the onboard LED provides immed
 
 ## Build and Flash
 
-Install dependencies and compile with zero warnings:
+Install the pinned dependencies and compile with all warnings enabled:
 
 ```bash
 arduino-cli core update-index \
   --additional-urls https://espressif.github.io/arduino-esp32/package_esp32_index.json
-arduino-cli core install esp32:esp32@3.3.10 \
+arduino-cli core install esp32:esp32@3.3.12 \
   --additional-urls https://espressif.github.io/arduino-esp32/package_esp32_index.json
-arduino-cli lib install NimBLE-Arduino@2.5.0
+arduino-cli lib install NimBLE-Arduino@2.5.1
 arduino-cli compile --warnings all --fqbn esp32:esp32:esp32 TrainerBridge
 ```
 
@@ -122,6 +122,26 @@ arduino-cli upload --port /dev/ttyUSB0 --fqbn esp32:esp32:esp32 TrainerBridge
 
 ---
 
+## Software Checks and Hardware Validation
+
+Run the host regression harness with a C++17 compiler:
+
+```bash
+bash tests/run.sh
+```
+
+The harness executes the firmware callbacks and loop with Arduino/NimBLE test doubles,
+including the existing module self-checks, scan retry timing, repeated reconnects,
+multi-client telemetry routing, and FTMS transactions. CI also compiles against the
+real pinned libraries with `--warnings all`. Test doubles do not validate the BLE
+stack, FreeRTOS concurrency, radio delivery, or scan-response timer lifetime.
+
+See [the patch upgrade validation notes](tests/VALIDATION.md) for coverage, known
+limitations, and the physical ESP32/trainer/Garmin/app checks still required for
+ESP32 Arduino 3.3.12 and NimBLE-Arduino 2.5.1.
+
+---
+
 ## Pairing Guide
 
 ### Connecting to Garmin Edge or Watch

tests/VALIDATION.md

@@ -0,0 +1,89 @@
+# Stable patch upgrade validation
+
+## Dependency and API review
+
+- [ESP32 Arduino 3.3.11](https://github.com/espressif/arduino-esp32/releases/tag/3.3.11)
+  and [3.3.12](https://github.com/espressif/arduino-esp32/releases/tag/3.3.12):
+  reviewed both intervening release notes, including ESP-IDF changes. The sketch
+  uses Arduino timing, Serial, GPIO, FreeRTOS critical sections and the classic
+  ESP32 brownout register. It uses NimBLE-Arduino, not the core's separate BLE
+  library, so the core BLE fixes are not proof of a TrainerBridge behavior change.
+- [NimBLE-Arduino 2.5.1](https://github.com/h2zero/NimBLE-Arduino/releases/tag/2.5.1)
+  and its [2.5.0 comparison](https://github.com/h2zero/NimBLE-Arduino/compare/2.5.0...2.5.1):
+  reviewed scan timer lifecycle changes and the public signatures used by this
+  sketch. Scan callbacks/start, client callbacks/connect/subscriptions/writes,
+  server callbacks, broadcast `notify(data, length)` and connection-specific
+  `indicate(data, length, handle)` remain compatible. No firmware migration needed.
+- The scan-response timer fix initializes the callout lazily and tears it down
+  before the host event queue. TrainerBridge calls `NimBLEDevice::init` once and
+  never calls `deinit`; ordinary reconnect reuses the host/client. This does not
+  reproduce or establish exposure to the upstream reinitialization crash.
+
+## Reproducible software checks
+
+```bash
+bash tests/run.sh
+arduino-cli compile --warnings all --fqbn esp32:esp32:esp32 TrainerBridge
+```
+
+`tests/run.sh` includes the actual sketch and links the actual helper modules.
+The stubs only provide hardware/stack boundaries; they are not a NimBLE emulator.
+The Arduino compile is the compatibility check against the real dependencies.
+
+Host assertions cover:
+
+- Existing matcher, LED and virtual-speed self-checks.
+- Nonmatching/matching advertisements, address copying, scan completion, 8-second
+  scans and 3-second retry boundaries; no extra retry on a matched scan stopping.
+- Twenty disconnect/rediscovery/reconnect cycles, without another host init,
+  including queue/filter cleanup, remote-pointer clearing, zero-power telemetry,
+  failed reconnect and failed telemetry subscription.
+- Two subscriber handles receive identical CPS/CSC/FTMS payloads; advertising
+  pauses at two clients and resumes below two after its existing throttle.
+- FTMS Request Control, Start and Reset responses; queued ERG and simulation
+  writes; response routing to the originating handle; successful/failed trainer
+  replies; failed writes; timeout and late-response suppression; a departing app's
+  pending response; empty/oversized writes; truncated telemetry; queue capacity,
+  payload bounds and existing coalescing behavior.
+
+## Results (2026-10-07)
+
+- Host regression harness: PASS, including twenty simulated reconnect cycles.
+- Arduino CLI 1.5.2-rc.1, ESP32 Arduino 3.3.12 and NimBLE-Arduino 2.5.1:
+  the exact compile command above passed with no warnings. Flash: 627,988 /
+  1,310,720 bytes (47%); static RAM: 37,172 / 327,680 bytes (11%).
+- AddressSanitizer and UndefinedBehaviorSanitizer: PASS for the host harness with
+  `ASAN_OPTIONS=detect_leaks=0`. LeakSanitizer cannot inspect processes in this
+  execution environment; neither this run nor the mocks validate physical leaks.
+- Hardware checks below: not executed.
+
+## Existing behavior outside this compatibility change
+
+These are not claims of complete FTMS correctness: the queue coalesces by opcode
+across clients, retaining the latest handle; pending trainer commands are cleared
+on trainer disconnect without an immediate failed indication. The initial Request
+Control attempt occurs before `realConnected` is set, and the write helper declines
+it. The response state machine does not compare a received opcode with the pending
+opcode. These pre-existing semantics are unchanged; concurrent controllers,
+out-of-order/late replies and trainer control acquisition require separate work
+if physical tests expose failures. This patch does not change protocol policy.
+
+## Physical checks still required
+
+No physical ESP32, trainer, Garmin or app is available in the software harness.
+
+1. Flash the build on the supported standard ESP32 and D100. Confirm serial module
+   self-checks, discovery by name/MAC, active scan responses and LED states.
+2. Power-cycle the trainer at least 20 times with Garmin and an app still paired.
+   Check zero power on loss, scan retry after three seconds, rediscovery, fresh
+   telemetry, recovered advertising, and no crash/watchdog/heap growth.
+3. Connect Garmin and an app concurrently. Verify CPS, CSC and FTMS subscriptions,
+   power/speed/distance delivery, disconnect/rejoin and continued delivery to the
+   remaining subscriber. Exercise subscription changes and MTU negotiation.
+4. Exercise Request Control, ERG target power, simulation grade, Start/Stop and
+   Reset against the real trainer. Verify resistance changes and response/status
+   delivery only as intended; disconnect each side during a pending transaction,
+   provoke timeout/late replies, and repeat reconnect plus control acquisition.
+5. The upstream init/deinit timer fix is not exercised by this firmware's normal
+   reconnect. A separate upstream reproduction is needed to validate that fix on
+   hardware; the host harness cannot certify the NimBLE timer or event queue.

tests/firmware_test.cpp

@@ -0,0 +1,169 @@
+#include <cassert>
+#include <iostream>
+#include "../TrainerBridge/TrainerBridge.ino"
+
+// Call private overrides through the public NimBLE callback interface.
+static void disconnectTrainer() {
+  static_cast<NimBLEClientCallbacks*>(&realClientCallbacks)->onDisconnect(realClient,19);
+}
+static void writeCp(std::initializer_list<uint8_t> bytes,uint16_t handle=11) {
+  std::vector<uint8_t> data(bytes);
+  virtualControlPointChr->setValue(data.data(),data.size());
+  NimBLEConnInfo info(handle);
+  static_cast<NimBLECharacteristicCallbacks*>(&virtualControlPointCallbacks)->onWrite(virtualControlPointChr,info);
+}
+static void clearDeliveries() {
+  for(auto* c:{virtualCpsMeasurementChr,virtualCscMeasurementChr,virtualIndoorBikeDataChr,virtualStatusChr,virtualControlPointChr}) {
+    c->notifications.clear(); c->indications.clear();
+  }
+}
+static void reply(uint8_t opcode,uint8_t result=FTMS_CP_RES_SUCCESS) {
+  uint8_t bytes[]={FTMS_CP_OP_RESPONSE_CODE,opcode,result};
+  realControlPointCallback(nullptr,bytes,sizeof(bytes),false);
+}
+static void assertCp(uint8_t opcode,uint8_t result,uint16_t handle) {
+  const auto& d=virtualControlPointChr->indications.back();
+  assert(d.handle==handle);
+  assert((d.bytes==std::vector<uint8_t>{FTMS_CP_OP_RESPONSE_CODE,opcode,result}));
+}
+static void completeScan() {
+  static_cast<NimBLEScanCallbacks*>(&realScanCallbacks)->onScanEnd(NimBLEScanResults{},0);
+}
+int main() {
+  assert(TrainerMatcher::selfCheck());
+  assert(StatusLed::selfCheck());
+  assert(VirtualSpeedModel::selfCheck());
+  setup();
+  assert(NimBLEDevice::initCalls==1);
+  loop();
+  auto* scan=NimBLEDevice::getScan();
+  assert(scanning && scan->starts==1 && scan->duration==8000 && scan->clears==1);
+  NimBLEAdvertisedDevice other; other.name="Other trainer";
+  static_cast<NimBLEScanCallbacks*>(&realScanCallbacks)->onResult(&other);
+  assert(!doConnectReal && scanning);
+  completeScan();
+  assert(doScan && !scanning);
+  const auto retry=lastScanAt;
+  testNow=retry-1; loop(); assert(scan->starts==1);
+  testNow=retry; loop(); assert(scan->starts==2);
+  NimBLEAdvertisedDevice trainer;
+  static_cast<NimBLEScanCallbacks*>(&realScanCallbacks)->onResult(&trainer);
+  assert(doConnectReal && !doScan && !scanning);
+  assert(realTrainerAddress.toString()==trainer.address.toString());
+  completeScan(); assert(!doScan); // Stop completion must not schedule another scan after match.
+  loop();
+  assert(realConnected && proxyReady && realClient);
+  assert(realIndoorBikeDataChr->callback==realIndoorBikeDataCallback);
+  assert(realControlPointChr->callback==realControlPointCallback);
+  proxyServer->connectedCount=2; loop(); assert(!NimBLEDevice::advertising.active);
+  proxyServer->connectedCount=1; testNow+=500; loop(); assert(NimBLEDevice::advertising.active);
+
+  clearDeliveries();
+  uint8_t telemetry[]={0x40,0,0,0,200,0}; // Speed and instantaneous power.
+  realIndoorBikeDataCallback(nullptr,telemetry,sizeof(telemetry),true);
+  for(auto* c:{virtualCpsMeasurementChr,virtualCscMeasurementChr,virtualIndoorBikeDataChr}) {
+    assert(c->notifications.size()==2);
+    assert(c->notifications[0].handle==11 && c->notifications[1].handle==22);
+    assert(c->notifications[0].bytes==c->notifications[1].bytes);
+  }
+  assert(readS16(virtualCpsMeasurementChr->notifications[0].bytes.data(),2)==200);
+  assert(readS16(virtualIndoorBikeDataChr->notifications[0].bytes.data(),7)==200);
+  int16_t parsed=0;
+  assert(!parseRealIndoorBikeData(telemetry,5,parsed));
+  assert(!parseRealIndoorBikeData(telemetry,1,parsed));
+
+  clearDeliveries();
+  writeCp({FTMS_CP_OP_REQUEST_CONTROL});
+  assertCp(FTMS_CP_OP_REQUEST_CONTROL,FTMS_CP_RES_SUCCESS,11);
+  assert(commandQueue.isEmpty());
+  writeCp({FTMS_CP_OP_START_RESUME});
+  assertCp(FTMS_CP_OP_START_RESUME,FTMS_CP_RES_SUCCESS,11);
+  writeCp({FTMS_CP_OP_RESET});
+  assertCp(FTMS_CP_OP_RESET,FTMS_CP_RES_SUCCESS,11);
+  writeCp({});
+  std::vector<uint8_t> oversized(21,FTMS_CP_OP_SET_TARGET_POWER);
+  virtualControlPointChr->setValue(oversized.data(),oversized.size());
+  NimBLEConnInfo info(22);
+  static_cast<NimBLECharacteristicCallbacks*>(&virtualControlPointCallbacks)->onWrite(virtualControlPointChr,info);
+  assertCp(FTMS_CP_OP_SET_TARGET_POWER,FTMS_CP_RES_INVALID_PARAMETER,22);
+  writeCp({FTMS_CP_OP_SET_TARGET_POWER,250,0},22);
+  processQueuedCommands();
+  assert(cpTxState==CpTxState::WAITING_RESPONSE);
+  assert(realControlPointChr->writes.back()==std::vector<uint8_t>({FTMS_CP_OP_SET_TARGET_POWER,250,0}));
+  auto statuses=virtualStatusChr->notifications.size();
+  reply(FTMS_CP_OP_SET_TARGET_POWER);
+  assertCp(FTMS_CP_OP_SET_TARGET_POWER,FTMS_CP_RES_SUCCESS,22);
+  assert(cpTxState==CpTxState::IDLE && activeTargetPower==250);
+  assert(virtualStatusChr->notifications.size()==statuses+2);
+  writeCp({FTMS_CP_OP_SET_INDOOR_BIKE_SIM,0,0,1,0,4,32});
+  processQueuedCommands(); reply(FTMS_CP_OP_SET_INDOOR_BIKE_SIM);
+  assertCp(FTMS_CP_OP_SET_INDOOR_BIKE_SIM,FTMS_CP_RES_SUCCESS,11);
+  assert(virtualStatusChr->notifications.back().bytes.size()==7);
+  writeCp({FTMS_CP_OP_SET_TARGET_POWER,100,0});
+  processQueuedCommands(); statuses=virtualStatusChr->notifications.size();
+  reply(FTMS_CP_OP_SET_TARGET_POWER,FTMS_CP_RES_OPERATION_FAILED);
+  assertCp(FTMS_CP_OP_SET_TARGET_POWER,FTMS_CP_RES_OPERATION_FAILED,11);
+  assert(virtualStatusChr->notifications.size()==statuses && activeTargetPower==250);
+  realControlPointChr->writeOk=false;
+  writeCp({FTMS_CP_OP_STOP_PAUSE,1}); processQueuedCommands();
+  assertCp(FTMS_CP_OP_STOP_PAUSE,FTMS_CP_RES_OPERATION_FAILED,11);
+  realControlPointChr->writeOk=true;
+  writeCp({FTMS_CP_OP_STOP_PAUSE,1}); processQueuedCommands();
+  testNow+=CP_TIMEOUT_MS+1; processQueuedCommands();
+  assertCp(FTMS_CP_OP_STOP_PAUSE,FTMS_CP_RES_OPERATION_FAILED,11);
+  const auto indications=virtualControlPointChr->indications.size();
+  reply(FTMS_CP_OP_STOP_PAUSE);
+  assert(virtualControlPointChr->indications.size()==indications && cpTxState==CpTxState::IDLE);
+
+  // A departing app must not receive the pending trainer response.
+  writeCp({FTMS_CP_OP_SET_TARGET_POWER,150,0}); processQueuedCommands();
+  NimBLEConnInfo departing(11);
+  static_cast<NimBLEServerCallbacks*>(&proxyServerCallbacks)->onDisconnect(proxyServer,departing,19);
+  reply(FTMS_CP_OP_SET_TARGET_POWER);
+  assert(virtualControlPointChr->indications.size()==indications);
+
+  // Repeat disconnect -> delayed scan -> rediscovery -> reconnect, keeping the same host/client.
+  auto* client=realClient;
+  for(int cycle=0;cycle<20;++cycle) {
+    writeCp({FTMS_CP_OP_SET_TARGET_POWER,180,0});
+    processQueuedCommands();
+    assert(cpTxState==CpTxState::WAITING_RESPONSE);
+    writeCp({FTMS_CP_OP_STOP_PAUSE,1});
+    assert(!commandQueue.isEmpty());
+    clearDeliveries();
+    disconnectTrainer();
+    assert(!realConnected && !proxyReady && !realIndoorBikeDataChr && !realControlPointChr);
+    assert(commandQueue.isEmpty() && cpTxState==CpTxState::IDLE);
+    assert(powerFilter.getLastOutput()==0 && virtualSpeedModel.getSpeedMps()==0);
+    assert(!NimBLEDevice::advertising.active && doScan);
+    assert(readS16(virtualCpsMeasurementChr->notifications.back().bytes.data(),2)==0);
+    assert(readS16(virtualIndoorBikeDataChr->notifications.back().bytes.data(),7)==0);
+    writeCp({FTMS_CP_OP_SET_TARGET_POWER,200,0});
+    assertCp(FTMS_CP_OP_SET_TARGET_POWER,FTMS_CP_RES_OPERATION_FAILED,11);
+    const auto starts=scan->starts;
+    testNow=lastScanAt-1; loop(); assert(scan->starts==starts);
+    testNow=lastScanAt; loop(); assert(scan->starts==starts+1);
+    static_cast<NimBLEScanCallbacks*>(&realScanCallbacks)->onResult(&trainer);
+    loop(); assert(realConnected && proxyReady && realClient==client);
+    assert(NimBLEDevice::initCalls==1);
+  }
+  CommandQueue queue;
+  uint8_t command[]={FTMS_CP_OP_SET_TARGET_POWER,1,0};
+  assert(queue.push(command,3,command[0],1,11));
+  command[1]=2; assert(queue.push(command,3,command[0],2,22));
+  QueuedCommand out{}; assert(queue.count()==1 && queue.pop(out));
+  assert(out.targetValue==2 && out.connHandle==22); // Existing cross-client coalescing behavior.
+  assert(!queue.push(nullptr,3,command[0]));
+  assert(!queue.push(command,21,command[0]));
+  // Failed reconnect and failed telemetry subscription return to the scan retry path.
+  disconnectTrainer(); realClient->connectOk=false; doConnectReal=true;
+  loop(); assert(!realClient && !realConnected && doScan);
+  realClient=NimBLEDevice::createClient();
+  realClient->service.data.subscribeOk=false; doConnectReal=true;
+  loop(); assert(!realClient && !proxyReady && doScan);
+  assert(queue.push(command,3,FTMS_CP_OP_STOP_PAUSE));
+  for(size_t i=1;i<CommandQueue::CAPACITY;++i) assert(queue.push(command,3,FTMS_CP_OP_STOP_PAUSE));
+  assert(queue.isFull() && !queue.push(command,3,FTMS_CP_OP_STOP_PAUSE));
+  queue.clear(); assert(queue.isEmpty() && !queue.pop(out));
+  std::cout << "PASS: self-checks, scanning, 20 reconnect cycles, multi-client routing and FTMS transactions\n";
+}

tests/run.sh

@@ -0,0 +1,10 @@
+#!/usr/bin/env bash
+set -euo pipefail
+cd "$(dirname "$0")/.."
+build_dir=$(mktemp -d)
+trap 'rm -rf "$build_dir"' EXIT
+"${CXX:-g++}" -std=c++17 -Wall -Wextra -Wno-unused-parameter \
+  -Itests/stubs -ITrainerBridge tests/firmware_test.cpp \
+  TrainerBridge/{CommandQueue,PowerFilter,TrainerMatcher,VirtualSpeedModel,StatusLed}.cpp \
+  -o "$build_dir/firmware_test"
+"$build_dir/firmware_test"

tests/stubs/Arduino.h

@@ -0,0 +1,37 @@
+#pragma once
+#include <algorithm>
+#include <cctype>
+#include <cstdint>
+#include <cstring>
+#include <string>
+#include <cstdlib>
+#include <cmath>
+inline unsigned long testNow = 0;
+inline unsigned long millis() { return testNow; }
+inline void delay(unsigned long ms) { testNow += ms; }
+constexpr int OUTPUT=1, HIGH=1, LOW=0, HEX=16;
+inline void pinMode(int, int) {}
+inline void digitalWrite(int, int) {}
+class String {
+  std::string s;
+public:
+  String(const char* v=""): s(v) {}
+  size_t length() const { return s.size(); }
+  void toLowerCase() { std::transform(s.begin(),s.end(),s.begin(),[](unsigned char c){return std::tolower(c);}); }
+  int indexOf(const String& v) const { auto p=s.find(v.s); return p==std::string::npos?-1:static_cast<int>(p); }
+  bool operator==(const String& v) const { return s==v.s; }
+  bool startsWith(const char* v) const { return s.rfind(v,0)==0; }
+  String substring(size_t i) const { return String(s.substr(i).c_str()); }
+  void trim() { auto a=s.find_first_not_of(" \t\r\n"); if(a==std::string::npos) {s.clear(); return;} s=s.substr(a,s.find_last_not_of(" \t\r\n")-a+1); }
+  float toFloat() const { return std::strtof(s.c_str(),nullptr); }
+  int toInt() const { return std::atoi(s.c_str()); }
+};
+struct TestSerial {
+  void begin(int) {}
+  template<class... T> void print(T...) {}
+  template<class... T> void println(T...) {}
+  template<class... T> void printf(T...) {}
+  int available() { return 0; }
+  int read() { return 0; }
+};
+inline TestSerial Serial;