From 7497a5d602f816268a75018024fe89c6859072fc Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Fri, 21 Aug 2026 19:36:34 +0200 Subject: [PATCH] Reconnect to remembered hardware instead of pairing every launch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `remembered` was a HashSet inside DeviceRegistry, so it lasted exactly as long as the process. Every launch started from nothing: find the trainer, press Connect, find the strap, press Connect, and only then ride. FR-1.5 has been a Should since the beginning and was never actually true. It is a file now — devices.json in the app data directory, written when a link actually comes up rather than when Connect is pressed. A Connect the hardware then refuses is not a pairing, and writing one down would mean a trainer the rider gave up on getting chased on every launch afterwards. Forgetting is recorded too, in its own list: absent means never seen, forgotten means the rider looked at this device and said no, and auto-connect has to keep honouring that on the next launch as well. The file is advisory — a corrupt one costs auto-connect, never a ride. Auto-connect is driven by the scan rather than fired once at startup. The hardware is asleep at startup — a trainer wakes when the cranks turn, a strap when it is put on (A-4) — so a remembered device is reconnected the moment it advertises, through the same path the rider's own click takes, scan suspension included. Bounded by AUTO_ATTEMPTS on an AUTO_RETRY cooldown and cleared when the link comes up or the rider connects by hand: an app that never stops trying can never honestly say it has stopped (FR-1.11). A device disconnected by hand is left alone for the rest of the session, since a disconnect that undoes itself two ticks later is not a disconnect. Pods now prefer the pod we know. Every Click advertises the same name and the same type byte, so before this a rider whose partner was warming up in the next room got whichever pod woke first. With nothing of that kind remembered anything still goes, or there could never be a first pairing. And the pair is one pod, not two. Confirmed on this hardware 2026-08-21: pairing the − pod alone delivers all ten buttons, its twin's included — which §2.3.1 had established for the frames but not for the pairing. So take_plus_pod holds the + pod back while a known − pod may merely be asleep, and connect_controller with no pod named means the − pod rather than both. The wait is bounded by PLUS_GRACE, because a flat − pod should cost the rider a D-pad and not a controller, and Buttons is untouched: it is what makes the handover between the two configurations invisible. Not yet tested against real hardware — nothing was advertising here. The store, the retry budget and the pod-preference rules have unit tests, and a seeded devices.json was confirmed to load and seed the − pod at launch, but the connect path itself waits for a ride. Co-Authored-By: Claude Opus 5 (1M context) --- REQUIREMENTS.md | 13 + src-tauri/src/commands.rs | 15 +- src-tauri/src/controller.rs | 146 +++++++++- src-tauri/src/devices.rs | 338 ++++++++++++++++++++-- src-tauri/src/known.rs | 311 ++++++++++++++++++++ src-tauri/src/lib.rs | 10 + ui/src/components/ClickPanel.svelte | 101 ++++--- ui/src/components/ConnectionScreen.svelte | 28 +- 8 files changed, 877 insertions(+), 85 deletions(-) create mode 100644 src-tauri/src/known.rs diff --git a/REQUIREMENTS.md b/REQUIREMENTS.md index ebda221..b285e7f 100644 --- a/REQUIREMENTS.md +++ b/REQUIREMENTS.md @@ -188,6 +188,17 @@ button we have found drives them. > so it is evidence of a wrong-way-round pair only while that pod has never sent its own — > otherwise the connection screen asks the rider to swap a pair that is filed correctly. +> **One link is the whole controller — confirmed 2026-08-21.** Pairing the `−` pod *alone* +> delivers all ten buttons on this hardware: its own paddle and D-pad, plus the `+` paddle and +> face buttons relayed from its twin. There is nothing a second link adds, and there is one +> thing it takes away — connected as a pair, the `−` pod stops reporting its own paddle. +> +> So the app pairs the `−` pod and stops there (`controller::take_plus_pod`). The `+` pod is a +> **substitute, not a second half**: it is connected on sight only when no `−` pod is known, or +> when a known one has been unreachable for `PLUS_GRACE`, so a flat `−` pod costs the rider a +> D-pad rather than a controller. `Buttons` stays exactly as it is — it is what makes the +> handover between the two configurations invisible. + ### 2.3.2 The D100's own Zwift service — telemetry, not shifting The trainer answers the same handshake (`RideOn 00 09` → `RideOn 02 00`) on the original @@ -390,6 +401,8 @@ bikecontrol/ | FR-1.3 | Connect to trainer and controller independently; either may connect first | Must | | FR-1.4 | Track the left and right pods as separate connections, since each is an independent peripheral | Must | | FR-1.5 | Remember paired devices and auto-connect on launch | Should | +| FR-1.5a | Remembering survives the process: trainer, `−` pod and heart rate monitor are written to `devices.json` in the app data directory when a link actually comes up, and a device the rider forgets is recorded as refused rather than merely dropped | Should | +| FR-1.5b | Auto-connect is driven by the scan, not by startup — the hardware is asleep at launch (A-4), so a remembered device is reconnected the moment it advertises. Bounded by `AUTO_ATTEMPTS`, so "gave up" (FR-1.11) is not contradicted two ticks later, and suspended for any device the rider disconnected by hand this session | Should | | FR-1.6 | Auto-reconnect on unexpected disconnect, with backoff, without ending the ride | Must | | FR-1.7 | Surface per-device connection state (scanning / connecting / connected / lost) | Must | | FR-1.8 | When nothing is found, prompt to wake the device (per A-4) — pedal the trainer, press a Click button | Must | diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 8377f04..54446b0 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -728,7 +728,14 @@ pub fn controller_status(state: State<'_, AppState>) -> ControllerStatus { state.controller().status() } -/// Connect a Click pod, or both when `pod` is omitted (FR-1.4). +/// Connect a Click pod, or the one that matters when `pod` is omitted (FR-1.4). +/// +/// **Omitting `pod` means the `−` pod, not both.** One link is the whole +/// controller: the `−` pod relays its twin's paddle and face buttons, so all +/// ten buttons arrive over it alone (§2.3.1, confirmed in the field +/// 2026-08-21). Connecting the pair adds nothing and breaks one thing — the +/// `−` pod stops reporting its own paddle. The `+` pod keeps its own button on +/// the connection screen for the case where the `−` pod is flat or absent. /// /// `device_id` is an address, for a specific pod the scanner has already /// listed. Without one the supervisor looks the pod up by the type byte in its @@ -753,12 +760,8 @@ pub fn connect_controller( if address.is_some() { return Err("An address names one pod, so say which pod it is".into()); } - // Both, each on its own schedule: a pod that is awake connects now - // rather than queueing behind its sleeping twin. let known = state.lock().devices.click_pod_addresses(); - for id in PodId::BOTH { - controller.connect(id, known.get(&id).cloned()); - } + controller.connect(PodId::Minus, known.get(&PodId::Minus).cloned()); } } Ok(()) diff --git a/src-tauri/src/controller.rs b/src-tauri/src/controller.rs index d1b45bd..f5d9bca 100644 --- a/src-tauri/src/controller.rs +++ b/src-tauri/src/controller.rs @@ -74,6 +74,19 @@ const NO_INPUT_AFTER: Duration = Duration::from_secs(150); /// 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). const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(3); +/// How long a known `−` pod is waited for before a lone `+` pod will do. +/// +/// One link is the whole controller: the `−` pod relays its twin's paddle and +/// face buttons (§2.3.1), so with a `−` pod in the house there is nothing for a +/// second link to add and one specific thing for it to break — connected as a +/// pair, the `−` pod stops reporting its *own* paddle. Confirmed in the field +/// 2026-08-21: pairing the `−` pod alone gives all ten buttons. +/// +/// But holding out forever would mean a flat or lost `−` pod costs the rider +/// their `+` paddle as well, which is a worse trade than a redundant link. So +/// the wait is bounded: long enough for a rider to wake both pods in whatever +/// order they like, short enough that half a controller beats none. +const PLUS_GRACE: Duration = Duration::from_secs(45); /// How long auto-reconnect keeps chasing a pod before it gives up and says so. /// /// FR-1.11. More generous than the trainer's, because a Click genuinely does @@ -379,6 +392,14 @@ enum Cmd { /// Connect one pod. `address` is used when the scanner has already seen it, /// which is both faster and unambiguous. Connect { pod: PodId, address: Option }, + /// This is the pod we paired with last time (FR-1.5). + /// + /// Sets the slot's address without touching the radio, so a pod that is + /// still asleep is nonetheless *known* — which is what lets the `+` pod + /// hold back for a `−` pod that has not woken up yet, and what gives a + /// manual connect an address to go straight to instead of a type byte to + /// go hunting with. + Remember { pod: PodId, address: String }, /// The scanner has this pod in view *right now* (FR-1.5). /// /// The whole difficulty with a Click is that it advertises for only a few @@ -456,6 +477,14 @@ impl ControllerHandle { let _ = self.cmd_tx.try_send(Cmd::Connect { pod, address }); } + /// Seed the pod we paired with last time, from the remembered-device store. + pub fn remember(&self, pod: PodId, address: &str) { + let _ = self.cmd_tx.try_send(Cmd::Remember { + pod, + address: address.to_string(), + }); + } + /// Tell the supervisor a pod is advertising right now (FR-1.5). /// /// Called from the device scan on every pass. Cheap and idempotent: the @@ -587,6 +616,10 @@ async fn run( let mut swapped = false; // One press is one press, whichever pods report it (see `Buttons`). let mut buttons = Buttons::default(); + // When a lone `+` pod stops being worse than no controller at all. Pushed + // back whenever the `−` pod is reachable, so it only ever expires against a + // `−` pod that is genuinely not coming (see `PLUS_GRACE`). + let mut plus_gate = tokio::time::Instant::now() + PLUS_GRACE; let (attempt_tx, mut attempt_rx) = mpsc::channel::(4); let mut housekeeping = tokio::time::interval(Duration::from_secs(5)); @@ -618,29 +651,43 @@ async fn run( let selector = selector_for(pod, address, &status_tx.borrow(), swapped); tracing::info!(pod = pod.as_str(), selector = %selector.describe(), "controller: connecting"); start_attempt(slot, pod, selector, &attempt_tx); + if pod == PodId::Minus { + plus_gate = tokio::time::Instant::now() + PLUS_GRACE; + } status_tx.send_modify(|s| s.get_mut(pod).reset_link(PodState::Searching)); } + Cmd::Remember { pod, address } => { + // Only fills a gap. A pod we have actually talked to + // this session knows its own address better than a file + // written last week does. + status_tx.send_modify(|s| { + let p = s.get_mut(pod); + if p.address.is_none() && !address.trim().is_empty() { + tracing::info!(pod = pod.as_str(), %address, "controller: remembered pod"); + p.address = Some(address); + } + }); + } Cmd::Seen { pod, address } => { - // One link is the whole controller. - // - // The `−` pod relays its twin: connected on its own it - // delivers its own paddle and D-pad *and* the `+` - // paddle and face buttons — all ten buttons, measured - // over 445 frames on one characteristic (§2.3.1). So - // the `+` pod is not connected while the `−` pod is - // there to speak for it. It stays a fallback for the - // case where the `−` pod is absent or the rider only - // owns that half. - // - // Connecting both is what the pair-merge in `Buttons` + // One link is the whole controller — see + // `take_plus_pod`, which owns this rule. The short of + // it: the `−` pod relays its twin, so all ten buttons + // arrive over it alone (§2.3.1, confirmed in the field + // 2026-08-21), and a `+` link is a substitute for a `−` + // pod that is missing rather than the other half of a + // pair. Connecting both is what the merge in `Buttons` // exists to paper over, and it is also the // configuration in which the `−` pod stops reporting // its own paddle — the failure that cost an evening. - // Not opening the second link removes both. - if pod == PodId::Plus && !slot_mut(&mut minus, &mut plus, PodId::Minus).idle() + if pod == PodId::Plus + && !take_plus_pod( + minus.idle(), + status_tx.borrow().minus.address.is_some(), + tokio::time::Instant::now() >= plus_gate, + ) { tracing::debug!( - "controller: + pod seen but the − pod is already speaking for it" + "controller: + pod seen; the − pod speaks for the pair" ); continue; } @@ -653,6 +700,9 @@ async fn run( // already found are the seconds it goes back to sleep in. tracing::info!(pod = pod.as_str(), %address, "controller: pod seen; connecting"); start_attempt(slot, pod, PodSelector::Address(address.clone()), &attempt_tx); + if pod == PodId::Minus { + plus_gate = tokio::time::Instant::now() + PLUS_GRACE; + } status_tx.send_modify(|s| { let p = s.get_mut(pod); p.address = Some(address); @@ -738,6 +788,13 @@ async fn run( } _ = housekeeping.tick() => { + // A `−` pod that is up, or on its way up, is a `−` pod worth + // waiting for. Only a slot that has been idle for the whole + // grace period lets the `+` pod in. + if !minus.idle() { + plus_gate = tokio::time::Instant::now() + PLUS_GRACE; + } + // Converge on one link. The `+` pod may have connected first — // it is the one the rider happened to wake — and once the `−` // pod is up it speaks for both, so the second link is redundant @@ -820,6 +877,25 @@ async fn run( } } +/// May a `+` pod the scan has just seen be connected? +/// +/// The `−` pod is the controller (§2.3.1): connected on its own it delivers all +/// ten buttons, its twin's included. So a `+` link is only ever a *substitute*, +/// and opening one alongside a working `−` link is the configuration in which +/// the `−` pod stops reporting its own paddle. +/// +/// Three inputs, in the order they decide: +/// - `minus_idle` — false when the `−` pod is connected or being connected. +/// Nothing else matters then: it is already speaking for both. +/// - `minus_known` — we have an address for a `−` pod, from this session or +/// from the remembered-device store. With none, there is no `−` pod to wait +/// for and the `+` pod is the whole controller. +/// - `gate_expired` — the `−` pod has been unreachable for [`PLUS_GRACE`]. +/// A flat or lost `−` pod must not cost the rider their `+` paddle too. +fn take_plus_pod(minus_idle: bool, minus_known: bool, gate_expired: bool) -> bool { + minus_idle && (!minus_known || gate_expired) +} + fn slot_mut<'a>(minus: &'a mut Slot, plus: &'a mut Slot, pod: PodId) -> &'a mut Slot { match pod { PodId::Minus => minus, @@ -1574,6 +1650,46 @@ mod tests { assert!(!s.plus.contradicted && !s.minus.contradicted); } + #[test] + fn one_link_is_the_whole_controller() { + // Confirmed in the field 2026-08-21: pairing the − pod alone gives all + // ten buttons, because it relays its twin (§2.3.1). So the + pod is a + // substitute, never a second half. + + // The − pod is up, or on its way up. Nothing else matters. + assert!(!take_plus_pod(false, true, true)); + assert!(!take_plus_pod(false, false, true)); + + // We know a − pod exists and it has not been out of reach for long. It + // is almost certainly just asleep — a Click only advertises while awake + // (A-4) — so wait rather than open a link we would only close again. + assert!(!take_plus_pod(true, true, false)); + + // No − pod has ever been seen or remembered: this rider's + pod *is* + // their controller, and making them wait for a pod they do not own + // would be waiting forever. + assert!(take_plus_pod(true, false, false)); + + // The − pod is known but has stayed out of reach. Flat, or left in the + // garage. Half a controller beats none. + assert!(take_plus_pod(true, true, true)); + } + + #[test] + fn waiting_for_the_minus_pod_is_shorter_than_giving_up_on_it() { + // The grace period is a pause, not a policy: it must expire long before + // the reconnect budget does, or a rider whose − pod is flat sits with no + // controller at all while the app keeps hoping. + assert!( + PLUS_GRACE <= Duration::from_secs(60), + "a rider with a flat − pod waits {PLUS_GRACE:?} for their + paddle" + ); + assert!( + PLUS_GRACE >= Duration::from_secs(20), + "shorter than the time it takes to wake two pods by hand" + ); + } + #[test] fn a_missing_pod_is_reported_as_something_to_do_not_as_not_found() { let advice = connect_advice(PodId::Minus, &FtmsError::NotFound("the − Click pod".into())); diff --git a/src-tauri/src/devices.rs b/src-tauri/src/devices.rs index 237f868..0e718c1 100644 --- a/src-tauri/src/devices.rs +++ b/src-tauri/src/devices.rs @@ -19,7 +19,8 @@ //! point is refused. use std::collections::{HashMap, HashSet}; -use std::time::Duration; +use std::path::PathBuf; +use std::time::{Duration, Instant}; use bikecontrol_ble::scan::{self, DiscoveredDevice, ScanKind, ZWIFT_SERVICE}; use bikecontrol_ble::uuids; @@ -31,6 +32,7 @@ use uuid::Uuid; use crate::controller::{ControllerHandle, PodState}; use crate::heart_rate::{HeartRateHandle, HeartRateStatus}; +use crate::known::KnownDevices; use crate::trainer::{TrainerHandle, TrainerStatus}; /// One pass of the scanner. Long enough for a trainer to advertise, short @@ -41,6 +43,23 @@ const IDLE_POLL: Duration = Duration::from_millis(400); /// How long to wait before looking for the adapter again. Longer than the scan /// cadence: nothing the rider can do about a missing radio happens in 400 ms. const ADAPTER_RETRY: Duration = Duration::from_secs(2); +/// How long before auto-connect tries a remembered device again (FR-1.5). +/// +/// An attempt only starts against a peripheral the scan can *see*, so the +/// common case needs no backing off at all. This is for the awkward one: a +/// trainer that advertises happily and then refuses the connect, which without +/// a cooldown would be retried every device tick — four failed connects a +/// second, all of them fighting the scan for the one adapter. +const AUTO_RETRY: Duration = Duration::from_secs(15); +/// How many times auto-connect will chase one device before leaving it alone. +/// +/// FR-1.11 in the small: giving up has to be terminal until the rider acts, and +/// an unbounded retry would quietly undo it — the trainer supervisor announces +/// that it has stopped trying, and two ticks later the device list starts the +/// whole thing again. Cleared the moment the link does come up, and by any +/// Connect the rider presses themselves, so the budget is per problem rather +/// than per lifetime. +const AUTO_ATTEMPTS: u32 = 5; /// What to suggest when there is no adapter. The remedy is platform-specific /// and telling an Android rider to check BlueZ is worse than saying nothing. @@ -126,6 +145,14 @@ pub struct PollResult { pub hr_changed: Option, } +/// Auto-connect's memory of one address: when it last tried, and how many +/// times it has tried since the last time the link actually came up. +#[derive(Debug, Clone, Copy)] +struct AutoAttempts { + last: Instant, + tries: u32, +} + /// What the scan task publishes. #[derive(Debug, Clone, Default)] pub struct ScanSnapshot { @@ -146,8 +173,23 @@ pub struct DeviceRegistry { hr: HeartRateHandle, scan_rx: watch::Receiver, scan_on: watch::Sender, - forgotten: HashSet, - remembered: HashSet, + /// Everything the rider has paired with, and everything they have refused. + /// Backed by a file, so it outlives the process (FR-1.5). + known: KnownDevices, + /// Links the rider closed by hand *this session*. Auto-connect must not + /// undo a deliberate disconnect two ticks later — that is not a disconnect, + /// it is a flicker. Deliberately not persisted: a fresh launch is a fresh + /// intention, and next time they start the app they do want their trainer. + auto_off: HashSet, + /// What auto-connect has already tried, per address, so a device that + /// advertises but will not connect is retried on a schedule and then let + /// be (see [`AUTO_RETRY`], [`AUTO_ATTEMPTS`]). + auto: HashMap, + /// Addresses the most recent scan pass actually saw. A remembered device is + /// only chased while it is advertising: a connect against a sleeping + /// peripheral burns the whole scan timeout for nothing, and the scan is + /// already telling us the moment it wakes (A-4). + seen_now: HashSet, /// The list published last tick, for change detection. published: Vec, last_trainer: TrainerStatus, @@ -173,8 +215,10 @@ impl DeviceRegistry { hr, scan_rx, scan_on, - forgotten: HashSet::new(), - remembered: HashSet::new(), + known: KnownDevices::default(), + auto_off: HashSet::new(), + auto: HashMap::new(), + seen_now: HashSet::new(), published: Vec::new(), scanning: false, scan_suspended: false, @@ -182,6 +226,29 @@ impl DeviceRegistry { } } + /// Load the remembered devices and tell the supervisors what we know + /// (FR-1.5). + /// + /// Separate from `new` because `AppState::new` runs before Tauri can hand + /// out an `AppHandle`, and without one there is no data directory to read. + /// Until this is called the registry remembers nothing, which is the + /// correct behaviour for the handful of milliseconds it lasts. + pub fn attach_store(&mut self, path: PathBuf) { + self.known = KnownDevices::load(path); + // Hand the controller the pod we paired with last time. It does not + // connect anything — it is what lets the supervisor hold out for *our* + // − pod instead of grabbing the first Click that happens to wake up. + for kind in [DeviceKind::ClickMinus, DeviceKind::ClickPlus] { + if let (Some(pod), Some(known)) = (kind.pod_id(), self.known.first_of(kind)) { + self.controller.remember(pod, &known.address); + } + } + tracing::info!( + count = self.known.len(), + "remembered devices loaded; they will reconnect as they advertise" + ); + } + /// Rider-initiated. Cancels any suspension: an explicit request outranks /// our own bookkeeping in both directions. pub fn start_scan(&mut self) { @@ -229,6 +296,10 @@ impl DeviceRegistry { let changed = next != self.published; self.published = next; + // After publishing, never before: `connect` reads the published list, + // and a device discovered this tick has to be in it to be connectable. + self.auto_connect(); + PollResult { changed, transitions, @@ -244,10 +315,12 @@ impl DeviceRegistry { let controller = self.controller.status(); + self.seen_now.clear(); let mut out: Vec = Vec::with_capacity(snapshot.devices.len() + 1); for d in &snapshot.devices { let id = d.address.clone(); - if self.forgotten.contains(&id) { + self.seen_now.insert(id.clone()); + if self.known.is_forgotten(&id) { continue; } let kind = classify(d); @@ -257,8 +330,15 @@ impl DeviceRegistry { // is the pod the rider already told us about by pressing a button // on it. So the scan connects it (FR-1.5), unless they disconnected // it on purpose, in which case the supervisor ignores this. + // + // Once we have paired with a pod, it is that pod we chase. Every + // Click advertises the same name and the same type byte, so without + // this a rider whose partner is warming up in the same room gets + // whichever pod woke first. if let Some(pod) = kind.pod_id() { - self.controller.pod_seen(pod, &id); + if is_ours(&self.known, kind, &id) { + self.controller.pod_seen(pod, &id); + } } out.push(DeviceInfo { kind, @@ -272,7 +352,9 @@ impl DeviceRegistry { }, control_acquired: false, services: d.services.iter().map(|u| describe_service(*u)).collect(), - remembered: self.remembered.contains(&id), + // Filled in below, once every row exists: the pass that records + // a live link is the same pass that reads the store back. + remembered: false, battery_pct: None, heart_rate_bpm: None, error: None, @@ -296,7 +378,7 @@ impl DeviceRegistry { state: ConnectionState::Idle, control_acquired: false, services: vec![describe_service(uuids::FITNESS_MACHINE_SERVICE)], - remembered: true, + remembered: false, battery_pct: None, heart_rate_bpm: None, error: None, @@ -308,7 +390,6 @@ impl DeviceRegistry { device.kind = DeviceKind::Trainer; device.state = trainer.state.clone(); device.control_acquired = trainer.control_acquired; - device.remembered = true; device.error = trainer.error.clone().or_else(|| { trainer .stale @@ -329,7 +410,7 @@ impl DeviceRegistry { let Some(address) = pod.address.clone() else { continue; }; - if self.forgotten.contains(&address) { + if self.known.is_forgotten(&address) { continue; } let kind = match id { @@ -348,7 +429,7 @@ impl DeviceRegistry { state: ConnectionState::Idle, control_acquired: false, services: vec![describe_service(ZWIFT_SERVICE)], - remembered: true, + remembered: false, battery_pct: None, heart_rate_bpm: None, error: None, @@ -360,7 +441,6 @@ impl DeviceRegistry { device.kind = kind; device.battery_pct = pod.battery_percent; device.error = pod.error.clone(); - device.remembered = true; device.state = match pod.state { PodState::Connected => ConnectionState::Connected, PodState::Searching => ConnectionState::Connecting, @@ -380,7 +460,7 @@ impl DeviceRegistry { // link. let hr = self.hr.status(); if let Some(address) = hr.address.clone() { - if !self.forgotten.contains(&address) { + if !self.known.is_forgotten(&address) { let idx = match out.iter().position(|d| d.id == address) { Some(i) => i, None => { @@ -396,7 +476,7 @@ impl DeviceRegistry { state: ConnectionState::Idle, control_acquired: false, services: vec![describe_service(HEART_RATE_SERVICE)], - remembered: true, + remembered: false, battery_pct: None, heart_rate_bpm: None, error: None, @@ -409,7 +489,6 @@ impl DeviceRegistry { device.battery_pct = hr.battery_percent; device.heart_rate_bpm = hr.bpm; device.error = hr.error.clone(); - device.remembered = true; if let Some(name) = &hr.name { device.name = name.clone(); } @@ -429,9 +508,95 @@ impl DeviceRegistry { .then(b.rssi.cmp(&a.rssi)) .then(a.id.cmp(&b.id)) }); + + // A link that actually came up is what "paired" means (FR-1.5) — not a + // Connect the rider pressed against a trainer that then refused them. + // So the store is written from the finished list rather than from the + // request, and the row's `remembered` flag is read straight back out of + // it, which keeps the screen and the file incapable of disagreeing. + for device in &out { + if matches!( + device.state, + ConnectionState::Connected | ConnectionState::Controlling + ) { + self.known + .remember(&device.address, Some(&device.name), device.kind); + // And a link that came up settles whatever auto-connect was + // struggling with, so the next problem starts from a full + // budget rather than inheriting the last one's. + self.auto.remove(&device.id); + } + } + for device in &mut out { + device.remembered = self.known.contains(&device.address); + } out } + /// Reconnect to remembered hardware as it turns up (FR-1.5). + /// + /// This is the whole feature from the rider's side: launch the app, get on + /// the bike, pedal, and the trainer and strap are simply there — no trip to + /// the device screen, no Connect button, no repairing what was paired last + /// week. Deliberately driven by the scan rather than fired once at startup, + /// because the hardware is asleep at startup: a trainer wakes when the + /// cranks turn and a strap when it is put on, and *that* is the moment + /// worth acting on (A-4, FR-1.8). + /// + /// Click pods are absent here on purpose. They already have this, in the + /// shape of `pod_seen` above — a pod's advertising window is too short for + /// anything that waits for the list to be published. + fn auto_connect(&mut self) { + if !self.scanning { + return; + } + let candidates: Vec = self + .published + .iter() + .filter(|d| matches!(d.kind, DeviceKind::Trainer | DeviceKind::HeartRate)) + // Advertising right now, so the connect has something to reach. + .filter(|d| self.seen_now.contains(&d.address)) + .filter(|d| d.remembered && !self.auto_off.contains(&d.id)) + .cloned() + .collect(); + + for device in candidates { + // Whatever is holding the supervisor — a live link, a connect in + // flight, a reconnect the BLE layer is running — outranks this. + let busy = match device.kind { + DeviceKind::Trainer => self.trainer.status().is_attached(), + _ => self.hr.status().is_attached(), + }; + if busy { + continue; + } + let previous = self.auto.get(&device.id).copied(); + if !may_auto_connect(previous.as_ref()) { + continue; + } + let tries = previous.map_or(0, |a| a.tries); + self.auto.insert( + device.id.clone(), + AutoAttempts { + last: Instant::now(), + tries: tries + 1, + }, + ); + tracing::info!( + name = %device.name, + address = %device.address, + attempt = tries + 1, + "reconnecting to a remembered device" + ); + // Through the same path the rider's own click takes, so a connect + // started by the scan and one started by a finger cannot diverge — + // including suspending the scan for a trainer. + if let Err(e) = self.connect(&device.id) { + tracing::debug!(address = %device.address, reason = %e, "auto-connect declined"); + } + } + } + pub fn list(&self) -> Vec { self.published.clone() } @@ -449,13 +614,12 @@ impl DeviceRegistry { // controller supervisor rather than handled here, so the device list // and the connection screen drive the same one link per pod (FR-1.4). if let Some(pod) = device.kind.pod_id() { - self.remembered.insert(id.to_string()); - self.forgotten.remove(id); + self.wanted(&device.address); self.controller.connect(pod, Some(device.address.clone())); let mut info = device; info.state = ConnectionState::Connecting; info.error = None; - info.remembered = true; + info.remembered = self.known.contains(&info.address); return Ok(info); } @@ -463,13 +627,12 @@ impl DeviceRegistry { // the heart rate supervisor so this list and whatever else drives the // link stay one path. if device.kind == DeviceKind::HeartRate { - self.remembered.insert(id.to_string()); - self.forgotten.remove(id); + self.wanted(&device.address); self.hr.connect(Some(device.address.clone())); let mut info = device; info.state = ConnectionState::Connecting; info.error = None; - info.remembered = true; + info.remembered = self.known.contains(&info.address); return Ok(info); } @@ -508,8 +671,7 @@ impl DeviceRegistry { // soon as the trainer is no longer attached (FR-1.12). self.set_scanning(false); self.scan_suspended = true; - self.remembered.insert(id.to_string()); - self.forgotten.remove(id); + self.wanted(&device.address); self.trainer .connect(scan::TrainerSelector::Address(device.address.clone())); @@ -517,7 +679,7 @@ impl DeviceRegistry { info.state = ConnectionState::Connecting; info.control_acquired = false; info.error = None; - info.remembered = true; + info.remembered = self.known.contains(&info.address); Ok(info) } @@ -525,6 +687,11 @@ impl DeviceRegistry { let mut device = self .get(id) .ok_or_else(|| format!("no such device: {id}"))?; + // The device stays remembered — the rider closed this link, not the + // pairing — but auto-connect stops chasing it for the rest of the + // session. A "disconnect" that reconnects itself half a second later is + // not a disconnect (FR-1.5). + self.auto_off.insert(id.to_string()); if device.kind == DeviceKind::Trainer { // SAF-2 runs inside the supervisor before the link drops. self.trainer.disconnect(); @@ -550,12 +717,29 @@ impl DeviceRegistry { if device.kind == DeviceKind::HeartRate { self.hr.disconnect(); } - self.remembered.remove(id); - self.forgotten.insert(id.to_string()); + self.known.forget(&device.address); + self.auto_off.remove(id); + self.auto.remove(id); self.published.retain(|d| d.id != id); Ok(()) } + /// The rider asked for this device: un-forget it and re-arm auto-connect. + /// + /// It is deliberately *not* remembered here. A Connect that the hardware + /// then refuses is not a pairing, and writing one down would mean a trainer + /// the rider gave up on getting chased on every launch afterwards. The + /// store is written when the link actually comes up — see `build`. + /// + /// A row's `id` *is* its address — see how every branch of `build` + /// constructs one — so the same string keys the store and the two + /// session-lifetime maps. + fn wanted(&mut self, address: &str) { + self.known.unforget(address); + self.auto_off.remove(address); + self.auto.remove(address); + } + /// The address of each Click pod the scanner has seen (FR-1.4). /// /// Connecting by address is both faster and unambiguous: the pods share a @@ -616,6 +800,33 @@ fn describe_service(uuid: Uuid) -> String { } } +/// May auto-connect try this address now? +/// +/// Two brakes, and they answer different questions. [`AUTO_RETRY`] is about +/// *rate*: a device that advertises and then refuses must not be hammered four +/// times a second. [`AUTO_ATTEMPTS`] is about *ending*: an app that never stops +/// trying can never honestly tell the rider it has stopped (FR-1.11). +fn may_auto_connect(previous: Option<&AutoAttempts>) -> bool { + match previous { + None => true, + Some(a) => a.tries < AUTO_ATTEMPTS && a.last.elapsed() >= AUTO_RETRY, + } +} + +/// Is this the device we paired with, or is the slot simply still empty? +/// +/// With nothing of this kind remembered, anything will do — that is a first +/// pairing, and refusing it would mean the rider could never make one. Once +/// something *is* remembered, only that peripheral is chased automatically; a +/// replacement is adopted the moment the rider connects it by hand, which is +/// what the Connect button on the row is for. +/// +/// A free function rather than a method so the rule can be checked without a +/// radio, two supervisors and a Tokio runtime. +fn is_ours(known: &KnownDevices, kind: DeviceKind, address: &str) -> bool { + known.contains(address) || !known.any_of_kind(kind) +} + /// May a scan we suspended for a connect be switched back on? /// /// FR-1.12. Split out from [`DeviceRegistry::poll`] so the rule is checkable @@ -819,6 +1030,79 @@ mod tests { assert!(!should_resume_scan(false, &idle)); } + #[test] + fn auto_connect_paces_itself_and_eventually_stops() { + // Never tried: go. + assert!(may_auto_connect(None)); + + // Just tried. A trainer that advertises and then refuses would + // otherwise be retried on every device tick, four times a second, each + // attempt fighting the scan for the one adapter. + let just_now = AutoAttempts { + last: Instant::now(), + tries: 1, + }; + assert!(!may_auto_connect(Some(&just_now))); + + // Long enough ago, and budget left. + let stale = AutoAttempts { + last: Instant::now() - AUTO_RETRY - Duration::from_secs(1), + tries: 1, + }; + assert!(may_auto_connect(Some(&stale))); + + // Out of budget. FR-1.11: the app has to be able to stop, or "gave up" + // is a message it contradicts two ticks later. The rider's own Connect + // clears this — see `wanted`. + let spent = AutoAttempts { + last: Instant::now() - AUTO_RETRY - Duration::from_secs(1), + tries: AUTO_ATTEMPTS, + }; + assert!(!may_auto_connect(Some(&spent))); + } + + #[test] + fn any_pod_will_do_until_one_has_been_paired_with() { + // FR-1.5. Every Click advertises the same name and the same type byte, + // so "a − pod is advertising" is not the same question as "*our* − pod + // is advertising" the moment there is more than one in the room. + let mut known = KnownDevices::default(); + assert!(is_ours(&known, DeviceKind::ClickMinus, "AA:BB:CC:DD:EE:01")); + + known.remember( + "AA:BB:CC:DD:EE:01", + Some("Zwift Click"), + DeviceKind::ClickMinus, + ); + assert!(is_ours(&known, DeviceKind::ClickMinus, "AA:BB:CC:DD:EE:01")); + assert!(is_ours(&known, DeviceKind::ClickMinus, "aa:bb:cc:dd:ee:01")); + // The one on the next bike. + assert!(!is_ours( + &known, + DeviceKind::ClickMinus, + "AA:BB:CC:DD:EE:02" + )); + // A kind we have never paired with is still wide open. + assert!(is_ours(&known, DeviceKind::ClickPlus, "AA:BB:CC:DD:EE:03")); + assert!(is_ours(&known, DeviceKind::Trainer, "AA:BB:CC:DD:EE:04")); + } + + #[test] + fn a_replacement_is_adopted_once_the_rider_connects_it() { + // The pod broke and a new one arrived. Auto-connect ignores it — it is + // not the one we know — but the rider's own Connect pairs it, and from + // then on it is chased like the old one. + let mut known = KnownDevices::default(); + known.remember("AA:BB:CC:DD:EE:01", None, DeviceKind::ClickMinus); + assert!(!is_ours( + &known, + DeviceKind::ClickMinus, + "AA:BB:CC:DD:EE:99" + )); + known.remember("AA:BB:CC:DD:EE:99", None, DeviceKind::ClickMinus); + assert!(is_ours(&known, DeviceKind::ClickMinus, "AA:BB:CC:DD:EE:99")); + } + #[test] fn well_known_services_are_named_and_zwift_is_recognised() { let ftms = describe_service(uuids::FITNESS_MACHINE_SERVICE); diff --git a/src-tauri/src/known.rs b/src-tauri/src/known.rs new file mode 100644 index 0000000..b84737c --- /dev/null +++ b/src-tauri/src/known.rs @@ -0,0 +1,311 @@ +//! Devices the rider has paired before, remembered across launches (FR-1.5). +//! +//! Until now "remembered" was a `HashSet` inside [`crate::devices::DeviceRegistry`], +//! which meant it lasted exactly as long as the process. Every launch started +//! from nothing: find the trainer, press Connect, find the strap, press +//! Connect, and only then ride. This is that set written down. +//! +//! ```text +//! app_data_dir()/devices.json +//! { "version": 1, +//! "devices": [ { address, name, kind }, … ], +//! "forgotten": [ address, … ] } +//! ``` +//! +//! Two lists rather than one, because *forgotten* is not merely "absent". +//! Absent means never seen; forgotten means the rider looked at this device and +//! said no, and auto-connect has to keep honouring that on the next launch too. +//! +//! The file is small and written only when something actually changes — a pair, +//! an unpair, a name learned — so this never lands in the ride loop's path. It +//! is also *advisory*: a corrupt or unreadable file costs the rider their +//! auto-connect, never their ride, so every failure here is logged and +//! swallowed rather than propagated. + +use std::collections::{BTreeMap, BTreeSet}; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; +use tauri::{AppHandle, Manager}; + +use crate::devices::DeviceKind; + +/// Bumped only if the shape changes incompatibly. An older file with a version +/// we do not know is discarded rather than guessed at. +const VERSION: u32 = 1; +const FILE: &str = "devices.json"; + +/// One device the rider has paired with. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct KnownDevice { + /// As the adapter reports it. Matching is case-insensitive — see [`key`] — + /// but what is written down is what we were told. + pub address: String, + pub name: Option, + pub kind: DeviceKind, +} + +/// The file on disk. Kept separate from the in-memory form so the indexes below +/// are never serialised. +#[derive(Debug, Default, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", default)] +struct Stored { + version: u32, + devices: Vec, + forgotten: Vec, +} + +/// Address as we match on it. BlueZ hands back `F4:C4:59:…` and Android +/// `f4:c4:59:…` for the same peripheral, and a pairing that survives a launch +/// but not a platform is not much of a pairing. +fn key(address: &str) -> String { + address.trim().to_ascii_uppercase() +} + +/// Everything the app remembers about the hardware in the room, plus where to +/// write it. +#[derive(Debug, Default)] +pub struct KnownDevices { + devices: BTreeMap, + forgotten: BTreeSet, + /// `None` before [`KnownDevices::load`] — `AppState::new` runs before there + /// is an `AppHandle` to ask for a data directory, so the registry spends + /// the first moments of the process with an in-memory-only store. + path: Option, +} + +impl KnownDevices { + /// Read the file, or start empty if it is missing, unreadable or from a + /// version we do not understand. + pub fn load(path: PathBuf) -> Self { + let mut out = Self { + path: Some(path.clone()), + ..Self::default() + }; + let text = match std::fs::read_to_string(&path) { + Ok(t) => t, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return out, + Err(e) => { + tracing::warn!(path = %path.display(), error = %e, "could not read remembered devices"); + return out; + } + }; + let stored: Stored = match serde_json::from_str(&text) { + Ok(s) => s, + Err(e) => { + tracing::warn!(path = %path.display(), error = %e, "remembered devices are unreadable; starting fresh"); + return out; + } + }; + if stored.version != VERSION { + tracing::warn!( + found = stored.version, + expected = VERSION, + "remembered devices are from another version; starting fresh" + ); + return out; + } + for device in stored.devices { + out.devices.insert(key(&device.address), device); + } + out.forgotten = stored.forgotten.iter().map(|a| key(a)).collect(); + out + } + + pub fn len(&self) -> usize { + self.devices.len() + } + + pub fn is_empty(&self) -> bool { + self.devices.is_empty() + } + + /// Has this device been paired with before? + pub fn contains(&self, address: &str) -> bool { + self.devices.contains_key(&key(address)) + } + + /// Did the rider say no to this device? + pub fn is_forgotten(&self, address: &str) -> bool { + self.forgotten.contains(&key(address)) + } + + /// The remembered device of this kind, if there is one. Used to prefer + /// *our* Click over the identically-named one in the next room. + pub fn first_of(&self, kind: DeviceKind) -> Option<&KnownDevice> { + self.devices.values().find(|d| d.kind == kind) + } + + pub fn any_of_kind(&self, kind: DeviceKind) -> bool { + self.first_of(kind).is_some() + } + + /// Record a pairing. No-op — and no write — when nothing changed, which is + /// the common case: this is called from the device poll, four times a + /// second. + pub fn remember(&mut self, address: &str, name: Option<&str>, kind: DeviceKind) { + // Nothing useful to auto-connect to, and a list full of every anonymous + // peripheral in the building helps nobody. + if kind == DeviceKind::Unknown || address.trim().is_empty() { + return; + } + let id = key(address); + let entry = KnownDevice { + address: address.to_string(), + name: name.map(str::to_owned).filter(|n| !n.trim().is_empty()), + kind, + }; + let unchanged = self.devices.get(&id) == Some(&entry); + let was_forgotten = self.forgotten.remove(&id); + if unchanged && !was_forgotten { + return; + } + tracing::info!(address, name, ?kind, "remembering device"); + self.devices.insert(id, entry); + self.save(); + } + + /// The rider said no. Both halves matter: drop the pairing *and* record the + /// refusal, so the next launch does not helpfully connect it again. + pub fn forget(&mut self, address: &str) { + let id = key(address); + let removed = self.devices.remove(&id).is_some(); + let added = self.forgotten.insert(id); + if removed || added { + tracing::info!(address, "forgetting device"); + self.save(); + } + } + + /// An explicit connect outranks an earlier refusal. + pub fn unforget(&mut self, address: &str) { + if self.forgotten.remove(&key(address)) { + self.save(); + } + } + + /// Write the file, atomically: a half-written `devices.json` would be + /// discarded whole on the next launch, and losing the pairings because the + /// power went out mid-`write` is exactly the failure this module exists to + /// prevent. + fn save(&self) { + let Some(path) = &self.path else { + return; + }; + let stored = Stored { + version: VERSION, + devices: self.devices.values().cloned().collect(), + forgotten: self.forgotten.iter().cloned().collect(), + }; + if let Err(e) = write_atomic(path, &stored) { + // Advisory, not fatal: the rider loses auto-connect on the next + // launch, never this ride. + tracing::warn!(path = %path.display(), error = %e, "could not save remembered devices"); + } + } +} + +fn write_atomic(path: &Path, stored: &Stored) -> std::io::Result<()> { + let text = serde_json::to_string_pretty(stored) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + let tmp = path.with_extension("json.tmp"); + std::fs::write(&tmp, text)?; + std::fs::rename(&tmp, path) +} + +/// Where the remembered devices live: beside the recorded rides, in the app's +/// own data directory. +pub fn store_path(app: &AppHandle) -> Result { + let dir = app + .path() + .app_data_dir() + .map_err(|e| format!("no app data directory: {e}"))?; + std::fs::create_dir_all(&dir) + .map_err(|e| format!("could not create {}: {e}", dir.display()))?; + Ok(dir.join(FILE)) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn temp() -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "bikecontrol-known-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + std::fs::create_dir_all(&dir).unwrap(); + dir.join(FILE) + } + + #[test] + fn a_pairing_survives_a_reload() { + let path = temp(); + let _ = std::fs::remove_file(&path); + let mut known = KnownDevices::load(path.clone()); + known.remember( + "F4:C4:59:03:A1:8E", + Some("Zwift Click"), + DeviceKind::ClickMinus, + ); + + let again = KnownDevices::load(path); + assert!(again.contains("F4:C4:59:03:A1:8E")); + // The same peripheral, as Android spells it. + assert!(again.contains("f4:c4:59:03:a1:8e")); + assert_eq!( + again + .first_of(DeviceKind::ClickMinus) + .unwrap() + .name + .as_deref(), + Some("Zwift Click") + ); + } + + #[test] + fn forgetting_outlives_the_process_too() { + // The whole point: "no" has to be remembered as firmly as "yes", or the + // next launch connects the neighbour's trainer again. + let path = temp().with_extension("forget.json"); + let _ = std::fs::remove_file(&path); + let mut known = KnownDevices::load(path.clone()); + known.remember("AA:BB:CC:DD:EE:FF", Some("D100"), DeviceKind::Trainer); + known.forget("aa:bb:cc:dd:ee:ff"); + + let again = KnownDevices::load(path); + assert!(!again.contains("AA:BB:CC:DD:EE:FF")); + assert!(again.is_forgotten("AA:BB:CC:DD:EE:FF")); + assert!(!again.any_of_kind(DeviceKind::Trainer)); + } + + #[test] + fn connecting_again_undoes_a_refusal() { + let mut known = KnownDevices::default(); + known.forget("AA:BB:CC:DD:EE:FF"); + assert!(known.is_forgotten("AA:BB:CC:DD:EE:FF")); + known.remember("AA:BB:CC:DD:EE:FF", None, DeviceKind::Trainer); + assert!(!known.is_forgotten("AA:BB:CC:DD:EE:FF")); + assert!(known.contains("AA:BB:CC:DD:EE:FF")); + } + + #[test] + fn an_unidentified_peripheral_is_not_worth_remembering() { + let mut known = KnownDevices::default(); + known.remember("AA:BB:CC:DD:EE:FF", None, DeviceKind::Unknown); + assert!(known.is_empty()); + } + + #[test] + fn a_corrupt_file_costs_the_pairings_and_nothing_else() { + let path = temp().with_extension("corrupt.json"); + std::fs::write(&path, b"{ this is not json").unwrap(); + let known = KnownDevices::load(path); + assert!(known.is_empty()); + } +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 20c8fb9..17eb3f9 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -14,6 +14,7 @@ pub mod derive; pub mod devices; pub mod events; pub mod heart_rate; +pub mod known; pub mod profile_view; pub mod recording; pub mod samples; @@ -120,6 +121,15 @@ pub fn run() { ]) .setup(|app| { let handle = app.handle().clone(); + // FR-1.5: the hardware the rider paired with last time, before the + // first scan pass so the very first thing the scanner sees can be + // reconnected rather than merely listed. A missing data directory + // costs auto-connect and nothing else, so it is a warning, not a + // failed launch. + match known::store_path(&handle) { + Ok(path) => handle.state::().lock().devices.attach_store(path), + Err(e) => tracing::warn!(error = %e, "remembered devices unavailable"), + } // NFR-7: scanning starts immediately, not on a user click. handle.state::().lock().devices.start_scan(); state::spawn_ride_loop(handle.clone()); diff --git a/ui/src/components/ClickPanel.svelte b/ui/src/components/ClickPanel.svelte index 88f33fd..642725a 100644 --- a/ui/src/components/ClickPanel.svelte +++ b/ui/src/components/ClickPanel.svelte @@ -1,24 +1,31 @@