From ff94b783751f6812bd2e93a7b5ffade13f5a07fd Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 20 Aug 2026 21:05:40 +0200 Subject: [PATCH] Never ride a link we did not open, and put a shifter on the other pod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes, all from the same evening on the hardware. **Never adopt an existing link.** `setup_session` skipped connecting when it found the peripheral already connected, which is not a shortcut: it means someone else left it that way, and after a shutdown that ran out of budget that someone is our own previous run. The inherited session answers the handshake and streams battery every five seconds while never delivering a button, which reads as broken hardware — it was diagnosed as a dead pod, a wrong bit map, mis-filed pods and a lapsed Zwift unlock before anyone looked at what the last run failed to close. Any pre-existing link is now dropped first, in all three actors, so every connection starts identical. **A watchdog for the same state, should it arise another way.** Deliberately narrow: a link that is plainly alive — frames arriving inside STALE_AFTER — and has never carried a button since it came up is recycled after 150 s. Not "the rider has not shifted lately", which is normal and would strand a pod that only advertises while awake. A link that has delivered even one press is exempt for its lifetime. **`Y` on the `+` pod shifts down.** Shifting down lived entirely on the `−` pod's paddle, so one pod was a single point of failure for half the drivetrain — and with no on-screen gear control on Android, a rider whose left pod goes quiet is stuck in whatever gear they were in, mid-interval, with no way out. This is RISK-9's documented mitigation and it should have been there from the start. Applied in Rust beside the paddles so a shift behaves the same wherever it comes from; removed from the webview so it cannot fire twice. Mode cycling keeps the `m` key. Verified on the tablet: `Y` moved the gear 12 -> 9, and the pods reconnected cleanly with no stale link to purge. Co-Authored-By: Claude Opus 5 --- crates/ble/src/click.rs | 16 +++++++- crates/ble/src/client.rs | 9 +++- crates/ble/src/heart_rate.rs | 9 +++- src-tauri/src/controller.rs | 61 ++++++++++++++++++++++++++++ src-tauri/src/state.rs | 12 ++++++ ui/src/App.svelte | 5 ++- ui/src/components/HelpOverlay.svelte | 2 +- 7 files changed, 105 insertions(+), 9 deletions(-) diff --git a/crates/ble/src/click.rs b/crates/ble/src/click.rs index 7a43076..8c01420 100644 --- a/crates/ble/src/click.rs +++ b/crates/ble/src/click.rs @@ -348,9 +348,21 @@ async fn setup_session( peripheral: Peripheral, asked_for: Option, ) -> Result<(Session, Notifications), FtmsError> { - if !peripheral.is_connected().await.unwrap_or(false) { - peripheral.connect().await?; + // Always ride a link we opened ourselves. Finding the peripheral already + // connected is not a shortcut worth taking — it means someone else left it + // that way, and after a shutdown that ran out of budget that someone is + // usually our own previous run (§7.1, SAF-9). An inherited half-closed + // session answers the handshake and streams battery every five seconds + // while never delivering a single button, which reads as broken hardware + // and is the single most expensive failure this project has had. + // + // Dropping it first costs one round trip on a path that already takes + // seconds, and buys the guarantee that every connection starts identical. + if peripheral.is_connected().await.unwrap_or(false) { + tracing::debug!("click: dropping a pre-existing link before connecting"); + let _ = tokio::time::timeout(DISCONNECT_TIMEOUT, peripheral.disconnect()).await; } + peripheral.connect().await?; peripheral.discover_services().await?; // A Click v2 carries 0xFC82; the trainer carries 00000001-19CA-…. Both hold diff --git a/crates/ble/src/client.rs b/crates/ble/src/client.rs index 0c914a3..53327e4 100644 --- a/crates/ble/src/client.rs +++ b/crates/ble/src/client.rs @@ -1335,9 +1335,14 @@ async fn connect_session( let _ = state_tx.send(ConnectionState::Connecting); - if !peripheral.is_connected().await.unwrap_or(false) { - peripheral.connect().await?; + // Never adopt a link we did not open. A peripheral found already connected + // usually means a previous run's shutdown ran out of budget before closing + // it, and an inherited half-closed session reports some frames and not + // others rather than failing outright (§7.1, SAF-9). + if peripheral.is_connected().await.unwrap_or(false) { + let _ = tokio::time::timeout(DISCONNECT_TIMEOUT, peripheral.disconnect()).await; } + peripheral.connect().await?; // From here on a failure leaves a live GATT link behind, and a peripheral // that accepts one connection at a time (A-3) would stay unavailable to the diff --git a/crates/ble/src/heart_rate.rs b/crates/ble/src/heart_rate.rs index fbd1b0c..13241dc 100644 --- a/crates/ble/src/heart_rate.rs +++ b/crates/ble/src/heart_rate.rs @@ -399,9 +399,14 @@ async fn open_session( } async fn setup_session(peripheral: Peripheral) -> Result<(Session, Notifications), FtmsError> { - if !peripheral.is_connected().await.unwrap_or(false) { - peripheral.connect().await?; + // Never adopt a link we did not open. A peripheral found already connected + // usually means a previous run's shutdown ran out of budget before closing + // it, and an inherited half-closed session reports some frames and not + // others rather than failing outright (§7.1, SAF-9). + if peripheral.is_connected().await.unwrap_or(false) { + let _ = tokio::time::timeout(DISCONNECT_TIMEOUT, peripheral.disconnect()).await; } + peripheral.connect().await?; peripheral.discover_services().await?; let chars = peripheral.characteristics(); diff --git a/src-tauri/src/controller.rs b/src-tauri/src/controller.rs index bb400ab..2f2926e 100644 --- a/src-tauri/src/controller.rs +++ b/src-tauri/src/controller.rs @@ -59,6 +59,17 @@ use tokio::sync::{mpsc, oneshot, watch}; /// explains, and the silence is logged when it fires so the next ride's log can /// replace this guess with the pods' real cadence. const STALE_AFTER: Duration = Duration::from_secs(180); +/// How long a *live* link may go without ever carrying a button before we stop +/// believing in it. +/// +/// This is not "the rider has not shifted lately" — that is normal, and tearing +/// down a working link over it would strand a pod that only advertises while +/// awake. It is the narrower and much stranger case from §7.1: frames arriving +/// steadily, battery every five seconds, and not one press since the link came +/// up. A healthy pod proves itself with its first button and is then never +/// touched by this; a wedged one never does, and recycling costs a few seconds +/// against a pod that is otherwise useless for the whole ride. +const NO_INPUT_AFTER: Duration = Duration::from_secs(150); /// Upper bound on closing the controller links at exit. Shorter than the /// trainer's: there is no reset sequence here, only an unsubscribe and a /// disconnect, and this budget is spent on the same window close (NFR-9). @@ -514,6 +525,11 @@ struct Slot { cancel: Option>, generation: u64, last_seen: Option, + /// When the current link was established, and whether it has ever carried a + /// button. Together these are the signature of the wedged session in §7.1: + /// frames arriving, and not one press among them. + connected_at: Option, + buttons_this_link: u32, /// May this pod be connected the moment the scan sees it? /// /// True until the rider disconnects it by hand, because a pod that @@ -528,6 +544,8 @@ impl Default for Slot { Self { client: None, events: None, + connected_at: None, + buttons_this_link: 0, cancel: None, generation: 0, last_seen: None, @@ -697,6 +715,39 @@ async fn run( } _ = housekeeping.tick() => { + // A link that is plainly alive and has never carried a button + // is the wedged session from §7.1, and no amount of waiting + // fixes it — the pod answers the handshake and streams battery + // for the rest of the ride while ignoring every press. Recycle + // it. `connect_pod` now drops any pre-existing link first, so + // the replacement starts genuinely clean. + // + // Deliberately narrow: a link that has delivered even one press + // is exempt for life, so a rider who simply is not shifting is + // never disturbed. + for id in PodId::BOTH { + let slot = slot_mut(&mut minus, &mut plus, id); + let alive = slot.client.is_some() + && slot.last_seen.is_some_and(|t| t.elapsed() < STALE_AFTER); + let mute = slot.buttons_this_link == 0 + && slot.connected_at.is_some_and(|t| t.elapsed() > NO_INPUT_AFTER); + if alive && mute && slot.auto { + tracing::warn!( + pod = id.as_str(), + "controller: link is alive but has never carried a button; \ + recycling it (see REQUIREMENTS §7.1)" + ); + slot.generation += 1; + slot.connected_at = None; + if let Some(client) = slot.client.take() { + slot.events = None; + tokio::spawn(async move { client.shutdown().await }); + } + status_tx.send_modify(|s| { + s.get_mut(id).state = PodState::Reconnecting; + }); + } + } for (id, slot) in [(PodId::Minus, &minus), (PodId::Plus, &plus)] { let silence = slot.last_seen.map(|t| t.elapsed()); let silent = slot.client.is_some() @@ -796,6 +847,10 @@ fn apply_attempt(attempt: Attempt, slot: &mut Slot, status_tx: &watch::Sender { slot.last_seen = Some(tokio::time::Instant::now()); + if matches!(event, ClickEvent::Button { .. }) { + // This link has now proven it carries input, which puts it + // beyond the no-input watchdog for as long as it lasts. + slot.buttons_this_link = slot.buttons_this_link.saturating_add(1); + } // A frame is proof the link is up, and it is the *only* proof that // ever arrives — the pod does not announce that it has started // talking again. Without this, the staleness sweep below was a one- @@ -994,6 +1054,7 @@ fn apply( pressed, "controller: button" ); + // One press is one press, however many pods reported it. let Some(pressed) = buttons.edge(pod, button, pressed) else { return; diff --git a/src-tauri/src/state.rs b/src-tauri/src/state.rs index 4ccf3ca..b58e552 100644 --- a/src-tauri/src/state.rs +++ b/src-tauri/src/state.rs @@ -323,6 +323,18 @@ pub fn spawn_controller_loop(app: AppHandle) { let delta = match input.button { "plus" => 1i32, "minus" => -1, + // The + pod's `Y`, as a second shift-down. + // + // Shifting down otherwise lives entirely on the + // `−` pod's paddle, which makes one pod a single + // point of failure for half the drivetrain — and + // a rider stuck in top gear mid-interval has no + // way out, because there is no on-screen gear + // control (FR-3.19 is unimplemented on Android). + // This is RISK-9's documented mitigation: put a + // shift on the right pod so the ride survives + // the left one misbehaving. + "y" => -1, _ => 0, }; if delta != 0 { diff --git a/ui/src/App.svelte b/ui/src/App.svelte index f17b61b..b744f84 100644 --- a/ui/src/App.svelte +++ b/ui/src/App.svelte @@ -190,8 +190,9 @@ return run(() => api.togglePause()); case 'b': return run(() => api.markLap()); - case 'y': - return run(() => api.cycleMode()); + // 'y' is shift-down, applied in Rust beside the paddles — see + // `spawn_controller_loop`. Handling it here as well would shift twice per + // press. Mode cycling keeps the 'm' key. case 'z': app.showProfiles = !app.showProfiles; return; diff --git a/ui/src/components/HelpOverlay.svelte b/ui/src/components/HelpOverlay.svelte index 83abb6e..312c372 100644 --- a/ui/src/components/HelpOverlay.svelte +++ b/ui/src/components/HelpOverlay.svelte @@ -14,7 +14,7 @@ ['D-pad ← / →', 'Device screen / ride screen'], ['A', 'Pause / resume — save the FIT on the summary'], ['B', 'Insert lap marker — start a new ride on the summary'], - ['Y', 'Cycle control mode'], + ['Y', 'Shift down a gear — the + pod\u2019s spare shifter'], ['Z', 'Profiles and routes'], ];