Close the link before the budget runs out, not after
🚴 Build and Test BikeControl / Workspace tests (push) Failing after 0s
🚴 Build and Test BikeControl / Android compile check (push) Skipped

A pod that was connected, reporting battery every five seconds, and
sending no button frames at all cost most of an evening. The cause was
not in the decode: it was that the previous run never let go of the link.

`Actor::teardown` unsubscribed every notifying characteristic before
disconnecting, each under its own two-second timeout. A Click carries
five of them, so the exit path could spend ten seconds on optional work
against a supervisor budget of three. The supervisor gave up first, the
process exited, and `disconnect` was never reached — leaving BlueZ
holding the pod with no application running. The next connect inherited
that half-dead session, and a half-dead session streams battery and no
buttons, which reads exactly like broken hardware.

Measured directly: both apps stopped, `bluetoothctl devices Connected`
still listing the pod. It also explains the connect failures that came
with it — `le-connection-abort-by-local`, then `service discovery timed
out` — and why the fault came and went, since it depended on whether the
last exit happened to time out.

Nothing is lost by dropping the unsubscribes. The link going down clears
the peripheral's CCCDs anyway, so they were only ever politeness toward a
connection about to be destroyed. The heart rate actor had the same
shape with one characteristic instead of five; the trainer's safety
writes stay, because SAF-2 is not optional.

All three now say what a timed-out disconnect costs. The old message
named the symptom and not the consequence, and the consequence lands on
the *next* run — which is precisely why this hid for so long.

Also logs which pod reported which button. A pod reporting a button that
belongs to the other one is invisible downstream, because the input
carries the button and never its source.

Fixes both platforms at once: this is `crates/ble`, and Android is the
worse case — a leaked link there survives the app being swiped away.

