refactor stable but polling rate still low

Makefile

@@ -26,7 +26,9 @@ FORMAT_FILES := $(shell find OpenPuck ReversePuckFirmware puck_sniffer pairtui \
 # or add your own defines:     make build EXTRA_FLAGS="-DOPK_LOG=1"
 FQBN ?= adafruit:nrf52:feather52840
 CFG_TUD_HID ?= 4
-CFG_TUD_TASK_QUEUE_SZ ?= 64
+# Deep so a loop()-context sendReport never blocks on a full usbd event queue (the watchdog path). Sends run
+# from loop() now (usbTxPump) for stable RF timing, so this depth is what keeps that block from happening.
+CFG_TUD_TASK_QUEUE_SZ ?= 512
 EXTRA_FLAGS ?=
 # {build.flags.usb} is expanded by arduino-cli (VID/PID/strings); pass it through verbatim.
 USB_EXTRA_FLAGS = -DNRF52840_XXAA {build.flags.usb} -DCFG_TUD_HID=$(CFG_TUD_HID) -DCFG_TUD_TASK_QUEUE_SZ=$(CFG_TUD_TASK_QUEUE_SZ) $(EXTRA_FLAGS)

OpenPuck/OpenPuck.ino

@@ -311,6 +311,7 @@ void loop()
 	ledTask();
 	acc[6] += (uint32_t)(micros() - t);
 	usbMountTask(); // dynamic mount/unmount of connected controllers (no-op unless enabled)
+	usbTxPump(); // drain queued device->host reports HERE, in loop -- never off-loop (jitters the RF poll)
 	loops++;
 	if (millis() - secMs >= 1000) {
 		g_loopPeriodUs = loops ? (uint16_t)(1000000UL / loops) : 0;
@@ -338,5 +339,6 @@ void loop()
 	hapticTask();
 	ledTask();
 	usbMountTask(); // dynamic mount/unmount of connected controllers (no-op unless enabled)
+	usbTxPump(); // drain queued device->host reports HERE, in loop -- never off-loop (jitters the RF poll)
 #endif
 }

OpenPuck/usb_tx.cpp

@@ -6,8 +6,8 @@
 // One outbound ring per active HID destination. Only one USB mode is live at a time, and a mode presents at
 // most CFG_TUD_HID HID interfaces plus the wake mouse, so a handful of channels covers every case. Channels are
 // allocated lazily on first send to a given Adafruit_USBD_HID* and never freed (the object set is fixed for the
-// session). The producer side runs under PRIMASK (loop AND the RF-decode path enqueue); the consumer is the
-// single usbd task (tud_sof_cb), so head/tail need no further locking beyond the enqueue critical section.
+// session). The producer side runs under PRIMASK (loop enqueue); the consumer (usbTxDrain) runs from the
+// usbtx task AND tud_hid_report_complete_cb, serialized by a guard flag inside usbTxDrain.
 #define TX_CHAN_MAX 8
 #define TX_RING_N \
 	4 // depth per destination; 250 Hz produced, ~1 kHz SOF drain -> stays shallow
@@ -73,12 +73,18 @@ void usbTxHid(Adafruit_USBD_HID *hid, uint8_t rid, const void *data,
 	__set_PRIMASK(pm);
 }
 
-// usbd task only (tud_sof_cb). Send at most one ready report per destination per SOF: after a send the
-// endpoint stays busy until the host polls it (~1 ms), and SOF fires every ~1 ms, so this naturally paces each
-// stream to the host's poll rate without ever blocking. A destination whose endpoint is busy is simply left
-// for the next frame; it does not hold up the others.
+// Send at most one ready report per destination per call. After a send the endpoint stays busy until the host
+// polls it, so the next report for that destination goes out from tud_hid_report_complete_cb (the instant the
+// host reads it) -- that chaining is what paces each stream to the host's poll rate. The usbtx task calls this
+// too, to kick a stream that's idle (no completion pending) and to keep non-busy destinations moving. A busy
+// destination is simply skipped; it does not hold up the others.
 void usbTxDrain(void)
 {
+	// Re-entrancy guard (defensive): usbTxPump only calls this from loop(), but sendReport below can yield,
+	// so guard against a nested loop()-context call ever racing a channel's tail. The loser just skips.
+	static volatile unsigned char draining;
+	if (__atomic_test_and_set(&draining, __ATOMIC_ACQUIRE))
+		return;
 	for (int i = 0; i < TX_CHAN_MAX; i++) {
 		TxChan *c = &g_chan[i];
 		if (!c->hid)
@@ -87,17 +93,17 @@ void usbTxDrain(void)
 		if (t == c->head)
 			continue; // empty
 		if (!c->hid->ready())
-			continue; // endpoint busy / not mounted -> retry next SOF
+			continue; // endpoint busy / not mounted -> retry on completion/next tick
 		TxItem &it = c->ring[t];
 		// sendReport copies into the endpoint buffer, so releasing the slot right after is safe.
 		if (c->hid->sendReport(it.rid, it.data, it.len))
 			c->tail = txNext(t);
 	}
+	__atomic_clear(&draining, __ATOMIC_RELEASE);
 }
 
-// Per-frame drain callbacks for non-HID senders (XInput endpoint, WebUSB blob). Registered at setup(),
-// invoked from tud_sof_cb on the usbd task. Fixed, tiny capacity; registration is setup-only (single-threaded)
-// so no locking is needed and the SOF reader sees a stable list.
+// Per-slot drain callbacks for non-HID senders (XInput endpoint, WebUSB blob). Registered at setup(), invoked
+// from usbTxPump() (loop context). Fixed, tiny capacity; registration is setup-only (single-threaded).
 #define TX_DRAIN_MAX 4
 static usbTxDrainFn g_drainFns[TX_DRAIN_MAX];
 static uint8_t g_nDrain;
@@ -108,26 +114,22 @@ void usbTxRegisterDrain(usbTxDrainFn fn)
 		g_drainFns[g_nDrain++] = fn;
 }
 
