fix: improve Bluetooth connection handling during device taps

Author: superwilso

CHANGELOG.md

@@ -16,6 +16,16 @@ level the commit history supports; from `v0.1.6` onward, entries are written as
 
 ## [Unreleased]
 
+### Fixed
+
+- **Tapping headphones in Bluetooth ▸ Devices often did nothing.** *Harness- and host-tested;
+  device-unverified.* A tap while the radio was already paging (its own reconnect attempt, or an
+  earlier tap) was refused by Sony's transmitter and simply dropped, and the row's "connecting"
+  spinner vanished within a frame, so it looked as if nothing had happened: on 2026-09-15, 23 of 24
+  taps in 21 seconds were refused that way. A tap during a page is now held and asked for the moment
+  the radio is free, a refused tap is retried, NFC taps on paired headphones follow the same rule,
+  and the spinner stays until the device connects or 30 seconds pass, without sending more requests.
+
 ## [0.3.7] — 2026-09-15
 
 
@@ -67,6 +77,9 @@ level the commit history supports; from `v0.1.6` onward, entries are written as
 
 ## [0.3.6] — 2026-09-14
 
+*Tagged (`4e1f76a`) but never pushed or published. v0.3.7 is the first release carrying these
+changes, so its links compare against that commit rather than a tag GitHub does not have.*
+
 ## [0.3.5] — 2026-09-14
 
 ### Added
@@ -1730,8 +1743,8 @@ First tagged release.
   the shell or any mixer control ([`docs/`](docs/)).
 
 [Unreleased]: https://github.com/superwilso/Cinder/compare/v0.3.7...HEAD
-[0.3.7]: https://github.com/superwilso/Cinder/compare/v0.3.6...v0.3.7
-[0.3.6]: https://github.com/superwilso/Cinder/compare/v0.3.5...v0.3.6
+[0.3.7]: https://github.com/superwilso/Cinder/compare/4e1f76a...v0.3.7
+[0.3.6]: https://github.com/superwilso/Cinder/compare/v0.3.5...4e1f76a
 [0.3.5]: https://github.com/superwilso/Cinder/compare/v0.3.4...v0.3.5
 [0.3.4]: https://github.com/superwilso/Cinder/compare/v0.3.3...v0.3.4
 [0.3.3]: https://github.com/superwilso/Cinder/compare/v0.3.2...v0.3.3

cinder-home/harness/fake_pst.cpp