Verified with four connect/disconnect cycles against the `−` pod: every
button registers on its documented bit (D-pad 0-3, paddle 8), and BlueZ
holds nothing afterwards. That satisfies TASK-0(d) and retires RISK-9.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-20 20:21:31 +02:00
co-authored by Claude Opus 5
parent 3274928a5f
commit a0190095fb
4 changed files with 51 additions and 9 deletions
+23 -5
View File
@@ -602,14 +602,32 @@ impl Actor {
/// (NFR-9). /// (NFR-9).
async fn teardown(&mut self, session: &Session) { async fn teardown(&mut self, session: &Session) {
self.release_held(); self.release_held();
for ch in &session.subscribed { // Straight to the disconnect, and deliberately no unsubscribe loop.
let unsubscribe = session.peripheral.unsubscribe(ch); //
let _ = tokio::time::timeout(DISCONNECT_TIMEOUT, unsubscribe).await; // A pod carries five notifying characteristics, and unsubscribing each
} // under its own DISCONNECT_TIMEOUT could spend 10 s before this even
// reached the disconnect — against a supervisor budget of 3 s. The
// caller gave up first, the process exited, and the link was left open:
// BlueZ then held the pod with no app running, and the next run
// inherited a half-dead session that streamed battery and no buttons.
// Observed exactly that, and it cost an evening.
//
// Nothing is lost by skipping it. Dropping the link clears the pod's
// CCCDs on its own, so the unsubscribes were only ever politeness
// toward a link we are about to destroy (NFR-9).
tracing::debug!(
subscribed = session.subscribed.len(),
"click: closing the link"
);
match tokio::time::timeout(DISCONNECT_TIMEOUT, session.peripheral.disconnect()).await { match tokio::time::timeout(DISCONNECT_TIMEOUT, session.peripheral.disconnect()).await {
Ok(Ok(())) => tracing::info!("click: disconnected"), Ok(Ok(())) => tracing::info!("click: disconnected"),
Ok(Err(e)) => tracing::debug!(error = %e, "click: disconnect failed"), Ok(Err(e)) => tracing::debug!(error = %e, "click: disconnect failed"),
Err(_) => tracing::warn!("click: disconnect timed out"), // Loud, because the cost lands on the *next* run rather than this
// one: a link we failed to close stays up in BlueZ after we exit.
Err(_) => tracing::warn!(
"click: disconnect timed out — the link may be left open, and a stale link \
makes the pod look connected while sending nothing"
),
} }
let _ = self.events_tx.send(ClickEvent::Disconnected); let _ = self.events_tx.send(ClickEvent::Disconnected);
} }
+8 -1
View File
@@ -1168,7 +1168,14 @@ impl Actor {
match tokio::time::timeout(DISCONNECT_TIMEOUT, session.peripheral.disconnect()).await { match tokio::time::timeout(DISCONNECT_TIMEOUT, session.peripheral.disconnect()).await {
Ok(Ok(())) => tracing::info!("disconnected from the trainer"), Ok(Ok(())) => tracing::info!("disconnected from the trainer"),
Ok(Err(e)) => tracing::warn!(error = %e, "disconnect failed"), Ok(Err(e)) => tracing::warn!(error = %e, "disconnect failed"),
Err(_) => tracing::warn!("disconnect timed out"), // The cost of this lands on the *next* run, not this one: a link we
// could not close stays up in BlueZ after the process exits, and
// the next connect inherits a half-dead session that reports some
// frames and not others. Worth saying plainly rather than as a
// bare timeout.
Err(_) => tracing::warn!(
"disconnect timed out — the link may be left open for the next run"
),
} }
self.set_state(ConnectionState::Idle); self.set_state(ConnectionState::Idle);
} }
+10 -3
View File
@@ -579,12 +579,19 @@ impl Actor {
/// Close the link. Bounded: this runs on the app's exit path (NFR-9). /// Close the link. Bounded: this runs on the app's exit path (NFR-9).
async fn teardown(&self, session: &Session) { async fn teardown(&self, session: &Session) {
let unsubscribe = session.peripheral.unsubscribe(&session.measurement); tracing::debug!(measurement = %session.measurement.uuid, "hr: closing the link");
let _ = tokio::time::timeout(DISCONNECT_TIMEOUT, unsubscribe).await; // No unsubscribe first. It is optional — dropping the link clears the
// strap's CCCD anyway — and on the exit path it competes for the same
// budget as the disconnect that actually matters. The Click actor lost
// that race and left links open in BlueZ; this path is one
// characteristic rather than five, but the failure is the same shape.
match tokio::time::timeout(DISCONNECT_TIMEOUT, session.peripheral.disconnect()).await { match tokio::time::timeout(DISCONNECT_TIMEOUT, session.peripheral.disconnect()).await {
Ok(Ok(())) => tracing::info!("hr: disconnected"), Ok(Ok(())) => tracing::info!("hr: disconnected"),
Ok(Err(e)) => tracing::debug!(error = %e, "hr: disconnect failed"), Ok(Err(e)) => tracing::debug!(error = %e, "hr: disconnect failed"),
Err(_) => tracing::warn!("hr: disconnect timed out"), Err(_) => tracing::warn!(
"hr: disconnect timed out — the link may be left open, and a stale link makes \
the strap look connected while sending nothing"
),
} }
let _ = self.events_tx.send(HeartRateEvent::Disconnected); let _ = self.events_tx.send(HeartRateEvent::Disconnected);
} }
+10
View File
@@ -984,6 +984,16 @@ fn apply(
} }
} }
}); });
// Which pod said what, before the merge collapses it. A pod
// reporting a button that belongs to the *other* one is invisible
// downstream — the input carries the button, never its source — and
// that is exactly the shape of a mis-mapped bit.
tracing::debug!(
pod = pod.as_str(),
button = button_name(button),
pressed,
"controller: button"
);
// One press is one press, however many pods reported it. // One press is one press, however many pods reported it.
let Some(pressed) = buttons.edge(pod, button, pressed) else { let Some(pressed) = buttons.edge(pod, button, pressed) else {
return; return;