Bläddra i källkod

more comment culling

Jariullah Safi 4 månader sedan
förälder
incheckning
8b7c73cbc6

+ 38 - 13
AGENTS.md

@@ -5,23 +5,48 @@
 - **Linux kernel style**, enforced by `.clang-format` (8-wide tabs, 80 columns).
   Pinned to **clang-format 18** — other versions reformat differently and CI
   will reject them.
-- **No long trailing comments.** clang-format can't relocate a trailing `//`
-  comment, so a long one either splits the statement or runs past 80 columns.
-  Instead put the comment on its own line(s) **above** the statement, with a
-  blank line before it for separation:
 
-  ```c
-  // why this value matters, at whatever length you need
-  static int g_foo = 0;
-  ```
+## Comments
 
-  not
+Comment to explain **why**, not **what** — the code already says what it does.
+This codebase reverse-engineers an RF/HID protocol, so the comments worth
+writing are the ones that capture knowledge the code can't show on its own.
 
-  ```c
-  static int g_foo = 0; // why this value matters, at whatever length ...
-  ```
+Write a comment only when it carries genuine nuance:
 
-  Short trailing comments that fit within 80 columns are fine.
+- the meaning of a magic value or protocol constant,
+- a hardware / host-OS quirk or gotcha,
+- a non-obvious constraint or consequence ("lower than 96 truncates the
+  battery report → CRC fail"),
+- why a non-obvious approach was chosen over the obvious one.
+
+Delete a comment if it:
+
+- restates the code (`i++; // increment i`),
+- narrates history or changelog ("was 64, bumped to 96"),
+- editorializes ("THE fix", "the magic part"),
+- leaves debug / A-B-test breadcrumbs.
+
+Keep the constraint, drop the story. Prefer no comment to an obvious one; when
+tempted to annotate, first see if clearer code makes the comment unnecessary.
+
+**No long trailing comments.** clang-format can't relocate a trailing `//`
+comment, so a long one either splits the statement or runs past 80 columns.
+Put it on its own line(s) **above** the statement, with a blank line before it:
+
+```c
+// 0xE7 selects the protocol-version handshake; 0xE3 is the steady-state poll
+uint8_t g_connType = 0xE7;
+```
+
+not
+
+```c
+uint8_t g_connType = 0xE7; // 0xE7 selects the protocol-version handshake ...
+```
+
+Short trailing comments that fit within 80 columns are fine. The lint
+(`make lint`) enforces only the column rule; the rest is on you and review.
 
 ## Before pushing
 

+ 18 - 24
OpenPuck/OpenPuck.ino

@@ -21,7 +21,7 @@ using namespace Adafruit_LittleFS_Namespace;
 
 #include "config.h"
 
-// OPK_GIT_HASH (used to tag the one-time factory-reset build)
+// OPK_GIT_HASH: tags the one-time factory-reset build
 #include "build_info.h"
 #include "identity.h"
 #include "bonds.h"
@@ -57,29 +57,27 @@ void setup()
 #endif
 	loadCfg();
 
-	// load persisted config + decide USB presentation BEFORE registering interfaces
+	// decide USB presentation BEFORE registering interfaces
 	g_xbox = !modeIsPuck(g_usbMode);
 	g_active = controllerFor(g_usbMode);
 
 	// ---- USB descriptor rebuild ----
-	// Puck mode NORMALLY drops the CDC serial console (clearConfiguration) to free a USB endpoint for the
-	// wake-mouse interface that lets it wake a sleeping Windows host. The one-shot debug arm (g_debugCdcThisBoot,
-	// set via the WebUSB panel / CDC 'D', auto-reverting after one boot) instead KEEPS the boot CDC composite
-	// for that boot and skips the wake mouse (no endpoint room for both). Clean modes always rebuild bare.
+	// Puck mode drops the CDC serial console (clearConfiguration) to free a USB endpoint for the wake-mouse
+	// interface that wakes a sleeping Windows host. The one-shot debug arm (g_debugCdcThisBoot, set via WebUSB
+	// panel / CDC 'D', auto-reverting after one boot) instead KEEPS the boot CDC composite and skips the wake
+	// mouse (no endpoint room for both). Clean modes always rebuild bare.
 	const bool puckMode = g_active->isPuck();
 	const bool keepCdc = puckMode && g_debugCdcThisBoot;
 	// The "game" PlayStation modes enumerate as a CLEAN single HID gamepad so games that classify off the raw HID
 	// device (Fortnite/UE GameInput, Windows.Gaming.Input) recognise them as PlayStation. A real Sony pad is just
 	// the gamepad HID (+ a USB-audio interface); our extra wake-mouse HID and WebUSB vendor interface make those
-	// classifiers refuse the PS glyph path even with the correct VID/PID (SDL/Steam tolerate the extras, which is
-	// why Steam still sees the normal PS modes). In clean-PS modes we skip BOTH the wake mouse and WebUSB -- the
-	// cost is no config panel / host-wake; chord back to Steam (back-paddle 4 + A) to reach the panel. The normal
-	// MODE_PS5 / MODE_HIDGYRO keep wake + panel (use those when you need host-wake; the *_GAME ones for games).
+	// classifiers refuse the PS glyph path even with the correct VID/PID (SDL/Steam tolerate the extras). So
+	// clean-PS modes skip BOTH the wake mouse and WebUSB -- no config panel / host-wake; chord back to Steam
+	// (back-paddle 4 + A) to reach the panel. Normal MODE_PS5 / MODE_HIDGYRO keep wake + panel.
 	const bool psClean = modeIsCleanPS(g_usbMode);
 	USBDevice.detach();
 	delay(30);
 	if (keepCdc) {
-		// keep boot CDC composite (debug boot)
 		USBDevice.setConfigurationBuffer(g_usbCfgDesc,
 						 sizeof g_usbCfgDesc);
 	} else {
@@ -111,7 +109,6 @@ void setup()
 	if (puckMode && !keepCdc)
 		wakeHidBegin();
 
-	// register this mode's USB interface(s) + set VID/PID/strings
 	g_active->begin();
 
 	// Boot-mouse wake interface for clean (non-puck) modes, and for puck on the one-shot debug boot (CDC on,
@@ -124,11 +121,10 @@ void setup()
 	// slots; other clean modes after controller. PS modes omit it to present a genuine single-HID PS controller.
 	if (!puckMode && !psClean)
 		usb_web.begin();
-	// Enable USB Remote Wakeup (bit 5) so the host lets us signal wake-from-sleep. Bit 7 is always required.
-	// bmAttributes: required(0x80) | remote_wakeup(0x20)
+	// bmAttributes: required(0x80) | remote_wakeup(0x20). Remote Wakeup lets us signal wake-from-sleep.
 	USBDevice.setConfigurationAttribute(0x80 | 0x20);
 
-	// re-connect with the final descriptor (host re-reads it fresh -> deterministic enumeration)
+	// re-attach with the final descriptor (host re-reads it fresh -> deterministic enumeration)
 	USBDevice.attach();
 	Serial.begin(115200);
 	for (int i = 0; i < 300 && !USBDevice.mounted(); i++)
@@ -138,8 +134,6 @@ void setup()
 		ledWakePulse();
 	} // wake host if bus was sleeping when we (re-)attached
 	loadBonds();
-
-	// clear relay/active flags + arm the reconnect block & initial stop burst
 	hapticInit();
 	static const char *MODE_NAME[] = {
 		"STEAM(puck)",	       "XBOX(xinput+mouse)",
@@ -161,9 +155,9 @@ void setup()
 		"# session addr %02X%02X%02X%02X/%02X ch%u (discovery on ibex/ch2)\n",
 		g_sessBase[0], g_sessBase[1], g_sessBase[2], g_sessBase[3],
 		g_sessPrefix, g_sessCh);
-	// Hardware watchdog: if loop() ever stops feeding it (a wedged radio busy-wait, a HardFault spin, a blocked
-	// CDC write) the WDT resets the nRF52 after ~8s -- re-enumerating USB and re-initialising RF on its own, so a
-	// hang no longer needs a physical replug. RUN keeps it counting in sleep; PAUSE freezes it under a debugger.
+	// Hardware watchdog: if loop() ever stops feeding it (wedged radio busy-wait, HardFault spin, blocked CDC
+	// write) the WDT resets the nRF52 after ~8s, re-enumerating USB + re-initialising RF, so a hang no longer
+	// needs a physical replug. RUN keeps it counting in sleep; PAUSE freezes it under a debugger.
 	NRF_WDT->CONFIG = (WDT_CONFIG_HALT_Pause << WDT_CONFIG_HALT_Pos) |
 			  (WDT_CONFIG_SLEEP_Run << WDT_CONFIG_SLEEP_Pos);
 	NRF_WDT->CRV = 8UL * 32768UL - 1; // timeout in 32.768 kHz ticks (~8 s)
@@ -171,9 +165,9 @@ void setup()
 	NRF_WDT->TASKS_START = 1;
 }
 
-// loop-timing diagnostics: the poll rate is capped by the loop ITERATION time (the pacing wants 4000us but a
-// slow loop fires the poll only as often as it comes around). Per-section us is accumulated and, each second,
-// the average loop period + the slowest section are published for the WebUSB panel to pinpoint the culprit.
+// loop-timing diagnostics: poll rate is capped by loop ITERATION time (pacing wants 4000us but the poll only
+// fires as often as the loop comes around). Per-section us accumulates and each second the avg loop period +
+// slowest section are published for the WebUSB panel.
 
 // avg loop iteration time last second (1e6/iterations)
 uint16_t g_loopPeriodUs = 0;