-// Drain trigger: a DEDICATED FreeRTOS task -- NOT tud_sof_cb. SOF proved unusable here: TinyUSB gates the SOF
-// callback on `sof_consumer`, which configuration_reset() wipes on every bus reset (tu_varclr(&_usbd_dev)), so
-// after the host's first reset tud_sof_cb silently stops firing and NO device->host data flows (0x45 input,
-// the 0x79 connect report, streamed reports, the WebUSB blob -- all dead, while RF keeps working). A plain task
-// is ungated and always runs. Crucially it is a SEPARATE task from loop(): if a sendReport ever blocks on a
-// full usbd event queue, this task just blocks/yields -- loop() keeps running and feeding the ~8 s watchdog,
-// which is the whole reason we moved sends off loop() in the first place.
-static void usbTxTask(void *arg)
+// Drain everything from loop() -- NOT from a SOF callback, a dedicated task, or report_complete. All three of
+// those were tried and all fight the RF poll, which is a microsecond-precise busy-wait running in loop(): SOF
+// got silently disabled by bus resets; the high-priority usbd task (report_complete) preempted the RF RX
+// windows -> missed replies + usbd self-blocking on a full event queue -> controllers dropped; a separate
+// low-priority task got starved under USB load -> latency. Draining in loop() is what stability-fix did and is
+// the only context that does NOT jitter the RF timing: the send and the poll run sequentially in one task, so
+// no send ever interrupts an RX window. The cost -- loop() can block if sendReport hits a full usbd event
+// queue -- is handled by a deep CFG_TUD_TASK_QUEUE_SZ (so it never fills) rather than by moving sends off loop.
+// Call once per loop() iteration.
+void usbTxPump(void)
 {
-	(void)arg;
-	for (;;) {
-		usbTxDrain();
-		for (uint8_t i = 0; i < g_nDrain; i++)
-			g_drainFns[i]();
-		vTaskDelay(1); // ~1 ms, matching the USB frame cadence
-	}
+	usbTxDrain();
+	for (uint8_t i = 0; i < g_nDrain; i++)
+		g_drainFns[i]();
 }
 void usbTxBegin(void)
 {
-	// 512 words (2 KB): covers the deepest drain path (webusbSendBlob's 115 B frame + the tud_* call chain).
-	// TASK_PRIO_LOW = same as loop(): cooperative, won't preempt the RF poll timing; vTaskDelay yields each ms.
-	xTaskCreate(usbTxTask, "usbtx", 512, NULL, TASK_PRIO_LOW, NULL);
+	// Nothing to arm: usbTxPump() is driven directly from loop(). Kept for call-site stability.
 }

OpenPuck/usb_tx.h

@@ -25,10 +25,15 @@ class Adafruit_USBD_HID;
 void usbTxHid(Adafruit_USBD_HID *hid, uint8_t rid, const void *data,
 	      uint16_t len);
 
-// Arm the SOF-driven drain. Call once, after USBDevice.attach(), from setup(). Idempotent.
+// Kept for call-site stability; no longer does anything (the drain runs from loop() via usbTxPump). Call once.
 void usbTxBegin(void);
 
-// Drain pending reports (one ready report per destination per call). usbd-task only -- invoked by tud_sof_cb.
+// Drain all pending sends (HID rings + registered non-HID drains). MUST be called from loop() once per
+// iteration -- sends run in loop() context, sequentially with the RF poll, so they never interrupt its
+// timing-critical RX windows (which is what off-loop drain contexts did, dropping controllers).
+void usbTxPump(void);
+
+// Drain pending HID reports (one ready report per destination per call). Internal -- called by usbTxPump().
 void usbTxDrain(void);
 
 // Register a callback to run every USB frame ON THE usbd TASK (from tud_sof_cb, right after usbTxDrain()).