Fix controller no longer shutting off regression (#252)

* Fix early return in hapticTask skipping host-sleep controller shutdown

Co-authored-by: safijari <5191844+safijari@users.noreply.github.com>

* Fix suspend power-off wake trade-off and stale-slot "off!" delivery

Two follow-ups to the PR #247 restore of the suspend power-off in puck modes.

1. Powering the controller off on host sleep removes the short-Steam-press
   remote wakeup (rf_link guards it on USBDevice.suspended()), leaving only the
   reconnect-wake. Make the behavior an explicit, persisted choice: g_suspendOff
   (default on, console "SO"), with the wake path documented either way.

2. relayEnqueue's 0xFF broadcast wrote into every slot ring, including unbonded
   ones. Only used slots are ever flushed, so a host-suspend "off!" queued for an
   empty slot sat there until a controller was later paired into it and was then
   powered off by its first flush. Skip unbonded slots on broadcast (the rule
   puckNotePowerOff already used) and clear a slot's ring when it becomes bonded.

Also correct the id9-steering doc blocks, which still described the puck-mode
rule (wantAuto = puckLizardActive()) removed in #230.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: safijari <5191844+safijari@users.noreply.github.com>

OpenPuck/config.cpp

@@ -106,6 +106,9 @@ struct Cfg {
 	uint8_t padStick[ET_COUNT][2];
 	// RUMBLE_STYLE_*; 0xFF (short pre-tail file) -> compiled default
 	uint8_t rumbleStyle;
+	// autonomous controller power-off on host sleep (see haptics.h g_suspendOff). 0/1; 0xFF (short
+	// pre-tail file) -> compiled default (on)
+	uint8_t suspendOff;
 }; // rsvd0 = ex-padSmooth, now the one-shot debug-CDC arm
 
 // Shortest cfg.bin we still accept: the layout as of CFG_MAGIC 0xCF, i.e. everything before the appended tail.
@@ -132,7 +135,8 @@ void saveCfg()
 		  { g_chordDpad[0], g_chordDpad[1], g_chordDpad[2],
 		    g_chordDpad[3] },
 		  {},
-		  g_rumbleStyle };
+		  g_rumbleStyle,
+		  g_suspendOff };
 	for (int i = 0; i < ET_COUNT; i++) {
 		c.type[i] = g_type[i];
 		c.padStick[i][0] = g_padStickCfg[i][0];
@@ -224,6 +228,9 @@ void loadCfg()
 			// host-rumble style (0xFF = short pre-tail file -> keep the default)
 			if (c.rumbleStyle <= RUMBLE_STYLE_MAX)
 				g_rumbleStyle = c.rumbleStyle;
+			// suspend power-off enable (0xFF = a cfg.bin predating this tail field -> keep the on default)
+			if (c.suspendOff <= 1)
+				g_suspendOff = c.suspendOff;
 			// The poll RX window is now FIXED (g_rxWin is const) -- any persisted rxWin10 is ignored.
 		}
 		f.close();

OpenPuck/haptics.cpp

@@ -3,7 +3,7 @@
 #include "config.h"
 #include "rf_link.h"
 #include "usb_tx.h" // usbTxBoost/Unboost -- flood-rate CDC prints share the dcd DMA claim window
-#include "puck_hid.h" // puckLizardActive() -- gate the lizard-suppression keepalive
+#include "puck_hid.h" // puckNotePowerOff() -- hold a powered-off slot disconnected to Steam
 #include "steam_commands.h"
 
 #include "fault_diag.h" // faultDiagTrace() -- flight recorder
@@ -22,11 +22,14 @@ volatile uint8_t g_hapticStop = 0;
 // Off by default -- opt in from the WebUSB panel if a controller's haptics come up degraded after connect.
 uint8_t g_hapticBlockOn = 0;
 uint16_t g_hapticBlockMs = HAPTIC_BLOCK_MS_DEFAULT;
-// id9 steering (see hapticTask): ON by default. Holds id9=0 while Steam is DRIVING (autonomous engine off,
-// Steam owns haptics, no reconnect buzz-latch) and lands id9=1 when lizard is PRESENTING (pure MODE_LIZARD
-// or Steam-closed fallback) so the autonomous pad layer -- the trackpad-tick source -- is on.
+// id9 steering (see hapticTask): ON by default. EMULATED modes only -- it lands id9=1 (autonomous pad layer,
+// the trackpad-tick source, on) or holds id9=0 every LIZKEEP_MS per the per-type g_padHaptics config. Puck
+// modes (STEAM/LIZARD) are left alone: Steam owns the controller's haptics there.
 // Persisted; console 'u' toggles for A/B.
 uint8_t g_lizKeep = 1;