@@ -183,7 +177,7 @@ uint8_t g_loopWorst = 0;
 uint16_t g_loopWorstUs = 0; // that section's avg us per iteration
 void loop()
 {
-	// feed the watchdog each loop; if we ever stop, the ~8s WDT auto-resets us
+	// feed the watchdog; if we ever stop, the ~8s WDT auto-resets us
 	NRF_WDT->RR[0] = WDT_RR_RR_Reload;
 	if (g_dirty) {
 		g_dirty = false;

+ 13 - 19
OpenPuck/config.cpp

@@ -76,18 +76,17 @@ void loadCfg()
 			for (int i = 0; i < 4; i++)
 				g_back[i] = c.back[i];
 			g_persistMode = c.persistMode ? true : false;
-			// one-shot debug-CDC (Cfg.rsvd0): honor it for THIS boot, then consume so the next boot reverts to normal.
+			// one-shot debug-CDC (Cfg.rsvd0): honor for THIS boot, then consume so the next boot reverts to normal.
 			g_debugCdcThisBoot = c.rsvd0 ? true : false;
 			if (c.rsvd0) {
 				g_debugCdc = 0;
 				consume = true;
 			}
-			// poll rate is fixed (g_pollUs const = POLL_US_DEFAULT). A stale rate from an older build is never
-			// applied; rewrite cfg so the persisted byte also matches the new default.
+			// poll rate is fixed; rewrite cfg so the persisted byte matches the new default.
 			if (c.pollU100 != (uint8_t)(POLL_US_DEFAULT / 100))
 				consume = true;
-			// boot-mode policy: a one-shot bootMode (set by an explicit switch when !persist) wins once and is then
-			// cleared; otherwise persist->last mode, else->Steam. (poll rate stays POLL_US_DEFAULT -- never restored from cfg.)
+			// boot-mode policy: a one-shot bootMode (explicit switch when !persist) wins once then clears;
+			// otherwise persist->last mode, else->Steam.
 			if (c.bootMode != 0xFF) {
 				g_usbMode = modeValid(c.bootMode) ? c.bootMode :
 								    0;
@@ -135,27 +134,22 @@ void armDebugCdcNextBoot()
 	saveCfg();
 } // next boot keeps CDC; loadCfg() consumes it after
 
-// FULL factory wipe: erase ALL persistent storage in one shot -- cfg.bin (modes/tunables/chords), bonds.bin
-// (the paired-controller record), and anything else living in the internal LittleFS. format() reformats the
-// whole filesystem, so there's nothing left to selectively miss. The caller is expected to reboot immediately
-// (NVIC_SystemReset): on the next boot loadCfg()/loadBonds() find no files and fall back to clean defaults,
-// and the controller must be re-paired. Destructive and irreversible -- gated behind explicit confirmation at
-// every call site (serial "ERASE-ALL", WebUSB op 0x0A magic, both with on-screen warnings).
+// FULL factory wipe: reformat the internal LittleFS, erasing cfg.bin (modes/tunables/chords) AND bonds.bin
+// (paired-controller record). Caller reboots: next boot finds no files and falls back to clean defaults, and
+// the controller must be re-paired. Irreversible -- gated behind explicit confirmation at every call site.
 void factoryErase()
 {
 	// ensure mounted before we reformat (no-op if already up)
 	InternalFS.begin();
-	InternalFS.format(); // wipes cfg.bin + bonds.bin + the entire FS
+	InternalFS.format();
 }
 
-// One-time factory reset for the -DOPK_FACTORY_RESET recovery build. The point is to clear a bad config/bond
-// ONCE (at the first boot after flashing) and then behave like a normal build that PERSISTS settings -- NOT to
-// wipe on every boot (which would make the board un-configurable). We track "already reset" with a tiny tag file
-// holding the build's git hash, written AFTER the wipe (so it survives in the freshly-formatted FS):
-//   - tag missing or != this build's hash  -> a reset is pending: wipe, then stamp the tag. Next boot persists.
+// One-time factory reset for the -DOPK_FACTORY_RESET recovery build: clear a bad config/bond ONCE (first boot
+// after flashing) then persist normally. "Already reset" is tracked by a tag file holding the build's git hash,
+// written AFTER the wipe (so it survives in the freshly-formatted FS):
+//   - tag missing or != this build's hash  -> wipe, then stamp the tag. Next boot persists.
 //   - tag == this build's hash             -> already reset for this build: skip, boot normally.
-// Because the tag is keyed to the git hash, flashing a DIFFERENT build re-triggers the one-time wipe; re-flashing
-// the identical image does not (use the WebUSB/serial erase for an on-demand wipe). buildTag is OPK_GIT_HASH.
+// Keying the tag to the git hash means flashing a DIFFERENT build re-triggers the wipe. buildTag is OPK_GIT_HASH.
 #define RESET_TAG_FILE "/rsttag"
 void factoryResetOnce(const char *buildTag)
 {

+ 13 - 21
OpenPuck/config.h

@@ -1,28 +1,22 @@
 // config.h -- USB-presentation modes + persisted, runtime-tunable settings.
 //
 // One puck speaks the SAME RF protocol no matter which USB face it wears; only the USB enumeration and the
-// input-report mapping change. The active face is g_usbMode, persisted to flash (cfg.bin). Tunables here
-// (mouse sensitivity, back-paddle mapping, A/B swap, chord assignments, QoS) are set over the CDC console
-// (serial_console.cpp) and the WebUSB panel (webusb_config.cpp) and likewise persist.
+// input-report mapping change. The active face is g_usbMode, persisted to flash (cfg.bin). Tunables here are
+// set over the CDC console (serial_console.cpp) and the WebUSB panel (webusb_config.cpp) and likewise persist.
 #pragma once
 #include <stdint.h>
 
-// ---- Compile-time logging / diagnostics ----
-// 0 (default) = PRODUCTION build: no host->controller capture ring (reclaims ~80KB RAM), no per-event
-// logging, no main-loop section timing, no WebUSB capture channel. The functional bits (haptic reconnect
-// block, the buzz-clear re-init, rate stats) stay. Build the diagnostic firmware with -DOPK_LOG=1.
-// The WebUSB panel reads this back in the status blob and hides its logging UI when it's 0.
+// Compile-time logging / diagnostics. 0 (default) = PRODUCTION build: no host->controller capture ring
+// (reclaims ~80KB RAM), no per-event logging, no main-loop section timing, no WebUSB capture channel.
+// Functional bits (haptic reconnect block, buzz-clear re-init, rate stats) stay. -DOPK_LOG=1 for diagnostics.
 #ifndef OPK_LOG
 #define OPK_LOG 0
 #endif
 
-// Build-time FACTORY RESET (recovery build). 0 (default) = normal. Build with -DOPK_FACTORY_RESET=1 to produce a
-// firmware that wipes ALL persistent storage (cfg.bin + bonds.bin) ONCE -- on the first boot after flashing --
-// then behaves exactly like a normal build that persists settings. It is NOT a wipe-every-boot image: a git-hash
-// tag file (written after the wipe) records that this build already did its one-time reset, so subsequent boots
-// skip it. This makes it a safe recovery image for a bad config/bond: flash it, boot once (cleaned), keep using
-// it. Flashing a DIFFERENT build re-triggers the one-time wipe; for an on-demand wipe use the serial "ERASE-ALL"
-// or WebUSB "Factory erase". Re-pair the controller after the reset. See factoryResetOnce() in config.cpp.
+// Build-time FACTORY RESET (recovery build). 0 (default) = normal. -DOPK_FACTORY_RESET=1 wipes ALL persistent
+// storage (cfg.bin + bonds.bin) ONCE -- on the first boot after flashing -- then persists normally. A git-hash
+// tag file (written after the wipe) records that this build already reset, so subsequent boots skip it. Flashing
+// a DIFFERENT build re-triggers the one-time wipe. Re-pair the controller after the reset. See factoryResetOnce().
 #ifndef OPK_FACTORY_RESET
 #define OPK_FACTORY_RESET 0
 #endif
@@ -69,18 +63,16 @@ extern bool g_xbox;
 extern uint8_t g_chordBtn[3]; // back4+B/X/Y -> these modes (A always STEAM)
 
 // Mode persistence policy: by DEFAULT every fresh power-on/reconnect lands in STEAM mode (0). An explicit
-// mode switch still works for the session via a ONE-SHOT bootMode (honored once, then cleared, so the next
-// cold boot reverts to Steam). The WebUI "persist last mode" toggle (g_persistMode) instead remembers the
-// last selected mode across reboots.
+// mode switch still works for the session via a ONE-SHOT bootMode (honored once, then cleared). g_persistMode
+// instead remembers the last selected mode across reboots.
 // false (default) = always boot Steam; true = boot into last mode
 extern bool g_persistMode;
 // one-shot: boot into this mode once then clear (!persistMode + explicit switch)
 extern uint8_t g_bootMode;
 
 // One-shot debug CDC. Puck mode normally DROPS the CDC serial console to free the USB endpoint its wake-mouse
-// interface needs (so puck can wake a sleeping Windows host). Arming this keeps CDC for the NEXT boot only --
-// dropping the wake mouse that boot -- so someone can attach the serial debugger; the boot after reverts to
-// normal automatically. Mirrors the g_bootMode one-shot. Armed from the WebUSB panel or CDC 'D' command.
+// interface needs (to wake a sleeping Windows host). Arming this keeps CDC for the NEXT boot only -- dropping
+// the wake mouse that boot -- to attach the serial debugger; the boot after reverts automatically.
 // decision for THIS boot: true => keep CDC, skip the wake interface
 extern bool g_debugCdcThisBoot;
 void armDebugCdcNextBoot(); // persist the one-shot (caller reboots)

+ 1 - 2
OpenPuck/controllers.cpp

@@ -10,8 +10,7 @@
 IController *g_active = nullptr;
 
 // Map a USB-presentation mode to its singleton controller. Steam and Lizard share the puck controller (the
-// lizard-vs-forward decision is made per-report inside it). This is the one place that knows the mode->class
-// mapping; everything else dispatches through g_active.
+// lizard-vs-forward decision is made per-report inside it).
 IController *controllerFor(uint8_t mode)
 {
 	switch (mode) {

+ 8 - 9
OpenPuck/controllers.h

@@ -1,19 +1,18 @@
 // controllers.h -- the emulated-USB-controller abstraction.
 //
 // Each USB personality (Steam puck, Xbox/XInput, Switch HORIPAD, Switch Pro, PS5 DualSense, DS4 gyro) is an
-// IController living in its own mode_*.cpp. Exactly ONE is active per boot, selected by g_usbMode. The split
-// of responsibilities:
-//   begin()       -- called once from setup(): set the USB device VID/PID/strings and register this mode's
-//                    interface(s). The shared USB lifecycle (detach, clear/keep CDC, serial suffix, WebUSB,
+// IController living in its own mode_*.cpp. Exactly ONE is active per boot, selected by g_usbMode. Methods:
+//   begin()       -- called once from setup(): set the USB VID/PID/strings and register this mode's
+//                    interface(s). Shared USB lifecycle (detach, clear/keep CDC, serial suffix, WebUSB,
 //                    attach) stays in setup(); begin() only adds what's specific to this controller.
 //   onReport45()  -- called from rf_link.cpp each time a controller input report 0x45 is decoded. PUSH-style
 //                    modes (Xbox, puck/lizard) build + send their host report here. STREAM-style modes ignore
-//                    it and instead emit at a fixed cadence from task(); they read the decoded g_in directly.
+//                    it and emit at a fixed cadence from task(); they read the decoded g_in directly.
 //   task()        -- called every loop(): streaming emit, handshake/subcommand draining, mode-specific upkeep.
 //   isPuck()      -- true for Steam/Lizard (keep the boot CDC composite; different USB lifecycle in setup()).
 //
 // To add a new controller: implement IController in a new mode_*.cpp, give it a singleton, and wire it into
-// controllerFor() in controllers.cpp. Nothing else needs to change.
+// controllerFor() in controllers.cpp.
 #pragma once
 #include <stdint.h>
 
@@ -30,9 +29,9 @@ class IController {
 		(void)fresh;
 		(void)bodyTlen;
 	}
-	// Other controller->host input reports decoded from the F1 reply (NOT 0x45): the periodic power/battery status
-	// report 0x43, the status event 0x44. The real puck forwards these to Steam verbatim -- that's how Steam reads
-	// battery. rid = report id, data/n = body after the id. Default no-op (clean modes don't expose these reports).
+	// Other controller->host input reports decoded from the F1 reply (NOT 0x45): power/battery status report
+	// 0x43, status event 0x44. The real puck forwards these to Steam verbatim -- that's how Steam reads battery.
+	// rid = report id, data/n = body after the id. Default no-op (clean modes don't expose these reports).
 	virtual void onAuxReport(uint8_t rid, const uint8_t *data, uint8_t n)
 	{
 		(void)rid;

+ 1 - 1
OpenPuck/gamepad_util.cpp

@@ -3,7 +3,7 @@
 #include "config.h"
 
 uint8_t swStick(int16_t v, bool invert)
-{ // int16 (center 0) -> uint8 (center 0x80)
+{
 	int32_t a = 0x80 + (invert ? -((int32_t)v >> 8) : ((int32_t)v >> 8));
 	if (a < 0)
 		a = 0;

+ 35 - 52
OpenPuck/haptics.cpp

@@ -9,21 +9,19 @@
 #include <string.h>
 
 uint8_t g_relayOp = 0xE3; // E3 poll
-// relay sub-TLV TYPE byte. Vestigial now: rfConnFlushRelay derives the on-air type from the report id (see the
-// CONFIRMED split there -- 0x05 for actuators/haptics <0x87, 0x01 for config/settings/LED >=0x87). Still exposed
-// over WebUSB (relaySub field) for the buzz-hunt diagnostics, but the flush path no longer reads it.
+// relay sub-TLV TYPE byte. Vestigial: rfConnFlushRelay derives the on-air type from the report id (0x05 for
+// actuators/haptics <0x87, 0x01 for config/settings/LED >=0x87). Still exposed over WebUSB diagnostics.
 uint8_t g_relaySub = 0x05;
 volatile uint8_t g_testHaptic = 0;
 volatile uint8_t g_hapticStop = 0;
 unsigned long g_hapticBlockUntil =
 	0; // drop Steam haptics briefly during reconnect settle
 
-// Controller power-off. CONFIRMED from a real Windows USB capture of the Valve puck (shutoff.pcapng): Steam's
-// "turn off controller" is the single feature-0x01 command 0x9F with payload ASCII "off!" (6F 66 66 21). The
-// dongle forwards host feature reports to the controller verbatim, so the controller acts on report 0x9F
-// directly -- we relay it the same way (E3 SET sub-TLV). The wire relay is NO-ACK, so send a small burst: a
-// single lost frame must not leave the controller on. Driven by the Steam 0x9F passthrough (puck_hid), the
-// host-suspend edge (hapticTask), and the WebUSB test button (op 0x08).
+// Controller power-off. CONFIRMED from a real Windows USB capture of the Valve puck: Steam's "turn off
+// controller" is the single feature-0x01 command 0x9F with payload ASCII "off!" (6F 66 66 21). The dongle
+// forwards host feature reports verbatim, so the controller acts on report 0x9F directly -- we relay it the
+// same way (E3 SET sub-TLV). The wire relay is NO-ACK, so send a small burst: a single lost frame must not
+// leave the controller on.
 void hapticSendShutdown()
 {
 	static const uint8_t OFF[4] = { 0x6f, 0x66, 0x66, 0x21 }; // "off!"
@@ -84,10 +82,8 @@ bool relayEnqueue(uint8_t rid, const uint8_t *payload, uint8_t plen)
 	__disable_irq();
 	uint8_t h = g_rqHead, nx = rqNext(h);
 	// Full -> evict the OLDEST entry, never the newest. Steam sends commands as bursts where the meaningful
-	// frame comes LAST (a settings transaction ends with its commit; a haptic stream ends with its stop). The
-	// old drop-newest policy threw away exactly those, so LED commits and haptic stops were the casualties --
-	// brightness never applied and a glide's final stop never reached the controller (stuck buzz). Dropping the
-	// oldest instead guarantees the most recent command always lands.
+	// frame comes LAST (a settings transaction ends with its commit; a haptic stream ends with its stop), so
+	// dropping the oldest guarantees the most recent command (LED commit, haptic stop) always lands.
 	if (nx == g_rqTail)
 		g_rqTail = rqNext(g_rqTail);
 	g_rq[h].rid = rid;
@@ -96,7 +92,7 @@ bool relayEnqueue(uint8_t rid, const uint8_t *payload, uint8_t plen)
 		memcpy(g_rq[h].data, payload, plen);
 	g_rqHead = nx;
 	// Any haptic relay (Steam OR Xbox rumble OR test) arms the idle-clear and refreshes its timer -- so the
-	// during-use buzz gets cleared in EVERY USB mode, not just Steam. The re-init's own 0x81/0x87 don't match.
+	// during-use buzz gets cleared in EVERY USB mode. The re-init's own 0x81/0x87 deliberately don't match.
 	if (rid == 0x82 || rid == 0x80) {
 		g_haptic82Ms = millis();
 		g_hapClearArmed = true;
@@ -107,14 +103,13 @@ bool relayEnqueue(uint8_t rid, const uint8_t *payload, uint8_t plen)
 
 #if OPK_LOG
 // diagnostic capture: a ring of the last OUTPUT reports Steam sends (rid/slot/bytes/ms), dumped with 'H'.
-// Reproduce the whine, press Steam to stop it, then 'H' to see the ON stream + the exact OFF frame Steam sends.
 struct HapLog {
 	uint32_t ms;
 	uint8_t slot, rid, n, b[16];
 }; // 16 payload bytes: capture full 0x87 settings frames
 // Big always-on ring: log EVERYTHING from boot (Steam writes, our TX-to-controller, link edges) so a rare
-// reconnect-buzz can be caught after the fact -- the trigger happens moments after the puck boots, while this
-// RAM is fresh, and we dump it once the panel reconnects. 4096 * 20B ~= 80KB (we have plenty of free RAM).
+// reconnect-buzz can be caught after the fact -- the trigger happens moments after boot, while this RAM is
+// fresh, and we dump it once the panel reconnects. 4096 * 20B ~= 80KB.
 #define HAPLOG_N 4096
 static HapLog g_hapLog[HAPLOG_N];
 static uint16_t g_hapHead = 0;
@@ -141,7 +136,6 @@ void hapLogAdd(uint8_t slot, uint8_t rid, const uint8_t *b, uint16_t n)
 }
 void hapticDumpLog()
 {
-	// dump the captured history (oldest->newest) over CDC for the haptic-whine hunt
 	const uint16_t N = HAPLOG_N;
 	uint32_t now = millis();
 	Serial.printf("# --- capture history (now=%lu, connSlot=%d) ---\n",
@@ -250,10 +244,8 @@ void haptic82HostReport(const uint8_t *p, uint16_t n)
 
 	// any haptic activity arms a clear when it next goes idle (kills a latch from this use)
 	g_hapClearArmed = true;
-	// Track on/off only. Do NOT synthesize a stop burst when Steam's own stop arrives: that stop is already
-	// forwarded verbatim, so adding 0x82-zero frames on top just makes the controller see the stop several times
-	// over. Each 0x82 is a discrete pad click, so the extra frames are exactly the spurious end-of-movement
-	// "click"/buzz that the real puck never produces. (Connect-time clearing still runs in hapticTask().)
+	// Track on/off only. Do NOT synthesize a stop burst: Steam's own stop is forwarded verbatim, and each extra
+	// 0x82 is a discrete pad click -- the spurious end-of-movement "click"/buzz the real puck never produces.
 	g_haptic82On = haptic82PayloadOn(p, n);
 }
 bool hapticSteamRumble(uint16_t lowFreq, uint16_t highFreq)
@@ -319,18 +311,16 @@ void rfConnFlushRelay(uint8_t ch, uint8_t s1)
 			if (rl > RELAY_MAXP)
 				rl = RELAY_MAXP;
 			// On-air sub-TLV framing. CONFIRMED from real puck<->controller sniffs (puck_sniffer): a command LANDS on
-			// the controller only with the type-01 + inner-len form  E3 [2+rl][01][rid][innerlen][data]; the legacy
-			// form  E3 [1+rl][05][rid][data]  (no inner-len) is what the long-working build shipped, and the controller
-			// DISCARDS any 0x87+ command sent that way (it reads data[0] as the length).
+			// the controller only with the type-01 + inner-len form  E3 [2+rl][01][rid][innerlen][data]; with the legacy
+			// form  E3 [1+rl][05][rid][data]  (no inner-len) the controller DISCARDS any 0x87+ command (it reads data[0]
+			// as the length).
 			//
-			// We WHITELIST the landing form to EXACTLY the two commands that need it -- and nothing else:
+			// WHITELIST the landing form to EXACTLY the two commands that need it; everything else keeps legacy form:
 			//   * LED brightness  -- report 0x87 whose first register byte is 0x2D
 			//   * controller power-off -- report 0x9F ("off!")
-			// Everything else keeps the legacy form. This is the crux: Steam ALSO sends 0x87 passthrough writes during
-			// normal play -- the haptic-config block (reg 0x30 = IMU/subsystem enable, 0x34/0x35 = haptic amplitude). On
-			// the legacy framing the controller discards them, which is WHY the gyro keeps streaming (a landed 0x30
-			// FREEZES the IMU) and the haptics stay quiet (landed 0x34/0x35 = the connect buzz). Landing ALL 0x87 (the
-			// naive "split on >=0x87") regresses BOTH: stuck gyro + buzz. So keep this list tight.
+			// Steam ALSO sends other 0x87 passthrough writes during play -- the haptic-config block (reg 0x30 =
+			// IMU/subsystem enable, 0x34/0x35 = haptic amplitude). Landing those regresses BOTH: a landed 0x30 FREEZES
+			// the IMU (gyro stops) and landed 0x34/0x35 = the connect buzz. So keep this list tight.
 			bool land01 =
 				(m.rid == 0x9F) ||
 				(m.rid == 0x87 && rl >= 1 && m.data[0] == 0x2D);
@@ -357,9 +347,8 @@ void rfConnFlushRelay(uint8_t ch, uint8_t s1)
 			// copied out -> release the slot before the TX
 			g_rqTail = rqNext(g_rqTail);
 			// s1 carries a PID distinct from the GET poll that follows (caller cycles it), so the controller's ESB
-			// dedup never mistakes the GET for a retransmit of this relay. 80us RX window: the relay is NO-ACK, the
-			// controller sends nothing back, so don't burn the full ~1.2ms reply window (that was halving the poll
-			// rate during haptics).
+			// dedup never mistakes the GET for a retransmit of this relay. 80us RX window: the relay is NO-ACK, so
+			// don't burn the full ~1.2ms reply window (that halves the poll rate during haptics).
 			rfConnTx(ch, s1, p, plen, 80);
 
 			// ONE relay per poll cycle (matches the real puck's pacing)
@@ -370,15 +359,13 @@ void rfConnFlushRelay(uint8_t ch, uint8_t s1)
 }
 
 // Haptic-subsystem RE-INIT: the exact sequence Steam sends (captured on hardware) when it (re)takes control,
-// and which clears a stuck haptic script on the controller -- a 0x81 reset action plus 0x87 writes to the
-// haptic registers (30/07/08/31/52, 18/2e/34/35). We replay it to recover from the latched-buzz the
-// controller falls into across a reconnect. Brightness (0x87 reg 2d) is deliberately OMITTED so we don't
-// stomp the LED. Enqueued onto the normal relay (drains in the poll cadence).
+// which clears a stuck haptic script on the controller -- a 0x81 reset action plus 0x87 writes to the haptic
+// registers (30/07/08/31/52, 18/2e/34/35). Replayed to recover from the latched-buzz the controller falls
+// into across a reconnect. Brightness (0x87 reg 2d) is deliberately OMITTED so we don't stomp the LED.
 //
-// NOTE: the 0x87 frames below go out on the LEGACY (type-05) framing -- the relay whitelist only lands 0x87
-// when its first register is brightness 0x2D -- so the controller DISCARDS them, exactly as on the long-working
-// build. They are kept verbatim so this path matches that build byte-for-byte; the effective re-init is the
-// three 0x81 frames, which is what cleared the buzz there. (Landing the 0x30 here would freeze the gyro.)
+// The 0x87 frames below go out on LEGACY (type-05) framing (the whitelist lands 0x87 only for brightness
+// 0x2D), so the controller DISCARDS them -- kept verbatim only to match the captured sequence. The effective
+// re-init is the three 0x81 frames. (Landing the 0x30 here would freeze the gyro.)
 void hapticReinit()
 {
 	static const uint8_t H30[] = { 0x30, 0x00, 0x00, 0x07, 0x07,
@@ -412,10 +399,8 @@ void hapticInit()
 	// boot: block stale Steam 0x82 until link stable
 	g_hapticBlockUntil = millis() + HAPTIC_RECONNECT_BLOCK_MS;
 	// NO fabricated stop burst. USB capture proves Steam only ever sends 0x82 [01 01 f7] pulses -- never a
-	// zero-gain [01 01 00] "stop". Injecting our invented stop frame (which the real puck never sends) at
-	// boot/connect is unfaithful and, given the controller's "haptic script already active -> ignore" behavior,
-	// a likelier cause of the connect-time buzz than a cure (the buzz happened WITH this burst in place). Be a
-	// pure pass-through of Steam's haptics, like the real puck.
+	// zero-gain [01 01 00] "stop". An invented stop frame at boot/connect is a likelier cause of the connect-time
+	// buzz than a cure. Be a pure pass-through of Steam's haptics, like the real puck.
 	g_hapticStop = 0;
 }
 // Arm the post-(re)connect haptic block + schedule the clearing re-init. Called from rf_link the moment a
@@ -430,11 +415,9 @@ void hapticOnReconnect()
 
 	// drop any haptic ON queued before the link came up
 	hapticCancelPendingOn();
-	// Be PROACTIVE: start re-initing the haptic engine right after the link is up and repeat across the whole
-	// settle window, instead of waiting for the block to end. The brief connect buzz engages early (during the
-	// block, controller-internal), so a single late shot misses it -- frequent early resets cut it short or
-	// preempt it. The re-init is settings only (no haptic play), and Steam haptics are blocked the whole time,
-	// so repeating it can't itself buzz.
+	// Re-init the haptic engine repeatedly across the settle window rather than waiting for the block to end: the
+	// brief connect buzz engages early (during the block, controller-internal), so a single late shot misses it.
+	// The re-init is settings only (no haptic play) and Steam haptics are blocked throughout, so it can't buzz.
 	g_reinitAt = millis() + 200u; // first reset ~200ms after (re)connect
 
 	// then every HAPTIC_REINIT_GAP_MS across the window
@@ -448,7 +431,7 @@ void hapticTask()
 {
 	static bool wasHapticLinkUp = false;
 	bool up = hapticLinkUp();
-	// Link-edge markers are diagnostic only now -- the block/re-init is armed reliably from rf_link
+	// Link-edge markers are diagnostic only -- the block/re-init is armed reliably from rf_link
 	// (hapticOnReconnect) on the first reply after a gap, which fires even when this 300ms edge doesn't.
 	if (up && !wasHapticLinkUp) {
 		uint8_t mk = 1;

+ 7 - 10
OpenPuck/haptics.h

@@ -6,17 +6,15 @@
 //
 // The queue is a small ring, NOT a single buffer: producers run in USB-ISR context while the consumer may be
 // mid-flush in loop context, and Steam writes settings/calibration as bursts of back-to-back feature reports.
-// The old single g_relayBuf both LOST every report that arrived before the previous one flushed and could be
-// TORN mid-copy by the ISR (a half-old-half-new command on the air) -- the settings-never-save and the
-// corrupted-haptic-latch bugs. Entries carry up to RELAY_MAXP payload bytes (the RF frame fits 60: [E3][len]
-// [05][rid] + payload <= MAXLEN 64); the old path truncated everything to 18, which chopped any multi-register
-// 0x87 settings block (LED brightness) and calibration write.
+// A single buffer LOSES reports arriving before the previous flush and can be TORN mid-copy by the ISR.
+// Entries carry up to RELAY_MAXP payload bytes (the RF frame fits 60: [E3][len][05][rid] + payload <= MAXLEN
+// 64); a smaller cap chops multi-register 0x87 settings blocks (LED brightness) and calibration writes.
 //
 // The controller's haptic LATCHES until told to stop. If the host's stop is lost over RF (or the link drops
 // mid-buzz) the actuator whines forever -- so we send 0x82-zero stop bursts on reconnect, but ONLY when a
 // haptic was actually active when the link dropped: every extra 0x82 frame is an audible click on the
-// controller (captured), so the old unconditional burst on each link-up edge was itself a buzz source on a
-// flapping link. The g_hapLog ring captures recent OUTPUT reports for the 'H' dump.
+// controller (captured), so an unconditional burst on each link-up edge is itself a buzz source on a flapping
+// link. The g_hapLog ring captures recent OUTPUT reports for the 'H' dump.
 #pragma once
 #include <stdint.h>
 #include "config.h" // OPK_LOG
@@ -25,7 +23,7 @@
 #define HAPTIC_QUIET_MS 300u
 // Block ALL haptic relays for this long after a (re)connect: a just-powered-on controller's haptic engine
 // isn't ready, and feeding it haptics in that window leaves it in a degraded/latched state (slow/stuck/missing
-// haptics until a re-init). 3s gives it time to initialize. (Was 1500.)
+// haptics until a re-init). 3s gives it time to initialize.
 #define HAPTIC_RECONNECT_BLOCK_MS 3000u
 // 0x82-zero relays per stop event (sent at poll cadence -- not loop rate)
 #define HAPTIC_STOP_BURST 4u
@@ -52,8 +50,7 @@ bool relayEnqueue(uint8_t rid, const uint8_t *payload, uint8_t plen);
 bool relayPending();
 extern uint8_t g_relayOp; // relay frame opcode (E3 poll)
 extern uint8_t g_relaySub; // relay sub-TLV type byte = SET
-extern volatile uint8_t
-	g_testHaptic; // 't<n>' injects n test haptics for the buzz hunt
+extern volatile uint8_t g_testHaptic; // 't<n>' injects n test haptics
 // pending haptic-STOP frames to relay (kill a latched whine)
 extern volatile uint8_t g_hapticStop;
 extern unsigned long g_hapticBlockUntil;

+ 1 - 1
OpenPuck/mode_hidgyro.cpp

@@ -102,7 +102,7 @@ void HidGyroController::begin()
 {
 	USBDevice.setID(0x054C, 0x05C4);
 
-	// bumped: clean-DS4 is now a separate mode (DS4_GAME); normal HIDGYRO keeps wake+WebUSB. Host re-reads config by VID:PID:serial (per-mode suffix) -- bump invalidates any cached 0x0103.
+	// Host re-reads config by VID:PID:serial (per-mode suffix); bump invalidates any cached descriptor
 	USBDevice.setDeviceVersion(0x0104);
 	USBDevice.setManufacturerDescriptor("Sony Computer Entertainment");
 	USBDevice.setProductDescriptor("Wireless Controller");

+ 1 - 2
OpenPuck/mode_lizard.cpp

@@ -65,8 +65,7 @@ void rfLizard(const uint8_t *r, Adafruit_USBD_HID *mdev,
 	if (dw < -15)
 		dw = -15; // finger up = wheel up (positive)
 	// --- mouse buttons: left=R-pad-click|R-trigger, right=L-trigger, middle=L-pad-click ---
-	// trigU8 (full-range scale + saturate) so a full pull reaches ~0xFF; a raw >>8 tops out ~0x80 and never
-	// crosses the threshold, leaving the trigger clicks dead.
+	// trigU8 scales full-range so a full pull reaches ~0xFF; a raw >>8 tops out ~0x80, never crossing threshold.
 	uint8_t rtrig = trigU8(u16off(r, 6)), ltrig = trigU8(u16off(r, 4));
 	uint8_t mbtn = 0;
 	if ((b & TB_RPADC) || rtrig > 180)

+ 1 - 2
OpenPuck/mode_lizard.h

@@ -4,8 +4,7 @@
 // closed), driving mouse(0x40)+keyboard(0x41) on the same puck HID slot. Canonical Valve SC1 map: right pad ->
 // mouse (trackball glide), left pad -> scroll wheel + middle-click, R-trigger/R-pad-click -> left mouse,
 // L-trigger -> right mouse; A=Enter B=Esc X=PgUp Y=PgDn, d-pad/left-stick -> arrows, LB=LeftCtrl RB=LeftAlt,
-// View=Tab, Menu=Esc. Mouse reuses the same velocity+friction+sub-pixel GLIDE model as Xbox mode (g_mDiv /
-// g_mFric, both live-tunable).
+// View=Tab, Menu=Esc. Mouse reuses the Xbox-mode velocity+friction+sub-pixel glide model (g_mDiv / g_mFric).
 #pragma once
 #include <Adafruit_TinyUSB.h>
 #include <stdint.h>

+ 1 - 1
OpenPuck/mode_ps5.cpp

@@ -152,7 +152,7 @@ void Ps5Controller::begin()
 {
 	USBDevice.setID(0x054C, 0x0CE6);
 
-	// bumped: clean-PS is now a separate mode (PS5_GAME); normal PS5 keeps wake+WebUSB. Host re-reads config by VID:PID:serial (per-mode suffix) -- bump invalidates any cached 0x0103.
+	// Host re-reads config by VID:PID:serial (per-mode suffix); bump invalidates any cached descriptor
 	USBDevice.setDeviceVersion(0x0104);
 	USBDevice.setManufacturerDescriptor("Sony Interactive Entertainment");
 	USBDevice.setProductDescriptor("DualSense Wireless Controller");

+ 1 - 1
OpenPuck/mode_switch_hori.cpp

@@ -165,7 +165,7 @@ void SwitchHoriController::begin()
 {
 	USBDevice.setID(0x0F0D, 0x0092);
 
-	// bumped from 0x0201 for 1ms poll interval (Windows caches config by VID:PID:bcdDevice)
+	// Windows caches config by VID:PID:bcdDevice, so any interface change MUST bump this
 	USBDevice.setDeviceVersion(0x0202);
 	USBDevice.setManufacturerDescriptor("HORI CO.,LTD.");
 	USBDevice.setProductDescriptor("POKKEN CONTROLLER");

+ 3 - 3
OpenPuck/mode_switch_hori.h

@@ -1,9 +1,9 @@
 // mode_switch_hori.h -- Switch HORIPAD / Pokkén Tournament Pro Pad personality (MODE_SW_HORI).
 //
 // Targets a REAL Switch console: a plain HID gamepad (HORI 0F0D:0092) the console accepts on plug-in with NO
-// handshake / SPI calibration / encryption -- unlike the genuine Pro Controller, whose USB init the console
-// handles fussily. This is the descriptor every DIY Switch controller copies (the LUFA "Splatoon printer"
-// descriptor). Also recognized by SDL/Steam on PC. 8-byte input report streamed at ~250Hz from task().
+// handshake / SPI calibration / encryption -- unlike the genuine Pro Controller. This is the descriptor every
+// DIY Switch controller copies (the LUFA "Splatoon printer" descriptor); also recognized by SDL/Steam on PC.
+// 8-byte input report streamed at ~250Hz from task().
 #pragma once
 #include "controllers.h"
 

+ 34 - 52
OpenPuck/mode_switch_pro.cpp

@@ -13,9 +13,8 @@ using namespace Adafruit_LittleFS_Namespace;
 SwitchProController g_switchPro;
 
 // Switch full-report (0x30) cadence. The console integrates the 3 IMU samples per report assuming ~5 ms/sample
-// (3 samples / 15 ms genuine); too high a rate over-integrates the gyro. With the accel scale fixed (gravity-
-// correction now works) 120 Hz is drift-free and lower-latency, so it is the DEFAULT. 66 Hz is the genuine
-// 2wiCC-matching compat fallback; "full" is the 4 ms PC rate for those who want lowest latency and tolerate it.
+// (3 samples / 15 ms genuine); too high a rate over-integrates the gyro. 120 Hz is drift-free and lower-latency
+// (default); 66 Hz is the genuine compat fallback; "full" is the 4 ms PC rate (lowest latency).
 #define SW_PRO_REPORT_MS 15u // 66 Hz
 #define SW_PRO_REPORT_MS_120 8u // ~120 Hz
 // SC2 accel is +/-2g (16384/g); /4 -> the genuine Pro +/-8g (4096/g) the Switch cal expects
@@ -152,8 +151,8 @@ static uint16_t jcRumbleAmp(const uint8_t r[4])
 {
 	// Nintendo packs a high band (r[0],r[1]) and low band (r[2],r[3]) of frequency+amplitude. We only need a
 	// magnitude for the Steam motor, so pull the two amplitude fields: HF amp = r[1]>>1, LF amp = r[3]&0x3F.
-	// Every canonical idle/neutral frame -- 00 01 40 40, 00 00 01 40, and all-zero -- decodes to 0 this way, so a
-	// steady idle rumble stream maps to "off" instead of latching the motor on. NO forced floor: 0 amplitude -> 0.
+	// Every canonical idle/neutral frame (00 01 40 40, 00 00 01 40, all-zero) decodes to 0 this way, so a steady
+	// idle rumble stream maps to "off" instead of latching the motor on. No forced floor: 0 amplitude -> 0.
 	uint8_t hf = r[1] >> 1; // high-band amplitude (neutral 0x01 -> 0)
 	uint8_t lf = r[3] & 0x3F; // low-band amplitude  (neutral 0x40 -> 0)
 	uint8_t a = hf > lf ? hf : lf; // 0..0x7F
@@ -165,12 +164,11 @@ static void jcRumble(const uint8_t *p, uint16_t pn)
 		return; // [timer][left rumble x4][right rumble x4]
 	uint16_t lo = jcRumbleAmp(p + 1), hi = jcRumbleAmp(p + 5);
 	static uint16_t lastLo = 0, lastHi = 0;
-	// only relay on change: the Switch streams rumble every frame, so
+	// only relay on change: the Switch streams rumble every frame; re-sending
+	// unchanged values would flood the RF relay and loop the motor
 	if (lo == lastLo && hi == lastHi)
 		return;
 	lastLo = lo;
-
-	// re-sending unchanged values would flood the RF relay and loop the motor
 	lastHi = hi;
 	hapticSteamRumble(lo, hi);
 }
@@ -249,7 +247,7 @@ static void jcInputPrefix(uint8_t *out)
 	out[0] = g_jcTimer++;
 
 	// battery full+charging (hi nibble 0x9), connection_info=1 (lo nibble): a wired/charging Pro Controller.
-	// A real Switch reads this to show the pad as connected; Steam/hid-nintendo accept it too. (2wiCC parity)
+	// A real Switch reads this to show the pad as connected; Steam/hid-nintendo accept it too.
 	out[1] = 0x91;
 	out[2] = (uint8_t)(jc);
 	out[3] = (uint8_t)(jc >> 8);
@@ -267,27 +265,21 @@ static void switchProBuild(uint8_t out[63])
 {
 	memset(out, 0, 63);
 	jcInputPrefix(out);
-	// Gyro slot order follows hid-nintendo: raw+6 = ROLL, raw+8 = PITCH, raw+10 = YAW. Steam-validated source
-	// routing (a proper rotation, det +1): SwitchX<-+gy, SwitchY<--gx, SwitchZ<-+gz.
+	// Gyro slot order follows hid-nintendo: raw+6 = ROLL, raw+8 = PITCH, raw+10 = YAW. Source routing (a proper
+	// rotation, det +1): SwitchX<-+gy, SwitchY<--gx, SwitchZ<-+gz.
 	//
-	// CRITICAL: the accel MUST use the SAME signed permutation as the gyro. Steam reads the gyro raw and ignores
-	// this, but the Switch console FUSES accel (gravity) with gyro to anchor absolute orientation -- and if the
-	// accel frame has the opposite handedness, the fusion's gravity-correction term points the wrong way and the
-	// attitude estimate latches into a rotated solution (the intermittent ~45deg axis-mixing, per-unit roll bias,
-	// and "only a replug clears it" behaviour). So accel Y must be -ax to match the gyro's -gx pitch axis; with
-	// that, accel is also det +1 and resting gravity lands cleanly on Switch Z (the up axis).
-	// The SC2 accelerometer is +/-2g full-scale (~16384 counts = 1g; measured |a|=16364 at rest). The factory cal we
-	// present (sensitivity 0x4000) tells the Switch it's a genuine +/-8g Pro Controller (4096 counts = 1g), so divide
-	// by SW_ACCEL_DIV=4 to report ~1g of gravity. Without this the console reads gravity as ~4g, REJECTS the accel for
-	// drift correction (it must be linear accel, not gravity), and the gyro's roll error accumulates into the slow
-	// ~45deg lean. Gyro is left at native scale (aiming sensitivity is right); only the accel magnitude was wrong.
-	// accel X <- +g_in.ay /4   (matches gyro +gy)
+	// The accel MUST use the SAME signed permutation as the gyro. Steam reads the gyro raw and ignores this, but the
+	// Switch console FUSES accel (gravity) with gyro to anchor absolute orientation -- if the accel frame has the
+	// opposite handedness, the gravity-correction term points the wrong way and the attitude estimate latches into a
+	// rotated solution (intermittent ~45deg axis-mixing, per-unit roll bias, cleared only by a replug). So accel Y
+	// must be -ax to match the gyro's -gx pitch axis; resting gravity then lands cleanly on Switch Z (the up axis).
+	// The SC2 accelerometer is +/-2g full-scale (~16384 counts = 1g). The factory cal we present (sensitivity 0x4000)
+	// tells the Switch it's a genuine +/-8g Pro Controller (4096 counts = 1g), so divide by SW_ACCEL_DIV=4 to report
+	// ~1g. Without this the console reads gravity as ~4g, REJECTS the accel for drift correction (it must be linear
+	// accel, not gravity), and gyro roll error accumulates into a slow ~45deg lean. Gyro is left at native scale.
+	// accel X <- +ay, accel Y <- -ax, accel Z <- +az (signs match gyro)
 	int16_t aX = (int16_t)(g_in.ay / SW_ACCEL_DIV);
-
-	// accel Y <- -g_in.ax /4   (matches gyro -gx)
 	int16_t aY = (int16_t)((-(int16_t)g_in.ax) / SW_ACCEL_DIV);
-
-	// accel Z <- +g_in.az /4   (matches gyro +gz)
 	int16_t aZ = (int16_t)(g_in.az / SW_ACCEL_DIV);
 
 	// gyro outputs scaled by the user sensitivity factor
@@ -297,20 +289,17 @@ static void switchProBuild(uint8_t out[63])
 	for (int k = 0; k < 3; k++) {
 		int o = 12 + k * 12;
 		out[o + 0] = aX & 0xFF;
-		out[o + 1] = (aX >> 8) & 0xFF; // accel X (Switch)
+		out[o + 1] = (aX >> 8) & 0xFF;
 		out[o + 2] = aY & 0xFF;
-		out[o + 3] = (aY >> 8) & 0xFF; // accel Y (Switch)
+		out[o + 3] = (aY >> 8) & 0xFF;
 		out[o + 4] = aZ & 0xFF;
-		out[o + 5] = (aZ >> 8) & 0xFF; // accel Z (Switch)
+		out[o + 5] = (aZ >> 8) & 0xFF;
 		out[o + 6] = groll & 0xFF;
-		out[o + 7] = (groll >> 8) &
-			     0xFF; // gyro ROLL  <- +g_in.gy (scaled)
+		out[o + 7] = (groll >> 8) & 0xFF;
 		out[o + 8] = gpitch & 0xFF;
-		out[o + 9] = (gpitch >> 8) &
-			     0xFF; // gyro PITCH <- -g_in.gx (scaled)
+		out[o + 9] = (gpitch >> 8) & 0xFF;
 		out[o + 10] = gyaw & 0xFF;
-		out[o + 11] = (gyaw >> 8) &
-			      0xFF; // gyro YAW   <- +g_in.gz (scaled)
+		out[o + 11] = (gyaw >> 8) & 0xFF;
 	}
 }
 // --- Canonical factory SPI dumps the host reads for calibration. Neutral IMU + centered sticks so a fresh
@@ -355,9 +344,8 @@ static void jcBuildStickCal()
 	jcPack12(&g_spiStickCal[9], Rr);
 }
 // Manual-pairing (subcommand 0x01) reply payloads. A real Switch runs this 3-stage BT key exchange over USB so it
-// can register the pad (Steam/hid-nintendo never do, which is why this was previously unneeded). The data is the
-// canonical hid handshake blob; the Switch only validates the shape, not the key contents. Each is the 31-byte
-// reply body that follows the 0x21 input prefix + ack(0x81) + echoed-subcommand(0x01). (adapted from 2wiCC)
+// can register the pad (Steam/hid-nintendo do not). The Switch only validates the shape, not the key contents. Each
+// is the 31-byte reply body following the 0x21 input prefix + ack(0x81) + echoed-subcommand(0x01).
 // type 1: echo type + controller BT addr + "Pro Controller" name
 static const uint8_t BT_PAIR_1[31] = {
 	0x01, JC_MAC[0], JC_MAC[1], JC_MAC[2], JC_MAC[3], JC_MAC[4], JC_MAC[5],
@@ -376,10 +364,8 @@ static const uint8_t BT_PAIR_3[31] = {
 }; // type 3: save pairing (all zero body)
 // User-calibration SPI mirror (0x8000-0x80FF). A real Pro Controller stores the gyro/accel (and stick) calibration
 // the Switch writes during "Calibrate Motion Controls" here, then reads it back and applies it -- that's how a
-// resting IMU/neutral offset gets cancelled. We previously DISCARDED SPI writes and returned 0xFF (blank), so the
-// Switch's calibration never stuck (it appeared to do nothing and reverted on replug) -- which is why calibrating
-// did not fix the Mario Kart roll/neutral offset. Now we store writes and serve them back, persisted to flash so
-// it survives unplug/replug. 0xFF = blank -> Switch falls back to our factory block at 0x6020 (default until cal'd).
+// resting IMU/neutral offset gets cancelled. We store writes and serve them back, persisted to flash so it survives
+// unplug/replug. 0xFF = blank -> Switch falls back to our factory block at 0x6020 (default until cal'd).
 #define SWCAL_FILE "/swimucal.bin"
 static uint8_t g_userCal[0x100];
 static void loadUserCal()
@@ -531,9 +517,7 @@ static void jcSubcmd(uint8_t sub, const uint8_t *args, uint16_t alen)
 			jcSpiWrite(a, args[4], &args[5],
 				   (alen > 5) ? (uint16_t)(alen - 5) : 0);
 		}
-		p[12] = 0x80;
-
-		// write ACK
+		p[12] = 0x80; // write ACK
 		break;
 
 	// set input report mode (0x30 = standard full) -> begin streaming
@@ -658,12 +642,10 @@ void SwitchProController::task()
 	}
 	if (g_swProReportMode != 0x30)
 		return; // not until the host has finished init + selected 0x30
-	// Stream the standard full report at ~66 Hz (15 ms), NOT the 250 Hz USB_STREAM_MS the PC modes use. The Switch
-	// integrates the report's 3 IMU samples by SAMPLE COUNT at a fixed ~5 ms/sample (it ignores the timer byte and
-	// assumes a 3-samples-per-15ms genuine cadence). At 250 Hz x 3 samples = 750 samples/s it credits ~3.75 s of
-	// gyro rotation per real second, so any residual gyro bias accumulates ~3.75x too fast -> the slow orientation
-	// lean that builds over minutes and resets on replug. 15 ms matches the genuine push (and 2wiCC's ~60 Hz /
-	// MissionControl), making integration 1:1. (Buttons/sticks on the Switch are genuine-rate-limited anyway.)
+	// The Switch integrates the report's 3 IMU samples by SAMPLE COUNT at a fixed ~5 ms/sample (it ignores the timer
+	// byte and assumes a 3-samples-per-15ms genuine cadence). Streaming faster (e.g. 250 Hz x 3 = 750 samples/s)
+	// over-credits gyro rotation ~3.75x, so residual bias accumulates into the slow orientation lean that builds over
+	// minutes and resets on replug. 15 ms matches the genuine push, making integration 1:1.
 	uint32_t interval = (g_swProRate == 2) ? USB_STREAM_MS :
 			    (g_swProRate == 1) ? SW_PRO_REPORT_MS_120 :
 						 SW_PRO_REPORT_MS;

+ 5 - 6
OpenPuck/mode_switch_pro.h

@@ -1,11 +1,10 @@
 // mode_switch_pro.h -- Nintendo Switch Pro Controller personality (MODE_SW_PRO): 057E:2009, report 0x30 + IMU.
 //
-// Unlike the HORIPAD, the genuine Pro Controller requires a USB handshake + subcommand exchange (device info,
-// SPI calibration reads, set-report-mode) before any host (or Steam) will bind it. jcSet() answers that
-// handshake from the USB ISR by enqueuing canonical 0x81/0x21 replies; task() drains them (and, once the host
-// selects report mode 0x30, streams the standard input report with gyro). Linux hid-nintendo hardcodes the
-// 0x30 layout, so we follow it exactly. macOS drops input reports shorter than the descriptor length, so every
-// reply is padded to the full 63 bytes.
+// The genuine Pro Controller requires a USB handshake + subcommand exchange (device info, SPI calibration reads,
+// set-report-mode) before any host (or Steam) will bind it. jcSet() answers that handshake from the USB ISR by
+// enqueuing canonical 0x81/0x21 replies; task() drains them (and, once the host selects report mode 0x30, streams
+// the standard input report with gyro). Linux hid-nintendo hardcodes the 0x30 layout, so we follow it exactly.
+// macOS drops input reports shorter than the descriptor length, so every reply is padded to the full 63 bytes.
 #pragma once
 #include "controllers.h"
 

+ 11 - 14
OpenPuck/mode_xinput.cpp

@@ -7,7 +7,7 @@
 #include <Adafruit_TinyUSB.h>
 #include <Arduino.h>
 #include <string.h>
-// custom class-driver API (usbd_class_driver_t, usbd_edpt_*) for the XInput interface
+// custom class-driver API for the XInput interface
 extern "C" {
 #include "device/usbd_pvt.h"
 }
@@ -34,8 +34,7 @@ enum {
 };
 
 // ===================== XInput custom TinyUSB class driver =====================
-// Custom class driver (registered via usbd_app_driver_get_cb) + an Adafruit_USBD_Interface subclass that emits
-// the interface + 0x21-blob + 2 endpoints. (Verified against Adafruit nRF52 1.7.0 / TinyUSB 0.18.)
+// Custom class driver + an Adafruit_USBD_Interface subclass emitting the interface + 0x21-blob + 2 endpoints.
 #define XINPUT_DESC_LEN \
 	(9 + 17 + 7 + 7) // interface(9)+vendor0x21(17)+IN ep(7)+OUT ep(7) = 40
 static uint8_t g_xiItf = 0xFF, g_xiEpIn = 0, g_xiEpOut = 0;
@@ -46,7 +45,7 @@ static volatile uint16_t g_rumbleLow = 0, g_rumbleHigh = 0;
 static volatile unsigned long g_rumbleMs =
 	0; // millis of last rumble OUT packet (stuck-rumble watchdog)
 
-// release a held rumble if no OUT packet refreshes it for this long (covers a lost stop without cutting normal short rumbles)
+// release a held rumble if no OUT packet refreshes it for this long (covers a lost stop)
 #define RUMBLE_STUCK_MS 2500u
 
 static void xi_init(void)
@@ -245,7 +244,7 @@ static void rfXboxGamepad(const uint8_t *r)
 		b &= ~(uint32_t)TB_QAM;
 		b |= tritonFromCode(g_qamMap);
 	}
-	uint16_t btn = 0; // map Triton buttons -> XInput button word
+	uint16_t btn = 0;
 	if (b & TB_DUP)
 		btn |= XB_DUP;
 	if (b & TB_DDN)
@@ -296,9 +295,8 @@ static void rfXboxGamepad(const uint8_t *r)
 		   (int16_t)s16off(r, 12),
 		   (int16_t)s16off(r, 14)); // R stick X/Y
 }
-// Right pad -> mouse, riding ALONGSIDE the XInput gamepad on a second HID-mouse interface. Same glide model as
-// Lizard's right pad. Purely a USB-side translation -- touches NOTHING about the RF poll or relay. RPad click =
-// left button, LPad click = right.
+// Right pad -> mouse on a second HID-mouse interface alongside the XInput gamepad. Same glide model as Lizard's
+// right pad; RPad click = left button, LPad click = right.
 static void rfXboxMouse(const uint8_t *r)
 {
 	uint32_t b = btnsOf(r);
@@ -360,16 +358,15 @@ void XboxController::begin()
 {
 	g_rumbleLow = g_rumbleHigh = 0;
 
-	// device-level 045E:028E match -> Windows xusb / SDL / Linux xpad all bind it
+	// 045E:028E -> Windows xusb / SDL / Linux xpad all bind it
 	USBDevice.setID(0x045E, 0x028E);
-	// bcdDevice 0x0115 (was 0x0114): Windows caches the config descriptor by VID:PID:bcdDevice, so any change to
-	// this mode's interfaces (here: the wake-mouse interface) MUST bump bcdDevice or Windows serves a stale
-	// descriptor and the change is invisible (no manual Device-Manager cache-clear should ever be needed).
+	// Windows caches the config descriptor by VID:PID:bcdDevice, so any interface change here MUST bump bcdDevice
+	// or Windows serves a stale descriptor.
 	USBDevice.setDeviceVersion(0x0115);
 	USBDevice.setManufacturerDescriptor("Microsoft");
 	USBDevice.setProductDescriptor("Controller");
 	g_xinput.setStringDescriptor("Controller");
-	g_xinput.begin(); // XInput vendor interface (FF/5D/01) -> MI_00
+	g_xinput.begin();
 	g_mouse.setStringDescriptor("OpenPuck Mouse");
 	g_mouse.setBootProtocol(HID_ITF_PROTOCOL_MOUSE);
 	g_mouse.setReportDescriptor(MOUSE_HID_DESC, sizeof MOUSE_HID_DESC);
@@ -382,7 +379,7 @@ void XboxController::onReport45(const uint8_t *rep, bool fresh,
 	(void)fresh;
 	(void)bodyTlen;
 	rfXboxGamepad(rep);
-	rfXboxMouse(rep); // standard gamepad + right-pad mouse (2nd interface)
+	rfXboxMouse(rep);
 }
 // Lost-stop watchdog: Steam/Triton rumble is latched, so force a zero report if the host stops refreshing.
 void XboxController::task()

+ 3 - 4
OpenPuck/mode_xinput.h

@@ -1,10 +1,9 @@
 // mode_xinput.h -- Xbox 360 wired (XInput) personality (MODE_XBOX): XInput gamepad + right-pad mouse.
 //
 // Real Xbox 360 pads are NOT HID: they use a vendor interface (class 0xFF / sub 0x5D / proto 0x01) carrying a
-// 20-byte XInput report. We register a custom TinyUSB class driver for that interface (see mode_xinput.cpp)
-// plus a second boot-mouse interface for the right trackpad. Steam/SDL/Windows xusb bind by VID/PID
-// (045E:028E) + the FF/5D/01 interface. The host's OUT endpoint carries rumble, relayed to the controller as
-// a haptic by task().
+// 20-byte XInput report. A custom TinyUSB class driver serves that interface plus a second boot-mouse interface
+// for the right trackpad. Host binds by VID/PID (045E:028E) + the FF/5D/01 interface. The OUT endpoint carries
+// rumble, relayed to the controller as a haptic by task().
 #pragma once
 #include "controllers.h"
 #include <stdint.h>

+ 61 - 76
OpenPuck/puck_hid.cpp

@@ -13,8 +13,8 @@
 uint8_t g_fwdNewOnly = 1;
 SteamPuckController g_steamPuck;
 
-// ---- 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. ----
+// 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[] = {
 	0x05, 0x01, 0x09, 0x02, 0xA1, 0x01, 0x85, 0x40, 0x09, 0x01, 0xA1, 0x00,
 	0x05, 0x09, 0x19, 0x01, 0x29, 0x02, 0x15, 0x00, 0x25, 0x01, 0x75, 0x01,
@@ -97,11 +97,10 @@ static Adafruit_USBD_HID hid[NSLOT];
 
 // millis of last Steam OUTPUT/settings write; 0 at boot => lizard until Steam appears
 static unsigned long g_steamAliveMs = 0;
-// Fall back to lizard this long after Steam's ~3s settings heartbeat stops. Keep >2x the cadence (7s): the
-// haptic relay is gated by !lizardActive(), so a shorter window lets a single jittered/delayed heartbeat flip
-// lizard mid-session while Steam is still running -> a haptic that arrives in that window gets gated out
-// (dropped/missing haptic). 7s tolerates one missed beat. (This matches main; do not shorten without a lizard
-// trigger that doesn't depend on the heartbeat timeout.)
+// Fall back to lizard this long after Steam's ~3s settings heartbeat stops. Keep >2x the cadence: the haptic
+// relay is gated by !lizardActive(), so a shorter window lets a jittered/delayed heartbeat flip lizard
+// mid-session while Steam is still running -> a haptic arriving in that window gets gated out (dropped). 7s
+// tolerates one missed beat. Do not shorten without a lizard trigger independent of the heartbeat timeout.
 #define LIZARD_WD_MS 7000u
 static bool g_autoLizard =
 	true; // master switch; false => Steam mode always forwards 0x45
@@ -123,8 +122,7 @@ static inline void hostStampAlive()
 }
 // Right after the host resumes from suspend, MUTE input forwarding briefly. Otherwise the controller's input
 // in that instant (a trackpad click/trigger, or residual button state from the wake gesture) gets forwarded
-// as a real click/keypress into the just-woken desktop -- which was activating the highlighted Start tile
-// (Edge) and closing the Start menu on every wake. Set when task() sees the suspended->active transition.
+// 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
 
@@ -133,45 +131,41 @@ static unsigned long g_resumeMs = 0;
 static void handleSet(int slot, uint8_t rid, hid_report_type_t type,
 		      uint8_t const *b, uint16_t n)
 {
-	// Steam OUTPUT reports 0x80-0x89. The haptic/actuator reports (0x80-0x86)
-	// are relayed to the controller, and ONLY when they arrive on the CONNECTED slot's interface. We have one
-	// controller but expose 4 puck slots, and a report aimed at a DIFFERENT slot made the controller buzz at
-	// random -> that is what the slot gate below fixes. (This used to also clamp to 0x82-ONLY, which silently
-	// dropped the ping / grip / test haptics: those ride other report IDs such as 0x85/0x86, so they never
-	// reached the controller.) The 63-byte settings/config reports 0x87/0x88/0x89 are NOT haptics and are not
-	// pushed here -- 0x87 (lizard-off/settings) reaches the controller through the feature-0x01 passthrough path.
+	// Steam OUTPUT reports 0x80-0x89. The haptic/actuator reports (0x80-0x86) are relayed to the controller,
+	// and ONLY when they arrive on the CONNECTED slot's interface: we have one controller but expose 4 puck
+	// slots, and a report aimed at a DIFFERENT slot made the controller buzz at random (the slot gate below
+	// fixes that). Must NOT clamp to 0x82-only: ping/grip/test haptics ride other report IDs (0x85/0x86). The
+	// 63-byte settings/config reports 0x87/0x88/0x89 are NOT haptics and reach the controller via the
+	// feature-0x01 passthrough path instead.
 	if (type == HID_REPORT_TYPE_OUTPUT) {
 		if (rid >= 0x80 && rid <= 0x89) {
 			// capture ALL OUTPUT reports (even un-relayed) for the 'H' dump
 			hapLogAdd((uint8_t)slot, rid, b, n);
 
-			// ANY Steam OUTPUT report (not just the 0x87 heartbeat) means Steam is present and
-			// driving -> leave lizard for gamepad NOW, so a haptic that arrives before the
-			// first 0x87 doesn't get relayed while we're still presenting lizard (-> buzz loop).
+			// ANY Steam OUTPUT report (not just the 0x87 heartbeat) means Steam is present and driving ->
+			// leave lizard for gamepad NOW, so a haptic arriving before the first 0x87 isn't relayed while
+			// we're still presenting lizard (-> buzz loop).
 			hostStampAlive();
 		}
 		// Post-resume mute also gates haptics: while onReport45 is muted Steam reads NO 0x45 back, which is the
-		// exact condition under which Steam loops the same haptic command (-> connect/wake buzz loop). Same logic
-		// as the lizard gate; the mute is short (1.5s), so a dropped wake chime is the cheap end of the trade.
+		// exact condition under which Steam loops the same haptic command (-> connect/wake buzz loop).
 		bool muted = g_resumeMs &&
 			     millis() - g_resumeMs < POST_RESUME_MUTE_MS;
 		if (rid >= 0x80 && rid <= 0x86 && n >= 1 &&
-		    hapticRelaySlotOk(slot) && !lizardActive() &&
-		    !muted) { // wrap as a SET sub-TLV like the report-01 path
+		    hapticRelaySlotOk(slot) && !lizardActive() && !muted) {
 			if (!haptic82Blocked()) {
 				relayEnqueue(rid, b,
 					     (uint8_t)(n > RELAY_MAXP ?
 							       RELAY_MAXP :
 							       n));
-				// Track on/off from what was actually RELAYED (= the controller's believed state), not from every host
-				// report: a BLOCKED stop must leave "on" set (the controller may be latched -> reconnect stop-burst),
-				// and a blocked ON must not set it (nothing reached the controller -> no spurious burst clicks).
+				// Track on/off from what was actually RELAYED (= the controller's believed state): a
+				// BLOCKED stop must leave "on" set (controller may be latched -> reconnect stop-burst),
+				// a blocked ON must not set it (nothing reached the controller -> no spurious clicks).
 				if (rid == 0x82)
 					haptic82HostReport(b, n);
 			}
 		}
 
-		// log so we can see what Steam actually sends (e.g. glide haptics)
 		if (Serial.availableForWrite() > 80) {
 			Serial.printf("# OUT if%d rid=%02X n=%u:", slot, rid,
 				      n);
@@ -199,24 +193,21 @@ static void handleSet(int slot, uint8_t rid, hid_report_type_t type,
 
 	// report 0x01 = raw passthrough -> queue for RF relay to the controller
 	if (rid == 1 && n >= 2) {
-		// capture EVERY relayed feature-1 command for the WebUSB capture view
-		// (haptics, LED SET_LED_COLOR, 0x87 settings, 0x9F power-off). The log
-		// shows cmd as "rid"; bytes start [cmd][len]...  Was gated to 0x80-0x89,
-		// which hid exactly the LED/power-off frames we now need to see.
+		// capture EVERY relayed feature-1 command for the WebUSB capture view (haptics, LED SET_LED_COLOR,
+		// 0x87 settings, 0x9F power-off). Log shows cmd as "rid"; bytes start [cmd][len]...
 		hapLogAdd((uint8_t)slot, cmd, b, n);
 		bool haptic82 = (cmd == 0x82 && len <= pln);
 
-		// see the OUTPUT path: no haptics while Steam can't read 0x45 back
 		bool muted = g_resumeMs &&
 			     millis() - g_resumeMs < POST_RESUME_MUTE_MS;
 
-		// never push haptics to the controller while presenting lizard (Steam isn't reading 0x45 -> would buzz-loop)
+		// never push haptics while presenting lizard (Steam isn't reading 0x45 -> would buzz-loop)
 		bool relayOk = hapticRelaySlotOk(slot) &&
 			       !(haptic82 && (lizardActive() || muted));
 		if (relayOk && (!haptic82 || !haptic82Blocked())) {
-			// Relay the DECLARED length (up to the 60B RF frame ceiling), not a truncation: Steam's multi-register
-			// 0x87 settings blocks (LED brightness) and calibration writes are longer than the old 18B cap, and the
-			// chopped frames were why those settings never landed on the controller.
+			// Relay the DECLARED length (up to the 60B RF frame ceiling), not a truncation: Steam's
+			// multi-register 0x87 settings blocks (LED brightness) and calibration writes exceed the old
+			// 18B cap, and the chopped frames were why those settings never landed on the controller.
 			uint8_t rl = (len <= pln) ? len : (uint8_t)pln;
 			if (len > RELAY_MAXP && Serial.availableForWrite() > 60)
 				Serial.printf(
@@ -229,8 +220,7 @@ static void handleSet(int slot, uint8_t rid, hid_report_type_t type,
 				haptic82HostReport(pl, len);
 		}
 	}
-	if (Serial.availableForWrite() >
-	    80) { // log host feature writes (non-blocking)
+	if (Serial.availableForWrite() > 80) {
 		Serial.printf("# SET if%d rid=%02X cmd=%02X len=%u:", slot, rid,
 			      cmd, len);
 		for (uint16_t i = 0; i < n && i < 14; i++)
@@ -359,10 +349,10 @@ void SteamPuckController::begin()
 {
 	USBDevice.setID(0x28DE, 0x1304);
 	// Distinct bcdDevice so Windows keys a FRESH usbflags entry (cache is VID:PID:bcdDevice) and actually runs
-	// MS OS 2.0 / WinUSB binding for the WebUSB vendor interface -- instead of reusing a stale "no WinUSB" entry
-	// tied to the real Steam Controller (28DE:1304), which has no WebUSB interface. The normal (wake-mouse) and
-	// one-shot debug (CDC) boots present DIFFERENT interface sets, so they use DIFFERENT bcdDevice values --
-	// otherwise Windows would serve one's cached descriptor for the other when you reboot between them.
+	// MS OS 2.0 / WinUSB binding for the WebUSB vendor interface, instead of reusing a stale "no WinUSB" entry
+	// tied to the real Steam Controller (28DE:1304, no WebUSB interface). The normal (wake-mouse) and one-shot
+	// debug (CDC) boots present DIFFERENT interface sets, so they need DIFFERENT bcdDevice values or Windows
+	// serves one's cached descriptor for the other across a reboot.
 	USBDevice.setDeviceVersion(
 		g_debugCdcThisBoot ?
 			0x0212 :
@@ -385,19 +375,18 @@ void SteamPuckController::begin()
 	}
 }
 
-// Forward the controller's report 0x45 to Steam, or drive lizard kb/mouse when Steam is closed. This is a
-// PURELY USB-SIDE decision -- it changes nothing about the RF poll or the host->controller relay.
+// Forward the controller's report 0x45 to Steam, or drive lizard kb/mouse when Steam is closed. PURELY a
+// USB-side decision -- changes nothing about the RF poll or the host->controller relay.
 void SteamPuckController::onReport45(const uint8_t *rep, bool fresh,
 				     uint8_t bodyTlen)
 {
-	// Host asleep -> forward NOTHING. While suspended, every sendReport attempt (a lizard-mouse report from a
-	// trackpad graze, a 0x45 forward) can translate into a host wake -- which made the PC wake on any controller
-	// movement and nearly impossible to keep asleep. Waking is an explicit gesture only (Steam-button short press
-	// / controller connect), handled in rf_link.cpp via the device-level USB resume signal.
+	// Host asleep -> forward NOTHING. While suspended, every sendReport attempt can translate into a host wake,
+	// making the PC wake on any controller movement. Waking is an explicit gesture only (Steam-button short
+	// press / controller connect), handled in rf_link.cpp via the device-level USB resume signal.
 	if (USBDevice.suspended())
 		return;
-	// Just woke? Hold off forwarding for a beat so the wake gesture's residual controller input doesn't click /
-	// type into the freshly-woken desktop (it was launching Edge from the Start menu on every wake).
+	// Just woke? Hold off forwarding so the wake gesture's residual controller input doesn't click/type into
+	// the freshly-woken desktop.
 	if (g_resumeMs && millis() - g_resumeMs < POST_RESUME_MUTE_MS)
 		return;
 	if (g_connSlot < 0 || g_connSlot >= NSLOT)
@@ -408,9 +397,9 @@ void SteamPuckController::onReport45(const uint8_t *rep, bool fresh,
 		uint8_t blen = bodyTlen - 1;
 		if (blen > 45)
 			blen = 45; // body after the 0x45 id byte
-		// forward the puck's raw pad coords untouched -- Steam does its own interpolation/smoothing. Forward only
-		// FRESH reports (the real puck dedupes -> Steam gets a clean unique stream; sending stale repeats makes
-		// Steam's velocity/smoothing stair-step). g_fwdNewOnly toggles for A/B.
+		// forward the puck's raw pad coords untouched (Steam does its own interpolation/smoothing). Forward
+		// only FRESH reports: the real puck dedupes, so stale repeats make Steam's velocity/smoothing
+		// stair-step. g_fwdNewOnly toggles for A/B.
 		if ((fresh || !g_fwdNewOnly) && g_slot[g_connSlot].used &&
 		    hid[g_connSlot].ready())
 			hid[g_connSlot].sendReport(
@@ -420,9 +409,8 @@ void SteamPuckController::onReport45(const uint8_t *rep, bool fresh,
 }
 
 // Forward the controller's NON-input status reports (0x43 power/battery, 0x44) to Steam verbatim -- the real
-// puck does this and it's how Steam reads battery. OpenPuck used to drop everything but 0x45, so Steam never got
-// the power report (-> default/unknown battery). Same host-asleep / post-resume gating as 0x45; no lizard path
-// (status reports aren't input, so they forward regardless of the lizard decision).
+// puck does this and it's how Steam reads battery. Same host-asleep / post-resume gating as 0x45; no lizard
+// path (status reports aren't input, so they forward regardless of the lizard decision).
 void SteamPuckController::onAuxReport(uint8_t rid, const uint8_t *data,
 				      uint8_t n)
 {
@@ -436,13 +424,11 @@ void SteamPuckController::onAuxReport(uint8_t rid, const uint8_t *data,
 		hid[g_connSlot].sendReport(rid, data, n);
 }
 
-// ---- wake nudge: a bare USB resume signal is NOT enough to wake some hosts (Windows in particular) -- they
-// only wake when actual mouse/keyboard input follows. So on a deliberate wake gesture we play a HARMLESS mouse
-// JIGGLE (move a few px right, then back -- NET ZERO cursor, NO button): real mouse activity wakes the host,
-// but it clicks nothing and doesn't close/activate anything (an open Start menu stays open). Earlier versions
-// also sent a click / space / Ctrl; those either activated the focused item or weren't needed. Queued by
-// wakeEvent() (rf_link, on a Steam short press / controller connect while suspended); reports can't cross a
-// suspended bus, so it's delivered once the bus has resumed.
+// wake nudge: a bare USB resume signal is NOT enough to wake some hosts (Windows in particular) -- they only
+// wake when actual mouse/keyboard input follows. So on a deliberate wake gesture we play a HARMLESS mouse
+// JIGGLE (move a few px right, then back -- NET ZERO cursor, NO button): real mouse activity wakes the host
+// but clicks/activates nothing (an open Start menu stays open). Queued by wakeEvent() (rf_link, on a Steam
+// short press / controller connect while suspended); delivered once the suspended bus has resumed.
 static uint8_t g_nudgeStep = 0; // 0=idle; 1=jiggle+, 2=jiggle-
 static unsigned long g_nudgeMs = 0;
 #define NUDGE_JIGGLE_PX 10
@@ -502,10 +488,10 @@ void SteamPuckController::task()
 	bool conn = (g_connSlot >= 0 && millis() - g_connReplyMs < 300);
 	if (g_connSlot >= 0 && g_connSlot < NSLOT && hid[g_connSlot].ready()) {
 		// 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; the old
-		// unconditional forever-resend could re-trigger Steam's connect handling (connect chime) every 750ms
-		// before Steam started consuming 0x45 -> the connect-time loop of haptic buzzes that only stopped on the
-		// first real input (the Steam button). Resending until acked still covers "Steam missed the edge".
+		// 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
+		// Steam consumes 0x45 -> a loop of connect-time haptic buzzes. Resending until acked still covers
+		// "Steam missed the edge".
 		bool steamAcked = g_steamAliveMs &&
 				  (int32_t)(g_steamAliveMs - connEdgeMs) >= 0;
 		if (conn != usbConn ||
@@ -517,16 +503,15 @@ void SteamPuckController::task()
 			usbConn = conn;
 			last79 = millis();
 		} else if (conn && millis() - last7B >= 2000) {
-			// 0x7B status, live-captured template. Byte 8 is the controller->puck signal strength as signed dBm
-			// (capture showed 0xDD = -35; replaying it verbatim is why Steam pinned -35dBm forever) -- patch in the
-			// smoothed RSSI the radio now samples on each controller reply (rf_link). 0 = no sample yet -> keep the
-			// capture value rather than report garbage.
+			// 0x7B status, live-captured template. Byte 8 is the controller->puck signal strength as signed
+			// dBm (capture showed 0xDD = -35) -- patch in the smoothed RSSI the radio samples on each
+			// controller reply (rf_link). 0 = no sample yet -> keep the capture value rather than garbage.
 			//
-			// CALIBRATION: our raw RSSISAMPLE reads ~RSSI_DBM_OFFSET dB lower than the real Valve puck at the same
-			// distance (the Pro Micro's PCB-trace antenna vs Valve's tuned front-end). The 2Mbit ESB link has ~55dB
-			// of margin, so a -75dBm reading still works across a house -- but Steam's bar maps raw dBm to "weak"
-			// long before that. Adding the offset lines our close-range value up with the puck's captured -35 so the
-			// bar tracks usable range instead of antenna gain. Clamp keeps it in a sane window (no wrap/garbage).
+			// CALIBRATION: our raw RSSISAMPLE reads ~RSSI_DBM_OFFSET dB lower than the real Valve puck at the
+			// same distance (Pro Micro PCB-trace antenna vs Valve's tuned front-end). The 2Mbit ESB link has
+			// ~55dB margin, so a -75dBm reading still works across a house, but Steam's bar maps raw dBm to
+			// "weak" long before that. The offset lines our close-range value up with the puck's captured -35
+			// so the bar tracks usable range, not antenna gain. Clamp keeps it in a sane window.
 			uint8_t s7b[12] = { 0xF7, 0x01, 0x89, 0x00, 0x00, 0x00,
 					    0x03, 0x00, 0xDD, 0x00, 0x3A, 0x02 };
 			if (g_linkRssi) {

+ 2 - 3
OpenPuck/puck_hid.h

@@ -14,8 +14,8 @@
 #include "controllers.h"
 #include <stdint.h>
 
-// Steam mode: forward report 0x45 only when the seq advanced (dedupe like the real puck). Sending stale
-// repeats makes Steam's trackpad smoothing stair-step. Toggle to A/B (WebUSB field 14).
+// Forward report 0x45 only when the seq advanced (dedupe like the real puck); sending stale
+// repeats makes Steam's trackpad smoothing stair-step. (WebUSB field 14)
 extern uint8_t g_fwdNewOnly;
 
 class SteamPuckController : public IController {
@@ -27,7 +27,6 @@ class SteamPuckController : public IController {
 		override; // forward controller 0x43/0x44 status to Steam
 	void task() override;
 
-	// queue the harmless post-resume wake nudge (mouse jiggle + Ctrl tap)
 	void wakeEvent() override;
 	bool isPuck() const override
 	{

+ 15 - 20
OpenPuck/radio.cpp

@@ -1,25 +1,24 @@
 #include "radio.h"
 
-// Pairing rendezvous bytes from IBEX rodata. The 0x91A2A793 key that precedes the "ibex" string is a
-// DIFFERENT key, NOT the discovery address (confirmed via PTR_s_ibexesb_common=0x56f99 -> "ibex...").
+// Pairing rendezvous bytes from IBEX rodata. The 0x91A2A793 key preceding "ibex" is a DIFFERENT key,
+// NOT the discovery address.
 const uint8_t PAIR_BASE[4] = { 0x69, 0x62, 0x65, 0x78 }; // "ibex"
 uint8_t g_rfPrefix = 0x10;
 uint8_t g_rfCh = 2;
 uint8_t g_rfBase[4] = { 0x69, 0x62, 0x65, 0x78 }; // "ibex"
-// ch18 measured clean (RSSI 95/90, PER 0) on the REAL PUCK's own channel scan and is in its active hop set
-// {18,2,80}; trackpad data bursts collide on bad channels -> reply-rate crash. Tunable 'C'.
+// ch18: clean channel in the real puck's active hop set {18,2,80}; bad channels collide with trackpad
+// data bursts -> reply-rate crash. Tunable 'C'.
 uint8_t g_sessCh = 18;
 
-// pre-init to "ibex"/0x10 as a safe default; rfGenSessionAddr() overwrites with the per-device value at boot.
+// Safe default; rfGenSessionAddr() overwrites with the per-device value at boot.
 uint8_t g_sessBase[4] = { 0x69, 0x62, 0x65, 0x78 };
 uint8_t g_sessPrefix = 0x10;
 
 void rfGenSessionAddr()
 {
-	// Mix the 64-bit FICR DEVICEID into a 4-byte base + a prefix. Deterministic per chip (stable across reboots,
-	// debuggable) and unique across chips (DEVICEID is unique) -> two OpenPucks never collide on the connected
-	// session. Avoid degenerate address bytes (0x00/0xFF) for clean preamble correlation, and never reproduce
-	// the shared discovery base "ibex" (else the isolation is lost).
+	// Mix the 64-bit FICR DEVICEID into a 4-byte base + prefix: deterministic per chip, unique across chips
+	// -> two OpenPucks never collide on the connected session. Avoid degenerate address bytes (0x00/0xFF) for
+	// clean preamble correlation, and never reproduce the shared discovery base "ibex" (else isolation is lost).
 	uint32_t h = NRF_FICR->DEVICEID[0] * 0x9E3779B1u ^
 		     NRF_FICR->DEVICEID[1];
 	uint32_t h2 = NRF_FICR->DEVICEID[1] * 0x85EBCA6Bu ^
@@ -36,25 +35,24 @@ void rfGenSessionAddr()
 	g_sessPrefix = p;
 	if (g_sessBase[0] == g_rfBase[0] && g_sessBase[1] == g_rfBase[1] &&
 	    g_sessBase[2] == g_rfBase[2] && g_sessBase[3] == g_rfBase[3])
-		g_sessBase[0] ^= 0x80; // collided with "ibex"
+		g_sessBase[0] ^= 0x80; // collided with discovery base "ibex"
 }
 
 uint8_t rfrx[100], rftx[100];
 uint32_t g_rfRxCount = 0;
 
-// HYBRID puck-link config (firmware-derived, definitive). DECODED from real-puck capture: puck TX = Ble_2Mbit,
-// ENDIAN=Big (MSB-first), NO whitening, addr "ibex", CRC16 0x11021/0xFFFF; frame = FUN_00027e9a (static length).
+// Puck-link config decoded from real-puck capture: puck TX = Ble_2Mbit, ENDIAN=Big (MSB-first), NO whitening,
+// addr "ibex", CRC16 0x11021/0xFFFF; static-length frame.
 uint32_t g_crcinit = 0xFFFF;
 uint8_t g_whiteiv = 37;
-// <-- was the bug (1Mbit). Real puck = 2Mbit.
 uint8_t g_mode = RADIO_MODE_MODE_Ble_2Mbit;
 
 // S0LEN0, LFLEN8, S1LEN3 (ESB DPL) - CRC-VALIDATED
 uint32_t g_pcnf0 = 0x00030008UL;
 uint8_t g_statlen = 0x20;
 
-// ENDIAN=Big, BALEN4, MAXLEN=96 (was 64 -> truncated the controller's
-// 66-byte 0x43-battery-augmented F1 -> CRC-fail -> battery never arrived)
+// ENDIAN=Big, BALEN4, MAXLEN=96. MAXLEN must be >=66 or the controller's 0x43-battery-augmented F1 (66B)
+// truncates -> CRC-fail -> battery never arrives.
 uint32_t g_pcnf1 = 0x01040060;
 uint32_t g_crcpoly = 0x11021UL;
 uint16_t g_crccnf = 0x2; // CRC16, address included
@@ -89,19 +87,16 @@ void rfConfig(uint8_t ch)
 	NRF_RADIO->TASKS_DISABLE = 1;
 	RWAIT_DISABLED();
 	NRF_RADIO->EVENTS_DISABLED = 0;
-	NRF_RADIO->MODE = ((uint32_t)g_mode
-			   << RADIO_MODE_MODE_Pos); // Ble_2Mbit (tunable 'M')
+	NRF_RADIO->MODE = ((uint32_t)g_mode << RADIO_MODE_MODE_Pos);
 	NRF_RADIO->FREQUENCY = ch;
 	NRF_RADIO->TXPOWER =
 		(RADIO_TXPOWER_TXPOWER_0dBm << RADIO_TXPOWER_TXPOWER_Pos);
 #if defined(RADIO_MODECNF0_RU_Fast)
 	NRF_RADIO->MODECNF0 = (RADIO_MODECNF0_RU_Fast << RADIO_MODECNF0_RU_Pos);
 #endif
-	// static length (tunable '0'); g_pcnf1=0 -> build from g_statlen/g_balen
 	NRF_RADIO->PCNF0 = g_pcnf0;
 
-	// ENDIAN=Big (puck transmits MSB-first)
-	// WHITEEN=0
+	// g_pcnf1=0 -> build static-length config from g_statlen/g_balen (ENDIAN=Big, WHITEEN=0)
 	NRF_RADIO->PCNF1 =
 		g_pcnf1 ? g_pcnf1 :
 			  ((1u << RADIO_PCNF1_ENDIAN_Pos) |

+ 10 - 11
OpenPuck/radio.h

@@ -1,9 +1,9 @@
 // radio.h -- bare-metal nRF52 RADIO (ESB-style) hardware layer.
 //
-// We drive the RADIO peripheral directly, with NO SoftDevice (BLE is never started), coexisting with TinyUSB.
-// This file owns the register-level config: PHY/CRC/whitening/address parameters (all live-tunable from the
-// CDC console for hardware iteration) plus rfConfig()/rfSetAddr() which program the radio, and the rfrx/rftx
-// DMA buffers. The puck *protocol* built on top lives in rf_link.cpp; the RE/calibration tooling in rf_diag.cpp.
+// Drives the RADIO peripheral directly with NO SoftDevice (BLE is never started), coexisting with TinyUSB.
+// Owns the register-level config (PHY/CRC/whitening/address, all live-tunable from the CDC console),
+// rfConfig()/rfSetAddr() to program the radio, and the rfrx/rftx DMA buffers. The puck protocol on top lives
+// in rf_link.cpp; the RE/calibration tooling in rf_diag.cpp.
 //
 // Register-confirmed config (decoded from real-puck capture): PHY=Ble_2Mbit, ENDIAN=Big (MSB-first), whitening
 // off, addr "ibex", CRC16 poly 0x11021 init 0xFFFF (address included). On-air address = bitrev8 of stored bytes.
@@ -11,9 +11,8 @@
 #include <stdint.h>
 #include <Arduino.h>
 
-// Bounded radio-disable wait: NEVER spin forever. A wedged RADIO peripheral would otherwise hang the whole
-// main loop (and USB stops being serviced -> device "dies" until replug). On timeout we bail and continue;
-// the next rfConfig re-inits the radio.
+// Bounded radio-disable wait: NEVER spin forever. A wedged RADIO would hang the main loop (USB stops being
+// serviced -> device "dies" until replug). On timeout we bail; the next rfConfig re-inits the radio.
 #define RWAIT_DISABLED()                                   \
 	do {                                               \
 		uint32_t _w = micros();                    \
@@ -31,10 +30,10 @@ extern uint8_t g_rfBase[4]; // "ibex"
 // connected-session channel: a CLEAN data channel off the congested adv ch2
 extern uint8_t g_sessCh;
 
-// Per-device SESSION address (base+prefix). Discovery/rendezvous stays on the SHARED "ibex" address (g_rfBase)
-// so any controller can find us; the host frame then advertises THIS unique address and the controller adopts
-// it for the connected session. Two OpenPucks therefore never share an on-air session address -> no crosstalk,
-// no spurious cross-wake. Derived from the FICR DEVICEID (stable per chip, unique across chips).
+// Per-device SESSION address (base+prefix). Discovery stays on the SHARED "ibex" address (g_rfBase) so any
+// controller can find us; the host frame advertises THIS unique address and the controller adopts it for the
+// session. Two OpenPucks therefore never share an on-air session address -> no crosstalk, no cross-wake.
+// Derived from the FICR DEVICEID (stable per chip, unique across chips).
 extern uint8_t g_sessBase[4];
 extern uint8_t g_sessPrefix;
 void rfGenSessionAddr();

+ 13 - 14
OpenPuck/rf_diag.cpp

@@ -30,8 +30,8 @@ static uint8_t g_schan[3] = { 78, 2, 80 };
 static uint8_t g_schi = 0;
 
 // ===================== promiscuous raw capture (calibration only) =====================
-// preamble-match on 0x55 bytes, fixed-length grab, no CRC. Catches any 2Mbit packet on a channel so we can
-// read the controller's reconnect addr/prefix/framing.
+// Preamble-match on 0x55, fixed-length grab, no CRC. Catches any 2Mbit packet on a channel so we can read the
+// controller's reconnect addr/prefix/framing.
 #define RAWCAP 48
 void rfRawStart(uint8_t ch)
 {
@@ -65,8 +65,8 @@ void rfRawStart(uint8_t ch)
 }
 
 // ===================== CRC-validating framing sweep =====================
-// Data looked bit-reversed (not whitened). Firmware-correct: ESB DPL (S0L0, LFLEN6or8, S1LEN3), BALEN4
-// (5-byte addr), CRC8/16 addr-included. Sweep PCNF0{LFLEN6,8} x CRC{8,16} x BALEN{4,3} x ENDIAN{Big,Lit}.
+// ESB DPL (S0L0, LFLEN6or8, S1LEN3), BALEN4 (5-byte addr), CRC8/16 addr-included.
+// Sweep PCNF0{LFLEN6,8} x CRC{8,16} x BALEN{4,3} x ENDIAN{Big,Lit}.
 static const uint32_t CAPP0B[] = {
 	0x00030006, 0x00030008
 }; // S1LEN3 + LFLEN6 / LFLEN8 (dynamic length)
@@ -109,9 +109,9 @@ void rfCapStart(uint8_t ch)
 	NRF_RADIO->EVENTS_END = 0;
 	NRF_RADIO->TASKS_RXEN = 1;
 }
-// REPLAY: capture one real-puck frame (raw, ENDIAN=Little) then re-transmit it VERBATIM (same bits, addr, ch)
-// to impersonate the puck bit-for-bit -- sidesteps unknown framing/CRC. Controller must be bonded to the REAL
-// puck (replayed frame carries real-puck uuids + its valid CRC).
+// REPLAY: capture one real-puck frame (raw, ENDIAN=Little) then re-transmit it VERBATIM to impersonate the puck
+// bit-for-bit -- sidesteps unknown framing/CRC. Controller must be bonded to the REAL puck (replayed frame
+// carries real-puck uuids + its valid CRC).
 static void rfCapPoll()
 {
 	if (!g_rfCap)
@@ -157,7 +157,7 @@ void rfReplayOnce()
 #if defined(RADIO_MODECNF0_RU_Fast)
 	NRF_RADIO->MODECNF0 = (RADIO_MODECNF0_RU_Fast << RADIO_MODECNF0_RU_Pos);
 #endif
-	NRF_RADIO->FREQUENCY = 2; // ch2
+	NRF_RADIO->FREQUENCY = 2;
 	NRF_RADIO->PCNF0 = 0; // static, ENDIAN=Little -> reproduce on-air bits
 	NRF_RADIO->PCNF1 = (3u << RADIO_PCNF1_BALEN_Pos) |
 			   ((uint32_t)g_replayLen << RADIO_PCNF1_STATLEN_Pos) |
@@ -202,7 +202,7 @@ static void rfPoll()
 	if (g_rfRaw) {
 		if (NRF_RADIO->EVENTS_END) {
 			NRF_RADIO->EVENTS_END = 0;
-			// filter noise: require some non-0x55/0xAA/0x00 structure
+			// filter noise: require non-preamble/idle byte structure
 			int nz = 0;
 			for (int i = 0; i < RAWCAP; i++) {
 				uint8_t b = rfrx[i];
@@ -240,8 +240,8 @@ static void rfPoll()
 }
 
 // ===================== autosweep candidate radio configs =====================
-// Cycle candidate radio configs while beaconing, so ONE controller search window covers the space. S0LEN=1 /
-// CRC16 0x11021 / addr "ibex" held fixed; sweep the residual unknowns (PHY, whitening, BALEN, prefix).
+// Cycle configs while beaconing so ONE controller search window covers the space. S0LEN=1 / CRC16 0x11021 /
+// addr "ibex" held fixed; sweep the residual unknowns (PHY, whitening, BALEN, prefix).
 struct RfCfg {
 	uint8_t mode;
 	uint32_t pcnf0, pcnf1;
@@ -289,7 +289,7 @@ void rfBeaconOnce()
 	memset(pl, 0, sizeof pl);
 	pl[0] = 0x01;
 	pl[1] = g_seq++;
-	pl[5] = 0xE2; // dongle beacon (FUN_00027ed8 format)
+	pl[5] = 0xE2; // dongle-beacon marker
 	rfConfig(g_rfCh);
 	rfSetAddr(g_rfBase, g_rfPrefix);
 	if (g_s1incl)
@@ -333,7 +333,7 @@ void rfBeaconOnce()
 }
 
 // ===================== scan-then-respond (dongle role) =====================
-// Listen on ch2 for the controller's advertisement, and the instant one arrives, TX the host frame (its ack).
+// Listen on ch2 for the controller's advertisement; the instant one arrives, TX the host frame (its ack).
 static void buildHostTx()
 {
 	int slot = -1;
@@ -399,7 +399,6 @@ static void rfRespondPoll()
 			for (uint8_t i = 0; i <= len && i < 32; i++)
 				Serial.printf("%02X", rfrx[i]);
 			Serial.println();
-			// re-arm RX
 			NRF_RADIO->PACKETPTR = (uint32_t)rfrx;
 			NRF_RADIO->SHORTS = RADIO_SHORTS_READY_START_Msk |
 					    RADIO_SHORTS_END_START_Msk;

+ 4 - 5
OpenPuck/rf_diag.h

@@ -1,10 +1,9 @@
 // rf_diag.h -- RF reverse-engineering & calibration scaffolding (NOT used in normal operation).
 //
-// The puck protocol is solved (see radio.cpp / rf_link.cpp), but the tooling that solved it is preserved here:
-// promiscuous raw capture, CRC-validating config sweeps, bit-exact frame replay, address/CRC listen, the
-// scan-then-respond dongle experiment, the dongle beacon, and the live-session sniffer. Each is driven by a
-// flag toggled from the CDC console (serial_console.cpp) and serviced by rfDiagTask() from loop(). Isolating
-// it keeps rf_link.cpp readable while keeping the hard-won RE history compilable and runnable.
+// Tooling that solved the puck protocol, preserved here: promiscuous raw capture, CRC-validating config sweeps,
+// bit-exact frame replay, address/CRC listen, the scan-then-respond dongle experiment, the dongle beacon, and
+// the live-session sniffer. Each is driven by a flag toggled from the CDC console (serial_console.cpp) and
+// serviced by rfDiagTask() from loop().
 #pragma once
 #include <stdint.h>
 

+ 14 - 19
OpenPuck/rf_link.cpp

@@ -13,12 +13,12 @@
 bool g_rfHost = true;
 bool g_connOn = true;
 uint8_t g_connType = 0xE7; // start with protocol-version handshake, then 0xE3
-// 0=current(slow/awake), 1=test protocol-version-1. 'V<n>' to toggle.
+// 0=current(slow/awake), 1=protocol-version-1. 'V<n>' to toggle.
 uint8_t g_e7b = 0;
 uint8_t g_connLen = 0x08;
-// GET report 0x45 param byte (try 0x00, fall back 0x2D) - 'q' cmd
+// GET report 0x45 param byte. 'q' cmd.
 uint8_t g_getParam = 0x00;
-// DEFAULT 1: cycling the ESB PID drains the controller's report queue (~400 new/s vs ~60 with a fixed PID) -- THE rate fix. 'e<n>' to A/B.
+// cycling the ESB PID drains the controller's report queue (~400 new/s vs ~60 with a fixed PID). 'e<n>' selects.
 uint8_t g_e3mode = 1;
 bool g_connVerbose = false;
 // poll RX-window (us); shorter=more polls/s but may miss DELAYED replies. Tunable 'r'.
@@ -42,7 +42,7 @@ uint16_t g_f1ps = 0;
 uint16_t g_newps = 0;
 // polls/s (GET+relay TXs) last second -- distinguishes loop-starvation from reply-loss
 uint16_t g_pollsps = 0;
-// MEASURED avg us between GET-poll fires (vs intended g_pollUs) -- ground truth
+// measured avg us between GET-poll fires (vs intended g_pollUs)
 uint16_t g_pollPeriodUs = 0;
 static uint32_t g_pollDtSum = 0;
 static uint16_t g_pollDtCnt = 0;
@@ -90,8 +90,8 @@ static void rfHostFrameOnce(int slot, bool discovery)
 	memcpy(rftx + 3, rec + 0, 4);
 	memcpy(rftx + 7, rec + 4, 4); // payload[5..9] ibex_uuid
 
-	// payload[9] session channel: tell the controller to run the session on
-	// the clean channel (it adopts buf[0xe]); discovery beacon still TXes on ch2
+	// payload[9] session channel: controller runs the session on this clean
+	// channel (adopts buf[0xe]); discovery beacon still TXes on ch2
 	rftx[11] = g_sessCh;
 	// payload[13..17] session base  (the per-device UNIQUE address)
 	memcpy(rftx + 15, g_sessBase, 4);
@@ -122,7 +122,7 @@ static void rfHostFrameOnce(int slot, bool discovery)
 	while (!NRF_RADIO->EVENTS_END && (micros() - t0) < bwin) {
 	}
 	if (NRF_RADIO->EVENTS_END) {
-		// ANY reception = controller answered our frame
+		// any reception = controller answered our frame
 		NRF_RADIO->EVENTS_END = 0;
 		g_rfRxCount++;
 		bool crcok = NRF_RADIO->CRCSTATUS & 1;
@@ -209,7 +209,6 @@ uint8_t rfConnTx(uint8_t ch, uint8_t s1, const uint8_t *payload, uint8_t plen,
 			// beacons (0xE1) + polls (0xE2/E3/E7) -- all E-type. Without this gate, puck A receives a SECOND puck's
 			// 0xE1 beacon (e.g. one just plugged into another computer), bumps g_connReplyMs, and the "new RF
 			// connection" wake in rfLinkTask() fires -> the second puck spuriously wakes this sleeping host.
-			// F-type reply (F1/F2/F3) -> our controller is alive
 			if (rtype >= 0xF0) {
 				// A reply after a long gap (or the first ever) = a (re)connect. Arm the haptic block + re-init HERE,
 				// directly off the reply stream -- reliable even when hapticTask's 300ms link-up edge doesn't fire
@@ -257,14 +256,12 @@ uint8_t rfConnTx(uint8_t ch, uint8_t s1, const uint8_t *payload, uint8_t plen,
 			bool isF1 = (rtype == 0xF1);
 			if (isF1) {
 				g_connF1++;
-				// walk ALL type6 TLVs (= HID report 0x45). idx is INT, not
+				// walk ALL type6 TLVs (= HID report 0x45); taking only [0] halves the rate. idx is INT,
+				// not uint8_t: tlen 0xFE would make idx+=tlen+2 wrap mod-256 -> infinite loop -> USB hang.
 				int idx = 3, end = rxlen + 2;
 				const uint8_t *lastRep = nullptr;
-				// uint8_t: a tlen of 0xFE would make idx+=tlen+2 wrap
 				uint8_t lastTlen = 0;
-				// mod-256 back to itself -> infinite loop -> USB hang/"crash".
 				while (idx + 1 < end) {
-					// pace with the real puck -- taking only [0] halved our rate.
 					uint8_t tlen = rfrx[idx],
 						ttype = rfrx[idx + 1];
 					if (tlen == 0)
@@ -433,8 +430,8 @@ uint8_t rfConnTx(uint8_t ch, uint8_t s1, const uint8_t *payload, uint8_t plen,
 						   (rfrx[idx + 2] == 0x43 ||
 						    rfrx[idx + 2] == 0x44)) {
 						// Controller STATUS reports (0x43 = periodic power/battery, ~every 2s; 0x44 = status event). The real
-						// puck forwards these to the host verbatim -- that's how Steam reads battery -- but OpenPuck used to
-						// drop everything but 0x45. Forward them (onAuxReport), and snapshot the battery % for the WebUSB panel.
+						// puck forwards these verbatim (onAuxReport) -- that's how Steam reads battery; also snapshot the
+						// battery % for the WebUSB panel.
 						// [rid][body...]
 						const uint8_t *rep =
 							&rfrx[idx + 2];
@@ -547,11 +544,9 @@ static void rfConnStep()
 		// CONTROLLED CADENCE: poll ~every g_pollUs
 		if ((uint32_t)(micros() - g_lastPollUs) < g_pollUs)
 			return;
-		// FIXED-RATE schedule: advance the deadline by exactly one interval. The old `g_lastPollUs = micros()`
-		// reset the reference to AFTER the gate passed, so each cycle's ~1ms of poll work + loop jitter was added
-		// to the next interval and never reclaimed -- the period drifted to ~5000us (200/s) instead of 4000us
-		// (250/s). Advancing by g_pollUs keeps the long-run average at exactly the target; the resync guard
-		// prevents a catch-up burst if we ever fall more than a full interval behind (a real stall).
+		// FIXED-RATE schedule: advance the deadline by exactly one interval (NOT reset-to-now, which would add
+		// each cycle's ~1ms of poll work + loop jitter to the next interval -- the period drifts to ~5000us
+		// instead of the 4000us target). The resync guard prevents a catch-up burst after a real stall.
 		{
 			uint32_t now = micros();
 			static uint32_t lastFire = 0;

+ 1 - 1
OpenPuck/rf_link.h

@@ -19,7 +19,7 @@ extern bool g_rfHost;
 extern bool g_connOn; // auto-start connected-mode poll on boot
 extern uint8_t g_connType; // start packet type (E7 handshake, then E3)
 
-// E7 payload B-byte: 0=current(slow/awake), 1=test protocol-version-1
+// E7 payload B-byte: 0=current(slow/awake), 1=protocol-version-1
 extern uint8_t g_e7b;
 extern uint8_t g_connLen;
 extern uint8_t g_getParam; // GET report 0x45 param byte

+ 4 - 5
OpenPuck/serial_console.cpp

@@ -11,8 +11,8 @@
 #include <string.h>
 
 // CDC commands: l=listen, s=stop, cN=channel, p<hex>=prefix, a<8hex>=base addr, b=bonds, etc.
-// NOTE: command order matters -- this is an else-if chain, so the FIRST matching letter wins. A couple of
-// letters ('C', 'H') appear twice; the second occurrence is unreachable (kept verbatim from the original).
+// Command order matters: else-if chain, FIRST matching letter wins. 'C' and 'H' appear twice; the second
+// occurrence is unreachable.
 void serialConsolePoll()
 {
 	static char line[24];
@@ -21,9 +21,8 @@ void serialConsolePoll()
 		char c = Serial.read();
 		if (c == '\n' || c == '\r') {
 			line[li] = 0;
-			// FULL factory wipe -- requires the exact word "ERASE-ALL" (not a single letter) so it can't be fat-
-			// fingered. Reformats the internal FS (cfg.bin + bonds.bin gone), then reboots into clean defaults; the
-			// controller must be re-paired afterwards.
+			// FULL factory wipe -- requires the exact word "ERASE-ALL" so it can't be fat-fingered. Reformats the
+			// internal FS (cfg.bin + bonds.bin gone), reboots into clean defaults; controller must be re-paired.
 			if (!strcmp(line, "ERASE-ALL")) {
 				Serial.println(
 					"# ERASING ALL persistent storage (config + bonds)...");

+ 7 - 9
OpenPuck/status_led.h

@@ -1,16 +1,14 @@
 // status_led.h -- LED indication of wake activity.
 //
 // The LED is DARK in all steady states -- including while wake is armed (host suspended) -- and flashes for
-// half a second at the moment a wake is actually sent (USBDevice.remoteWakeup() fired by a Steam-button short
-// press, a controller connect, or the boot-time wake). This turns the LED into a wake debugger you can read
-// from the couch: flash + PC stays asleep = the resume signal was sent and the HOST ignored it (fix on the
-// host: powercfg /deviceenablewake); no flash = the firmware never fired (it didn't see the gesture, or it
-// didn't consider the bus suspended).
+// half a second when a wake is actually sent (USBDevice.remoteWakeup()). It's a wake debugger: flash + PC
+// stays asleep = resume signal was sent and the HOST ignored it (fix host-side: powercfg /deviceenablewake);
+// no flash = firmware never fired (didn't see the gesture, or didn't consider the bus suspended).
 //
-// Board note: the sketch is built with the Feather nRF52840 variant, but the usual hardware is a SuperMini
-// "Pro Micro" clone. The Feather's user LED is P1.15 (D3, active high); the SuperMini's blue user LED is
-// P0.15 (= D24 in the Feather pin map -- SPI MISO, unused by this project). We drive BOTH pins so the
-// indicator works on either board. Override the pins/polarity below if your board differs.
+// Board note: built with the Feather nRF52840 variant, but the usual hardware is a SuperMini "Pro Micro"
+// clone. The Feather's user LED is P1.15 (D3, active high); the SuperMini's blue user LED is P0.15 (= D24 in
+// the Feather pin map -- SPI MISO, unused here). We drive BOTH pins so the indicator works on either board.
+// Override the pins/polarity below if your board differs.
 #pragma once
 
 #ifndef WAKE_LED_PIN_A

+ 4 - 7
OpenPuck/triton.h

@@ -1,8 +1,8 @@
 // triton.h -- the controller's native input as it comes off the RF link, and the decoders for it.
 //
 // "Triton" is the Steam Controller 2 controller. Its HID input report 0x45 is what the puck relays; rf_link.cpp
-// decodes one into the shared g_in struct each fresh frame, and every USB personality (the IController
-// implementations in mode_*.cpp) reads g_in to build its own host report. report 0x45 layout:
+// decodes one into the shared g_in struct each fresh frame, and every USB personality reads g_in to build its
+// own host report. report 0x45 layout:
 //   [0]=0x45 [1]=seq [2..5]=buttons u32; analog offsets below are from the buttons low byte (rep[2]).
 #pragma once
 #include <stdint.h>
@@ -55,9 +55,8 @@ static inline int u16off(const uint8_t *r, int off)
 {
 	return r[2 + off] | (r[2 + off + 1] << 8);
 }
-// Controller trigger analog: the u16 in report 0x45 tops out near half-scale (~0x8000) at a full pull, so a
-// straight >>8 reads only ~0x80 (host sees a half-pressed trigger). Scale x2 (>>7) and saturate so a full
-// pull maps to the full 0xFF.
+// Controller trigger u16 tops out near half-scale (~0x8000) at a full pull, so a straight >>8 reads only ~0x80
+// (host sees a half-pressed trigger). Scale x2 (>>7) and saturate so a full pull maps to 0xFF.
 static inline uint8_t trigU8(int u16v)
 {
 	int v = u16v >> 7;
@@ -73,8 +72,6 @@ void imuFrom45(const uint8_t *r, int16_t *ax, int16_t *ay, int16_t *az,
 	       int16_t *gx, int16_t *gy, int16_t *gz);
 
 // ---- shared decoded input (filled by rf_link.cpp once per fresh report 0x45, read by every mode) ----
-// This single struct replaces the per-mode duplicates the firmware used to carry (the IMU/stick/button state
-// was copied verbatim into g_swPro*, g_ps5*, and g_gyro*). One source, many consumers.
 struct PuckInput {
 	// raw Triton buttons (TB_*); per-mode builders apply their own chord masking
 	uint32_t buttons;

+ 3 - 4
OpenPuck/wake_hid.cpp

@@ -1,10 +1,9 @@
 #include "wake_hid.h"
 #include <Adafruit_TinyUSB.h>
 
-// A bog-standard boot MOUSE descriptor -- the same shape Xbox mode's right-pad mouse uses, which is proven to
-// enumerate and wake Windows. We never send a report on it; it exists purely so the host enumerates a
-// "HID-compliant mouse" child and grants the device wake-from-sleep privileges. (A boot keyboard was tried
-// first and was worse: it didn't enumerate on Windows and suppressed the wake that the mouse class provides.)
+// Boot MOUSE descriptor -- proven to enumerate and wake Windows. We never send a report on it; it exists so the
+// host enumerates a "HID-compliant mouse" child and grants the device wake-from-sleep privileges. (A boot
+// keyboard didn't enumerate on Windows and suppressed the wake the mouse class provides.)
 static const uint8_t WAKE_HID_DESC[] = { TUD_HID_REPORT_DESC_MOUSE() };
 static Adafruit_USBD_HID g_wakeHid;
 

+ 9 - 11
OpenPuck/wake_hid.h

@@ -1,19 +1,17 @@
 // wake_hid.h -- a minimal boot-protocol HID interface whose ONLY job is to make the device a host-recognized
 // USB wake source.
 //
-// Every mode already advertises remote-wakeup in its config descriptor (the Adafruit core sets
-// TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP), so the device is *armed* to wake the host. But hosts only *honor* a
-// remote-wakeup resume signal from an allow-listed input device class -- a HID keyboard/mouse. Windows
-// (especially under Modern Standby) ignores the wake from a bare gamepad / vendor / composite presentation
-// even though it's armed. Exposing a boot MOUSE interface (the same shape Xbox mode already uses to wake
-// Windows) puts the device in that allow-list. It never sends reports -- the actual wake is the device-level
-// USBDevice.remoteWakeup() resume signal driven from rf_link.cpp; this interface only changes how the host
-// classifies us. (A boot keyboard was tried and was worse -- it didn't enumerate on Windows.)
+// Every mode already advertises remote-wakeup in its config descriptor, so the device is *armed* to wake the
+// host. But hosts only *honor* a resume signal from an allow-listed input device class -- a HID keyboard/mouse.
+// Windows (especially Modern Standby) ignores the wake from a bare gamepad / vendor / composite presentation
+// even though it's armed. Exposing a boot MOUSE interface puts the device in that allow-list. It never sends
+// reports -- the actual wake is the device-level USBDevice.remoteWakeup() resume signal driven from
+// rf_link.cpp; this interface only changes how the host classifies us. (A boot keyboard didn't enumerate on
+// Windows.)
 //
 // Added for every clean mode AND for puck mode on a normal boot: puck mode drops its CDC serial console by
-// default (freeing the endpoint this interface needs) so it too can wake Windows. The exception is the one-shot
-// debug boot, where CDC is kept and this interface is skipped (no endpoint room for both) -- see config.h
-// (g_debugCdcThisBoot) and ARCHITECTURE.md.
+// default (freeing the endpoint this interface needs) so it too can wake Windows. The one-shot debug boot keeps
+// CDC and skips this interface (no endpoint room for both) -- see config.h (g_debugCdcThisBoot).
 #pragma once
 
 // register the boot-keyboard wake interface (call from setup() for clean modes)

+ 13 - 16
OpenPuck/webusb_config.cpp

@@ -60,20 +60,18 @@ static void webusbSendBlob()
 	// poll TX rate (vs delivered/new -> starvation vs reply-loss)
 	p[29] = (uint8_t)(g_pollsps >> 8);
 	p[30] = (uint8_t)g_loopPeriodUs;
-	p[31] = (uint8_t)(g_loopPeriodUs >>
-			  8); // loop diag: avg iteration time + slowest section
+	p[31] = (uint8_t)(g_loopPeriodUs >> 8); // avg loop iteration time
 	p[32] = g_loopWorst;
 	p[33] = (uint8_t)g_loopWorstUs;
 	p[34] = (uint8_t)(g_loopWorstUs >> 8);
 	p[35] = (uint8_t)g_pollPeriodUs;
 	p[36] = (uint8_t)(g_pollPeriodUs >>
-			  8); // measured actual poll period (vs intended 4000)
+			  8); // measured poll period (intended 4000)
 	p[37] = OPK_LOG; // logging build? panel shows/hides its log UI
 	p[38] = g_battery; // controller battery % (report 0x43); 0=unknown
 	p[39] = g_linkRssi; // RAW signal strength |dBm| (0=no sample)
-	// Build provenance: which git commit this firmware was built from, and whether the tree was dirty. Lets the
-	// panel confirm exactly what's flashed. Injected at build time (build_info.h / gen_version.sh); "unknown" if
-	// the version header wasn't generated.
+	// git commit this firmware was built from + dirty flag; injected at build time
+	// (build_info.h / gen_version.sh); "unknown" if the version header wasn't generated.
 	p[40] = OPK_GIT_DIRTY ? 1 : 0;
 	memset(&p[41], 0, 12); // 12B ASCII git hash, NUL-padded
 	{
@@ -96,8 +94,7 @@ static void webusbSendBlob()
 	usb_web.flush();
 }
 #if OPK_LOG
-// Stream the capture ring (haptics / relayed host commands) to the panel as 0xA6 frames, so the browser can
-// pull it with no CDC. Frame formats:
+// Stream the capture ring (haptics / relayed host commands) to the panel as 0xA6 frames. Frame formats:
 //   entry: [0xA6][L][T=1][age u32 LE][slot][rid][n][bytes n]   (L = 8 + n)
 //   end:   [0xA6][1][T=0]
 static void webusbCapFrame(uint32_t ms, uint8_t slot, uint8_t rid, uint8_t nb,
@@ -127,7 +124,7 @@ static void webusbCapEnd()
 	usb_web.write(e, 3);
 	usb_web.flush();
 }
-// 0x06: drain -- entries since the last drain (or since the 0x05 start/rewind). The panel polls this on a
+// 0x06: drain entries since the last drain (or since the 0x05 start/rewind). The panel polls this on a
 // timer and accumulates, so a dump-from-boot of the whole ring streams over many polls without blocking the
 // loop. A per-call budget bounds how long one poll spends here.
 static void webusbDrainCapture()
@@ -182,19 +179,19 @@ void webusbPoll()
 			} // drain entries since the rewind (panel polls this)
 #endif
 
-			// clear a stuck/latched haptic buzz on the controller (functional)
+			// clear a stuck/latched haptic buzz on the controller
 			else if (op == 0x07) {
 				hapticReinit();
 			}
 
-			// TEST: trigger the controller power-off (same path Steam 0x9F / host-suspend use)
+			// trigger controller power-off (same path Steam 0x9F / host-suspend use)
 			else if (op == 0x08) {
 				hapticSendShutdown();
 			}
 
-			// FULL factory wipe: erase cfg.bin + bonds.bin, reboot to clean defaults.
-			// Guarded by a 3-byte magic ("ERS") so a stray/corrupt byte can never trigger it; the panel double-
-			// confirms before sending. Irreversible -- the controller must be re-paired afterwards.
+			// factory wipe: erase cfg.bin + bonds.bin, reboot to clean defaults.
+			// Guarded by a 3-byte magic ("ERS") so a stray/corrupt byte can never trigger it.
+			// Irreversible -- the controller must be re-paired afterwards.
 			else if (op == 0x0A) {
 				if (buf[1] == 0x45 && buf[2] == 0x52 &&
 				    buf[3] == 0x53) {
@@ -232,12 +229,12 @@ void webusbPoll()
 					g_e7b = v ? 1 : 0;
 					break;
 
-				// haptic-relay opcode (buzz hunt)
+				// haptic-relay opcode
 				case 11:
 					g_relayOp = v;
 					break;
 
-				// haptic-relay sub-type (buzz hunt)
+				// haptic-relay sub-type
 				case 12:
 					g_relaySub = v;
 					break;

+ 3 - 5
OpenPuck/webusb_config.h

@@ -1,12 +1,10 @@
-// webusb_config.h -- WebUSB configuration channel.
-//
-// A dedicated vendor interface present in EVERY mode so the browser panel (the GitHub-Pages config UI) can
-// read/set the runtime tunables. Binary framed protocol:
+// WebUSB configuration channel: a vendor interface present in EVERY mode so the browser config panel can
+// read/set runtime tunables. Binary framed protocol:
 //   host->dev:  0x01                 GET  -> reply status blob
 //               0x02 <field> <value> SET one byte field (see the switch in webusb_config.cpp)
 //               0x03 <mode>          switch USB mode (0..6): persist + reboot
 //   dev->host:  0xA5 <len> <payload>  status blob (mode/tunables/link state/rates)
-// No setLandingPage() on purpose -- that would pop a Chrome "open <url>?" notification on every plug-in.
+// No setLandingPage() on purpose -- it would pop a Chrome "open <url>?" notification on every plug-in.
 #pragma once
 #include <Adafruit_TinyUSB.h>