Reconnect issue regression fix (#161)

* Fixed but with some issues

* fixed and confirmed working

* ci fix

OpenPuck/haptics.cpp

@@ -64,6 +64,11 @@ void hapticSendShutdown(uint8_t slot)
 	faultDiagTrace(FR_OFF, slot);
 	for (uint8_t i = 0; i < HAPTIC_SHUTDOWN_SHOTS; i++)
 		relayEnqueue(0x9F, OFF, sizeof OFF, slot);
+	// Tell the puck presentation layer to show this slot (or all, for 0xFF broadcast) cleanly DISCONNECTED to
+	// Steam and hold it there through the controller's post-off F1 tail -- otherwise the dying replies bounce
+	// the connection state (phantom reconnect / never-removed). Covers every power-off path (Steam 0x9F, the
+	// Steam+Y chord, the panel button, host suspend) since they all funnel through here.
+	puckNotePowerOff(slot);
 }
 
 // millis of last 0x82 haptic OUTPUT relayed (Steam mode)

OpenPuck/puck_hid.cpp

@@ -23,6 +23,29 @@ uint8_t g_fwdNewOnly = 1;
 uint8_t g_fwdContentDedup = 1;
 SteamPuckController g_steamPuck;
 
+// millis() a power-off (0x9F) was last relayed to each slot; 0 = never. See puckNotePowerOff() / task().
+static volatile unsigned long g_powerOffMs[NSLOT] = { 0 };
+// Present a powered-off slot as DISCONNECTED to Steam for this long, to ride out the controller's post-off
+// F1 tail (measured ~<=1s of dying replies after the "off!" command) without bouncing the connection state.
+#define POWEROFF_HOLD_MS 2000u
+void puckNotePowerOff(uint8_t slot)
+{
+	if (slot >=
+	    NSLOT) { // 0xFF broadcast ("all off": host suspend, panel/test button)
+		for (int s = 0; s < NSLOT; s++)
+			if (g_slot[s].used)
+				g_powerOffMs[s] = millis();
+	} else {
+		g_powerOffMs[slot] = millis();
+	}
+}
+// True while `slot` is inside its post-power-off hold window (present it disconnected, ignore dying replies).
+static inline bool slotPoweringOff(int slot)
+{
+	return slot >= 0 && slot < NSLOT && g_powerOffMs[slot] &&
+	       (millis() - g_powerOffMs[slot] < POWEROFF_HOLD_MS);
+}
+
 // Cloned puck HID report descriptor: mouse(0x40)+keyboard(0x41)+vendor(FF00) with the 63-byte FEATURE
 // command reports on report id 1/2. Each of the 4 interfaces uses this.
 static const uint8_t PUCK_HID_DESC[] = {
@@ -207,6 +230,9 @@ static inline void hostStampAlive()
 // as a real click/keypress into the just-woken desktop. Set when task() sees the suspended->active transition.
 static unsigned long g_resumeMs = 0;
 #define POST_RESUME_MUTE_MS 1500u
+// How long to keep re-asserting the 0x79 DISCONNECT edge to Steam after a controller drops, so a single
+// lost report can't strand Steam in "connected" (see task()). Bounded to avoid forever-spamming disc.
+#define DISC_RESEND_MS 6000u
 
 // ===================== puck feature command channel =====================
 // `slot` is the interface index (interface N == bond slot N).
@@ -420,8 +446,11 @@ static void handleSet(int slot, uint8_t rid, hid_report_type_t type,
 		hostStampAlive();
 		S.resp[0] = 0xB4;
 		S.resp[1] = 0x01;
+		// Report disconnected during the post-power-off hold so B4 agrees with the 0x79 disconnect (they used
+		// different windows -- 500ms here vs the 300ms conn/DOWN edge -- so right after a power-off Steam's B4
+		// probe answered "still connected" and contradicted the disconnect we just pushed).
 		S.resp[2] = (slot >= 0 && slot < NSLOT && !g_xbox &&
-			     g_slot[slot].used &&
+			     g_slot[slot].used && !slotPoweringOff(slot) &&
 			     (millis() - g_connReplyMs[slot] < 500)) ?
 				    0x02 :
 				    0x01;
@@ -919,11 +948,31 @@ void SteamPuckController::task()
 	// slot. The real puck's per-slot edge-triggered 0x79 prevents re-triggering Steam's connect-chime loop.
 	static bool usbConn[NSLOT] = { 0 };
 	static unsigned long last79[NSLOT] = { 0 }, last7B[NSLOT] = { 0 },
-			     connEdgeMs[NSLOT] = { 0 }, last43[NSLOT] = { 0 };
+			     connEdgeMs[NSLOT] = { 0 },
+			     discEdgeMs[NSLOT] = { 0 }, last43[NSLOT] = { 0 },
+			     poHandled[NSLOT] = { 0 };
 	for (int s = 0; s < NSLOT; s++) {
 		if (!g_slot[s].used || !hid[s].ready())
 			continue;
-		bool conn = (millis() - g_connReplyMs[s] < 300);
+		// Power-off just relayed to this slot -> force ONE clean disconnect to Steam now, regardless of the
+		// prior edge state. The controller's noisy shutdown (it keeps streaming F1 for ~1s after "off!") can
+		// leave usbConn desynced from what Steam shows, so the ordinary conn!=usbConn edge sometimes never
+		// fired -> Steam kept the controller in its list (the "doesn't get removed" case). Anchor discEdgeMs
+		// too so the bounded resend covers a dropped packet.
+		if (g_powerOffMs[s] && g_powerOffMs[s] != poHandled[s]) {
+			poHandled[s] = g_powerOffMs[s];
+			uint8_t st = 0x01;
+			hapLogAdd(0xFB, 0x79, &st, 1); // ->host push (capture)
+			usbTxHid(&hid[s], 0x79, &st, 1);
+			usbConn[s] = false;
+			discEdgeMs[s] = millis();
+			last79[s] = millis();
+		}
+		// Hold the slot DISCONNECTED through the controller's post-power-off F1 tail (see slotPoweringOff);
+		// otherwise a stray dying reply bounces conn true -> a phantom 0x79=02 that Steam reads as a reconnect
+		// and answers by re-running its connect config (the "reappears for a split second").
+		bool conn = !slotPoweringOff(s) &&
+			    (millis() - g_connReplyMs[s] < 300);
 		// 0x79 connection state: on edge, then repeated every 750ms ONLY until Steam reacts (its first OUTPUT/
 		// settings write after the edge -- g_steamAliveMs). The real puck sends 0x79 ONCE, edge-triggered; an
 		// unconditional forever-resend re-triggers Steam's connect handling (connect chime) every 750ms before
@@ -932,10 +981,22 @@ void SteamPuckController::task()
 		bool steamAcked = g_steamAliveMs &&
 				  (int32_t)(g_steamAliveMs - connEdgeMs[s]) >=
 					  0;
+		// The DISCONNECT edge (0x79=01) also needs resending: it used to be sent exactly ONCE, so a single
+		// dropped report left Steam believing the controller was still connected indefinitely -- observed as
+		// "WebUSB panel shows the slot DOWN (g_connReplyMs stale) while Steam still lists it connected, and the
+		// controller light is solid (it re-adopted the beacon) but no input flows." Resend disc every 750ms but
+		// BOUNDED to DISC_RESEND_MS so a lost packet converges without the forever-spam the "real puck sends
+		// 0x79 once" note warns against.
+		bool discResend = !conn &&
+				  (millis() - discEdgeMs[s] < DISC_RESEND_MS);
 		if (conn != usbConn[s] ||
-		    (conn && !steamAcked && millis() - last79[s] >= 750)) {
+		    (conn && !steamAcked && millis() - last79[s] >= 750) ||
+		    (discResend && millis() - last79[s] >= 750)) {
 			if (conn && !usbConn[s])
 				connEdgeMs[s] = millis();
+			if (!conn &&
+			    usbConn[s]) // connect->disc edge: anchor the resend window
+				discEdgeMs[s] = millis();
 			uint8_t st = conn ? 0x02 : 0x01;
 			hapLogAdd(0xFB, 0x79, &st, 1); // ->host push (capture)
 			usbTxHid(&hid[s], 0x79, &st, 1);

OpenPuck/puck_hid.h

@@ -38,6 +38,14 @@ void puckCmdLogDrain(void);
 // config; OpenPuck doesn't need it). On by default; console "S81" toggles for A/B.
 extern bool g_drop81;
 
+// Note that a controller power-off (0x9F "off!") was just relayed to `slot` (or ALL used slots for the 0xFF
+// broadcast). The puck then presents that slot as cleanly DISCONNECTED to Steam and HOLDS it there through the
+// controller's messy shutdown -- a real controller keeps streaming F1 for up to ~1s after the off command, and
+// without this the puck faithfully reports "still connected" -> conn bounces -> Steam sees a phantom reconnect
+// (the "reappears for a split second") and re-runs its connect config, or never sees a clean disconnect and
+// leaves the controller in its list. Called from hapticSendShutdown() so every power-off path is covered.
+void puckNotePowerOff(uint8_t slot);
+
 class SteamPuckController : public IController {
     public:
 	void begin() override;

OpenPuck/rf_link.cpp

@@ -887,30 +887,53 @@ static void rfConnStep()
 		g_connPoll++;
 	};
 
-	// Poll a slot every cycle (full rate) only when its controller is actually here. Throttle the rest to
-	// every SLOT_COLD_RETRY_MS:
-	//   - "cold": WAS connected but silent > SLOT_COLD_MS (controller powered off / out of range);
-	//   - "phantom": NEVER replied AND another controller is already connected. This is the cloned-puck case
-	//     (issue: bonds copied from a backup include controllers that aren't present). Polling a phantom slot
-	//     every cycle was doubling/tripling Polls/s, flooding noRx, and stealing reply windows from the live
-	//     controller. A never-replied slot with nothing else connected still polls full-rate, so the FIRST
-	//     controller connects instantly; a phantom only backs off once a real link exists to protect.
 	bool linkUp = anySlotLinkUp();
-	for (int k = 0; k < NSLOT; k++) {
-		if (!g_slot[k].used)
-			continue;
-		bool everReplied = g_connReplyMs[k] != 0;
-		unsigned long silentMs =
-			everReplied ? (nowMs - g_connReplyMs[k]) : 0;
-		bool cold = everReplied && silentMs > SLOT_COLD_MS;
-		bool quiet = everReplied && !cold && silentMs > SLOT_QUIET_MS;
-		bool phantom = !everReplied && linkUp;
-		unsigned long retry = (cold || phantom) ? SLOT_COLD_RETRY_MS :
-				      quiet		? SLOT_QUIET_RETRY_MS :
-							  0;
-		if (retry && nowMs - g_slotLastAttemptMs[k] < retry)
-			continue;
-		doPoll(k);
+	if (!linkUp) {
+		// (RE)ACQUIRE MODE -- nothing is linked. Poll used slots to catch a (re)connecting controller fast,
+		// but ROUND-ROBIN: exactly ONE slot per cycle, so radio duty stays bounded to a single ~g_rxWin no-reply
+		// wait per cycle no matter how many bonds are absent.
+		//
+		// Why not poll every absent slot per cycle: doing so was the fix for the solid-light reconnect livelock
+		// (a slot silent > SLOT_COLD_MS had dropped to a 2s poll that kept missing the controller's re-adopt
+		// window), BUT with two bonds absent it put TWO full ~g_rxWin no-reply waits + the every-loop beaconing
+		// on every cycle -- a radio-duty spike that starved loop()/USB into a watchdog reset. The puck then
+		// re-enumerated on every disconnect, so Steam saw a "new controller" and popped its setup wizard every
+		// time. One slot per cycle keeps acquire snappy (N bonds => each polled at 250/N Hz, still far faster
+		// than the old 2s cold cadence) while capping duty. A single used slot is still polled every cycle.
+		static int acqIdx = 0;
+		for (int n = 0; n < NSLOT; n++) {
+			int k = (acqIdx + n) % NSLOT;
+			if (g_slot[k].used) {
+				acqIdx = (k + 1) % NSLOT;
+				doPoll(k);
+				break;
+			}
+		}
+	} else {
+		// CONNECTED -- at least one controller is live. Poll every WARM slot per cycle (all bonded controllers
+		// at full rate -- the real puck services them all per cycle), and throttle the SILENT ones so they don't
+		// steal reply windows from the live controller(s) (the issue-#72 radio-duty concern):
+		//   - "cold": WAS connected but silent > SLOT_COLD_MS (controller powered off / out of range);
+		//   - "quiet": briefly silent (fade) -> back off to SLOT_QUIET_RETRY_MS until it recovers or goes cold;
+		//   - "phantom": NEVER replied while another controller is live (e.g. a stale bond from a cloned backup).
+		for (int k = 0; k < NSLOT; k++) {
+			if (!g_slot[k].used)
+				continue;
+			bool everReplied = g_connReplyMs[k] != 0;
+			unsigned long silentMs =
+				everReplied ? (nowMs - g_connReplyMs[k]) : 0;
+			bool cold = everReplied && silentMs > SLOT_COLD_MS;
+			bool quiet = everReplied && !cold &&
+				     silentMs > SLOT_QUIET_MS;
+			bool phantom = !everReplied;
+			unsigned long retry = (cold || phantom) ?
+						      SLOT_COLD_RETRY_MS :
+					      quiet ? SLOT_QUIET_RETRY_MS :
+						      0;
+			if (retry && nowMs - g_slotLastAttemptMs[k] < retry)
+				continue;
+			doPoll(k);
+		}
 	}
 }