@@ -163,7 +163,14 @@ long h_GetPairedDeviceInfo(void*, void* pl, void*, void*) {
 // GetConnectInformation returns. The two conventions live side by side in the same client.
 bool g_retry_mode = false;
 
-bool connect_refused() { return g_retry_mode; }
+// A page on the air until this virtual time, set by cinder_harness_bt_page_until(). While it is, the
+// A2DP source reports CONNECTING and refuses any new connect with rc=0 — the busy refusal measured on
+// the device for the ladder (2026-08-26) and for 23 of 24 Devices-row taps (2026-09-15).
+long long g_page_until_ms = 0;
+
+bool paging() { return cinder_harness_now_ms() < g_page_until_ms; }
+
+bool connect_refused() { return g_retry_mode || paging(); }
 
 // Bring the link up if anything could. A connect against a powered-down radio is ACCEPTED AND
 // SILENTLY DROPPED — the device behaviour behind "Bluetooth doesn't connect automatically".
@@ -207,7 +214,7 @@ long h_GetAvSrcConnectionStatus(void*, void*, void*, void*) {
     long long scripted = 0;
     int st = cinder_harness_scripted("BtXmit::GetAvSrcConnectionStatus", &scripted)
                  ? (int)scripted
-                 : (!g_radio_on ? 0 : (g_connected ? 5 : 2));
+                 : (!g_radio_on ? 0 : (g_connected ? 5 : (paging() ? 3 : 2)));
     cinder_harness_record("BtXmit::GetAvSrcConnectionStatus", st);
     return st;
 }
@@ -340,7 +347,7 @@ int _ZN3pst8services12mediascanner12MediaScanner4ScanEPNS1_21IMediaScannerListen
 
 // ── the fake radio, as a test fixture ────────────────────────────────────────────────────────
 void cinder_harness_bt_reset(void) {
-    g_radio_on = false; g_connected = false; g_retry_mode = false;
+    g_radio_on = false; g_connected = false; g_retry_mode = false; g_page_until_ms = 0;
     g_peer_name.clear(); g_peer_addr.clear(); g_paired.clear();
 }
 
@@ -352,6 +359,10 @@ void cinder_harness_bt_set_radio(int on) { g_radio_on = on != 0; }
 void cinder_harness_bt_set_retry_mode(int on) { g_retry_mode = on != 0; }
 int  cinder_harness_bt_retry_mode(void) { return g_retry_mode ? 1 : 0; }
 
+// Hold a page on the air until `ms` of virtual time: a radio already busy connecting, as the
+// ladder's own attempt had left it when the user started tapping on 2026-09-15.
+void cinder_harness_bt_page_until(long long ms) { g_page_until_ms = ms; }
+
 // Add a device to the radio's pairing table. `addr_last` is the final byte of a synthetic
 // 00:00:5E:00:53:xx address (RFC 7042 documentation range), so two fixtures are distinguishable.
 void cinder_harness_bt_add_paired(const char* name, int addr_last) {

cinder-home/harness/harness.h

@@ -145,6 +145,9 @@ void cinder_harness_bt_set_radio(int on);
 // armed; read it to assert the app cleared it.
 void cinder_harness_bt_set_retry_mode(int on);
 int  cinder_harness_bt_retry_mode(void);
+// A page on the air until `ms` of virtual time: the A2DP source reports CONNECTING (3) and refuses
+// every connect request with rc=0 until then, as the device does while one is already in flight.
+void cinder_harness_bt_page_until(long long ms);
 void cinder_harness_bt_add_paired(const char* name, int addr_last);
 int  cinder_harness_bt_connected(void);
 int  cinder_harness_bt_radio_on(void);

cinder-home/harness/scenarios.cpp

@@ -219,13 +219,43 @@ static void s_bt_waits_for_a_page_in_flight(void) {
 
     check(cinder_harness_count("BtXmit::GetAvSrcConnectionStatus") >= 1,
           "asked the radio whether it was already trying");
-    // The skip cap is 6, so a handful of attempts do get through — that is deliberate, a status
+    // The wait is capped at 30 s, so a handful of attempts do get through — that is deliberate, a status
     // stuck at 3 must not mean silence. What must not happen is a request every time round.
     check_range(cinder_harness_count("BtXmit::RequestLastDeviceConnection")
                   + cinder_harness_count("BtXmit::RequestConnection"),
                 0, 3, "stood aside for the page already in flight");
 }
 
+// ── a Devices tap while a page is on the air ─────────────────────────────────────────────────
+// 2026-09-15 on the device: 24 taps on the headphones' row in 21 s, 23 of them refused
+// `RequestConnection rc=0`, the busy refusal the ladder already stands aside for. The tap path never
+// asked, so every tap requested into the page, was refused, and nothing kept the device the user
+// wanted. The tap must hand its request to the ladder instead, which asks for THAT device once the
+// page ends. (Against the old tap path this fails its first check with five refused requests.)
+static void s_bt_tap_waits_for_the_page(void) {
+    healthy_device();
+    cinder_harness_bt_set_radio(1);
+    cinder_harness_bt_add_paired("WH-1000XM4", 0x91);   // row 0
+    cinder_harness_bt_add_paired("WONDERBOOM", 0x92);   // row 1, the one tapped
+    cinder_harness_bt_page_until(40000);
+    cinder_harness_input_enable();
+    cinder_harness_script("cinder_tap", 28 /* CINDER_ACT_BT_CONNECT_DEVICE */);
+    cinder_harness_script("cinder_pending_bt_device", 1);
+    for (int i = 0; i < 5; i++)
+        cinder_harness_tap_at(20000 + i * 400, 240, 400);   // impatient, like the real log
+    cinder_harness_set_budget_ms(60000);
+    cinder_harness_run();
+
+    check(cinder_harness_count_between("cinder_tap", 20000, 23000) >= 1, "the taps reached carry_out");
+    check_eq(cinder_harness_count_between("BtXmit::RequestConnection", 20000, 40000), 0,
+             "no connect was asked into the page");
+    check(cinder_harness_count_between("BtXmit::RequestConnection", 40000, 43000) >= 1,
+          "the device was asked for within a few seconds of the page ending");
+    check_eq(cinder_harness_arg("BtXmit::RequestConnection", 0), 1,
+             "…and it was the device that was tapped");
+    check(cinder_harness_bt_connected() == 1, "and it connected");
+}
+
 // ── an idle radio reports 3, and 3 is not a connection ───────────────────────────────────────
 // `GetBtStatus` reaches 3 with nothing on the other end (measured 0.61 s after powering an idle
 // radio). The route used to be `st == 3`, so it flipped to BLUETOOTH the moment the radio came up:
@@ -1044,6 +1074,7 @@ static const Scenario kScenarios[] = {
     {"bt-stale-jam",      s_bt_clears_a_stale_jam,   "a retry mode left armed by anything else is cleared"},
     {"bt-idle-not-link",  s_bt_idle_radio_is_not_a_link, "GetBtStatus 3 with no peer is not a connection"},
     {"bt-page-in-flight", s_bt_waits_for_a_page_in_flight, "the ladder defers to a connect already on the air"},
+    {"bt-tap-in-page",    s_bt_tap_waits_for_the_page, "a Devices tap during a page is asked for when it ends"},
     {"bt-idle-poll",      s_bt_idle_poll_rate,       "the idle Bluetooth poll backs off"},
     {"stalled-bringup",   s_stalled_bringup,         "bring-up that never completes must not freeze the app"},
     {"dark-playing",      s_dark_playing,            "panel dark, BT playing: the state the device lives in"},

cinder-home/src/main.cpp

@@ -4522,17 +4522,54 @@ static int bt_avsrc_status() {
     } catch (...) { return -1; }
 }
 
-// How many times in a row the ladder will stand aside for an attempt that is already in flight
-// before asking anyway. Bounded so a status wedged at 3 cannot silence the ladder for good — the
-// same rule the rest of this file applies to any service state it cannot verify.
-#define BT_AVSRC_MAX_SKIPS 6
+// How long the ladder will stand aside for an attempt that is already in flight before asking
+// anyway. Bounded so a status wedged at 3 cannot silence the ladder for good — the same rule the
+// rest of this file applies to any service state it cannot verify. A TIME rather than a count of
+// skips (it was 6 skips at 5 s apiece): the ladder now looks every second while a user is waiting,
+// and a count would have made the same wedge five times shorter for exactly that case.
+#define BT_AVSRC_MAX_BUSY_MS 30000
+// How soon the ladder looks again after a Devices tap it could not act on.
+#define BT_RECONNECT_SOON_S 2
+
+// A TAP THE RADIO CANNOT TAKE IS HANDED TO THE LADDER, NOT DROPPED. Measured 2026-09-15 on the
+// device, 24 taps on the headphones' row in 21 s:
+//
+//   377.557 bt-paired: row 1: RequestConnection rc=0 (REJECTED — nothing will reach the air …)
+//     … 22 more like it …
+//   394.696 bt-paired: row 1: RequestConnection rc=1 (accepted)
+//   401.904 bt-vol: rocker now drives BLUETOOTH (GetBtStatus=3, peer named)   <- 7.2 s later
+//
+// rc=0 there is the busy refusal the ladder already stands aside for (see bt_reconnect_tick): a page
+// was on the air, first the ladder's own earlier attempt and then the one tap that got through. The
+// Devices-row path never asked, so each tap requested into the page, was refused, and nothing kept
+// the device the user wanted — while every tap also re-armed the ladder and pushed its next look ten
+// seconds out. The UI's spinner vanished with the list refresh each tap triggers, so nothing on
+// screen said a connect was under way either; hence 24 taps.
+//
+// Now a tap during a page, or one refused for any other reason, makes that device the ladder's
+// target, and the ladder looks every second until the radio is free and then asks for it.
+static bool g_bt_user_pending = false;
 
 // Called wherever the user asks for a link, so the retry stops being parked.
 static void bt_reconnect_rearm() {
     g_bt_user_disconnected = false;
     g_bt_reconnect_at = 0;
     g_bt_reconnect_wait_s = 0;
     g_bt_reconnect_tries = 0;   // a user gesture buys a fresh set of attempts
+    g_bt_user_pending = false;  // …and replaces whatever an earlier gesture was still waiting on
+}
+
+// The ladder takes over a connect the user asked for and the radio refused. It sets up what the
+// ladder's own first notice of a drop would (retry mode off; connect-wait on, but not while a link is
+// up, which would let a second device walk in), aims at `addr`, and schedules a look soon.
+static void bt_connect_when_free(const std::vector<unsigned char>& addr) {
+    g_bt_target_addr = addr;
+    bt_service_retry(false, false);
+    if (!g_bt_have_name) bt_connect_wait(true);
+    g_bt_reconnect_wait_s = BT_RECONNECT_SOON_S;
+    g_bt_reconnect_at = now_ms() + BT_RECONNECT_SOON_S * 1000L;
+    g_bt_reconnect_tries = 0;
+    g_bt_user_pending = true;
 }
 
 // Runs from the 1 Hz housekeeping. Cheap: the common paths are two boolean tests.
@@ -4543,6 +4580,7 @@ static void bt_reconnect_tick() {
         // broken.
         bt_connect_wait(false);
         bt_service_retry(false, false);
+        g_bt_user_pending = false;
         return;
     }
     if (g_bt_have_name) {                     // connected — reset so the next drop starts fresh
@@ -4551,6 +4589,7 @@ static void bt_reconnect_tick() {
         g_bt_target_addr.clear();             // reached it; a later drop retries the ordinary way
         g_bt_zeroarg_fails = 0;
         g_bt_reconnect_tries = 0;
+        g_bt_user_pending = false;
         if (g_bt_reconnect_wait_s) {
             clog_("bt-reconnect: link is up again — ladder disarmed");
             g_bt_reconnect_at = 0;
@@ -4606,16 +4645,19 @@ static void bt_reconnect_tick() {
     // any non-1 as decisive and jumps to naming a device, on evidence that says nothing about which
     // device is right. Standing aside costs one scalar IPC and keeps the two refusals apart.
     {
-        static int avsrc_skips = 0;
+        static long avsrc_busy_since = 0;
         const int avsrc = bt_avsrc_status();
-        if (avsrc == BT_AVSRC_CONNECTING && avsrc_skips < BT_AVSRC_MAX_SKIPS) {
-            avsrc_skips++;
-            g_bt_reconnect_at = now + 5000;   // look again shortly; the page times out in ~5-20 s
-            return;
-        }
-        if (avsrc_skips >= BT_AVSRC_MAX_SKIPS)
+        if (avsrc == BT_AVSRC_CONNECTING) {
+            if (avsrc_busy_since == 0) avsrc_busy_since = now;
+            if (now - avsrc_busy_since < BT_AVSRC_MAX_BUSY_MS) {
+                // The page times out in ~5-20 s. Look again in 5, or in 1 while a user is waiting on
+                // a tap, so the device they asked for is asked for the moment the radio is free.
+                g_bt_reconnect_at = now + (g_bt_user_pending ? 1000 : 5000);
+                return;
+            }
             clog_("bt-reconnect: AvSrc has said 'connecting' for 30 s without a link — asking anyway");
-        avsrc_skips = 0;
+        }
+        avsrc_busy_since = 0;
     }
     g_bt_reconnect_tries++;
 
@@ -4657,6 +4699,7 @@ static void bt_reconnect_tick() {
         if (rc != 1) g_bt_zeroarg_fails = BT_ZEROARG_GIVEUP;   // rejected: decisive
         else         g_bt_zeroarg_fails++;
     }
+    if (rc == 1) g_bt_user_pending = false;   // on the air now; the ordinary backoff takes over
 
     if (g_bt_reconnect_wait_s < BT_RECONNECT_MAX_S) {
         g_bt_reconnect_wait_s *= 2;
@@ -5396,6 +5439,28 @@ static int bt_request_connection(const std::vector<unsigned char>& addr, const c
     return rc;
 }
 
+// A connect the USER asked for: a Devices row, or an NFC tap on bonded headphones. It never asks into
+// a page already on the air (the ladder's rule, which these paths lacked) and never drops a refusal:
+// either way the device becomes the ladder's target, asked for as soon as the radio is free. See
+// bt_connect_when_free for the 2026-09-15 log that found this.
+static void bt_user_connect(const std::vector<unsigned char>& addr, const char* who) {
+    const bool again = g_bt_user_pending && addr == g_bt_target_addr;
+    bt_reconnect_rearm();   // an explicit connect re-arms the retry
+    char m[160];
+    if (bt_avsrc_status() == BT_AVSRC_CONNECTING) {
+        bt_connect_when_free(addr);
+        if (!again) {       // a repeat tap on the same device says nothing new
+            std::snprintf(m, sizeof m, "%s: a connect is already on the air — asking for this "
+                          "device the moment it ends", who);
+            clog_(m);
+        }
+    } else if (bt_request_connection(addr, who) != 1) {
+        bt_connect_when_free(addr);
+        std::snprintf(m, sizeof m, "%s: refused — asking again once the radio is free", who);
+        clog_(m);
+    }
+}
+
 // Called from the render loop when a tap landed.
 //
 // A TAP IS NOT ALWAYS A PAIRING. This used to call `Pairing` unconditionally, which is right only
@@ -5499,8 +5564,7 @@ static void nfc_service_tap() {
             // must stay armed — for the new one, which bt_request_connection is about to name.
             refresh_bt_connected();
         }
-        bt_reconnect_rearm();
-        bt_request_connection(addr, "nfc");
+        bt_user_connect(addr, "nfc");
         // The link comes up asynchronously; the route poll notices it and refreshes the name.
         return;
     }
@@ -5532,10 +5596,9 @@ void apply_bt_connect_device() {
     int i = cinder_pending_bt_device();
     if (i < 0 || (size_t)i >= g_bt_paired.size()) { clog_("bt-paired: connect for an unknown row"); return; }
     const std::vector<unsigned char> addr = g_bt_paired[(size_t)i];
-    bt_reconnect_rearm();          // an explicit connect re-arms the retry
     char who[48];
     std::snprintf(who, sizeof who, "bt-paired: row %d", i);
-    bt_request_connection(addr, who);
+    bt_user_connect(addr, who);
     // The connection completes asynchronously; the 3 s route poll notices it and refreshes the name.
     refresh_bt_paired();
 }