+// Power the controllers off when host sleep persists (see haptics.h for the wake trade-off). Persisted;
+// console "SO" toggles.
+uint8_t g_suspendOff = 1;
 // Host-rumble shaping (persisted in cfg.bin; console "RS<pct>" / "RY<n>").
 uint16_t g_rumbleScale = RUMBLE_SCALE_PCT;
 uint8_t g_rumbleStyle = RUMBLE_STYLE_NORMAL;
@@ -117,12 +120,19 @@ bool relayEnqueue(uint8_t rid, const uint8_t *payload, uint8_t plen,
 		return false;
 	uint32_t pm = __get_PRIMASK();
 	__disable_irq();
-	// slot=0xFF: broadcast -- enqueue into every slot's ring.
+	// slot=0xFF: broadcast -- enqueue into every BONDED slot's ring. Unbonded slots are skipped: only used
+	// slots are ever flushed (rfConnStep/doPoll), so anything queued for an empty slot sits in its ring
+	// indefinitely and is then delivered to whatever controller later pairs INTO that slot -- e.g. a host
+	// suspend with one controller bonded left "off!" (0x9F) frames in the empty rings, and the next
+	// controller paired there powered itself off on its first flush. puckNotePowerOff() already gates its
+	// broadcast on used; this is the same rule for the wire side.
 	// Full queue: evict the oldest entry, never the newest. Steam bursts end with the commit/stop, so
 	// dropping the oldest keeps the most-recent (meaningful) frame.
 	uint8_t s0 = (slot == 0xFF) ? 0 : slot;
 	uint8_t s1 = (slot == 0xFF) ? NSLOT : slot + 1;
 	for (uint8_t s = s0; s < s1; s++) {
+		if (slot == 0xFF && !g_slot[s].used)
+			continue;
 		uint8_t h = g_rqHead[s], nx = rqNext(h);
 		if (nx == g_rqTail[s])
 			g_rqTail[s] = rqNext(g_rqTail[s]);
@@ -139,6 +149,15 @@ bool relayEnqueue(uint8_t rid, const uint8_t *payload, uint8_t plen,
 	faultDiagTrace(FR_RELAY, (uint16_t)((slot << 8) | rid));
 	return true;
 }
+void relayClearSlot(uint8_t slot)
+{
+	if (slot >= NSLOT)
+		return;
+	uint32_t pm = __get_PRIMASK();
+	__disable_irq();
+	g_rqHead[slot] = g_rqTail[slot] = 0;
+	__set_PRIMASK(pm);
+}
 
 #if OPK_LOG
 // diagnostic capture: a ring of the last OUTPUT reports Steam sends (rid/slot/bytes/ms), dumped with 'H'.
@@ -687,12 +706,12 @@ void hapticTask()
 	// controller's revert timer (which also resets all settings = audible pop).
 	//
 	// wantAuto = should the controller run its own pad layer (trackpad ticks) for the ACTIVE mode?
-	//  - puck modes (STEAM/LIZARD): ON while we present lizard (pure MODE_LIZARD, or MODE_STEAM with Steam
-	//    closed) so the pad keeps its ticks; OFF when Steam is driving (Steam owns haptics, and holding the
-	//    engine off stops it latching the deep-inside reconnect buzz). = puckLizardActive().
+	//  - puck modes (STEAM/LIZARD): NOT STEERED AT ALL. Steam owns the controller's haptics in puck mode
+	//    (it writes id9 itself), so the whole block below is skipped -- we used to drive id9 from
+	//    puckLizardActive() here, which fought Steam's own writes.
 	//  - emulated modes (Xbox/Switch/DS): follow the per-type trackpad-haptics config g_padHaptics (default
-	//    ON; Switch defaults OFF). This is the MIRROR of the lizard case -- here holding id9=0 is how we turn
-	//    the controller's autonomous trackpad haptics OFF for a type that doesn't want them.
+	//    ON; Switch defaults OFF). Holding id9=0 is how we turn the controller's autonomous trackpad
+	//    haptics OFF for a type that doesn't want them.
 	if (g_lizKeep) {
 		static unsigned long lastKeep[NSLOT] = { 0 };
 		static bool landedAuto[NSLOT] = { false };
@@ -701,43 +720,41 @@ void hapticTask()
 		static const uint8_t DATA_LIZARD_ON[3] = { SETTING_LIZARD_MODE,
 							   0x01, 0x00 };
 
-		if (modeIsPuck(g_usbMode)) {
-			// We're in puck mode. Leave the haptics to Steam.
-			return;
-		}
-
-		// We're in emulated controller mode.
-		bool wantAuto = (g_padHaptics != 0);
-		for (int s = 0; s < NSLOT; s++) {
-			if (!g_slot[s].used || !hapticLinkUp(s)) {
-				// re-land id9 on the next (re)connect: a fresh controller defaults to
-				// autonomous, but one carrying our previous session's id9 does not
-				landedAuto[s] = false;
-				lastKeep[s] = 0;
-				continue;
-			}
-			if (wantAuto) {
-				if (!landedAuto[s]) {
-					landedAuto[s] = true;
+		// In puck mode Steam owns haptics; skip id9 steering.
+		if (!modeIsPuck(g_usbMode)) {
+			bool wantAuto = (g_padHaptics != 0);
+			for (int s = 0; s < NSLOT; s++) {
+				if (!g_slot[s].used || !hapticLinkUp(s)) {
+					// re-land id9 on the next (re)connect: a fresh controller defaults to
+					// autonomous, but one carrying our previous session's id9 does not
+					landedAuto[s] = false;
+					lastKeep[s] = 0;
+					continue;
+				}
+				if (wantAuto) {
+					if (!landedAuto[s]) {
+						landedAuto[s] = true;
+						relayEnqueue(
+							IBEX_CMD_SET_SETTINGS_VALUES,
+							DATA_LIZARD_ON,
+							sizeof DATA_LIZARD_ON,
+							false, (uint8_t)s);
+					}
+				} else {
+					landedAuto[s] = false;
+					if (lastKeep[s] &&
+					    (uint32_t)(millis() - lastKeep[s]) <
+						    LIZKEEP_MS)
+						continue;
+					lastKeep[s] = millis();
 					relayEnqueue(
 						IBEX_CMD_SET_SETTINGS_VALUES,
-						DATA_LIZARD_ON,
-						sizeof DATA_LIZARD_ON, false,
+						DATA_LIZARD_OFF,
+						sizeof DATA_LIZARD_OFF, false,
 						(uint8_t)s);
 				}
-			} else {
-				landedAuto[s] = false;
-				if (lastKeep[s] &&
-				    (uint32_t)(millis() - lastKeep[s]) <
-					    LIZKEEP_MS)
-					continue;
-				lastKeep[s] = millis();
-				relayEnqueue(IBEX_CMD_SET_SETTINGS_VALUES,
-					     DATA_LIZARD_OFF,
-					     sizeof DATA_LIZARD_OFF, false,
-					     (uint8_t)s);
 			}
-		}
+		} // !modeIsPuck
 	}
 	// Per-slot link-edge detect (backup for hapticOnReconnect in rf_link).
 	static bool wasHapticLinkUp[NSLOT] = { 0 };
@@ -785,8 +802,12 @@ void hapticTask()
 			faultDiagTrace(FR_RESUME, 0);
 		suspArmed = false;
 	}
+	// g_suspendOff: opt out of the power-off entirely -- keeps the controller awake through host sleep so its
+	// short-Steam-press remote wakeup stays available (see haptics.h). Disarm either way so re-enabling it
+	// mid-suspend can't fire late.
 	if (suspArmed && vbus && (millis() - suspSinceMs) >= SUSPEND_OFF_MS) {
-		hapticSendShutdown();
+		if (g_suspendOff)
+			hapticSendShutdown();
 		suspArmed = false; // fire once per suspend
 	}
 	wasSusp = susp;

OpenPuck/haptics.h

@@ -66,16 +66,33 @@ void hapticTestRumble();
 // that nothing will ever wait on a reply for. rfConnFlushRelay uses this.
 bool relayEnqueue(uint8_t rid, const uint8_t *payload, uint8_t plen,
 		  bool isHaptic, uint8_t slot = 0xFF, bool expectReply = false);
+// Drop everything queued for one bond slot. Called when a slot becomes BONDED (Steam's 0xA2 pairing write,
+// the panel's bond import): whatever was queued while the slot was empty was aimed at a controller that no
+// longer -- or never did -- live there, and an unbonded slot's ring is never flushed, so it would otherwise
+// be handed straight to the freshly paired controller (a stale "off!" powered it back off). ISR-safe.
+void relayClearSlot(uint8_t slot);
 
-// id9=0 hold (MODE_STEAM only): land the controller's SET_SETTINGS index 9 (digital-mappings/lizard-active)
-// at 0, once per LIZKEEP_MS per connected slot, like the real puck. This holds the controller's autonomous
-// mapping/haptic engine OFF so it can't latch into the deep-inside buzz seen after repeated reconnects
-// (capture-for-haptics.txt: the buzz is controller-internal; OpenPuck relays no haptics in that state). It
-// also disables the controller's autonomous touchpad ticks (id9 gates the whole pad layer) -- fine in Steam
-// mode (Steam owns haptics), so it is scoped to MODE_STEAM; pure MODE_LIZARD is left alone to keep its ticks.
+// id9 steering (EMULATED modes only): land the controller's SET_SETTINGS index 9 (digital-mappings /
+// lizard-active) at 0 once per LIZKEEP_MS per connected slot to hold its autonomous mapping/haptic engine
+// OFF, or at 1 once per connect episode to turn it on -- per the active type's g_padHaptics config. id9
+// gates the whole autonomous pad layer, including the trackpad ticks, and holding it off also stops the
+// engine latching the deep-inside buzz seen after repeated reconnects (capture-for-haptics.txt: that buzz is
+// controller-internal; OpenPuck relays no haptics in that state). PUCK modes (STEAM/LIZARD) are not steered:
+// Steam writes id9 itself there, and driving it from the puck side fought those writes.
 #define LIZKEEP_MS 2000u
 extern uint8_t
 	g_lizKeep; // 1 = hold on (default, persisted); console 'u' toggles for A/B
+// Autonomous controller power-off on host sleep (see hapticTask). 1 = power the controllers off once the
+// USB suspend has persisted SUSPEND_OFF_MS (default -- what the real puck does, so the controllers don't sit
+// awake draining while the host sleeps). 0 = leave them on.
+//
+// TRADE-OFF, deliberate: with this ON the controller is off while the host sleeps, so the short-Steam-press
+// remote-wakeup gesture in rf_link (guarded on USBDevice.suspended()) can no longer be sent from it. Waking
+// the host from the controller then goes through the OTHER path: pressing Steam on a powered-off controller
+// turns it back on, the link comes up, and the reconnect-wake in rfConnStep issues the remote wakeup. Same
+// physical gesture; the power-off fires once per suspend so the returning controller is not shut off again.
+// Turn this OFF (console "SO") if a controller must stay awake through host sleep. Persisted.
+extern uint8_t g_suspendOff;
 // Master enable for the puck->controller haptic relay (Steam 0x80-0x86 rumble/pad-feedback). Console "HR"
 // toggles it to isolate the drag-smoothness cost of relaying Steam's trackpad haptics. See haptics.cpp.
 extern bool g_hapticRelay;

OpenPuck/puck_hid.cpp

@@ -505,8 +505,14 @@ static void handleSet(int slot, uint8_t rid, hid_report_type_t type,
 				S.used = false;
 				memset(S.rec, 0, 24);
 			} else {
+				bool wasUsed = S.used;
 				memcpy(S.rec, pl, 24);
 				S.used = true;
+				// Fresh bond into this slot: drop anything queued while it was empty (an
+				// unbonded slot's ring is never flushed, so a stale frame -- e.g. the "off!"
+				// from a host-suspend broadcast -- would land on the new controller).
+				if (!wasUsed)
+					relayClearSlot((uint8_t)slot);
 			}
 			g_dirty = true;
 #if OPK_LOG

OpenPuck/serial_console.cpp

@@ -64,6 +64,16 @@ void serialConsolePoll()
 				Serial.printf(
 					"# poll rate -> %ld Hz (%lu us)\n", hz,
 					(unsigned long)g_pollUs);
+			} else if (!strcmp(line, "SO")) {
+				// toggle the autonomous controller power-off on host sleep. Off = the controller
+				// stays awake through host sleep, keeping its short-Steam-press remote wakeup
+				// usable (see haptics.h g_suspendOff). Persisted.
+				g_suspendOff = !g_suspendOff;
+				saveCfg();
+				Serial.printf(
+					"# power off controllers on host sleep (>%ums) %s\n",
+					(unsigned)SUSPEND_OFF_MS,
+					g_suspendOff ? "ON" : "off");
 			} else if (!strcmp(line, "HR")) {
 				// A/B: disable the puck->controller haptic relay (Steam 0x80-0x86 rumble/pad feedback)
 				// to isolate whether relaying Steam's trackpad texture haptics degrades drag smoothness.

OpenPuck/webusb_config.cpp

@@ -792,9 +792,14 @@ void webusbPoll()
 				uint8_t slot = buf[1], used = buf[2];
 				if (slot < NSLOT) {
 					if (used && !recEmpty(buf + 3)) {
+						bool wasUsed =
+							g_slot[slot].used;
 						memcpy(g_slot[slot].rec,
 						       buf + 3, 24);
 						g_slot[slot].used = true;
+						// see puck_hid 0xA2: never hand a stale queue to a new bond
+						if (!wasUsed)
+							relayClearSlot(slot);
 					} else {
 						memset(g_slot[slot].rec, 0, 24);
 						g_slot[slot].used = false;