diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c3de97..70e87c0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -86,8 +86,8 @@ pico_set_program_version(PIOKMbox "0.1") # Modify the below lines to enable/disable output over UART/USB # UART0 is used for KMBox serial input via RP2350 USB Bridge # USB CDC is disabled to avoid breaking HID enumeration -# Debug output can be sent through the KMBox interface if needed -pico_enable_stdio_uart(PIOKMbox 1) +# The protocol UART is owned by the response queue; debug stdio must not write it. +pico_enable_stdio_uart(PIOKMbox 0) pico_enable_stdio_usb(PIOKMbox 0) # Optimization flags for release performance at 240MHz diff --git a/README.md b/README.md index 48822dd..d16f282 100644 --- a/README.md +++ b/README.md @@ -472,6 +472,56 @@ RaspberryKMBox/ Append `clean` to force a rebuild: `./build.sh all clean` +### Host Regression Checks + +These checks need Python 3 and a C compiler with address and undefined-behavior +sanitizers. They run production functions against mocked peripherals; they do +not replace firmware builds or testing on the boards. + +```bash +python3 tests/test_xbox_transfers.py +python3 tests/test_bridge_parsers.py +python3 tests/test_uart_timing.py +python3 tests/test_display_io.py +python3 tests/test_bridge_protocol.py +python3 tests/test_proxy_tx.py +python3 tests/test_proxy_rx.py +python3 tests/test_dma_rx.py +``` + +Build and flash the bridge and proxy from the same revision. Both bridge targets +use eight-byte fast packets. Humanization configuration uses opcode `0x10`, and +info requests use `0x11` to avoid the carriage-return opcode `0x0D`; info replies +retain `0x0D`. The variable-length wire-protocol library is not the proxy's UART +parser and must not be mixed into that stream. + +The default software TFT path advances refreshes one DMA row per main-loop pass. +Rendering runs in the main loop, while the timer interrupt only schedules it. +The optional `TFT_HW_ACCEL` path and explicit splash/error refreshes remain +synchronous. + +The proxy's protocol UART has stdio disabled. Command replies use an explicit +output callback into a shared record queue; debug `printf` calls do not write +onto that UART. The queue supports seven pending records of up to 256 bytes, +including any CRLF suffix. A full queue rejects whole records and accounts for +their bytes in `kmbox_get_tx_dropped_bytes()`. Startup and DMA-allocation failure +use polled RX and IRQ TX, preserving pending data when DMA starts. + +DMA RX on both boards uses RP2350 self-trigger mode with a decrementing count, +so complete ring-buffer laps cannot appear empty. Copies are checked against a +second coherent count/address snapshot before delivery. A loss discards pending +input and partial parser state; the proxy also releases injected controls and +cancels queued/timed movement. Reception resumes after at least 1 ms without DMA +progress and an empty UART FIFO. The sender must provide that gap at a packet +boundary; continuous traffic is discarded during recovery. + +The bridge exposes overrun events through `hw_uart_get_stats()` and discarded +bytes/ambiguous polls through `hw_uart_get_rx_loss_stats()`. The proxy exposes +all three through `kmbox_get_rx_loss_stats()` on Core 0. If a scheduling pause +could hide an entire hardware count period, reception fails closed and the byte +count is a lower bound. These counters describe discarded data, not retransmission +or flow control. No additional DMA channels or RX interrupts are required. + ### Build Configuration Presets defined in `defines.h`: diff --git a/bridge/core1_translator.c b/bridge/core1_translator.c index 411a1d2..0af9309 100644 --- a/bridge/core1_translator.c +++ b/bridge/core1_translator.c @@ -14,51 +14,53 @@ static queued_makcu_frame_t makcu_queue[MAKCU_FRAME_QUEUE_SIZE] __attribute__((aligned(4))); static queued_kmbox_cmd_t kmbox_queue[KMBOX_CMD_QUEUE_SIZE] __attribute__((aligned(4))); -// Ring buffer indices (volatile for inter-core visibility) -static volatile uint8_t makcu_head = 0; -static volatile uint8_t makcu_tail = 0; -static volatile uint8_t kmbox_head = 0; -static volatile uint8_t kmbox_tail = 0; +// SPSC indices: acquire/release orders slot data and prevents early slot reuse. +static uint8_t makcu_head = 0; +static uint8_t makcu_tail = 0; +static uint8_t kmbox_head = 0; +static uint8_t kmbox_tail = 0; // Statistics static volatile uint32_t frames_queued = 0; static volatile uint32_t cmds_produced = 0; static volatile uint32_t queue_overflows = 0; -// Core1 main translation loop +// Only fixed-size UART commands belong in this queue. Text responses (such +// as VERSION) must not be copied into packet slots or forwarded to the UART. +static bool queue_translated_packet(const translated_cmd_t *result) { + if (result->result != TRANSLATE_OK || + result->length != sizeof(kmbox_queue[0].packet)) return false; + uint8_t head = __atomic_load_n(&kmbox_head, __ATOMIC_RELAXED); + uint8_t next = (head + 1) & (KMBOX_CMD_QUEUE_SIZE - 1); + if (next == __atomic_load_n(&kmbox_tail, __ATOMIC_ACQUIRE)) { + __atomic_add_fetch(&queue_overflows, 1, __ATOMIC_RELAXED); + return false; + } + memcpy(kmbox_queue[head].packet, result->buffer, result->length); + __atomic_store_n(&kmbox_head, next, __ATOMIC_RELEASE); + __atomic_add_fetch(&cmds_produced, 1, __ATOMIC_RELAXED); + return true; +} + +// Consume one input frame. Kept separate from idle waiting so wake-ups always +// recheck the queue, including events received just before WFE. +static bool translate_one_frame(void) { + uint8_t tail = __atomic_load_n(&makcu_tail, __ATOMIC_RELAXED); + uint8_t head = __atomic_load_n(&makcu_head, __ATOMIC_ACQUIRE); + if (head == tail) return false; + + queued_makcu_frame_t *frame = &makcu_queue[tail]; + translated_cmd_t result; + makcu_translate_command(frame->cmd, frame->payload, frame->payload_len, &result); + queue_translated_packet(&result); + __atomic_store_n(&makcu_tail, (tail + 1) & (MAKCU_FRAME_QUEUE_SIZE - 1), __ATOMIC_RELEASE); + return true; +} + static void __not_in_flash_func(core1_main)(void) { - // Run translation loop at max speed while (1) { - // Check for Makcu frames to translate - uint8_t head = makcu_head; - uint8_t tail = makcu_tail; - - if (head != tail) { - // Get frame from queue - queued_makcu_frame_t* frame = &makcu_queue[tail & (MAKCU_FRAME_QUEUE_SIZE - 1)]; - - // Translate to KMBox command(s) - translated_cmd_t result; - makcu_translate_command(frame->cmd, frame->payload, frame->payload_len, &result); - - // Queue translated wire protocol packet (max 7 bytes, fits in 8-byte slot) - if (result.result == TRANSLATE_OK && result.length > 0) { - uint8_t next_kmbox_head = (kmbox_head + 1) & (KMBOX_CMD_QUEUE_SIZE - 1); - - if (next_kmbox_head != kmbox_tail) { - memcpy(kmbox_queue[kmbox_head].packet, result.buffer, result.length); - kmbox_head = next_kmbox_head; - __atomic_add_fetch(&cmds_produced, 1, __ATOMIC_RELAXED); - } else { - __atomic_add_fetch(&queue_overflows, 1, __ATOMIC_RELAXED); - } - } - - // Advance tail (consume frame) - makcu_tail = (tail + 1) & (MAKCU_FRAME_QUEUE_SIZE - 1); - } else { - // No work - yield to save power - __wfi(); // Wait for interrupt (low power idle) + if (!translate_one_frame()) { + __wfe(); // Matches the producer's SEV; a pending event prevents sleep. } } } @@ -83,47 +85,46 @@ void core1_translator_init(void) { // Queue Makcu frame for translation bool core1_queue_makcu_frame(uint8_t cmd, const uint8_t* payload, uint16_t len) { - if (len > 256) { + if (len > 256 || (len > 0 && !payload)) { return false; // Payload too large } - uint8_t next_head = (makcu_head + 1) & (MAKCU_FRAME_QUEUE_SIZE - 1); + uint8_t head = __atomic_load_n(&makcu_head, __ATOMIC_RELAXED); + uint8_t next_head = (head + 1) & (MAKCU_FRAME_QUEUE_SIZE - 1); - if (next_head == makcu_tail) { + if (next_head == __atomic_load_n(&makcu_tail, __ATOMIC_ACQUIRE)) { __atomic_add_fetch(&queue_overflows, 1, __ATOMIC_RELAXED); return false; // Queue full } // Copy frame to queue - queued_makcu_frame_t* frame = &makcu_queue[makcu_head]; + queued_makcu_frame_t* frame = &makcu_queue[head]; frame->cmd = cmd; frame->payload_len = len; if (payload && len > 0) { memcpy(frame->payload, payload, len); } - // Fix #4: Data memory barrier - ensure all writes complete before publishing index - __dmb(); - - // Advance head (publish frame) - makcu_head = next_head; + __atomic_store_n(&makcu_head, next_head, __ATOMIC_RELEASE); __atomic_add_fetch(&frames_queued, 1, __ATOMIC_RELAXED); // Wake Core1 if idle - __sev(); // Send event to wake WFI + __dsb(); // Complete publication before waking the other core. + __sev(); return true; } // Check if KMBox commands available bool core1_has_kmbox_commands(void) { - return kmbox_head != kmbox_tail; + return __atomic_load_n(&kmbox_head, __ATOMIC_ACQUIRE) != + __atomic_load_n(&kmbox_tail, __ATOMIC_RELAXED); } // Dequeue translated KMBox command bool core1_dequeue_kmbox_command(uint8_t* packet_out) { - uint8_t head = kmbox_head; - uint8_t tail = kmbox_tail; + uint8_t tail = __atomic_load_n(&kmbox_tail, __ATOMIC_RELAXED); + uint8_t head = __atomic_load_n(&kmbox_head, __ATOMIC_ACQUIRE); if (head == tail) { return false; // Queue empty @@ -133,7 +134,7 @@ bool core1_dequeue_kmbox_command(uint8_t* packet_out) { memcpy(packet_out, kmbox_queue[tail].packet, 8); // Advance tail (consume command) - kmbox_tail = (tail + 1) & (KMBOX_CMD_QUEUE_SIZE - 1); + __atomic_store_n(&kmbox_tail, (tail + 1) & (KMBOX_CMD_QUEUE_SIZE - 1), __ATOMIC_RELEASE); return true; } diff --git a/bridge/ferrum_translator.c b/bridge/ferrum_translator.c index 85d2fd6..8c92d60 100644 --- a/bridge/ferrum_translator.c +++ b/bridge/ferrum_translator.c @@ -19,6 +19,7 @@ #include #include #include +#include // Persistent button state — buttons are sticky (press stays until release) static uint8_t g_button_state = 0; @@ -41,12 +42,15 @@ static bool parse_int(const char **p, int32_t *out) { if (!isdigit((unsigned char)**p)) return false; - int32_t val = 0; + uint32_t val = 0; + uint32_t limit = (uint32_t)INT32_MAX + (neg ? 1u : 0u); while (isdigit((unsigned char)**p)) { - val = val * 10 + (**p - '0'); + uint32_t digit = (uint32_t)(**p - '0'); + if (val > (limit - digit) / 10u) return false; + val = val * 10u + digit; (*p)++; } - *out = neg ? -val : val; + *out = neg ? (val == (uint32_t)INT32_MAX + 1u ? INT32_MIN : -(int32_t)val) : (int32_t)val; return true; } @@ -74,7 +78,13 @@ static void emit_button_packet(ferrum_translated_t *out, uint8_t mask, bool pres // ============================================================================ bool ferrum_translate_line(const char *line, size_t len, ferrum_translated_t *out) { - if (!line || len == 0 || !out) return false; + if (!line || len == 0 || len >= FERRUM_MAX_LINE || !out) return false; + + // Respect the supplied span; callers need not provide a trailing NUL. + char bounded_line[FERRUM_MAX_LINE]; + memcpy(bounded_line, line, len); + bounded_line[len] = '\0'; + line = bounded_line; memset(out, 0, sizeof(ferrum_translated_t)); @@ -94,7 +104,8 @@ bool ferrum_translate_line(const char *line, size_t len, ferrum_translated_t *ou if (!parse_int(&p, &x)) return false; skip_separator(&p); if (!parse_int(&p, &y)) return false; - if (*p != ')') return false; + if (*p != ')' || x < INT16_MIN || x > INT16_MAX || + y < INT16_MIN || y > INT16_MAX) return false; // Direct accumulator (0x01) — the coalescer in main.c intercepts // move packets and re-routes through smooth queue (0x07) at a @@ -113,7 +124,7 @@ bool ferrum_translate_line(const char *line, size_t len, ferrum_translated_t *ou int32_t wheel; if (!parse_int(&p, &wheel)) return false; - if (*p != ')') return false; + if (*p != ')' || wheel < INT8_MIN || wheel > INT8_MAX) return false; out->length = (uint16_t)fast_build_mouse_move( out->buffer, 0, 0, g_button_state, (int8_t)wheel); diff --git a/bridge/hw_uart.c b/bridge/hw_uart.c index e0ed445..883ff87 100644 --- a/bridge/hw_uart.c +++ b/bridge/hw_uart.c @@ -13,7 +13,6 @@ #include "pico/stdlib.h" #include "hardware/uart.h" #include "hardware/dma.h" -#include "hardware/irq.h" #include "hardware/gpio.h" #include "hardware/clocks.h" #include "hardware/structs/uart.h" @@ -27,30 +26,20 @@ static int tx_dma_chan = -1; static int rx_dma_chan = -1; -// TX double-buffer for continuous transmission -static uint8_t tx_buffer_a[HW_UART_TX_BUFFER_SIZE] __attribute__((aligned(4))); -static uint8_t tx_buffer_b[HW_UART_TX_BUFFER_SIZE] __attribute__((aligned(4))); -static volatile bool using_buffer_a = true; -static volatile bool tx_busy = false; +// One staging buffer is sufficient: it is only reused after DMA is idle. +static uint8_t tx_buffer[HW_UART_TX_BUFFER_SIZE] __attribute__((aligned(4))); // RX circular buffer (must be aligned to size for DMA ring mode) static uint8_t rx_buffer[HW_UART_RX_BUFFER_SIZE] __attribute__((aligned(HW_UART_RX_BUFFER_SIZE))); -static volatile uint32_t rx_read_pos = 0; +static dma_rx_state_t rx_state; +static uint32_t rx_reported_loss_epoch; // Statistics static volatile uint32_t total_tx_bytes = 0; static volatile uint32_t total_rx_bytes = 0; -static volatile uint32_t rx_overflows = 0; - -// IRQ handler for TX DMA completion -static void tx_dma_irq_handler(void) { - if (tx_dma_chan >= 0 && dma_channel_get_irq0_status(tx_dma_chan)) { - dma_channel_acknowledge_irq0(tx_dma_chan); - tx_busy = false; - } -} bool hw_uart_init(uint tx_pin, uint rx_pin, uint baud) { + if (baud == 0) return false; // NOTE: We intentionally skip peri_clock_configure_stable() here. // 3Mbaud divides exactly from 240MHz (div=5), and keeping clk_peri // at 240MHz gives the TFT SPI bus 40MHz instead of 24MHz. @@ -69,7 +58,7 @@ bool hw_uart_init(uint tx_pin, uint rx_pin, uint baud) { uint actual_baud = uart_init(HW_UART, baud); // Calculate and display baud rate accuracy - int32_t baud_error_ppm = ((int32_t)actual_baud - (int32_t)baud) * 1000000 / (int32_t)baud; + int32_t baud_error_ppm = (int32_t)(((int64_t)actual_baud - baud) * 1000000 / baud); printf("[HW_UART] Requested: %u, Actual: %u, Error: %ld ppm\n", baud, actual_baud, baud_error_ppm); // Warn if baud rate error is too high (>2% is problematic) @@ -116,11 +105,9 @@ bool hw_uart_init(uint tx_pin, uint rx_pin, uint baud) { false ); - // Set up TX DMA interrupt - dma_channel_set_irq0_enabled(tx_dma_chan, true); - irq_set_exclusive_handler(DMA_IRQ_0, tx_dma_irq_handler); - irq_set_enabled(DMA_IRQ_0, true); - + // TX completion is polled from the DMA BUSY bit. No per-packet IRQ or + // software flag is needed, and a delayed IRQ cannot mark a new TX idle. + // Claim RX DMA channel rx_dma_chan = dma_claim_unused_channel(false); if (rx_dma_chan < 0) { @@ -141,6 +128,7 @@ bool hw_uart_init(uint tx_pin, uint rx_pin, uint baud) { uint ring_size_bits = __builtin_ctz(HW_UART_RX_BUFFER_SIZE); channel_config_set_ring(&rx_cfg, true, ring_size_bits); + channel_config_set_irq_quiet(&rx_cfg, true); channel_config_set_high_priority(&rx_cfg, true); printf("[HW_UART] RX DMA: ch=%d dreq=%u ring_bits=%u buf_size=%u\n", @@ -149,13 +137,16 @@ bool hw_uart_init(uint tx_pin, uint rx_pin, uint baud) { // Print UART hardware register addresses for debugging printf("[HW_UART] UART base=%p DR=%p\n", uart_hw, &uart_hw->dr); - // Start continuous RX DMA + dma_rx_init(&rx_state, HW_UART_RX_BUFFER_SIZE - 1, 0, 0, actual_baud, time_us_64()); + rx_state.fifo_status = &uart_hw->fr; + rx_reported_loss_epoch = 0; + // Self-triggered count reloads preserve continuous reception and expose progress. dma_channel_configure( rx_dma_chan, &rx_cfg, rx_buffer, &uart_hw->dr, // Read from UART data register - 0xFFFFFFFF, + dma_encode_transfer_count_with_self_trigger(DMA_RX_TRANSFER_COUNT), true ); @@ -171,7 +162,7 @@ bool hw_uart_init(uint tx_pin, uint rx_pin, uint baud) { } bool hw_uart_send(const uint8_t *data, size_t len) { - if (len == 0 || len > HW_UART_TX_BUFFER_SIZE) { + if (!data || len == 0 || len > HW_UART_TX_BUFFER_SIZE) { return false; } @@ -183,23 +174,10 @@ bool hw_uart_send(const uint8_t *data, size_t len) { return true; } - // Non-blocking: if DMA is still busy, copy to the other buffer - // and return false so caller knows to retry or drop - if (tx_busy) { - // Check if DMA actually finished (IRQ may be delayed) - if (!dma_channel_is_busy(tx_dma_chan)) { - tx_busy = false; - } else { - return false; // Non-blocking: DMA still active, caller should retry - } - } - - uint8_t *buf = using_buffer_a ? tx_buffer_a : tx_buffer_b; - memcpy(buf, data, len); - using_buffer_a = !using_buffer_a; - - tx_busy = true; - dma_channel_set_read_addr(tx_dma_chan, buf, false); + if (dma_channel_is_busy(tx_dma_chan)) return false; + + memcpy(tx_buffer, data, len); + dma_channel_set_read_addr(tx_dma_chan, tx_buffer, false); dma_channel_set_trans_count(tx_dma_chan, len, true); total_tx_bytes += len; @@ -207,69 +185,57 @@ bool hw_uart_send(const uint8_t *data, size_t len) { } void hw_uart_putc(uint8_t c) { - // Non-blocking with brief spin (max ~10µs at 2Mbaud) - uint32_t timeout = 100; // ~100 iterations - while (!uart_is_writable(HW_UART) && --timeout) { - tight_loop_contents(); - } - if (timeout) { - uart_putc_raw(HW_UART, c); - total_tx_bytes++; - } + // Control text must follow queued binary DMA data, never interleave with it. + hw_uart_tx_wait(); + uart_putc_raw(HW_UART, c); + total_tx_bytes++; } void hw_uart_puts(const char *str) { + if (!str) return; + hw_uart_tx_wait(); size_t len = strlen(str); - if (len <= HW_UART_TX_BUFFER_SIZE) { - hw_uart_send((const uint8_t *)str, len); - } else { - while (*str) { - hw_uart_putc(*str++); - } - } -} - -#define RX_MASK (HW_UART_RX_BUFFER_SIZE - 1) - -static inline uint32_t get_rx_write_pos(void) { - return dma_ring_write_pos(rx_dma_chan, rx_buffer, RX_MASK); + uart_write_blocking(HW_UART, (const uint8_t *)str, len); + total_tx_bytes += len; } bool hw_uart_rx_available(void) { - return dma_ring_available(rx_read_pos, get_rx_write_pos(), RX_MASK) > 0; + return hw_uart_rx_count() > 0; } size_t hw_uart_rx_count(void) { - uint32_t avail = dma_ring_available(rx_read_pos, get_rx_write_pos(), RX_MASK); - if (avail >= HW_UART_RX_BUFFER_SIZE - 16) { - rx_overflows++; - } - return avail; + return rx_dma_chan < 0 ? 0 : dma_rx_poll(&rx_state, rx_dma_chan); } int hw_uart_getc(void) { - if (!hw_uart_rx_available()) return -1; - - uint8_t c = rx_buffer[rx_read_pos]; - rx_read_pos = (rx_read_pos + 1) & RX_MASK; - total_rx_bytes++; - return c; + uint8_t byte; + return hw_uart_read(&byte, 1) == 1 ? byte : -1; } size_t hw_uart_read(uint8_t *buf, size_t maxlen) { - if (maxlen == 0) return 0; - uint32_t wp = get_rx_write_pos(); - size_t n = dma_ring_read(rx_buffer, &rx_read_pos, wp, RX_MASK, buf, maxlen); + if (rx_dma_chan < 0) return 0; + size_t n = dma_rx_read(&rx_state, rx_dma_chan, rx_buffer, buf, maxlen); total_rx_bytes += n; return n; } +bool hw_uart_rx_take_loss(void) { + bool lost = rx_reported_loss_epoch != rx_state.loss_epoch; + rx_reported_loss_epoch = rx_state.loss_epoch; + return lost; +} + +void hw_uart_get_rx_loss_stats(uint64_t *discarded_bytes, uint32_t *ambiguous_polls) { + if (discarded_bytes) *discarded_bytes = rx_state.discarded_bytes; + if (ambiguous_polls) *ambiguous_polls = rx_state.ambiguous_polls; +} + bool hw_uart_tx_ready(void) { - return !tx_busy; + return tx_dma_chan < 0 || !dma_channel_is_busy(tx_dma_chan); } void hw_uart_tx_wait(void) { - while (tx_busy) { + while (!hw_uart_tx_ready()) { tight_loop_contents(); } } @@ -277,5 +243,5 @@ void hw_uart_tx_wait(void) { void hw_uart_get_stats(uint32_t *tx_bytes, uint32_t *rx_bytes, uint32_t *overflows) { if (tx_bytes) *tx_bytes = total_tx_bytes; if (rx_bytes) *rx_bytes = total_rx_bytes; - if (overflows) *overflows = rx_overflows; + if (overflows) *overflows = rx_state.overruns; } diff --git a/bridge/hw_uart.h b/bridge/hw_uart.h index 39fde3d..7c0be2b 100644 --- a/bridge/hw_uart.h +++ b/bridge/hw_uart.h @@ -9,7 +9,7 @@ * - Hardware UART0 for reliable, high-speed communication * - DMA TX for non-blocking, zero-CPU transmission * - DMA RX with circular buffer for continuous reception - * - IRQ-based fallback for compatibility + * - Polled DMA completion without per-packet interrupts */ #ifndef HW_UART_H @@ -22,7 +22,12 @@ // DMA buffer sizes (must be power of 2 for ring buffer) #define HW_UART_TX_BUFFER_SIZE 512 // 512B for burst TX at 3 Mbaud -#define HW_UART_RX_BUFFER_SIZE PICO_STDIO_STACK_BUFFER_SIZE +#ifndef HW_UART_RX_BUFFER_SIZE +#define HW_UART_RX_BUFFER_SIZE 2048 // Independent of SDK printf stack buffers +#endif +_Static_assert(HW_UART_RX_BUFFER_SIZE >= 2 && HW_UART_RX_BUFFER_SIZE <= 32768 && + (HW_UART_RX_BUFFER_SIZE & (HW_UART_RX_BUFFER_SIZE - 1)) == 0, + "UART RX DMA ring must be a power of two, at most 32768 bytes"); /** * Initialize hardware UART with DMA support @@ -87,6 +92,11 @@ int hw_uart_getc(void); */ size_t hw_uart_read(uint8_t *buf, size_t maxlen); +// Consume a loss notification after read/count; reset any partial protocol state. +bool hw_uart_rx_take_loss(void); +// Discarded bytes are a lower bound when ambiguous_polls is nonzero. +void hw_uart_get_rx_loss_stats(uint64_t *discarded_bytes, uint32_t *ambiguous_polls); + /** * Check if TX is ready for more data * diff --git a/bridge/input_classifier.h b/bridge/input_classifier.h index 006a7b6..ced857a 100644 --- a/bridge/input_classifier.h +++ b/bridge/input_classifier.h @@ -46,8 +46,8 @@ static uint8_t ic_hysteresis = 0; static inline void input_classifier_record_move(int16_t dx, int16_t dy) { ic_move_count++; // Use integer abs - int16_t ax = dx < 0 ? -dx : dx; - int16_t ay = dy < 0 ? -dy : dy; + int32_t ax = dx < 0 ? -(int32_t)dx : dx; + int32_t ay = dy < 0 ? -(int32_t)dy : dy; ic_total_magnitude += (uint32_t)(ax + ay); } diff --git a/bridge/main.c b/bridge/main.c index 5c21852..1e6a868 100644 --- a/bridge/main.c +++ b/bridge/main.c @@ -482,6 +482,11 @@ static void uart_rx_task(void) { // Batch read up to 64 bytes at once from DMA ring buffer (single memcpy) uint8_t rx_batch[64]; size_t batch_count = hw_uart_read(rx_batch, sizeof(rx_batch)); + if (hw_uart_rx_take_loss()) { + in_binary_packet = false; + binary_idx = 0; + status_buffer_idx = 0; + } if (batch_count == 0) return; for (size_t batch_i = 0; batch_i < batch_count; batch_i++) { @@ -497,7 +502,9 @@ static void uart_rx_task(void) { } // Check for start of binary response packet (0xFF, 0xFE, 0x0D, or 0x0E from KMBox) - if (!in_binary_packet && (c == 0xFF || c == 0xFE || c == 0x0D || c == 0x0E)) { + if (!in_binary_packet && (c == FAST_CMD_RESPONSE || c == FAST_CMD_PING || + (c == FAST_CMD_INFO && status_buffer_idx == 0) || + c == FAST_CMD_INFO_EXT || c == FAST_CMD_XBOX_STATUS)) { in_binary_packet = true; binary_idx = 0; binary_packet[binary_idx++] = c; @@ -517,7 +524,7 @@ static void uart_rx_task(void) { binary_packet[binary_idx++] = c; if (binary_idx >= 8) { // Complete packet received - if (binary_packet[0] == 0xFF) { + if (binary_packet[0] == FAST_CMD_RESPONSE) { // Ping/response packet kmbox_response_count++; @@ -672,7 +679,7 @@ static void kmbox_connection_task(void) { last_humanization_request_ms = now - HUMANIZATION_REQUEST_INTERVAL_MS + HUMANIZATION_INITIAL_DELAY_MS; // Send initial info request right away (binary) - uint8_t info_req[8] = {0x0D, 0, 0, 0, 0, 0, 0, 0}; + uint8_t info_req[8] = {FAST_CMD_INFO_REQUEST, 0, 0, 0, 0, 0, 0, 0}; send_uart_packet(info_req, 8); } } @@ -700,7 +707,7 @@ static void kmbox_connection_task(void) { static uint8_t info_cycle = 0; if (info_cycle % 2 == 0) { // Primary: humanization mode, inject mode, queue depth, temperature, flags - uint8_t info_req[8] = {0x0D, 0, 0, 0, 0, 0, 0, 0}; + uint8_t info_req[8] = {FAST_CMD_INFO_REQUEST, 0, 0, 0, 0, 0, 0, 0}; send_uart_packet(info_req, 8); } else { // Extended: total injected, queue overflows, queue capacity @@ -739,7 +746,7 @@ static void kmbox_connection_task(void) { // Send 0x0D info request even when disconnected to test UART RX if (now - last_humanization_request_ms >= HUMANIZATION_REQUEST_INTERVAL_MS) { last_humanization_request_ms = now; - uint8_t info_req[8] = {0x0D, 0, 0, 0, 0, 0, 0, 0}; + uint8_t info_req[8] = {FAST_CMD_INFO_REQUEST, 0, 0, 0, 0, 0, 0, 0}; bool sent = send_uart_packet(info_req, 8); // Debug: confirm we're sending the request @@ -923,8 +930,9 @@ static void handle_text_command(const char* cmd) { // km.click(btn) - mouse click (convert to binary) int btn; if (kmbox_parse_click_command(cmd, &btn)) { - uint8_t pkt[8] = {0x02, (uint8_t)btn, 1, 0, 0, 0, 0, 0}; - send_uart_packet(pkt, 8); + uint8_t pkt[FAST_CMD_PACKET_SIZE]; + fast_build_mouse_click(pkt, (uint8_t)(1u << btn), 1); + send_uart_packet(pkt, sizeof(pkt)); return; } @@ -1404,7 +1412,7 @@ static void tft_submit_stats_task(void) { stats.button_presses = button_press_count; stats.commands_per_sec = commands_per_sec; - stats.uart_errors = uart_error_count; + stats.uart_errors = uart_error_count + uart_rx_overflows; stats.frame_errors = frame_error_count; stats.cpu_freq_mhz = clock_get_hz(clk_sys) / 1000000; @@ -1519,9 +1527,9 @@ int main(void) { // Process menu requests (from TFT menu view) if (tft_menu_request.set_humanization_mode) { tft_menu_request.set_humanization_mode = false; - // WIRE_CONFIG: [0x0B][param=0x02][value] - uint8_t config_pkt[3] = {0x0B, 0x02, tft_menu_request.humanization_mode_val}; - send_uart_packet(config_pkt, 3); + uint8_t config_pkt[FAST_CMD_PACKET_SIZE]; + fast_build_set_human(config_pkt, tft_menu_request.humanization_mode_val); + send_uart_packet(config_pkt, sizeof(config_pkt)); printf("[Bridge] Menu: set humanization mode -> %d\n", tft_menu_request.humanization_mode_val); } if (tft_menu_request.set_max_per_frame || tft_menu_request.set_velocity_matching) { @@ -1530,9 +1538,9 @@ int main(void) { (kmbox_velocity_matching ? 1 : 0); tft_menu_request.set_max_per_frame = false; tft_menu_request.set_velocity_matching = false; - // WIRE_SMOOTH_CFG: [0x0C][max_per_frame][vel_match] - uint8_t smooth_cfg_pkt[3] = {0x0C, mpf, vel}; - send_uart_packet(smooth_cfg_pkt, 3); + uint8_t smooth_cfg_pkt[FAST_CMD_PACKET_SIZE]; + fast_build_smooth_config(smooth_cfg_pkt, mpf, vel); + send_uart_packet(smooth_cfg_pkt, sizeof(smooth_cfg_pkt)); printf("[Bridge] Menu: smooth cfg mpf=%d vel=%d\n", mpf, vel); } if (tft_menu_request.set_inject_mode) { diff --git a/bridge/main_feather.c b/bridge/main_feather.c index 6b561ae..5855d9b 100644 --- a/bridge/main_feather.c +++ b/bridge/main_feather.c @@ -5,7 +5,7 @@ * No TFT, no tracker, no Makcu. Just CDC -> Wire Protocol -> KMBox. * * Architecture: - * PC (km.move text) -> USB CDC -> Feather RP2350 -> UART0 (wire_protocol) -> KMBox + * PC (km.move text) -> USB CDC -> Feather RP2350 -> UART0 (fixed-size fast protocol) -> KMBox * * Wiring: * Feather GPIO 0 (UART0 TX) -> KMBox GPIO 9 (UART1 RX) @@ -28,7 +28,7 @@ #include "ferrum_protocol.h" #include "ferrum_translator.h" #include "neopixel_dma.h" -#include "wire_protocol.h" +#include "fast_commands.h" #include "kmbox_commands.h" #include "input_classifier.h" @@ -67,21 +67,21 @@ static uint8_t cmd_buffer_idx = 0; // UART Helpers // ============================================================================ -static inline bool send_wire_packet(const uint8_t *data, size_t len) { +static inline bool send_fast_packet(const uint8_t *data, size_t len) { return hw_uart_send(data, len); } // ============================================================================ -// CDC RX Task — Parse Ferrum and KMBox text, emit wire protocol +// CDC RX Task — Parse Ferrum and KMBox text, emit fixed-size fast packets // ============================================================================ static void handle_text_command(const char *cmd) { int x, y; if (kmbox_parse_move_command(cmd, &x, &y)) { input_classifier_record_move((int16_t)x, (int16_t)y); - uint8_t pkt[WIRE_MAX_PACKET]; - size_t len = wire_build_move(pkt, (int16_t)x, (int16_t)y); - send_wire_packet(pkt, len); + uint8_t pkt[FAST_CMD_PACKET_SIZE]; + size_t len = fast_build_mouse_move(pkt, (int16_t)x, (int16_t)y, 0, 0); + send_fast_packet(pkt, len); return; } @@ -118,15 +118,13 @@ static void cdc_rx_task(void) { ferrum_translated_t translated; if (ferrum_translate_line(ferrum_line, ferrum_idx, &translated)) { if (translated.length > 0) { - if (translated.buffer[0] == WIRE_MOVE8 && translated.length >= 3) { - input_classifier_record_move((int8_t)translated.buffer[1], - (int8_t)translated.buffer[2]); - } else if (translated.buffer[0] == WIRE_MOVE16 && translated.length >= 5) { + if (translated.buffer[0] == FAST_CMD_MOUSE_MOVE && + translated.length == FAST_CMD_PACKET_SIZE) { int16_t mx = (int16_t)(translated.buffer[1] | (translated.buffer[2] << 8)); int16_t my = (int16_t)(translated.buffer[3] | (translated.buffer[4] << 8)); input_classifier_record_move(mx, my); } - send_wire_packet(translated.buffer, translated.length); + send_fast_packet(translated.buffer, translated.length); } if (translated.needs_response && tud_cdc_connected()) { const char *resp = FERRUM_RESPONSE; @@ -186,60 +184,55 @@ static void cdc_rx_task(void) { } // ============================================================================ -// UART RX Task — Read wire protocol responses from KMBox +// UART RX Task — Read fixed-size responses from KMBox // ============================================================================ static void uart_rx_task(void) { + // Keep parser state across DMA reads; a UART packet need not arrive in one batch. + static uint8_t packet[FAST_CMD_PACKET_SIZE]; + static uint8_t packet_idx; + static char line[128]; + static size_t line_len; + static uint32_t packet_started_ms; uint8_t batch[128]; size_t n = hw_uart_read(batch, sizeof(batch)); + if (hw_uart_rx_take_loss()) { + packet_idx = 0; + line_len = 0; + } if (n == 0) return; - uint32_t now = to_ms_since_boot(get_absolute_time()); - kmbox_last_rx_time = now; - - if (bridge_conn_state == BRIDGE_DISCONNECTED) { - bridge_conn_state = BRIDGE_CONNECTED; - } - - // Process wire protocol responses - size_t pos = 0; - while (pos < n) { - uint8_t first = batch[pos]; - uint8_t pkt_len = wire_get_packet_len(first); - - if (pkt_len > 0 && pos + pkt_len <= n) { - // Wire protocol response — forward key info to CDC debug - if (first == WIRE_RESPONSE && pkt_len == 6) { - uint8_t status = batch[pos + 1]; - if (status == WIRE_RESP_INFO) { - // Info response: [0xFF][0x0C][hmode][imode][max_pf][queue] - // Could log or display via NeoPixel - } + if (packet_idx && now - packet_started_ms > 100) packet_idx = 0; + for (size_t i = 0; i < n; i++) { + uint8_t c = batch[i]; + if (packet_idx) { + packet[packet_idx++] = c; + if (packet_idx == sizeof(packet)) { + packet_idx = 0; + kmbox_last_rx_time = now; + bridge_conn_state = BRIDGE_CONNECTED; } - pos += pkt_len; continue; } - - // Text response from KMBox (KMBOX_PONG, etc.) - // Forward to CDC for debugging - if (first >= 0x20 && first <= 0x7E) { - // Find line end - size_t line_start = pos; - while (pos < n && batch[pos] != '\n' && batch[pos] != '\r') pos++; - if (pos < n) { - // Forward text response to CDC + // Feather only requests ping replies. Other bytes are text, including CR/LF. + if (c == FAST_CMD_RESPONSE || c == FAST_CMD_PING) { + packet_idx = 1; + packet[0] = c; + packet_started_ms = now; + } else if (c == '\r' || c == '\n') { + if (line_len) { + kmbox_last_rx_time = now; + bridge_conn_state = BRIDGE_CONNECTED; if (tud_cdc_connected()) { - tud_cdc_write(&batch[line_start], pos - line_start); + tud_cdc_write(line, line_len); tud_cdc_write_str("\r\n"); tud_cdc_write_flush(); } - while (pos < n && (batch[pos] == '\n' || batch[pos] == '\r')) pos++; + line_len = 0; } - continue; + } else if (c >= 0x20 && c <= 0x7E && line_len < sizeof(line)) { + line[line_len++] = (char)c; } - - // Skip unknown byte - pos++; } } @@ -259,9 +252,9 @@ static void connection_task(void) { // Periodic ping if (now - kmbox_last_ping_time >= KMBOX_PING_INTERVAL_MS) { kmbox_last_ping_time = now; - uint8_t pkt[1]; - wire_build_ping(pkt); - send_wire_packet(pkt, 1); + uint8_t pkt[FAST_CMD_PACKET_SIZE]; + fast_build_ping(pkt); + send_fast_packet(pkt, sizeof(pkt)); } // Evaluate input classification (~1Hz) @@ -274,10 +267,9 @@ static void connection_task(void) { if (cls != INPUT_UNKNOWN && cls != last_sent_class && bridge_conn_state == BRIDGE_CONNECTED) { uint8_t hmode = (cls == INPUT_PRE_HUMANIZED) ? 1 : 2; - uint8_t cfg[3]; - wire_build_config(cfg, WIRE_CFG_HUMANIZATION, hmode); - send_wire_packet(cfg, 3); - last_sent_class = cls; + uint8_t cfg[FAST_CMD_PACKET_SIZE]; + fast_build_set_human(cfg, hmode); + if (send_fast_packet(cfg, sizeof(cfg))) last_sent_class = cls; printf("[Bridge] Auto-detected input: %s -> humanization %s\n", cls == INPUT_PRE_HUMANIZED ? "pre-humanized" : "raw", hmode == 1 ? "MICRO" : "FULL"); diff --git a/bridge/neopixel_dma.c b/bridge/neopixel_dma.c index 0677319..1a2a332 100644 --- a/bridge/neopixel_dma.c +++ b/bridge/neopixel_dma.c @@ -187,7 +187,7 @@ void neopixel_dma_init(PIO pio, uint pin) { channel_config_set_transfer_data_size(&c, DMA_SIZE_32); channel_config_set_read_increment(&c, false); channel_config_set_write_increment(&c, false); - channel_config_set_dreq(&c, DREQ_FORCE); + channel_config_set_dreq(&c, pio_get_dreq(s_pio, s_sm, true)); dma_channel_configure( s_dma_chan, diff --git a/bridge/tft.c b/bridge/tft.c index 8f104a7..145df4f 100644 --- a/bridge/tft.c +++ b/bridge/tft.c @@ -161,52 +161,32 @@ static void __unused write_buffer_dma(void *bstr, size_t len) void tft_control(uint8_t reg, const uint8_t *bstr, int len) { + if (len < 0 || (len > 0 && !bstr)) return; + // Register commands and parameters are byte streams, even though pixels + // use 16-bit DMA. This also handles odd parameter counts without padding. + spi_set_format(TFT_SPI_DEV, 8, 0, 0, SPI_MSB_FIRST); select_register(); select_chip(true); spi_write_blocking(TFT_SPI_DEV, ®, 1); - select_data(); - - for (int i = 0; i < len; i += 2) { - uint16_t tmp = (bstr[i] << 8) | bstr[i + 1]; - spi_write16_blocking(TFT_SPI_DEV, &tmp, 1); - } - - if (len & 1) { - uint16_t tmp = bstr[len - 1] << 8; - spi_write16_blocking(TFT_SPI_DEV, &tmp, 1); - } - + if (len > 0) spi_write_blocking(TFT_SPI_DEV, bstr, (size_t)len); select_chip(false); + spi_set_format(TFT_SPI_DEV, 16, 0, 0, SPI_MSB_FIRST); } #if defined(TFT_MISO_PIN) void tft_read(uint8_t reg, uint8_t *bstr, int len, int delay_us) { + if (len < 0 || (len > 0 && !bstr)) return; + spi_set_format(TFT_SPI_DEV, 8, 0, 0, SPI_MSB_FIRST); select_register(); select_chip(true); spi_write_blocking(TFT_SPI_DEV, ®, 1); - - if (delay_us >= 0) - sleep_us(delay_us); - + if (delay_us >= 0) sleep_us(delay_us); select_data(); - - const uint16_t blank = 0; - uint16_t tmp; - - for (int i = 0; i < len; i += 2) { - spi_read16_blocking(TFT_SPI_DEV, blank, &tmp, 1); - bstr[i] = tmp >> 8; - bstr[i + 1] = tmp; - } - - if (len & 1) { - spi_read16_blocking(TFT_SPI_DEV, blank, &tmp, 1); - bstr[len - 1] = tmp >> 8; - } - + if (len > 0) spi_read_blocking(TFT_SPI_DEV, 0, bstr, (size_t)len); select_chip(false); + spi_set_format(TFT_SPI_DEV, 16, 0, 0, SPI_MSB_FIRST); } #endif @@ -349,8 +329,62 @@ void tft_swap_buffers(void) tft_input = tmp; } +static bool async_active; +#if !TFT_HW_ACCEL +static int async_row; +static int async_repeat; +#endif + +bool tft_sync_start(void) +{ + if (async_active) return false; +#if TFT_HW_ACCEL + // The optional chained PIO path retains its synchronous implementation. + tft_sync(); +#else + tft_begin_sync(); + select_chip(true); + async_row = 0; + async_repeat = 0; + async_active = true; +#endif + return true; +} + +bool tft_sync_poll(void) +{ +#if !TFT_HW_ACCEL + if (!async_active) return true; + if (dma_channel_is_busy(dma_ch_spi)) return false; + if (async_row == TFT_RAW_HEIGHT / TFT_SCALE) { + // DMA completion can precede the final bits leaving the SPI shifter. + if (spi_is_busy(TFT_SPI_DEV)) return false; + select_chip(false); + async_active = false; + return true; + } + if (async_repeat == 0) { + const uint8_t *src = tft_committed + async_row * (TFT_RAW_WIDTH / TFT_SCALE); + uint16_t *dst = txbuf[0]; + for (int x = 0; x < TFT_RAW_WIDTH / TFT_SCALE; x++) { + uint16_t color = tft_palette[src[x]]; + for (int n = 0; n < TFT_SCALE; n++) *dst++ = color; + } + } + dma_channel_transfer_from_buffer_now(dma_ch_spi, txbuf[0], TFT_RAW_WIDTH); + if (++async_repeat == TFT_SCALE) { + async_repeat = 0; + async_row++; + } + return false; +#else + return true; +#endif +} + void tft_sync(void) { + while (!tft_sync_poll()) tight_loop_contents(); #if TFT_HW_ACCEL tft_begin_sync(); select_chip(true); @@ -404,6 +438,7 @@ void tft_sync(void) void tft_swap_sync(void) { + while (!tft_sync_poll()) tight_loop_contents(); tft_swap_buffers(); tft_sync(); } diff --git a/bridge/tft.h b/bridge/tft.h index 85d7829..2ee2d86 100644 --- a/bridge/tft.h +++ b/bridge/tft.h @@ -87,6 +87,9 @@ void tft_swap_buffers(void); * Blocks until it's done so the you do not clobber the back buffer. */ void tft_sync(void); +// Cooperative row-DMA refresh (software LUT path). Poll once per main-loop pass. +bool tft_sync_start(void); +bool tft_sync_poll(void); /* Shortcut to swap buffers and sync at once. */ void tft_swap_sync(void); diff --git a/bridge/tft_display.c b/bridge/tft_display.c index 0705b65..52b6abe 100644 --- a/bridge/tft_display.c +++ b/bridge/tft_display.c @@ -163,10 +163,9 @@ static inline int fast_isqrt(int v) { // // Architecture: // Main loop calls tft_display_submit_stats() to copy stats into shared buf. -// A repeating hardware timer fires every 100ms, picks up the latest stats, -// formats strings, and draws into the tft_input framebuffer. -// Main loop calls tft_display_flush() which does tft_swap_sync() (SPI DMA) -// only when a new frame has been rendered. +// A repeating hardware timer marks rendering due every 100ms. +// The main loop formats/draws the latest stats and advances one row of SPI +// DMA per flush call. No rendering runs in IRQ context. // ============================================================================ static bool initialized = false; @@ -219,12 +218,14 @@ static bool menu_values_initialized = false; static int8_t menu_highlight_item = -1; // Item being highlighted (-1 = none) static uint32_t menu_highlight_time = 0; // When highlight started -// Double-buffered stats: main loop writes, timer ISR reads +// Latest stats are submitted and rendered by the main loop. static tft_stats_t shared_stats; // Latest stats from main loop -static volatile bool stats_pending = false; // Main loop set, timer clears +static bool stats_pending = false; // Main loop sets and clears -// Frame ready flag: timer ISR sets after drawing, main loop clears after DMA -static volatile bool frame_ready = false; +// Hold the rendered frame until its asynchronous DMA transfer completes. +static bool frame_ready = false; +static volatile bool render_due = false; +static bool transfer_pending = false; // Stats hash for frame-skip optimization: skip render if stats unchanged static uint32_t last_stats_hash = 0; @@ -1551,8 +1552,12 @@ static void draw_menu_view(const tft_stats_t *stats) { // Public API // ============================================================================ -// Forward declaration — timer callback defined below -static bool tft_render_timer_callback(repeating_timer_t *rt); +// Keep alarm IRQ work bounded. Formatting and drawing run in the main loop. +static bool tft_render_timer_callback(repeating_timer_t *rt) { + (void)rt; + render_due = true; + return true; +} bool tft_display_init(void) { if (initialized) return true; @@ -1590,26 +1595,23 @@ bool tft_display_init(void) { } // ============================================================================ -// Timer callback — fires every 100ms from hardware alarm IRQ -// Does all CPU-intensive work: formatting + drawing into framebuffer. -// Does NOT touch SPI/DMA (that stays on the main loop via flush). +// Main-loop rendering, scheduled by the lightweight alarm callback. // ============================================================================ -static bool tft_render_timer_callback(repeating_timer_t *rt) { - (void)rt; - if (!initialized) return true; +static void tft_render_frame(void) { + if (!initialized) return; // Skip if the previous frame hasn't been flushed yet - if (frame_ready) return true; + if (frame_ready) return; - // Grab latest stats (atomic-enough: single-word flag guard) - if (!stats_pending) return true; + // Stats submission and rendering both run on the main loop. + if (!stats_pending) return; tft_stats_t local_stats = shared_stats; // Snapshot stats_pending = false; // Skip render if stats haven't meaningfully changed (saves CPU in steady state) uint32_t h = stats_quick_hash(&local_stats); - if (h == last_stats_hash && menu_highlight_item < 0) return true; + if (h == last_stats_hash && menu_highlight_item < 0) return; last_stats_hash = h; // Render into the back buffer @@ -1632,7 +1634,7 @@ static bool tft_render_timer_callback(repeating_timer_t *rt) { // Signal main loop that a frame is ready for DMA frame_ready = true; - return true; // Keep timer running + return; } // ============================================================================ @@ -1640,20 +1642,27 @@ static bool tft_render_timer_callback(repeating_timer_t *rt) { // ============================================================================ void tft_display_submit_stats(const tft_stats_t *stats) { - // Copy stats into shared buffer — called from main loop. - // The timer ISR will pick these up on its next tick. + // Keep the latest snapshot for the next main-loop render. shared_stats = *stats; stats_pending = true; } bool tft_display_flush(void) { - // If the timer ISR has rendered a new frame, push it to the display. - // This does the SPI DMA transfer (must run on Core0, not in ISR). - if (!initialized || !frame_ready) return false; - - tft_swap_sync(); - frame_ready = false; - return true; + if (!initialized) return false; + if (transfer_pending) { + if (!tft_sync_poll()) return false; + transfer_pending = false; + frame_ready = false; + return true; + } + if (render_due && !frame_ready) { + render_due = false; + tft_render_frame(); + } + if (!frame_ready) return false; + tft_swap_buffers(); + transfer_pending = tft_sync_start(); + return false; } void tft_display_update(const tft_stats_t *stats) { @@ -1686,6 +1695,7 @@ void tft_display_refresh(const tft_stats_t *stats) { last_tx_bytes = stats->tx_bytes; last_rx_bytes = stats->rx_bytes; frame_ready = false; + transfer_pending = false; } void tft_display_splash(void) { diff --git a/kmbox_serial_handler.c b/kmbox_serial_handler.c index b3999e5..8a87104 100644 --- a/kmbox_serial_handler.c +++ b/kmbox_serial_handler.c @@ -14,6 +14,7 @@ #include "xbox_device.h" #include "xbox_host.h" #include "uart_buffers.h" +#include "dma_uart.h" #include "dcp_helpers.h" #include "pico/stdlib.h" #include "hardware/uart.h" @@ -22,6 +23,7 @@ #include "hardware/clocks.h" #include "hardware/adc.h" #include "hardware/dma.h" +#include "hardware/sync.h" #include "pico/time.h" #include #include @@ -70,47 +72,99 @@ static __force_inline int16_t read_i16_le(const uint8_t *p) { static uint8_t uart_rx_buffer[UART_RX_BUFFER_SIZE] __attribute__((aligned(UART_RX_BUFFER_SIZE))); static volatile uint32_t uart_rx_read_pos = 0; static int uart_rx_dma_chan = -1; +static uint32_t uart_rx_poll_write_pos; +static bool text_discarding; +static dma_rx_state_t uart_rx_state; +static uint32_t uart_rx_reported_epoch; +static bool uart_rx_loss_pending; //--------------------------------------------------------------------+ -// TX DMA (zero-CPU transmission) -// Stages data into a linear buffer, then fires a DMA transfer. -// Much faster than per-byte IRQ TX at 2 Mbaud. +// Queued UART transmission (DMA when available, otherwise TX IRQ). +// Records remain owned until DMA completes; queue overflow drops whole records. //--------------------------------------------------------------------+ -#define DMA_TX_BUFFER_SIZE 1024 -static uint8_t dma_tx_buffer[DMA_TX_BUFFER_SIZE] __attribute__((aligned(4))); -static volatile uint16_t dma_tx_len = 0; +// Fixed-size records preserve message boundaries across DMA/IRQ and both cores. +#define UART_TX_RECORD_SIZE 256 +#define UART_TX_RECORD_COUNT 8 +#define UART_TX_RECORD_MASK (UART_TX_RECORD_COUNT - 1) +typedef struct { + uint8_t data[UART_TX_RECORD_SIZE]; + uint16_t len; +} uart_tx_record_t; +static uart_tx_record_t tx_records[UART_TX_RECORD_COUNT]; +static uint8_t tx_head, tx_tail; +static uint16_t tx_offset; +static bool tx_dma_active; static int uart_tx_dma_chan = -1; -static volatile bool dma_tx_busy = false; +static spin_lock_t *tx_lock; +static uint32_t g_uart_tx_dropped; // DMA-based RX: get current write position from DMA write address // __force_inline guarantees inlining even at -Os (Pico SDK macro) static __force_inline uint32_t rx_get_write_pos(void) { - if (uart_rx_dma_chan < 0) return 0; + if (uart_rx_dma_chan < 0) { + // Startup and allocation-failure fallback. Reserve one ring slot so + // a full buffer cannot masquerade as empty; leave remaining bytes in FIFO. + while (uart_is_readable(KMBOX_UART)) { + uint32_t next = (uart_rx_poll_write_pos + 1) & UART_RX_BUFFER_MASK; + if (next == uart_rx_read_pos) break; + uart_rx_buffer[uart_rx_poll_write_pos] = (uint8_t)uart_getc(KMBOX_UART); + uart_rx_poll_write_pos = next; + } + return uart_rx_poll_write_pos; + } uintptr_t write_addr = (uintptr_t)dma_channel_hw_addr(uart_rx_dma_chan)->write_addr; return (uint32_t)((write_addr - (uintptr_t)uart_rx_buffer) & UART_RX_BUFFER_MASK); } +static __force_inline void rx_sync_state(void) { + uart_rx_read_pos = uart_rx_state.consumed & UART_RX_BUFFER_MASK; + if (uart_rx_reported_epoch != uart_rx_state.loss_epoch) { + uart_rx_reported_epoch = uart_rx_state.loss_epoch; + uart_rx_loss_pending = true; + text_discarding = false; + } +} + static __force_inline uint16_t rx_available(void) { - return (uint16_t)((rx_get_write_pos() - uart_rx_read_pos) & UART_RX_BUFFER_MASK); + if (uart_rx_dma_chan < 0) { + return (uint16_t)((rx_get_write_pos() - uart_rx_read_pos) & UART_RX_BUFFER_MASK); + } + uint32_t avail = dma_rx_poll(&uart_rx_state, uart_rx_dma_chan); + rx_sync_state(); + return (uint16_t)avail; } static __force_inline uint8_t rx_peek_at(uint16_t offset) { return uart_rx_buffer[(uart_rx_read_pos + offset) & UART_RX_BUFFER_MASK]; } +static bool rx_copy_peek(uint8_t *dest, size_t len) { + if (len > rx_available()) return false; + if (uart_rx_dma_chan >= 0) { + bool valid = dma_rx_peek_copy(&uart_rx_state, uart_rx_dma_chan, + uart_rx_buffer, dest, 0, len); + rx_sync_state(); + return valid; + } + for (size_t i = 0; i < len; i++) dest[i] = rx_peek_at((uint16_t)i); + return true; +} + static __force_inline void rx_consume(uint16_t count) { - uart_rx_read_pos = (uart_rx_read_pos + count) & UART_RX_BUFFER_MASK; + if (count > rx_available()) return; + if (uart_rx_dma_chan >= 0) { + uart_rx_state.consumed += count; + rx_sync_state(); + } else { + uart_rx_read_pos = (uart_rx_read_pos + count) & UART_RX_BUFFER_MASK; + } } //--------------------------------------------------------------------+ -// TX Ring Buffer (Non-blocking IRQ-based transmission) +// Forward declarations for the shared TX queue. //--------------------------------------------------------------------+ - -static uart_tx_buffer_t g_uart_tx_buffer; -static volatile uint32_t g_uart_tx_dropped = 0; - -// Forward declaration for DMA TX completion handler -static void __not_in_flash_func(on_dma_tx_complete)(void); +static void on_uart_irq(void); +static void tx_service_locked(void); //--------------------------------------------------------------------+ // Connection State @@ -176,95 +230,92 @@ static uint32_t fast_cmd_count = 0; static uint32_t fast_cmd_errors = 0; //--------------------------------------------------------------------+ -// DMA TX Completion IRQ Handler +// TX queue service. Call with tx_lock held; never wait for the peripheral. //--------------------------------------------------------------------+ - -static void __not_in_flash_func(on_dma_tx_complete)(void) { - // Clear the interrupt - if (uart_tx_dma_chan >= 0) { - dma_hw->ints0 = (1u << uart_tx_dma_chan); +static void tx_service_locked(void) { + if (uart_tx_dma_chan < 0) { + uart_set_irq_enables(KMBOX_UART, false, tx_head != tx_tail); + return; + } + if (tx_dma_active) { + if (dma_channel_is_busy(uart_tx_dma_chan)) return; + tx_dma_active = false; + tx_tail = (tx_tail + 1) & UART_TX_RECORD_MASK; + tx_offset = 0; } - dma_tx_busy = false; + if (tx_head == tx_tail) return; + uart_tx_record_t *record = &tx_records[tx_tail]; + dma_channel_set_read_addr(uart_tx_dma_chan, record->data + tx_offset, false); + tx_dma_active = true; + __dmb(); // Publish slot contents to the DMA bus master before triggering. + dma_channel_set_trans_count(uart_tx_dma_chan, record->len - tx_offset, true); } -//--------------------------------------------------------------------+ -// UART TX IRQ Handler (fallback before DMA TX is initialized) -//--------------------------------------------------------------------+ +static void tx_service(void) { + if (!tx_lock) return; + uint32_t save = spin_lock_blocking(tx_lock); + tx_service_locked(); + spin_unlock(tx_lock, save); +} static void __not_in_flash_func(on_uart_irq)(void) { - // Handle TX (only used before DMA TX is initialized) - if (uart_is_writable(KMBOX_UART) && g_uart_tx_buffer.tx_active) { - while (uart_is_writable(KMBOX_UART) && tx_buffer_has_data(&g_uart_tx_buffer)) { - int byte = tx_buffer_get(&g_uart_tx_buffer); - if (byte >= 0) { - uart_putc_raw(KMBOX_UART, (uint8_t)byte); - } else { - break; + uint32_t save = spin_lock_blocking(tx_lock); + if (uart_tx_dma_chan < 0) { + while (tx_head != tx_tail && uart_is_writable(KMBOX_UART)) { + uart_tx_record_t *record = &tx_records[tx_tail]; + uart_putc_raw(KMBOX_UART, record->data[tx_offset++]); + if (tx_offset == record->len) { + tx_offset = 0; + tx_tail = (tx_tail + 1) & UART_TX_RECORD_MASK; } } - if (!tx_buffer_has_data(&g_uart_tx_buffer)) { - uart_set_irq_enables(KMBOX_UART, false, false); - g_uart_tx_buffer.tx_active = false; - } + uart_set_irq_enables(KMBOX_UART, false, tx_head != tx_tail); } + spin_unlock(tx_lock, save); } -// Non-blocking TX: Use DMA if available, fall back to IRQ ring buffer -static bool uart_send_bytes(const uint8_t *data, size_t len) { - if (!data || len == 0) return true; - - // DMA TX path: zero-CPU, zero-wait transmission - if (uart_tx_dma_chan >= 0) { - // Non-blocking: if DMA is still sending the previous packet, drop this one. - // At 3Mbaud, an 8-byte packet takes ~27µs — the previous DMA is almost - // always done by the next main loop iteration. The old 500µs spin-wait - // blocked tud_task() and hid_device_task(), adding up to 500µs of input latency. - // Bridge retries on missed responses, so dropping is safe. - if (dma_tx_busy) { - g_uart_tx_dropped += len; - return false; - } - - // Copy data to DMA TX staging buffer - size_t to_send = (len > DMA_TX_BUFFER_SIZE) ? DMA_TX_BUFFER_SIZE : len; - memcpy(dma_tx_buffer, data, to_send); - dma_tx_len = to_send; - dma_tx_busy = true; - - // Fire DMA transfer - dma_channel_set_read_addr(uart_tx_dma_chan, dma_tx_buffer, false); - dma_channel_set_trans_count(uart_tx_dma_chan, to_send, true); - - if (to_send < len) { - g_uart_tx_dropped += (len - to_send); - return false; - } - return true; +// Enqueue all bytes or none. CRLF is part of the same record as its body. +static bool uart_enqueue_record(const uint8_t *data, size_t len, bool crlf) { + size_t suffix = crlf ? 2u : 0u; + if (!tx_lock) return false; + uint32_t save = spin_lock_blocking(tx_lock); + if (!data || len > UART_TX_RECORD_SIZE - suffix) { + g_uart_tx_dropped += (uint32_t)len + (uint32_t)suffix; + spin_unlock(tx_lock, save); + return false; } - - // Fallback: IRQ-based ring buffer TX (before DMA is initialized) - int uart_irq = (KMBOX_UART == uart0) ? UART0_IRQ : UART1_IRQ; - irq_set_enabled(uart_irq, false); - - uint16_t written = tx_buffer_write(&g_uart_tx_buffer, data, len); - - if (written > 0 && !g_uart_tx_buffer.tx_active) { - g_uart_tx_buffer.tx_active = true; - uart_set_irq_enables(KMBOX_UART, false, true); + if (len + suffix == 0) { + spin_unlock(tx_lock, save); + return true; } - - irq_set_enabled(uart_irq, true); - - if (written < len) { - g_uart_tx_dropped += (len - written); + // Reclaim a completed DMA record before deciding whether the queue is full. + tx_service_locked(); + uint8_t next = (tx_head + 1) & UART_TX_RECORD_MASK; + if (next == tx_tail) { + g_uart_tx_dropped += (uint32_t)len + (uint32_t)suffix; + spin_unlock(tx_lock, save); return false; } + uart_tx_record_t *record = &tx_records[tx_head]; + memcpy(record->data, data, len); + if (crlf) { + record->data[len] = '\r'; + record->data[len + 1] = '\n'; + } + record->len = (uint16_t)(len + suffix); + tx_head = next; + tx_service_locked(); + spin_unlock(tx_lock, save); return true; } -static bool uart_send_string(const char *str) { - if (!str) return true; - return uart_send_bytes((const uint8_t*)str, strlen(str)); +static bool uart_send_bytes(const uint8_t *data, size_t len) { + if (len == 0) return true; + return uart_enqueue_record(data, len, false); +} + +static bool command_reply_write(const char *data, size_t len) { + return uart_send_bytes((const uint8_t *)data, len); } //--------------------------------------------------------------------+ @@ -273,9 +324,7 @@ static bool uart_send_string(const char *str) { void kmbox_send_response(const char *response) { if (!response) return; - uart_send_string(response); - uart_send_string("\r\n"); - // Non-blocking: TX IRQ handles actual transmission + uart_enqueue_record((const uint8_t *)response, strlen(response), true); } void kmbox_send_status(const char *message) { @@ -290,7 +339,11 @@ void kmbox_send_ping_to_bridge(void) { // Get TX buffer stats (for debugging) uint32_t kmbox_get_tx_dropped_bytes(void) { - return g_uart_tx_dropped; + if (!tx_lock) return 0; + uint32_t save = spin_lock_blocking(tx_lock); + uint32_t dropped = g_uart_tx_dropped; + spin_unlock(tx_lock, save); + return dropped; } //--------------------------------------------------------------------+ @@ -400,7 +453,8 @@ static const uint32_t g_fast_cmd_bitmap[8] = { // 0x01 MOUSE_MOVE, 0x02 MOUSE_CLICK, 0x07 SMOOTH_MOVE, 0x08 SMOOTH_CONFIG, // 0x09 SMOOTH_CLEAR, 0x0B MULTI_MOVE, 0x0C KEY_COMBO, 0x0E INFO_EXT, 0x0F CYCLE_HUMAN (1u << 0x01) | (1u << 0x02) | (1u << 0x07) | (1u << 0x08) | - (1u << 0x09) | (1u << 0x0B) | (1u << 0x0C) | (1u << 0x0E) | (1u << 0x0F), + (1u << 0x09) | (1u << 0x0B) | (1u << 0x0C) | (1u << 0x0E) | (1u << 0x0F) | + (1u << FAST_CMD_SET_HUMAN) | (1u << FAST_CMD_INFO_REQUEST), // Bits 32-63: Xbox commands 0x20-0x28 (1u << (0x20 - 32)) | (1u << (0x22 - 32)) | (1u << (0x23 - 32)) | (1u << (0x27 - 32)) | (1u << (0x28 - 32)), @@ -421,7 +475,7 @@ static bool __not_in_flash_func(process_fast_command)(const uint8_t *pkt) { // - Skips neopixel flash (too expensive for high-rate moves) if (__builtin_expect(pkt[0] == FAST_CMD_MOUSE_MOVE, 1)) { const fast_cmd_move_t *m = (const fast_cmd_move_t *)pkt; - if (m->buttons) kmbox_update_physical_buttons(m->buttons & 0x1F); + kmbox_set_injected_buttons(m->buttons); if (m->x || m->y) kmbox_add_mouse_movement(m->x, m->y); if (m->wheel) kmbox_add_wheel_movement(m->wheel); // Passive LED activity — single volatile store, DMA pushes to PIO at ~30 Hz @@ -434,22 +488,22 @@ static bool __not_in_flash_func(process_fast_command)(const uint8_t *pkt) { case FAST_CMD_MOUSE_CLICK: { const fast_cmd_click_t *c = (const fast_cmd_click_t *)pkt; - // Map button number to kmbox_button_t enum + // The fast protocol carries a HID button mask, not a button number. // FAST_BTN: 1=left, 2=right, 4=middle, 8=side1, 16=side2 kmbox_button_t btn; switch (c->button) { case 0: case 1: btn = KMBOX_BUTTON_LEFT; break; case 2: btn = KMBOX_BUTTON_RIGHT; break; - case 3: btn = KMBOX_BUTTON_MIDDLE; break; - case 4: btn = KMBOX_BUTTON_SIDE1; break; - case 5: btn = KMBOX_BUTTON_SIDE2; break; - default: btn = KMBOX_BUTTON_LEFT; break; + case 4: btn = KMBOX_BUTTON_MIDDLE; break; + case 8: btn = KMBOX_BUTTON_SIDE1; break; + case 16: btn = KMBOX_BUTTON_SIDE2; break; + default: return false; } // Use timed click mechanism — press/hold/release with randomized // timing, handled by kmbox_update_states() on each main loop tick. // Only single-click is supported; the count field is ignored for now // because multi-click requires waiting for each click to complete. - uint32_t click_now = time_us_32() / 1000; + uint32_t click_now = to_ms_since_boot(get_absolute_time()); kmbox_start_button_click(btn, click_now); fast_cmd_count++; return true; @@ -558,6 +612,13 @@ static bool __not_in_flash_func(process_fast_command)(const uint8_t *pkt) { return true; } + case FAST_CMD_SET_HUMAN: + if (pkt[1] < HUMANIZATION_MODE_COUNT) { + smooth_set_humanization_mode((humanization_mode_t)pkt[1]); + } + return true; + + case FAST_CMD_INFO_REQUEST: case FAST_CMD_INFO: { uint32_t now_ms = to_ms_since_boot(get_absolute_time()); int16_t temp = get_cached_temperature(now_ms); @@ -753,9 +814,9 @@ static bool handle_text_command(const char *line, size_t len, uint32_t now_ms) { // Echo: E if (len >= 2 && line[0] == 'E') { - uart_send_string("ECHO:"); - uart_send_string(line + 1); - uart_send_string("\r\n"); + char response[KMBOX_CMD_BUFFER_SIZE + 8]; + snprintf(response, sizeof(response), "ECHO:%s", line + 1); + kmbox_send_response(response); return true; } @@ -859,40 +920,59 @@ static bool handle_text_command(const char *line, size_t len, uint32_t now_ms) { // Parse a complete line from the ring buffer static bool parse_text_line(char *buf, size_t bufsize, size_t *out_len) { + if (!buf || bufsize < 2 || !out_len) return false; + *out_len = 0; uint16_t avail = rx_available(); if (avail == 0) return false; - - // Scan for line terminator + uint16_t line_end = 0; - bool found = false; - for (uint16_t i = 0; i < avail && i < bufsize - 1; i++) { - uint8_t ch = rx_peek_at(i); - if (ch == '\n' || ch == '\r') { - line_end = i; - found = true; - break; + while (line_end < avail && rx_peek_at(line_end) != '\n' && + rx_peek_at(line_end) != '\r') line_end++; + if (line_end == avail) { + if (text_discarding || avail >= bufsize) { + // Discard an oversized line until its terminator, across task calls. + rx_consume(avail); + text_discarding = true; } + return false; } - if (!found) return false; - - // Copy line to buffer - for (uint16_t i = 0; i < line_end; i++) { - buf[i] = rx_peek_at(i); + if (!text_discarding && line_end < bufsize) { + if (!rx_copy_peek((uint8_t *)buf, line_end)) return false; + buf[line_end] = '\0'; + *out_len = line_end; } - buf[line_end] = '\0'; - *out_len = line_end; - - // Consume line + terminator(s) + text_discarding = false; uint16_t consume = line_end + 1; if (consume < avail) { uint8_t next = rx_peek_at(consume); - if (next == '\n' || next == '\r') consume++; // Handle \r\n + if (next == '\n' || next == '\r') consume++; } rx_consume(consume); - return true; } +static uint16_t process_text_commands(uint32_t now_ms, uint16_t budget) { + uint16_t processed = 0; + char line[KMBOX_CMD_BUFFER_SIZE]; + size_t len; + while (rx_available() > 0 && processed < budget) { + // Recheck after every line: a binary packet may follow text in the same + // DMA batch, and its payload may contain CR or LF. + uint8_t peek = rx_peek_at(0); + if (!text_discarding && (is_fast_cmd_start(peek) || peek == BRIDGE_SYNC_BYTE)) break; + if (!parse_text_line(line, sizeof(line), &len)) break; + processed++; + if (len > 0) { + mark_activity(now_ms); + if (!handle_text_command(line, len, now_ms)) { + kmbox_process_serial_line(line, len, "\n", 1, now_ms); + } + } + } + + return processed; +} + //--------------------------------------------------------------------+ // Initialization //--------------------------------------------------------------------+ @@ -907,9 +987,17 @@ void kmbox_serial_init(void) { // Initialize state uart_rx_read_pos = 0; - - // Initialize TX ring buffer - tx_buffer_init(&g_uart_tx_buffer); + uart_rx_poll_write_pos = 0; + text_discarding = false; + uart_rx_reported_epoch = 0; + uart_rx_loss_pending = false; + memset(&uart_rx_state, 0, sizeof(uart_rx_state)); + + // Claim once for firmware lifetime; both USB cores may enqueue status text. + if (!tx_lock) tx_lock = spin_lock_init((unsigned)spin_lock_claim_unused(true)); + tx_head = tx_tail = 0; + tx_offset = 0; + tx_dma_active = false; g_uart_tx_dropped = 0; g_connection_state = BRIDGE_STATE_WAITING; @@ -945,6 +1033,7 @@ void kmbox_serial_init(void) { // Initialize kmbox commands library kmbox_commands_init(); + kmbox_set_output_callback(command_reply_write); kmbox_update_states(g_last_data_time_ms); // Set initial LED @@ -970,15 +1059,19 @@ void kmbox_serial_init_dma(void) { channel_config_set_write_increment(&rx_cfg, true); // Increment write pointer channel_config_set_dreq(&rx_cfg, uart_get_dreq(KMBOX_UART, false)); // RX DREQ channel_config_set_ring(&rx_cfg, true, __builtin_ctz(UART_RX_BUFFER_SIZE)); // Ring on write side + channel_config_set_irq_quiet(&rx_cfg, true); channel_config_set_high_priority(&rx_cfg, true); // High priority for low latency - // Start continuous RX DMA - transfers indefinitely into circular buffer + dma_rx_init(&uart_rx_state, UART_RX_BUFFER_MASK, uart_rx_read_pos, + uart_rx_poll_write_pos, KMBOX_UART_BAUDRATE, time_us_64()); + uart_rx_state.fifo_status = &uart_get_hw(KMBOX_UART)->fr; + // Self-trigger mode exposes byte progress while reloading without CPU rearming. dma_channel_configure( uart_rx_dma_chan, &rx_cfg, - uart_rx_buffer, // Write to ring buffer + uart_rx_buffer + uart_rx_poll_write_pos, // Preserve unread polled bytes &uart_get_hw(KMBOX_UART)->dr, // Read from UART data register - 0xFFFFFFFF, // Transfer "forever" + dma_encode_transfer_count_with_self_trigger(DMA_RX_TRANSFER_COUNT), true // Start immediately ); @@ -987,13 +1080,13 @@ void kmbox_serial_init_dma(void) { //--------------------------------------------------------------------+ // DMA TX: Zero-CPU UART transmission //--------------------------------------------------------------------+ - uart_tx_dma_chan = dma_claim_unused_channel(false); - if (uart_tx_dma_chan < 0) { + int new_tx_dma_chan = dma_claim_unused_channel(false); + if (new_tx_dma_chan < 0) { printf("[KMBOX] WARNING: Failed to claim TX DMA channel, using IRQ fallback\n"); return; } - dma_channel_config tx_cfg = dma_channel_get_default_config(uart_tx_dma_chan); + dma_channel_config tx_cfg = dma_channel_get_default_config(new_tx_dma_chan); channel_config_set_transfer_data_size(&tx_cfg, DMA_SIZE_8); channel_config_set_read_increment(&tx_cfg, true); // Increment through TX buffer channel_config_set_write_increment(&tx_cfg, false); // Always write to UART DR @@ -1002,25 +1095,44 @@ void kmbox_serial_init_dma(void) { // Configure channel but don't start yet (will be triggered per-transfer) dma_channel_configure( - uart_tx_dma_chan, + new_tx_dma_chan, &tx_cfg, &uart_get_hw(KMBOX_UART)->dr, // Write to UART data register - dma_tx_buffer, // Read from TX staging buffer (updated per-transfer) + NULL, // Set from the queued record at submission 0, // Transfer count set per-transfer false // Don't start yet ); - // Set up DMA TX completion interrupt - dma_channel_set_irq0_enabled(uart_tx_dma_chan, true); - irq_set_exclusive_handler(DMA_IRQ_0, on_dma_tx_complete); - irq_set_priority(DMA_IRQ_0, 1); // High priority (just below UART) - irq_set_enabled(DMA_IRQ_0, true); - - // Disable UART TX IRQ since DMA handles TX now + // Switch ownership under the same lock as the IRQ consumer. If a record is + // partly written, DMA resumes at tx_offset after the bytes already in FIFO. + uint32_t save = spin_lock_blocking(tx_lock); uart_set_irq_enables(KMBOX_UART, false, false); - g_uart_tx_buffer.tx_active = false; - - printf("[KMBOX] DMA TX initialized: ch=%d, buf=%u bytes\n", uart_tx_dma_chan, DMA_TX_BUFFER_SIZE); + uart_tx_dma_chan = new_tx_dma_chan; + tx_service_locked(); + spin_unlock(tx_lock, save); + +} + +// Called on Core 0 after RX polling has reported a discontinuity. +static void rx_handle_loss(void) { + if (!uart_rx_loss_pending) return; + uart_rx_loss_pending = false; + kmbox_set_injected_buttons(0); + smooth_clear_queue(); + if (g_timed_move_alarm_id >= 0) { + cancel_alarm(g_timed_move_alarm_id); + g_timed_move_alarm_id = -1; + } + pending_timed_move.pending = false; + pending_timed_move.fired = false; + if (g_xbox_mode) xbox_inject_clear(); +} + +void kmbox_get_rx_loss_stats(uint32_t *overruns, uint64_t *discarded_bytes, + uint32_t *ambiguous_polls) { + if (overruns) *overruns = uart_rx_state.overruns; + if (discarded_bytes) *discarded_bytes = uart_rx_state.discarded_bytes; + if (ambiguous_polls) *ambiguous_polls = uart_rx_state.ambiguous_polls; } //--------------------------------------------------------------------+ @@ -1028,10 +1140,11 @@ void kmbox_serial_init_dma(void) { //--------------------------------------------------------------------+ void kmbox_serial_task(void) { - // Use cheap microsecond timer — avoid to_ms_since_boot(get_absolute_time()) - // which has significant overhead. Convert to ms only when needed. - uint32_t now_us = time_us_32(); - uint32_t now_ms = now_us / 1000; + tx_service(); + rx_handle_loss(); + // Share the boot-time millisecond clock with click/report processing. + // Dividing time_us_32() first would reset this clock every ~71 minutes. + uint32_t now_ms = to_ms_since_boot(get_absolute_time()); // Periodic ping to bridge (every 5 seconds) if (now_ms - g_last_ping_time_ms > 5000) { @@ -1053,6 +1166,7 @@ void kmbox_serial_task(void) { const uint16_t MAX_PACKETS_PER_CALL = 64; while (rx_available() > 0 && packets_processed < MAX_PACKETS_PER_CALL) { + if (text_discarding) break; uint8_t first = rx_peek_at(0); //-------------------------------------------------------------- @@ -1064,18 +1178,8 @@ void kmbox_serial_task(void) { uint8_t pkt[8]; size_t copy = (avail < 8) ? avail : 8; - // Optimized peek - use memcpy if data is contiguous - uint16_t tail = (uint16_t)(uart_rx_read_pos & UART_RX_BUFFER_MASK); - uint16_t first_chunk = UART_RX_BUFFER_SIZE - tail; - if (first_chunk >= copy) { - // Data is contiguous, use fast memcpy - memcpy(pkt, &uart_rx_buffer[tail], copy); - } else { - // Data wraps around, copy in two chunks - memcpy(pkt, &uart_rx_buffer[tail], first_chunk); - memcpy(pkt + first_chunk, uart_rx_buffer, copy - first_chunk); - } - + if (!rx_copy_peek(pkt, copy)) break; + uint8_t consumed = process_bridge_packet(pkt, copy); if (consumed >= 2) { rx_consume(consumed); @@ -1090,24 +1194,13 @@ void kmbox_serial_task(void) { } //-------------------------------------------------------------- - // Fast binary commands — zero-copy when data is contiguous + // Fast binary commands — copy and validate before executing. //-------------------------------------------------------------- if (is_fast_cmd_start(first)) { if (rx_available() >= 8) { - uint16_t tail = (uint16_t)(uart_rx_read_pos & UART_RX_BUFFER_MASK); - const uint8_t *pkt; - uint8_t pkt_stack[8]; - - // Zero-copy: if all 8 bytes are contiguous in the circular buffer, - // pass a pointer directly into the DMA buffer (no memcpy) - if (tail + 8 <= UART_RX_BUFFER_SIZE) { - pkt = &uart_rx_buffer[tail]; - } else { - // Wraps around — must copy to stack (rare at 512-byte buffer) - for (int i = 0; i < 8; i++) pkt_stack[i] = rx_peek_at(i); - pkt = pkt_stack; - } - + uint8_t pkt[8]; + if (!rx_copy_peek(pkt, sizeof(pkt))) break; + rx_consume(8); process_fast_command(pkt); mark_activity(now_ms); @@ -1148,27 +1241,12 @@ void kmbox_serial_task(void) { // packet. The remaining bytes arrive within ~27µs at 3 Mbaud, so the // next kmbox_serial_task() call will process the completed packet. //------------------------------------------------------------------ - bool buffer_starts_with_binary = false; - if (rx_available() > 0) { - uint8_t peek = rx_peek_at(0); - buffer_starts_with_binary = is_fast_cmd_start(peek) || - peek == BRIDGE_SYNC_BYTE; - } + packets_processed += process_text_commands(now_ms, MAX_PACKETS_PER_CALL - packets_processed); - char line[KMBOX_CMD_BUFFER_SIZE]; - size_t len; - while (!buffer_starts_with_binary && parse_text_line(line, sizeof(line), &len)) { - if (len > 0) { - mark_activity(now_ms); - if (!handle_text_command(line, len, now_ms)) { - // Pass to kmbox library for additional parsing - kmbox_process_serial_line(line, len, "\n", 1, now_ms); - } - } - } - + rx_handle_loss(); // Update button states kmbox_update_states(now_ms); + tx_service(); } //--------------------------------------------------------------------+ diff --git a/kmbox_serial_handler.h b/kmbox_serial_handler.h index b1da342..dd1b4e5 100644 --- a/kmbox_serial_handler.h +++ b/kmbox_serial_handler.h @@ -82,5 +82,8 @@ void kmbox_send_xbox_status_to_bridge(void); // Get TX buffer stats (for debugging/monitoring) uint32_t kmbox_get_tx_dropped_bytes(void); +// Core 0 only. Discarded bytes are a lower bound if ambiguous_polls is nonzero. +void kmbox_get_rx_loss_stats(uint32_t *overruns, uint64_t *discarded_bytes, + uint32_t *ambiguous_polls); #endif // KMBOX_SERIAL_HANDLER_H \ No newline at end of file diff --git a/led_control.c b/led_control.c index 628ce49..0d5ec2e 100644 --- a/led_control.c +++ b/led_control.c @@ -129,14 +129,14 @@ static bool led_refresh_timer_callback(struct repeating_timer *t) // If an activity flash is pending, check expiration if (s_led_activity_pending) { - if (time_us_32() >= s_led_activity_expire_us) { + if ((int32_t)(time_us_32() - s_led_activity_expire_us) >= 0) { // Activity flash expired — clear it so status task resumes normal color s_led_activity_pending = false; } } - // Only DMA-push if there's something in the shadow register and channel is idle - if (s_led_shadow_grb != 0 && s_led_dma_chan >= 0 && !dma_channel_is_busy(s_led_dma_chan)) { + // Push black as well as lit colors, so an OFF state actually reaches the LED. + if (s_led_dma_chan >= 0 && !dma_channel_is_busy(s_led_dma_chan)) { // Re-arm transfer count to 1 before triggering — after the previous // transfer completes, trans_count is 0 and triggering does nothing. dma_channel_set_trans_count(s_led_dma_chan, 1, false); @@ -160,8 +160,8 @@ static void led_dma_refresh_init(void) channel_config_set_transfer_data_size(&c, DMA_SIZE_32); channel_config_set_read_increment(&c, false); // always read from shadow register channel_config_set_write_increment(&c, false); // always write to same FIFO addr - // Use DREQ_FORCE so the transfer completes immediately (we gate it via the timer) - channel_config_set_dreq(&c, DREQ_FORCE); + // Pace writes from the PIO FIFO so a full FIFO cannot lose an LED update. + channel_config_set_dreq(&c, pio_get_dreq(g_led_controller.pio_instance, g_led_controller.state_machine, true)); dma_channel_configure( s_led_dma_chan, diff --git a/lib/dma-uart/CMakeLists.txt b/lib/dma-uart/CMakeLists.txt index 8ba708b..02eb6c2 100644 --- a/lib/dma-uart/CMakeLists.txt +++ b/lib/dma-uart/CMakeLists.txt @@ -1,3 +1,3 @@ add_library(dma_uart INTERFACE) target_include_directories(dma_uart INTERFACE include) -target_link_libraries(dma_uart INTERFACE hardware_dma) +target_link_libraries(dma_uart INTERFACE hardware_dma hardware_sync pico_time) diff --git a/lib/dma-uart/include/dma_uart.h b/lib/dma-uart/include/dma_uart.h index ffbbc05..64cf3a3 100644 --- a/lib/dma-uart/include/dma_uart.h +++ b/lib/dma-uart/include/dma_uart.h @@ -2,9 +2,9 @@ * DMA Ring Buffer Helpers for UART RX * * Shared inline utilities for reading from a DMA-driven circular receive - * buffer. The DMA channel writes continuously into a power-of-2 aligned - * buffer using ring mode; these helpers inspect the DMA write pointer to - * determine how many bytes are available and read them out. + * buffer. RP2350 self-trigger mode exposes a decrementing byte counter while + * continuously reloading. The tracker distinguishes complete buffer laps, + * validates copies, and discards damaged input until an idle gap. * * Used by both the KMBox serial handler and the bridge HW UART driver. */ @@ -13,9 +13,136 @@ #define DMA_UART_H #include +#include #include #include #include "hardware/dma.h" +#include "hardware/regs/uart.h" +#include "hardware/sync.h" +#include "pico/time.h" + +// RP2350 self-trigger mode reloads this count while retaining ring addresses. +// A power of two makes count reloads and 32-bit software totals wrap safely. +#define DMA_RX_TRANSFER_COUNT (1u << 27) +#define DMA_RX_TRANSFER_MASK (DMA_RX_TRANSFER_COUNT - 1u) +#define DMA_RX_IDLE_US 1000u + +typedef struct { + uint32_t produced; + uint32_t consumed; + uint32_t remaining; + uint32_t mask; + uint32_t loss_epoch; + uint32_t overruns; + uint32_t ambiguous_polls; + uint64_t discarded_bytes; // Known discarded bytes; excludes unknown count laps. + uint64_t sampled_us; + uint64_t progress_us; + uint64_t max_poll_gap_us; + volatile const uint32_t *fifo_status; + bool resync; + bool sample_valid; +} dma_rx_state_t; + +static inline void dma_rx_init(dma_rx_state_t *s, uint32_t mask, + uint32_t read_pos, uint32_t write_pos, + uint32_t baud, uint64_t now_us) { + memset(s, 0, sizeof(*s)); + s->mask = mask; + s->consumed = read_pos; + s->produced = read_pos + ((write_pos - read_pos) & mask); + s->remaining = 0; // TRANSFER_COUNT & TRANSFER_MASK at the initial reload. + s->sampled_us = s->progress_us = now_us; + // Conservative upper bound: eight bit times per received byte, plus room + // for a full UART FIFO. A stale sample cannot silently alias a count lap. + s->max_poll_gap_us = baud ? ((uint64_t)(DMA_RX_TRANSFER_COUNT - 64u) * 8000000u) / baud : 0; +} + +/** Sample a live count, retaining absolute unread length across ring laps. */ +static inline uint32_t dma_rx_update(dma_rx_state_t *s, uint32_t remaining, + uint64_t now_us) { + s->sample_valid = true; + remaining &= DMA_RX_TRANSFER_MASK; // Strip mode bits and normalize reload/zero. + uint32_t delta = (s->remaining - remaining) & DMA_RX_TRANSFER_MASK; + bool ambiguous = now_us - s->sampled_us >= s->max_poll_gap_us; + s->remaining = remaining; + s->sampled_us = now_us; + s->produced += delta; + if (delta) s->progress_us = now_us; + uint32_t unread = s->produced - s->consumed; + // Reserve a slot for a possibly in-flight DMA write. Full is never empty. + if (ambiguous || unread > s->mask) { + s->overruns++; + if (ambiguous) s->ambiguous_polls++; + s->loss_epoch++; + s->resync = true; + s->progress_us = now_us; + } + if (s->resync) { + s->discarded_bytes += unread; + s->consumed = s->produced; + if (!delta && now_us - s->progress_us >= DMA_RX_IDLE_US) s->resync = false; + return 0; + } + return unread; +} + +static inline uint32_t dma_rx_poll(dma_rx_state_t *s, int channel) { + // WRITE_ADDR updates on completed writes. Match it to the count so an + // in-flight bus beat or a torn snapshot cannot publish unwritten bytes. + // Buffers must be aligned to mask+1, as required by DMA ring addressing. + for (unsigned attempt = 0; attempt < 3; attempt++) { + uint64_t before = time_us_64(); + uint32_t remaining = dma_channel_hw_addr(channel)->transfer_count & DMA_RX_TRANSFER_MASK; + uintptr_t wa = dma_channel_hw_addr(channel)->write_addr; + bool fifo_pending = s->fifo_status && !(*s->fifo_status & UART_UARTFR_RXFE_BITS); + uint64_t after = time_us_64(); + if ((dma_channel_hw_addr(channel)->transfer_count & DMA_RX_TRANSFER_MASK) != remaining) continue; + uint32_t delta = (s->remaining - remaining) & DMA_RX_TRANSFER_MASK; + if ((wa & s->mask) != ((s->produced + delta) & s->mask)) continue; + if (fifo_pending) s->progress_us = after; + __dmb(); + uint32_t avail = dma_rx_update(s, remaining, after); + // Bracket the reads: a scheduling stall on either side must not hide a lap. + s->sampled_us = before; + return avail; + } + s->sample_valid = false; + return 0; // Retry next task pass rather than spin or publish uncertain data. +} + +/** Copy without consuming. Recheck after copying so a concurrent lap is rejected. */ +static inline bool dma_rx_peek_copy(dma_rx_state_t *s, int channel, + const uint8_t *buffer, uint8_t *dest, + size_t offset, size_t len) { + uint32_t avail = dma_rx_poll(s, channel); + if (!s->sample_valid || s->resync || offset > avail || len > avail - offset) return false; + uint32_t start = s->consumed; + uint32_t epoch = s->loss_epoch; + if (len) { + uint32_t pos = (start + (uint32_t)offset) & s->mask; + size_t first = s->mask + 1u - pos; + if (first > len) first = len; + memcpy(dest, buffer + pos, first); + if (len > first) memcpy(dest + first, buffer, len - first); + } + __dmb(); + dma_rx_poll(s, channel); + return s->sample_valid && !s->resync && s->loss_epoch == epoch && s->consumed == start; +} + +static inline size_t dma_rx_read(dma_rx_state_t *s, int channel, + const uint8_t *buffer, uint8_t *dest, size_t maxlen) { + if (!dest || !maxlen) return 0; + uint32_t avail = dma_rx_poll(s, channel); + size_t len = avail < maxlen ? avail : maxlen; + if (!len || !dma_rx_peek_copy(s, channel, buffer, dest, 0, len)) return 0; + s->consumed += (uint32_t)len; + return len; +} + +// Legacy index-only helpers below cannot detect producer laps. Receive paths +// must use the tracked API above; these remain for index arithmetic compatibility. /** Get the current DMA write position in the ring buffer. */ static inline uint32_t dma_ring_write_pos(int dma_chan, diff --git a/lib/fast-protocol/include/fast_protocol.h b/lib/fast-protocol/include/fast_protocol.h index 73c468f..d8f1d41 100644 --- a/lib/fast-protocol/include/fast_protocol.h +++ b/lib/fast-protocol/include/fast_protocol.h @@ -35,6 +35,8 @@ #define FAST_CMD_KEY_PRESS 0x0C // Single key press (alias for KEY_COMBO) #define FAST_CMD_INFO 0x0D // Request system info #define FAST_CMD_INFO_EXT 0x0E // Request extended stats +#define FAST_CMD_SET_HUMAN 0x10 // [cmd][mode][pad*6], mode 0..2 +#define FAST_CMD_INFO_REQUEST 0x11 // Text-safe request; response remains FAST_CMD_INFO #define FAST_CMD_CYCLE_HUMAN 0x0F // Cycle humanization mode #define FAST_CMD_XBOX_INPUT 0x20 // Xbox gamepad: buttons + triggers #define FAST_CMD_XBOX_STICK_L 0x22 // Xbox left stick X/Y @@ -193,4 +195,26 @@ static inline size_t fast_build_mouse_click(uint8_t *buf, uint8_t button, uint8_ return FAST_CMD_PACKET_SIZE; } +// Shared builders for fixed-size control packets. +static inline size_t fast_build_ping(uint8_t *buf) { + memset(buf, 0, FAST_CMD_PACKET_SIZE); + buf[0] = FAST_CMD_PING; + return FAST_CMD_PACKET_SIZE; +} + +static inline size_t fast_build_set_human(uint8_t *buf, uint8_t mode) { + memset(buf, 0, FAST_CMD_PACKET_SIZE); + buf[0] = FAST_CMD_SET_HUMAN; + buf[1] = mode; + return FAST_CMD_PACKET_SIZE; +} + +static inline size_t fast_build_smooth_config(uint8_t *buf, uint8_t max_per_frame, uint8_t vel_match) { + memset(buf, 0, FAST_CMD_PACKET_SIZE); + buf[0] = FAST_CMD_SMOOTH_CONFIG; + buf[1] = max_per_frame; + buf[2] = vel_match; + return FAST_CMD_PACKET_SIZE; +} + #endif // FAST_PROTOCOL_H diff --git a/lib/kmbox-commands/kmbox_commands.c b/lib/kmbox-commands/kmbox_commands.c index f93b726..6ca4d46 100644 --- a/lib/kmbox-commands/kmbox_commands.c +++ b/lib/kmbox-commands/kmbox_commands.c @@ -8,15 +8,35 @@ #include #include #include +#include #include "hardware/sync.h" +// Protocol replies use a dedicated sink; diagnostic printf stays on stdio. +static kmbox_output_callback_t output_callback; + +void kmbox_set_output_callback(kmbox_output_callback_t callback) +{ + output_callback = callback; +} + +static bool reply_printf(const char *format, ...) +{ + char buffer[256]; + va_list args; + va_start(args, format); + int len = vsnprintf(buffer, sizeof(buffer), format, args); + va_end(args); + if (len < 0 || (size_t)len >= sizeof(buffer)) return false; + if (output_callback) return output_callback(buffer, (size_t)len); + return fwrite(buffer, 1, (size_t)len, stdout) == (size_t)len; +} + //--------------------------------------------------------------------+ // Hardware Spinlock for Cross-Core Accumulator Safety // Core0 (bridge fast commands + hid_device_task) and Core1 (physical mouse // forwarding) both read/write the mouse accumulators. A hardware spinlock // ensures atomic read-modify-write without disabling interrupts globally. //--------------------------------------------------------------------+ -#define KMBOX_ACCUMULATOR_SPINLOCK_NUM 16 // Use spinlock 16 (0-15 reserved by SDK) static spin_lock_t *g_acc_spinlock; //--------------------------------------------------------------------+ @@ -53,6 +73,7 @@ static const char* lock_button_names[KMBOX_BUTTON_COUNT] = { // Static Variables //--------------------------------------------------------------------+ +static uint8_t g_injected_buttons; static kmbox_state_t g_kmbox_state; // zero-initialized by default (static storage) static kmbox_parser_t g_parser; // zero-initialized by default (static storage) @@ -186,6 +207,9 @@ static void set_button_state(kmbox_button_t button, bool pressed, uint32_t curre if (btn_state->is_forced && btn_state->is_pressed) { btn_state->is_pressed = false; btn_state->release_time = current_time_ms + get_random_release_time(); + // Zero is reserved for an indefinite press; defer this rare deadline + // by one millisecond if it lands exactly on the clock rollover. + if (btn_state->release_time == 0) btn_state->release_time = 1; btn_state->is_clicking = false; // Cancel any ongoing click } } @@ -239,7 +263,7 @@ static void send_button_state_callback(uint8_t button_state) { // Send the callback in the format: km.[button_state]\r\n>>> // where button_state is the raw character representing the bitmap - printf("km.%c\r\n>>> ", button_state); + reply_printf("km.%c\r\n>>> ", button_state); } //--------------------------------------------------------------------+ @@ -293,7 +317,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) size_t arg_len = paren_end - arg_start; if (arg_len == 0) { // No argument - return monitor state with result - printf("%d\r\n>>> ", g_kmbox_state.monitor_enabled ? 1 : 0); + reply_printf("%d\r\n>>> ", g_kmbox_state.monitor_enabled ? 1 : 0); return; } @@ -313,7 +337,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) g_kmbox_state.monitor_enabled = (state != 0); // Send the prompt - printf(">>> "); + reply_printf(">>> "); return; } @@ -357,7 +381,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) } // Output format: result then prompt - printf("%d\r\n>>> ", state); + reply_printf("%d\r\n>>> ", state); return; } @@ -379,12 +403,12 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) sum_movement_since(since, now, &sx, &sy); // Output format: (x, y) then prompt - printf("(%ld, %ld)\r\n>>> ", (long)sx, (long)sy); + reply_printf("(%ld, %ld)\r\n>>> ", (long)sx, (long)sy); return; } // Echo the command back with the original line terminator - printf("%s%.*s", cmd, g_parser.terminator_len, g_parser.command_terminator); + reply_printf("%s%.*s", cmd, g_parser.terminator_len, g_parser.command_terminator); // Check if this is a move command if ((is_km && strncmp(cmd + 3, "move(", 5) == 0) || is_alias_move) { @@ -451,7 +475,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) kmbox_add_mouse_movement(x_amount, y_amount); // Send the prompt - printf(">>> "); + reply_printf(">>> "); return; } @@ -471,7 +495,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) kmbox_add_wheel_movement((int8_t)wheel_amount); // Send the prompt - printf(">>> "); + reply_printf(">>> "); return; } @@ -489,7 +513,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) size_t arg_len = paren_end - arg_start; if (arg_len == 0) { // No argument - return lock state with result - printf("%d\r\n>>> ", g_kmbox_state.lock_mx ? 1 : 0); + reply_printf("%d\r\n>>> ", g_kmbox_state.lock_mx ? 1 : 0); return; } @@ -512,7 +536,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) g_kmbox_state.lock_mx = (state == 1); // Send the prompt - printf(">>> "); + reply_printf(">>> "); return; } @@ -530,7 +554,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) size_t arg_len = paren_end - arg_start; if (arg_len == 0) { // No argument - return lock state with result - printf("%d\r\n>>> ", g_kmbox_state.lock_my ? 1 : 0); + reply_printf("%d\r\n>>> ", g_kmbox_state.lock_my ? 1 : 0); return; } @@ -553,7 +577,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) g_kmbox_state.lock_my = (state == 1); // Send the prompt - printf(">>> "); + reply_printf(">>> "); return; } @@ -571,7 +595,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) size_t arg_len = paren_end - arg_start; if (arg_len == 0) { // No argument - return transform state - printf("(%d, %d, %d)\r\n>>> ", + reply_printf("(%d, %d, %d)\r\n>>> ", g_kmbox_state.transform_scale_x, g_kmbox_state.transform_scale_y, g_kmbox_state.transform_enabled ? 1 : 0); @@ -583,7 +607,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) if (sscanf(arg_start, "%d , %d , %d", &scale_x, &scale_y, &enabled) == 3 || sscanf(arg_start, "%d,%d,%d", &scale_x, &scale_y, &enabled) == 3) { kmbox_set_transform((int16_t)scale_x, (int16_t)scale_y, enabled != 0); - printf(">>> "); + reply_printf(">>> "); } return; } @@ -604,7 +628,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) start_button_click((kmbox_button_t)button_num, current_time_ms); // Send the prompt - printf(">>> "); + reply_printf(">>> "); return; } @@ -622,7 +646,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) size_t arg_len = paren_end - arg_start; if (arg_len == 0) { // No argument - return callback state with result - printf("%d\r\n>>> ", g_kmbox_state.button_callback_enabled ? 1 : 0); + reply_printf("%d\r\n>>> ", g_kmbox_state.button_callback_enabled ? 1 : 0); return; } @@ -645,7 +669,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) g_kmbox_state.button_callback_enabled = (state == 1); // Send the prompt - printf(">>> "); + reply_printf(">>> "); return; } @@ -687,7 +711,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) if (arg_len == 0) { // No argument - return lock state with result bool is_locked = get_button_lock(button); - printf("%d\r\n>>> ", is_locked ? 1 : 0); + reply_printf("%d\r\n>>> ", is_locked ? 1 : 0); return; } @@ -710,7 +734,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) set_button_lock(button, state == 1); // Send the prompt - printf(">>> "); + reply_printf(">>> "); return; } @@ -760,7 +784,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) if (*p == '\0') { // Getter: print current state (1 pressed, 0 released) int pressed = g_kmbox_state.buttons[button].is_pressed ? 1 : 0; - printf("%d\r\n>>> ", pressed); + reply_printf("%d\r\n>>> ", pressed); return; } } @@ -775,7 +799,7 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) set_button_state(button, state == 1, current_time_ms); // Per spec, echo the command then print the prompt only - printf(">>> "); + reply_printf(">>> "); } //--------------------------------------------------------------------+ @@ -785,8 +809,9 @@ static void parse_command(const char* cmd, uint32_t current_time_ms) void kmbox_commands_init(void) { // Claim hardware spinlock for accumulator protection - g_acc_spinlock = spin_lock_init(KMBOX_ACCUMULATOR_SPINLOCK_NUM); + if (!g_acc_spinlock) g_acc_spinlock = spin_lock_init((unsigned)spin_lock_claim_unused(true)); // Initialize state + __atomic_store_n(&g_injected_buttons, 0, __ATOMIC_RELEASE); memset(&g_kmbox_state, 0, sizeof(g_kmbox_state)); memset(&g_parser, 0, sizeof(g_parser)); @@ -936,7 +961,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Handle click sequence if (btn->is_clicking) { - if (current_time_ms >= btn->click_end_time) { + if ((int32_t)(current_time_ms - btn->click_end_time) >= 0) { // Click sequence complete, return to hardware state btn->is_clicking = false; btn->is_forced = false; @@ -946,7 +971,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Restore physical button state bool physical_pressed = (g_kmbox_state.physical_buttons & 0x01) != 0; btn->is_pressed = physical_pressed; - } else if (current_time_ms >= btn->click_release_start) { + } else if ((int32_t)(current_time_ms - btn->click_release_start) >= 0) { // In release phase of click btn->is_pressed = false; } @@ -954,7 +979,7 @@ void kmbox_update_states(uint32_t current_time_ms) } // Check if a forced release has expired (for non-click commands) else if (btn->is_forced && !btn->is_pressed && btn->release_time > 0) { - if (current_time_ms >= btn->release_time) { + if ((int32_t)(current_time_ms - btn->release_time) >= 0) { // Release time expired, button returns to physical state btn->is_forced = false; btn->release_time = 0; @@ -979,7 +1004,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Handle click sequence if (btn->is_clicking) { - if (current_time_ms >= btn->click_end_time) { + if ((int32_t)(current_time_ms - btn->click_end_time) >= 0) { // Click sequence complete, return to hardware state btn->is_clicking = false; btn->is_forced = false; @@ -989,7 +1014,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Restore physical button state bool physical_pressed = (g_kmbox_state.physical_buttons & 0x02) != 0; btn->is_pressed = physical_pressed; - } else if (current_time_ms >= btn->click_release_start) { + } else if ((int32_t)(current_time_ms - btn->click_release_start) >= 0) { // In release phase of click btn->is_pressed = false; } @@ -997,7 +1022,7 @@ void kmbox_update_states(uint32_t current_time_ms) } // Check if a forced release has expired (for non-click commands) else if (btn->is_forced && !btn->is_pressed && btn->release_time > 0) { - if (current_time_ms >= btn->release_time) { + if ((int32_t)(current_time_ms - btn->release_time) >= 0) { // Release time expired, button returns to physical state btn->is_forced = false; btn->release_time = 0; @@ -1022,7 +1047,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Handle click sequence if (btn->is_clicking) { - if (current_time_ms >= btn->click_end_time) { + if ((int32_t)(current_time_ms - btn->click_end_time) >= 0) { // Click sequence complete, return to hardware state btn->is_clicking = false; btn->is_forced = false; @@ -1032,7 +1057,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Restore physical button state bool physical_pressed = (g_kmbox_state.physical_buttons & 0x04) != 0; btn->is_pressed = physical_pressed; - } else if (current_time_ms >= btn->click_release_start) { + } else if ((int32_t)(current_time_ms - btn->click_release_start) >= 0) { // In release phase of click btn->is_pressed = false; } @@ -1040,7 +1065,7 @@ void kmbox_update_states(uint32_t current_time_ms) } // Check if a forced release has expired (for non-click commands) else if (btn->is_forced && !btn->is_pressed && btn->release_time > 0) { - if (current_time_ms >= btn->release_time) { + if ((int32_t)(current_time_ms - btn->release_time) >= 0) { // Release time expired, button returns to physical state btn->is_forced = false; btn->release_time = 0; @@ -1065,7 +1090,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Handle click sequence if (btn->is_clicking) { - if (current_time_ms >= btn->click_end_time) { + if ((int32_t)(current_time_ms - btn->click_end_time) >= 0) { // Click sequence complete, return to hardware state btn->is_clicking = false; btn->is_forced = false; @@ -1075,7 +1100,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Restore physical button state bool physical_pressed = (g_kmbox_state.physical_buttons & 0x08) != 0; btn->is_pressed = physical_pressed; - } else if (current_time_ms >= btn->click_release_start) { + } else if ((int32_t)(current_time_ms - btn->click_release_start) >= 0) { // In release phase of click btn->is_pressed = false; } @@ -1083,7 +1108,7 @@ void kmbox_update_states(uint32_t current_time_ms) } // Check if a forced release has expired (for non-click commands) else if (btn->is_forced && !btn->is_pressed && btn->release_time > 0) { - if (current_time_ms >= btn->release_time) { + if ((int32_t)(current_time_ms - btn->release_time) >= 0) { // Release time expired, button returns to physical state btn->is_forced = false; btn->release_time = 0; @@ -1108,7 +1133,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Handle click sequence if (btn->is_clicking) { - if (current_time_ms >= btn->click_end_time) { + if ((int32_t)(current_time_ms - btn->click_end_time) >= 0) { // Click sequence complete, return to hardware state btn->is_clicking = false; btn->is_forced = false; @@ -1118,7 +1143,7 @@ void kmbox_update_states(uint32_t current_time_ms) // Restore physical button state bool physical_pressed = (g_kmbox_state.physical_buttons & 0x10) != 0; btn->is_pressed = physical_pressed; - } else if (current_time_ms >= btn->click_release_start) { + } else if ((int32_t)(current_time_ms - btn->click_release_start) >= 0) { // In release phase of click btn->is_pressed = false; } @@ -1126,7 +1151,7 @@ void kmbox_update_states(uint32_t current_time_ms) } // Check if a forced release has expired (for non-click commands) else if (btn->is_forced && !btn->is_pressed && btn->release_time > 0) { - if (current_time_ms >= btn->release_time) { + if ((int32_t)(current_time_ms - btn->release_time) >= 0) { // Release time expired, button returns to physical state btn->is_forced = false; btn->release_time = 0; @@ -1170,12 +1195,7 @@ void kmbox_get_mouse_report(uint8_t* buttons, int8_t* x, int8_t* y, int8_t* whee } // Build button byte from current states - UNROLLED for performance - uint8_t button_byte = - (g_kmbox_state.buttons[KMBOX_BUTTON_LEFT].is_pressed ? 0x01 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_RIGHT].is_pressed ? 0x02 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_MIDDLE].is_pressed ? 0x04 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_SIDE1].is_pressed ? 0x08 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_SIDE2].is_pressed ? 0x10 : 0); + uint8_t button_byte = kmbox_get_current_buttons(); *buttons = button_byte; @@ -1231,12 +1251,7 @@ void kmbox_get_mouse_report_16(uint8_t* buttons, int16_t* x, int16_t* y, int8_t* } // Build button byte - uint8_t button_byte = - (g_kmbox_state.buttons[KMBOX_BUTTON_LEFT].is_pressed ? 0x01 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_RIGHT].is_pressed ? 0x02 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_MIDDLE].is_pressed ? 0x04 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_SIDE1].is_pressed ? 0x08 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_SIDE2].is_pressed ? 0x10 : 0); + uint8_t button_byte = kmbox_get_current_buttons(); *buttons = button_byte; @@ -1380,7 +1395,8 @@ bool kmbox_has_forced_buttons(void) { // Check if any button is currently forced by a bridge/kmbox command. // Unrolled for performance — KMBOX_BUTTON_COUNT is always 5. - return g_kmbox_state.buttons[KMBOX_BUTTON_LEFT].is_forced | + return (__atomic_load_n(&g_injected_buttons, __ATOMIC_ACQUIRE) != 0) | + g_kmbox_state.buttons[KMBOX_BUTTON_LEFT].is_forced | g_kmbox_state.buttons[KMBOX_BUTTON_RIGHT].is_forced | g_kmbox_state.buttons[KMBOX_BUTTON_MIDDLE].is_forced | g_kmbox_state.buttons[KMBOX_BUTTON_SIDE1].is_forced | @@ -1392,12 +1408,18 @@ void kmbox_start_button_click(kmbox_button_t button, uint32_t current_time_ms) start_button_click(button, current_time_ms); } +void kmbox_set_injected_buttons(uint8_t buttons) +{ + __atomic_store_n(&g_injected_buttons, buttons & 0x1F, __ATOMIC_RELEASE); +} + uint8_t kmbox_get_current_buttons(void) { // Return the current combined button byte (physical | forced) without // draining any accumulators. Used by hid_device_task() to detect // button-only state changes that need an immediate report. - return (g_kmbox_state.buttons[KMBOX_BUTTON_LEFT].is_pressed ? 0x01 : 0) | + return __atomic_load_n(&g_injected_buttons, __ATOMIC_ACQUIRE) | + (g_kmbox_state.buttons[KMBOX_BUTTON_LEFT].is_pressed ? 0x01 : 0) | (g_kmbox_state.buttons[KMBOX_BUTTON_RIGHT].is_pressed ? 0x02 : 0) | (g_kmbox_state.buttons[KMBOX_BUTTON_MIDDLE].is_pressed ? 0x04 : 0) | (g_kmbox_state.buttons[KMBOX_BUTTON_SIDE1].is_pressed ? 0x08 : 0) | @@ -1409,12 +1431,7 @@ bool kmbox_try_drain_mouse_16(uint8_t last_sent_buttons, int8_t *wheel, int8_t *pan) { // Build button byte (no lock needed — single-core writes) - uint8_t button_byte = - (g_kmbox_state.buttons[KMBOX_BUTTON_LEFT].is_pressed ? 0x01 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_RIGHT].is_pressed ? 0x02 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_MIDDLE].is_pressed ? 0x04 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_SIDE1].is_pressed ? 0x08 : 0) | - (g_kmbox_state.buttons[KMBOX_BUTTON_SIDE2].is_pressed ? 0x10 : 0); + uint8_t button_byte = kmbox_get_current_buttons(); *buttons = button_byte; bool buttons_changed = (button_byte != last_sent_buttons); @@ -1562,34 +1579,51 @@ int8_t kmbox_clamp_wheel_i8(int16_t value) // Command Parsing Utilities //--------------------------------------------------------------------+ -bool kmbox_parse_move_command(const char* cmd, int* x, int* y) +static const char *skip_argument_space(const char *p) { - if (!cmd || !x || !y) { - return false; - } - - // Check for km.move(x,y) format - allow optional spaces - if (strncmp(cmd, "km.move(", 8) != 0) { - return false; + while (isspace((unsigned char)*p)) p++; + return p; +} + +static const char *parse_i16_argument(const char *p, int *value) +{ + p = skip_argument_space(p); + bool negative = *p == '-'; + if (*p == '-' || *p == '+') p++; + if (!isdigit((unsigned char)*p)) return NULL; + uint32_t limit = negative ? 32768u : 32767u; + uint32_t result = 0; + while (isdigit((unsigned char)*p)) { + uint32_t digit = (uint32_t)(*p++ - '0'); + if (result > (limit - digit) / 10u) return NULL; + result = result * 10u + digit; } - - // sscanf with space in format allows optional whitespace - return sscanf(cmd, "km.move(%d , %d)", x, y) == 2 || - sscanf(cmd, "km.move(%d,%d)", x, y) == 2; + *value = negative ? -(int)result : (int)result; + return skip_argument_space(p); +} + +bool kmbox_parse_move_command(const char* cmd, int* x, int* y) +{ + if (!cmd || !x || !y || strncmp(cmd, "km.move(", 8) != 0) return false; + int parsed_x, parsed_y; + const char *p = parse_i16_argument(cmd + 8, &parsed_x); + if (!p || *p != ',') return false; + p = parse_i16_argument(p + 1, &parsed_y); + if (!p || *p != ')' || *skip_argument_space(p + 1) != '\0') return false; + *x = parsed_x; + *y = parsed_y; + return true; } bool kmbox_parse_click_command(const char* cmd, int* button) { - if (!cmd || !button) { - return false; - } - - // Check for km.click(btn) format - if (strncmp(cmd, "km.click(", 9) != 0) { - return false; - } - - return sscanf(cmd, "km.click(%d)", button) == 1; + if (!cmd || !button || strncmp(cmd, "km.click(", 9) != 0) return false; + int parsed_button; + const char *p = parse_i16_argument(cmd + 9, &parsed_button); + if (!p || *p != ')' || *skip_argument_space(p + 1) != '\0' || + parsed_button < 0 || parsed_button >= KMBOX_BUTTON_COUNT) return false; + *button = parsed_button; + return true; } bool kmbox_is_version_command(const char* cmd) diff --git a/lib/kmbox-commands/kmbox_commands.h b/lib/kmbox-commands/kmbox_commands.h index e163da0..d6950e3 100644 --- a/lib/kmbox-commands/kmbox_commands.h +++ b/lib/kmbox-commands/kmbox_commands.h @@ -95,6 +95,9 @@ typedef struct { // Initialize the kmbox commands module void kmbox_commands_init(void); +// NULL restores stdout output for hosted tools. Firmware supplies its UART queue. +typedef bool (*kmbox_output_callback_t)(const char *data, size_t len); +void kmbox_set_output_callback(kmbox_output_callback_t callback); // Process incoming serial data (call this with each received character) void kmbox_process_serial_char(char c, uint32_t current_time_ms); @@ -172,6 +175,8 @@ const char* kmbox_get_button_name(kmbox_button_t button); // Update physical button states (call this with actual hardware button states) void kmbox_update_physical_buttons(uint8_t physical_buttons); +// Absolute synthetic button mask; releasing it preserves physical holds. +void kmbox_set_injected_buttons(uint8_t buttons); // Monitor mode control (polling-based button state queries) void kmbox_set_monitor_enabled(bool enabled); diff --git a/tests/dma_rx_mock.py b/tests/dma_rx_mock.py new file mode 100644 index 0000000..fe8682a --- /dev/null +++ b/tests/dma_rx_mock.py @@ -0,0 +1,33 @@ +"""Small SDK register/time stubs shared by the DMA receive regression tests.""" +from pathlib import Path + + +def write_headers(directory): + directory = Path(directory) + (directory / 'hardware').mkdir(exist_ok=True) + (directory / 'pico').mkdir(exist_ok=True) + (directory / 'hardware/regs').mkdir(exist_ok=True) + (directory / 'hardware/regs/uart.h').write_text('#pragma once\n#define UART_UARTFR_RXFE_BITS (1u << 4)\n') + (directory / 'hardware/dma.h').write_text(''' +#pragma once +#include +static struct {uintptr_t write_addr; uint32_t transfer_count;} mock_dma; +#define dma_channel_hw_addr(channel) ((void)(channel), &mock_dma) +''') + (directory / 'hardware/sync.h').write_text(''' +#pragma once +static inline void __dmb(void) {__asm__ volatile ("" ::: "memory");} +''') + (directory / 'pico/time.h').write_text(''' +#pragma once +#include +static uint64_t mock_us; +static uint64_t mock_time_step; +static void (*mock_time_hook)(void); +static inline uint64_t time_us_64(void) { + uint64_t now = mock_us; + mock_us += mock_time_step; + if (mock_time_hook) mock_time_hook(); + return now; +} +''') diff --git a/tests/test_bridge_parsers.py b/tests/test_bridge_parsers.py new file mode 100644 index 0000000..062c85b --- /dev/null +++ b/tests/test_bridge_parsers.py @@ -0,0 +1,135 @@ +"""Host regression checks for bridge parsing and translated queue bounds.""" +from pathlib import Path +import subprocess +import tempfile + +ROOT = Path(__file__).resolve().parents[1] +TEST = r''' +#include +#include +#include +#include +#include +#include +#include "core1_translator.c" +#include "input_classifier.h" + +static jmp_buf idle_exit; +static unsigned idle_calls; +static void test_wfe(void) { + if (idle_calls++ == 0) { + uint8_t move[] = {7, 0, 9, 0}; + assert(core1_queue_makcu_frame(MAKCU_CMD_MOVE, move, sizeof(move))); + return; + } + longjmp(idle_exit, 1); +} + +#define STRESS_COUNT 100000 +static void *consume_frames(void *unused) { + (void)unused; + for (unsigned i = 0; i < STRESS_COUNT; i++) { + while (!translate_one_frame()) sched_yield(); + uint8_t packet[8]; + assert(core1_dequeue_kmbox_command(packet)); + assert(packet[1] == (uint8_t)i && packet[2] == (uint8_t)(i >> 8)); + assert(packet[3] == (uint8_t)~i && packet[4] == (uint8_t)(~i >> 8)); + } + return NULL; +} + +static void *produce_packets(void *unused) { + (void)unused; + for (unsigned i = 0; i < STRESS_COUNT; i++) { + translated_cmd_t result = {.length = 8, .result = TRANSLATE_OK}; + memcpy(result.buffer, &i, sizeof(i)); + memcpy(result.buffer + 4, &i, sizeof(i)); + while (!queue_translated_packet(&result)) sched_yield(); + } + return NULL; +} + +int main(void) { + ferrum_translated_t out; + const char raw[] = {'k','m','.','m','o','v','e','(','1',',','2',')'}; + assert(ferrum_translate_line(raw, sizeof(raw), &out)); + assert(out.length == 8 && out.buffer[1] == 1 && out.buffer[3] == 2); + // A truncated span must not read the rest of its backing string. + assert(!ferrum_translate_line("km.move(1,2)", 7, &out)); + const char *bad[] = { + "km.move(99999999999999999999999999999,0)", + "km.move(-2147483649,0)", "km.move(2147483648,0)", + "km.move(-2147483648,0)", "km.move(32768,0)", + "km.move(0,-32769)", "km.wheel(128)", "km.wheel(-129)" + }; + for (unsigned i = 0; i < sizeof(bad)/sizeof(bad[0]); i++) + assert(!ferrum_translate_line(bad[i], strlen(bad[i]), &out)); + assert(ferrum_translate_line("km.move(-32768,32767)", 21, &out)); + input_classifier_reset(); + input_classifier_record_move(INT16_MIN, 0); + assert(ic_total_magnitude == 32768); + input_classifier_record_move(INT16_MIN, INT16_MIN); + assert(ic_total_magnitude == 98304); + + translated_cmd_t result; + makcu_translate_command(MAKCU_CMD_VERSION, NULL, 0, &result); + assert(result.length > 8); + assert(!queue_translated_packet(&result)); + assert(!core1_has_kmbox_commands()); + assert(!core1_queue_makcu_frame(MAKCU_CMD_MOVE, NULL, 4)); + uint8_t move[] = {1, 0, 2, 0}; + makcu_translate_command(MAKCU_CMD_MOVE, move, sizeof(move), &result); + assert(queue_translated_packet(&result)); + uint8_t packet[8]; + assert(core1_dequeue_kmbox_command(packet)); + assert(memcmp(packet, result.buffer, 8) == 0); + assert(!core1_dequeue_kmbox_command(packet)); + + // Exercise the actual idle loop: wake, consume, then return to waiting. + if (setjmp(idle_exit) == 0) core1_main(); + assert(idle_calls == 2 && core1_dequeue_kmbox_command(packet)); + assert(packet[1] == 7 && packet[3] == 9); + + pthread_t thread; + assert(pthread_create(&thread, NULL, consume_frames, NULL) == 0); + for (unsigned i = 0; i < STRESS_COUNT; i++) { + uint8_t move[] = {(uint8_t)i, (uint8_t)(i >> 8), + (uint8_t)~i, (uint8_t)(~i >> 8)}; + while (!core1_queue_makcu_frame(MAKCU_CMD_MOVE, move, sizeof(move))) sched_yield(); + } + assert(pthread_join(thread, NULL) == 0); + + assert(pthread_create(&thread, NULL, produce_packets, NULL) == 0); + for (unsigned i = 0; i < STRESS_COUNT; i++) { + while (!core1_dequeue_kmbox_command(packet)) sched_yield(); + unsigned first, second; + memcpy(&first, packet, sizeof(first)); + memcpy(&second, packet + 4, sizeof(second)); + assert(first == i && second == i); + } + assert(pthread_join(thread, NULL) == 0); + return 0; +} +''' +with tempfile.TemporaryDirectory() as temp: + temp = Path(temp) + (temp / 'pico').mkdir() + (temp / 'pico/stdlib.h').write_text(''' +#include +#define __not_in_flash_func(x) x +static inline void __dmb(void) {} +static inline void __sev(void) {} +static void test_wfe(void); +static inline void __wfe(void) {test_wfe();} +static inline void __dsb(void) {} +''') + (temp / 'pico/sync.h').write_text('') + (temp / 'pico/multicore.h').write_text('static inline void multicore_launch_core1(void (*f)(void)) {(void)f;}') + (temp / 'test.c').write_text(TEST) + includes = [temp, ROOT / 'bridge', ROOT / 'lib/fast-protocol/include', ROOT / 'lib/hid-defs/include'] + subprocess.run(['cc', '-std=c11', '-pthread', '-fsanitize=address,undefined', + *[f'-I{p}' for p in includes], str(temp / 'test.c'), + str(ROOT / 'bridge/ferrum_translator.c'), + str(ROOT / 'bridge/makcu_translator.c'), '-o', str(temp / 'test')], check=True) + subprocess.run([str(temp / 'test')], check=True) + print('bridge parsers and queue bounds: passed') diff --git a/tests/test_bridge_protocol.py b/tests/test_bridge_protocol.py new file mode 100644 index 0000000..7c94b04 --- /dev/null +++ b/tests/test_bridge_protocol.py @@ -0,0 +1,152 @@ +"""Check fixed-size bridge commands and Feather replies split across DMA reads.""" +from pathlib import Path +import re +import subprocess +import tempfile + +ROOT = Path(__file__).resolve().parents[1] + + +def function(file, name): + source = (ROOT / file).read_text() + match = re.search(r'^static void ' + name + r'\([^;]*?\)\s*\{', source, re.M) + assert match, name + end, depth = match.end(), 1 + while depth: + depth += (source[end] == '{') - (source[end] == '}') + end += 1 + return source[match.start():end] + + +serial = (ROOT / 'kmbox_serial_handler.c').read_text() +bitmap = re.search(r'static const uint32_t g_fast_cmd_bitmap\[8\] = \{.*?\n\};', serial, re.S)[0] +click_case = serial[serial.index(' case FAST_CMD_MOUSE_CLICK: {'): + serial.index(' case FAST_CMD_KEY_COMBO: {')] +prefix = r''' +#include +#include +#include +#include +#include "fast_protocol.h" +#include "kmbox_commands.h" +static uint8_t incoming[128], forwarded[256]; +static size_t incoming_len, forwarded_len; +static uint32_t now = 1000, kmbox_last_rx_time; +static bool loss_pending; +static bool hw_uart_rx_take_loss(void) {bool lost = loss_pending; loss_pending = false; return lost;} +enum {BRIDGE_DISCONNECTED, BRIDGE_CONNECTED}; +static int bridge_conn_state; +static uint64_t get_absolute_time(void) {return now;} +static uint32_t to_ms_since_boot(uint64_t t) {return (uint32_t)t;} +static size_t hw_uart_read(uint8_t *dst, size_t capacity) { + assert(capacity >= incoming_len); + memcpy(dst, incoming, incoming_len); + size_t n = incoming_len; incoming_len = 0; return n; +} +static bool tud_cdc_connected(void) {return true;} +static void tud_cdc_write(const void *p, size_t n) { + memcpy(forwarded + forwarded_len, p, n); forwarded_len += n; +} +static void tud_cdc_write_str(const char *p) {tud_cdc_write(p, strlen(p));} +static void tud_cdc_write_flush(void) {} +static unsigned fast_cmd_count; +static kmbox_button_t clicked; +static void kmbox_start_button_click(kmbox_button_t button, uint32_t ms) { + assert(ms == now); clicked = button; +} +''' +prefix = prefix.replace('static void kmbox_start_button_click', 'void kmbox_start_button_click') +click_handler = 'static bool process_click(const uint8_t *pkt) {switch(pkt[0]) {\n' + click_case + 'default: return false;}}\n' +test = r''' +static void feed(const void *p, size_t n) { + memcpy(incoming, p, n); incoming_len = n; uart_rx_task(); +} +int main(void) { + uint8_t packet[8]; + assert(fast_build_ping(packet) == 8 && packet[0] == FAST_CMD_PING); + assert(fast_build_set_human(packet, 2) == 8 && packet[1] == 2); + assert(fast_build_smooth_config(packet, 16, 1) == 8 && packet[0] == 8 && packet[2] == 1); + const unsigned commands[] = {FAST_CMD_PING, FAST_CMD_SET_HUMAN, FAST_CMD_INFO_REQUEST, + FAST_CMD_SMOOTH_CONFIG, FAST_CMD_MOUSE_MOVE}; + for (unsigned i = 0; i < sizeof(commands)/sizeof(commands[0]); i++) + assert((g_fast_cmd_bitmap[commands[i] >> 5] >> (commands[i] & 31)) & 1); + assert(!((g_fast_cmd_bitmap['\r' >> 5] >> ('\r' & 31)) & 1)); + for (unsigned i = 0; i < KMBOX_BUTTON_COUNT; i++) { + fast_build_mouse_click(packet, (uint8_t)(1u << i), 1); + assert(process_click(packet) && clicked == (kmbox_button_t)i); + } + fast_build_mouse_click(packet, 0xFF, 1); + assert(!process_click(packet)); + const uint8_t reply[] = {FAST_CMD_RESPONSE, 0, 0x0A, 0x0D, 'k', 'm', '.', 0}; + for (size_t split = 1; split < sizeof(reply); split++) { + bridge_conn_state = BRIDGE_DISCONNECTED; + feed(reply, split); + assert(bridge_conn_state == BRIDGE_DISCONNECTED); + feed(reply + split, sizeof(reply) - split); + assert(bridge_conn_state == BRIDGE_CONNECTED && kmbox_last_rx_time == now); + } + assert(forwarded_len == 0); + feed("KMBOX_", 6); assert(forwarded_len == 0); + feed("PONG\r\n", 6); + assert(forwarded_len == 12 && memcmp(forwarded, "KMBOX_PONG\r\n", 12) == 0); + // An abandoned partial response must not eat the next complete reply. + feed(reply, 3); now += 101; + bridge_conn_state = BRIDGE_DISCONNECTED; + feed(reply, sizeof(reply)); assert(bridge_conn_state == BRIDGE_CONNECTED); + feed(reply, 3); loss_pending = true; + feed("", 0); // Even an empty read must reset partial state after DMA loss. + bridge_conn_state = BRIDGE_DISCONNECTED; + feed(reply, sizeof(reply)); assert(bridge_conn_state == BRIDGE_CONNECTED); +} +''' +with tempfile.TemporaryDirectory() as temp: + source = Path(temp) / 'test.c' + binary = Path(temp) / 'test' + source.write_text(prefix + bitmap + click_handler + function('bridge/main_feather.c', 'uart_rx_task') + test) + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', + f'-I{ROOT / "lib/fast-protocol/include"}', f'-I{ROOT / "lib/hid-defs/include"}', + f'-I{ROOT / "lib/kmbox-commands"}', + str(source), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('Fixed-size bridge protocol and fragmented replies: passed') + + metro_state = r''' +#define STATUS_BUFFER_SIZE 128 +enum {KMBOX_CONNECTED = 1}; +static char status_buffer[STATUS_BUFFER_SIZE], last_status[STATUS_BUFFER_SIZE]; +static unsigned status_buffer_idx, status_count, kmbox_response_count, kmbox_state; +static unsigned kmbox_humanization_mode, kmbox_inject_mode, kmbox_max_per_frame; +static unsigned kmbox_queue_depth, kmbox_queue_capacity, kmbox_total_injected, kmbox_queue_overflows; +static unsigned kmbox_gamepad_buttons; +static int16_t kmbox_gamepad_sticks[4]; +static float kmbox_temperature_c; +static bool kmbox_humanization_valid, kmbox_jitter_enabled, kmbox_velocity_matching; +static bool kmbox_console_mode, kmbox_console_auth_complete; +static void process_status_message(const char *msg, size_t len) { + assert(len < sizeof(last_status)); memcpy(last_status, msg, len + 1); status_count++; +} +''' + metro_test = r''' +static void feed(const void *p, size_t n) { + memcpy(incoming, p, n); incoming_len = n; uart_rx_task(); +} +int main(void) { + const uint8_t reply[] = {FAST_CMD_RESPONSE, 0, 10, 13, 'k', 'm', '.', 0}; + feed(reply, 3); loss_pending = true; feed("", 0); + feed(reply, sizeof(reply)); + assert(kmbox_response_count == 1 && forwarded_len == 8 && memcmp(forwarded, reply, 8) == 0); + feed("OLD", 3); loss_pending = true; feed("", 0); + feed("PONG\r\n", 6); + assert(status_count == 1 && strcmp(last_status, "PONG") == 0); + const uint8_t info[] = {FAST_CMD_INFO, 2, 1, 16, 4, 250, 0, 3}; + feed(info, 4); loss_pending = true; feed("", 0); feed(info, sizeof(info)); + assert(kmbox_humanization_valid && kmbox_humanization_mode == 2 && kmbox_max_per_frame == 16); + assert(kmbox_temperature_c == 25.0f); +} +''' + source.write_text(prefix + metro_state + function('bridge/main.c', 'uart_rx_task') + metro_test) + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', + f'-I{ROOT / "lib/fast-protocol/include"}', f'-I{ROOT / "lib/hid-defs/include"}', + f'-I{ROOT / "lib/kmbox-commands"}', str(source), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('Metro binary/text parser reset after DMA loss: passed') diff --git a/tests/test_display_io.py b/tests/test_display_io.py new file mode 100644 index 0000000..d810141 --- /dev/null +++ b/tests/test_display_io.py @@ -0,0 +1,188 @@ +"""Check exact TFT register byte counts and LED-off/rollover behavior.""" +from pathlib import Path +import re +import subprocess +import tempfile + +ROOT = Path(__file__).resolve().parents[1] + + +def function(file, name): + source = (ROOT / file).read_text() + match = re.search(r'^(?:static )?(?:bool|void) ' + name + r'\([^;]*?\)\s*\{', source, re.M) + assert match, name + end, depth = match.end(), 1 + while depth: + depth += (source[end] == '{') - (source[end] == '}') + end += 1 + return source[match.start():end] + + +prefix = r''' +#include +#include +#include +#include +#define TFT_SPI_DEV 0 +#define SPI_MSB_FIRST 0 +static unsigned bits = 16; +static bool selected, data_mode; +static uint8_t written[128]; +static size_t written_len; +static void select_register(void) {data_mode = false;} +static void select_data(void) {data_mode = true;} +static void select_chip(bool on) {selected = on;} +static void spi_set_format(int spi, unsigned n, int p, int h, int order) { + assert(spi == 0 && p == 0 && h == 0 && order == 0 && !selected); bits = n; +} +static void spi_write_blocking(int spi, const uint8_t *src, size_t n) { + assert(spi == 0 && bits == 8 && selected); + memcpy(written + written_len, src, n); written_len += n; +} +static void spi_read_blocking(int spi, uint8_t fill, uint8_t *dst, size_t n) { + assert(spi == 0 && fill == 0 && bits == 8 && selected && data_mode); + for (size_t i = 0; i < n; i++) dst[i] = (uint8_t)(i + 1); +} +static void sleep_us(int delay) {(void)delay;} +struct repeating_timer {int unused;}; +static uint32_t now_us, s_led_activity_expire_us; +static volatile uint32_t s_led_shadow_grb; +static bool s_led_activity_pending, dma_busy; +static int s_led_dma_chan = 1; +static unsigned pushes; +static uint32_t time_us_32(void) {return now_us;} +static bool dma_channel_is_busy(int ch) {assert(ch == 1); return dma_busy;} +static void dma_channel_set_trans_count(int ch, unsigned n, bool trigger) { + assert(ch == 1 && n == 1 && !trigger); +} +static void dma_channel_set_read_addr(int ch, const volatile void *src, bool trigger) { + assert(ch == 1 && src == &s_led_shadow_grb && trigger); pushes++; +} +''' +test = r''' +int main(void) { + const unsigned sizes[] = {0, 1, 2, 3, 15}; + for (unsigned k = 0; k < sizeof(sizes)/sizeof(sizes[0]); k++) { + unsigned n = sizes[k]; + uint8_t *src = n ? malloc(n) : NULL; + for (unsigned i = 0; i < n; i++) src[i] = (uint8_t)(i + 1); + written_len = 0; + tft_control(0x26, src, (int)n); + assert(bits == 16 && !selected && written_len == n + 1 && written[0] == 0x26); + if (n) assert(memcmp(written + 1, src, n) == 0); + written_len = 0; + tft_read(0x45, src, (int)n, 3); + assert(bits == 16 && !selected && written_len == 1); + for (unsigned i = 0; i < n; i++) assert(src[i] == i + 1); + free(src); + } + // Zero must be transmitted to turn a previously lit LED off. + s_led_shadow_grb = 0; + assert(led_refresh_timer_callback(NULL) && pushes == 1); + dma_busy = true; led_refresh_timer_callback(NULL); assert(pushes == 1); + s_led_activity_pending = true; + now_us = UINT32_MAX - 10; s_led_activity_expire_us = 20; + led_refresh_timer_callback(NULL); assert(s_led_activity_pending); + now_us = 21; + led_refresh_timer_callback(NULL); assert(!s_led_activity_pending); +} +''' +with tempfile.TemporaryDirectory() as temp: + source = Path(temp) / 'test.c' + binary = Path(temp) / 'test' + source.write_text('#include \n' + prefix + function('bridge/tft.c', 'tft_control') + + function('bridge/tft.c', 'tft_read') + + function('led_control.c', 'led_refresh_timer_callback') + test) + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', str(source), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('TFT byte transactions and LED rollover/off: passed') + + async_prefix = r''' +#include +#include +#include +#include +#define TFT_HW_ACCEL 0 +#define TFT_RAW_WIDTH 4 +#define TFT_RAW_HEIGHT 4 +#define TFT_SPI_DEV 0 +static int dma_ch_spi = 1; +static bool async_active, busy, shifter_busy, selected; +static int async_row, async_repeat; +static uint8_t frame[16]; +static uint8_t *tft_committed = frame; +static uint16_t tft_palette[16], txbuf[2][4], sent[16]; +static const uint16_t *pending; +static size_t sent_count; +static void tft_sync(void) {assert(0);} +static void tft_begin_sync(void) {assert(!selected && !busy);} +static void select_chip(bool on) {selected = on;} +static bool dma_channel_is_busy(int ch) {assert(ch == 1); return busy;} +static bool spi_is_busy(int spi) {assert(spi == 0); return shifter_busy;} +static void dma_channel_transfer_from_buffer_now(int ch, const void *src, size_t n) { + assert(ch == 1 && n == 4 && !busy && selected); + pending = src; busy = true; +} +''' + async_test = r''' +int main(void) { + for (unsigned i = 0; i < 16; i++) {frame[i] = (uint8_t)i; tft_palette[i] = (uint16_t)(100 + i);} + assert(tft_sync_poll()); + assert(tft_sync_start() && !tft_sync_start()); + for (unsigned row = 0; row < 4; row++) { + assert(!tft_sync_poll() && busy); + uint16_t snapshot[4]; memcpy(snapshot, pending, sizeof(snapshot)); + assert(!tft_sync_poll()); // Busy DMA must keep the staging row unchanged. + assert(memcmp(snapshot, pending, sizeof(snapshot)) == 0); + memcpy(sent + sent_count, pending, sizeof(snapshot)); sent_count += 4; + busy = false; + } + shifter_busy = true; + assert(!tft_sync_poll() && selected); // Keep CS until the last bits leave SPI. + shifter_busy = false; + assert(tft_sync_poll() && !selected && !async_active); + assert(sent_count == 16); + for (unsigned y = 0; y < 4; y++) + for (unsigned x = 0; x < 4; x++) + assert(sent[y * 4 + x] == tft_palette[frame[(y / TFT_SCALE) * (4 / TFT_SCALE) + x / TFT_SCALE]]); +} +''' + source.write_text(async_prefix + function('bridge/tft.c', 'tft_sync_start') + + function('bridge/tft.c', 'tft_sync_poll') + async_test) + for scale in (1, 2): + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', f'-DTFT_SCALE={scale}', + str(source), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('Cooperative TFT DMA, framebuffer scaling, and SPI completion: passed') + + flush_prefix = r''' +#include +#include +typedef struct {int unused;} repeating_timer_t; +static bool initialized = true, frame_ready, render_due, transfer_pending; +static bool dma_active, dma_done; +static unsigned renders, swaps; +static void tft_render_frame(void) {assert(!dma_active); renders++; frame_ready = true;} +static void tft_swap_buffers(void) {assert(!dma_active); swaps++;} +static bool tft_sync_start(void) {assert(!dma_active); dma_active = true; dma_done = false; return true;} +static bool tft_sync_poll(void) { + if (!dma_done) return false; + dma_active = false; return true; +} +''' + flush_test = r''' +int main(void) { + assert(tft_render_timer_callback(0) && render_due && renders == 0); + assert(!tft_display_flush() && renders == 1 && swaps == 1 && transfer_pending); + assert(tft_render_timer_callback(0)); + assert(!tft_display_flush() && renders == 1 && swaps == 1 && frame_ready); + dma_done = true; + assert(tft_display_flush() && !transfer_pending && !frame_ready); + assert(!tft_display_flush() && renders == 2 && swaps == 2); +} +''' + source.write_text(flush_prefix + function('bridge/tft_display.c', 'tft_render_timer_callback') + + function('bridge/tft_display.c', 'tft_display_flush') + flush_test) + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', str(source), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('Display IRQ scheduling and framebuffer ownership: passed') diff --git a/tests/test_dma_rx.py b/tests/test_dma_rx.py new file mode 100644 index 0000000..de8e996 --- /dev/null +++ b/tests/test_dma_rx.py @@ -0,0 +1,173 @@ +"""Model RP2350 self-trigger DMA, including laps and overwrites during copying.""" +from pathlib import Path +import re +import subprocess +import tempfile +from dma_rx_mock import write_headers + +ROOT = Path(__file__).resolve().parents[1] + + +def function(name): + source = (ROOT / 'bridge/hw_uart.c').read_text() + match = re.search(r'^(?:bool|void|size_t|int) ' + name + r'\([^;]*?\)\s*\{', source, re.M) + assert match, name + end, depth = match.end(), 1 + while depth: + depth += (source[end] == '{') - (source[end] == '}') + end += 1 + return source[match.start():end] + + +prefix = r''' +#include +#include +#include +#include +static void *copy_with_dma(void *dest, const void *src, size_t n); +#define memcpy copy_with_dma +#include "dma_uart.h" +#undef memcpy +static uint8_t rx_buffer[64] __attribute__((aligned(64))); +static dma_rx_state_t rx_state; +static int rx_dma_chan = 1; +static uint32_t rx_reported_loss_epoch, total_rx_bytes; +static uint64_t model_bytes; +static unsigned model_start, overwrite_on_copy; +static void registers(void) { + mock_dma.transfer_count = (1u << 28) | (DMA_RX_TRANSFER_COUNT - (model_bytes & DMA_RX_TRANSFER_MASK)); + mock_dma.write_addr = (uintptr_t)(rx_buffer + ((model_start + model_bytes) & 63)); +} +static void write_bytes(unsigned n, uint8_t first) { + for (unsigned i = 0; i < n; i++) rx_buffer[(model_start + model_bytes++) & 63] = (uint8_t)(first + i); + registers(); +} +static void *copy_with_dma(void *dest, const void *src, size_t n) { + memcpy(dest, src, n); + if (overwrite_on_copy) { + unsigned count = overwrite_on_copy; overwrite_on_copy = 0; + write_bytes(count, 0x80); + } + return dest; +} +static void reset(unsigned read_pos, unsigned write_pos) { + mock_us = 1000; mock_time_step = 0; mock_time_hook = NULL; model_bytes = 0; model_start = write_pos; + overwrite_on_copy = 0; rx_reported_loss_epoch = total_rx_bytes = 0; + memset(rx_buffer, 0, sizeof(rx_buffer)); + registers(); + dma_rx_init(&rx_state, 63, read_pos, write_pos, 3000000, mock_us); +} +static unsigned time_calls; +static void advance_during_snapshot(void) { + if (++time_calls == 2) { + mock_time_hook = NULL; + write_bytes(1, 77); + } +} +size_t hw_uart_rx_count(void); +size_t hw_uart_read(uint8_t *buf, size_t maxlen); +''' +test = r''' +int main(void) { + uint8_t out[64]; + reset(0, 0); + assert(hw_uart_rx_count() == 0); + write_bytes(63, 1); assert(hw_uart_rx_count() == 63 && rx_state.overruns == 0); + assert(hw_uart_read(out, 63) == 63); + for (unsigned i = 0; i < 63; i++) assert(out[i] == i + 1); + assert(hw_uart_getc() == -1); + + reset(0, 0); write_bytes(64, 1); + assert(hw_uart_rx_count() == 0 && rx_state.overruns == 1 && rx_state.resync); + assert(rx_state.discarded_bytes == 64 && hw_uart_rx_take_loss()); + assert(!hw_uart_rx_take_loss()); + assert(hw_uart_rx_count() == 0 && rx_state.overruns == 1); + write_bytes(10, 1); assert(hw_uart_rx_count() == 0 && rx_state.resync); + assert(rx_state.discarded_bytes == 74); + mock_us += DMA_RX_IDLE_US; + assert(hw_uart_rx_count() == 0 && !rx_state.resync); + write_bytes(8, 10); assert(hw_uart_read(out, 8) == 8); + for (unsigned i = 0; i < 8; i++) assert(out[i] == 10 + i); + + reset(0, 0); write_bytes(64 * 3, 1); // Same write pointer, three complete laps. + assert(hw_uart_read(out, 8) == 0 && rx_state.overruns == 1); + assert(rx_state.discarded_bytes == 192); + + reset(60, 60); write_bytes(8, 40); + assert(hw_uart_read(out, 8) == 8 && !hw_uart_rx_take_loss()); + for (unsigned i = 0; i < 8; i++) assert(out[i] == 40 + i); + + // Preserve unread polled bytes when DMA starts at a nonzero ring position. + reset(50, 10); write_bytes(2, 70); + assert(hw_uart_rx_count() == 26 && rx_state.overruns == 0); + assert(hw_uart_read(out, 26) == 26 && out[24] == 70 && out[25] == 71); + + // Hardware counter reload and software byte-total rollover happen together. + reset(0, 0); model_bytes = DMA_RX_TRANSFER_COUNT - 3; + rx_state.remaining = 3; rx_state.produced = rx_state.consumed = UINT32_MAX - 2; + registers(); write_bytes(6, 20); + assert(hw_uart_rx_count() == 6 && rx_state.produced == 3 && rx_state.overruns == 0); + assert(hw_uart_read(out, 6) == 6); + for (unsigned i = 0; i < 6; i++) assert(out[i] == 20 + i); + + reset(0, 0); write_bytes(8, 1); overwrite_on_copy = 64; + assert(hw_uart_read(out, 8) == 0 && rx_state.resync && hw_uart_rx_take_loss()); + assert(rx_state.discarded_bytes == 72); // Never deliver a copy crossed by a lap. + + reset(0, 0); write_bytes(8, 1); overwrite_on_copy = 3; + assert(hw_uart_read(out, 8) == 8 && rx_state.overruns == 0); + for (unsigned i = 0; i < 8; i++) assert(out[i] == i + 1); + assert(hw_uart_read(out, 3) == 3 && out[0] == 0x80 && out[2] == 0x82); + + reset(0, 0); time_calls = 0; mock_time_hook = advance_during_snapshot; + assert(hw_uart_getc() == 77 && rx_state.overruns == 0); // Retry a changing register snapshot. + + reset(0, 0); + mock_dma.transfer_count--; // Count/address disagree while a bus write is pending. + assert(hw_uart_rx_count() == 0 && !rx_state.sample_valid); + assert(rx_state.produced == 0 && rx_state.overruns == 0); + rx_buffer[0] = 0x55; model_bytes = 1; registers(); + assert(hw_uart_getc() == 0x55); + + // Too much elapsed time makes the modulo count ambiguous: fail closed. + reset(0, 0); model_bytes = DMA_RX_TRANSFER_COUNT + 5; registers(); + mock_us += rx_state.max_poll_gap_us; + assert(hw_uart_rx_count() == 0 && rx_state.ambiguous_polls == 1 && rx_state.resync); + uint64_t discarded; uint32_t ambiguous, overflows; + hw_uart_get_rx_loss_stats(&discarded, &ambiguous); + hw_uart_get_stats(NULL, NULL, &overflows); + assert(discarded == 5 && ambiguous == 1 && overflows == 1); + mock_us += DMA_RX_IDLE_US; + assert(hw_uart_rx_count() == 0 && !rx_state.resync); + write_bytes(8, 60); assert(hw_uart_read(out, 8) == 8); + for (unsigned i = 0; i < 8; i++) assert(out[i] == 60 + i); + + // A scheduling pause inside the sampling operation must also fail closed. + reset(0, 0); mock_time_step = rx_state.max_poll_gap_us + 1; + assert(hw_uart_rx_count() == 0 && rx_state.ambiguous_polls == 1); + + // An idle DMA counter is insufficient if bytes remain in the UART FIFO. + reset(0, 0); write_bytes(64, 0); hw_uart_rx_count(); + volatile uint32_t fifo_status = 0; + rx_state.fifo_status = &fifo_status; + mock_us += DMA_RX_IDLE_US * 2; + assert(hw_uart_rx_count() == 0 && rx_state.resync); + fifo_status = UART_UARTFR_RXFE_BITS; + assert(hw_uart_rx_count() == 0 && rx_state.resync); + mock_us += DMA_RX_IDLE_US; + assert(hw_uart_rx_count() == 0 && !rx_state.resync); +} +''' +with tempfile.TemporaryDirectory() as temp: + temp = Path(temp) + write_headers(temp) + unit = temp / 'test.c' + binary = temp / 'test' + # get_stats also exposes the existing TX statistic. + unit.write_text(prefix + '\nstatic uint32_t total_tx_bytes;\n' + '\n'.join(function(f) for f in [ + 'hw_uart_rx_available', 'hw_uart_rx_count', 'hw_uart_getc', 'hw_uart_read', + 'hw_uart_rx_take_loss', 'hw_uart_get_rx_loss_stats', 'hw_uart_get_stats']) + test) + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', f'-I{temp}', + f'-I{ROOT / "lib/dma-uart/include"}', str(unit), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('DMA RX laps, count rollover, concurrent overwrite, recovery, and bridge statistics: passed') diff --git a/tests/test_proxy_rx.py b/tests/test_proxy_rx.py new file mode 100644 index 0000000..1fb5b58 --- /dev/null +++ b/tests/test_proxy_rx.py @@ -0,0 +1,145 @@ +"""Check polled RX, ring handoff, overlong text, and text/binary boundaries.""" +from pathlib import Path +import re +import subprocess +import tempfile +from dma_rx_mock import write_headers + +ROOT = Path(__file__).resolve().parents[1] +source = (ROOT / 'kmbox_serial_handler.c').read_text() + + +def function(name): + match = re.search(r'^(?:static )?(?:__force_inline )?(?:bool|void|uint\d+_t) ' + + name + r'\([^;]*?\)\s*\{', source, re.M) + assert match, name + end, depth = match.end(), 1 + while depth: + depth += (source[end] == '{') - (source[end] == '}') + end += 1 + return source[match.start():end] + + +prefix = r''' +#include +#include +#include +#include +#include +#include "fast_protocol.h" +#include "dma_uart.h" +#define __force_inline inline +#define UART_RX_BUFFER_SIZE 128 +#define UART_RX_BUFFER_MASK 127 +#define KMBOX_CMD_BUFFER_SIZE 64 +#define KMBOX_UART 0 +#define BRIDGE_SYNC_BYTE 0xBD +static uint8_t uart_rx_buffer[UART_RX_BUFFER_SIZE] __attribute__((aligned(UART_RX_BUFFER_SIZE))); +static uint32_t uart_rx_read_pos, uart_rx_poll_write_pos; +static int uart_rx_dma_chan = -1; +static bool text_discarding; +static dma_rx_state_t uart_rx_state; +static uint32_t uart_rx_reported_epoch; +static bool uart_rx_loss_pending; +static bool g_xbox_mode; +static int g_timed_move_alarm_id = -1; +static struct {bool pending, fired;} pending_timed_move; +static unsigned released, cleared, cancelled, xbox_cleared; +static void kmbox_set_injected_buttons(uint8_t mask) {assert(mask == 0); released++;} +static void smooth_clear_queue(void) {cleared++;} +static void cancel_alarm(int id) {assert(id == 7); cancelled++;} +static void xbox_inject_clear(void) {xbox_cleared++;} +static uint8_t fifo[512]; +static size_t fifo_pos, fifo_len; +static unsigned commands; +static char last_command[64]; +static bool uart_is_readable(int uart) {assert(uart == 0); return fifo_pos < fifo_len;} +static int uart_getc(int uart) {assert(uart == 0 && fifo_pos < fifo_len); return fifo[fifo_pos++];} +static void mark_activity(uint32_t now_ms) {assert(now_ms == 100);} +static bool handle_text_command(const char *line, size_t len, uint32_t now_ms) { + (void)line; (void)len; assert(now_ms == 100); return false; +} +static void kmbox_process_serial_line(const char *line, size_t len, const char *term, unsigned n, uint32_t now_ms) { + assert(n == 1 && *term == '\n' && now_ms == 100 && len < sizeof(last_command)); + memcpy(last_command, line, len + 1); commands++; +} +''' +bitmap = re.search(r'static const uint32_t g_fast_cmd_bitmap\[8\] = \{.*?\n\};', source, re.S)[0] +test = r''' +static void reset(const void *input, size_t len) { + memcpy(fifo, input, len); fifo_pos = 0; fifo_len = len; + uart_rx_read_pos = uart_rx_poll_write_pos = 0; + uart_rx_dma_chan = -1; text_discarding = false; commands = 0; + uart_rx_reported_epoch = 0; uart_rx_loss_pending = false; mock_us = 1000; +} +int main(void) { + const uint8_t mixed[] = {'M','1',',','2','\n', 1,10,13,0,1,2,3,4, 'P','\n'}; + reset(mixed, sizeof(mixed)); + assert(process_text_commands(100, 64) == 1 && commands == 1); + assert(strcmp(last_command, "M1,2") == 0 && rx_available() == 10); + for (unsigned i = 0; i < 8; i++) assert(rx_peek_at(i) == mixed[5 + i]); + rx_consume(8); + assert(process_text_commands(100, 64) == 1 && strcmp(last_command, "P") == 0); + + uint8_t oversized[203]; memset(oversized, 'X', sizeof(oversized)); + oversized[127] = 1; oversized[200] = '\n'; oversized[201] = 'P'; oversized[202] = '\n'; + reset(oversized, sizeof(oversized)); + process_text_commands(100, 64); assert(text_discarding && commands == 0); + process_text_commands(100, 64); + assert(!text_discarding && commands == 1 && strcmp(last_command, "P") == 0); + + uint8_t maximum[64]; memset(maximum, 'A', 63); maximum[63] = '\n'; + reset(maximum, sizeof(maximum)); process_text_commands(100, 64); + assert(commands == 1 && strlen(last_command) == 63); + reset("P\nP\n", 4); + assert(process_text_commands(100, 1) == 1 && rx_available() == 2); + + // A full polled ring stays nonempty and leaves excess bytes in the FIFO. + uint8_t burst[150]; for (unsigned i = 0; i < 150; i++) burst[i] = (uint8_t)i; + reset(burst, sizeof(burst)); + assert(rx_available() == 127 && fifo_pos == 127); + assert(rx_available() == 127 && fifo_pos == 127); + rx_consume(100); assert(rx_available() == 50 && fifo_pos == 150); + for (unsigned i = 0; i < 50; i++) assert(rx_peek_at(i) == (uint8_t)(100 + i)); + // DMA begins at the polled producer position, retaining unread ring data. + uint32_t pos = uart_rx_poll_write_pos; + uart_rx_buffer[pos] = 0xAA; uart_rx_buffer[(pos + 1) & 127] = 0xBB; + dma_rx_init(&uart_rx_state, 127, uart_rx_read_pos, pos, 3000000, mock_us); + mock_dma.write_addr = (uintptr_t)(uart_rx_buffer + ((pos + 2) & 127)); + mock_dma.transfer_count = (1u << 28) | (DMA_RX_TRANSFER_COUNT - 2); + uart_rx_dma_chan = 1; + assert(rx_available() == 52 && rx_peek_at(50) == 0xAA && rx_peek_at(51) == 0xBB); + uint8_t copy[52]; + assert(rx_copy_peek(copy, sizeof(copy)) && copy[50] == 0xAA && copy[51] == 0xBB); + text_discarding = true; + // Another complete lap has the same address but a different transfer count. + mock_dma.transfer_count -= 128; + assert(rx_available() == 0 && uart_rx_state.overruns == 1); + assert(uart_rx_loss_pending && !text_discarding && uart_rx_state.discarded_bytes == 180); + assert(!rx_copy_peek(copy, 1)); + g_xbox_mode = true; g_timed_move_alarm_id = 7; + pending_timed_move.pending = pending_timed_move.fired = true; + rx_handle_loss(); + assert(!uart_rx_loss_pending && released == 1 && cleared == 1 && cancelled == 1 && xbox_cleared == 1); + assert(g_timed_move_alarm_id == -1 && !pending_timed_move.pending && !pending_timed_move.fired); + rx_handle_loss(); assert(released == 1 && cleared == 1); + uint32_t overruns, ambiguous; uint64_t dropped; + kmbox_get_rx_loss_stats(&overruns, &dropped, &ambiguous); + assert(overruns == 1 && dropped == 180 && ambiguous == 0); + mock_us += DMA_RX_IDLE_US; + assert(rx_available() == 0 && !uart_rx_state.resync); +} +''' +with tempfile.TemporaryDirectory() as temp: + write_headers(temp) + unit = Path(temp) / 'test.c' + binary = Path(temp) / 'test' + unit.write_text(prefix + bitmap + '\n'.join(function(f) for f in [ + 'rx_get_write_pos', 'rx_sync_state', 'rx_available', 'rx_peek_at', 'rx_copy_peek', 'rx_consume', 'is_fast_cmd_start', + 'parse_text_line', 'process_text_commands', 'rx_handle_loss', 'kmbox_get_rx_loss_stats']) + test) + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', f'-I{temp}', + f'-I{ROOT / "lib/dma-uart/include"}', + f'-I{ROOT / "lib/fast-protocol/include"}', f'-I{ROOT / "lib/hid-defs/include"}', + str(unit), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('Proxy polled RX, ring handoff, oversized lines, and text/binary boundaries: passed') diff --git a/tests/test_proxy_tx.py b/tests/test_proxy_tx.py new file mode 100644 index 0000000..6676d7d --- /dev/null +++ b/tests/test_proxy_tx.py @@ -0,0 +1,171 @@ +"""Exercise the proxy's production TX queue with deferred DMA and two producers.""" +from pathlib import Path +import re +import subprocess +import tempfile + +ROOT = Path(__file__).resolve().parents[1] +assert 'pico_enable_stdio_uart(PIOKMbox 0)' in (ROOT / 'CMakeLists.txt').read_text() +source = (ROOT / 'kmbox_serial_handler.c').read_text() +queue_state = source[source.index('#define UART_TX_RECORD_SIZE'):source.index('// DMA-based RX:')] + + +def function(name): + match = re.search(r'^(?:static )?(?:bool|void|uint32_t)\s+(?:__not_in_flash_func\()?' + + name + r'\)?\([^;]*?\)\s*\{', source, re.M) + assert match, name + end, depth = match.end(), 1 + while depth: + depth += (source[end] == '{') - (source[end] == '}') + end += 1 + return source[match.start():end] + + +prefix = r''' +#include +#include +#include +#include +#include +#include +#include "hardware/sync.h" +#include "kmbox_commands.c" +#define __not_in_flash_func(x) x +static void __dmb(void) {} +#define KMBOX_UART 0 +static uint8_t output[65536]; +static size_t output_len, dma_len; +static const uint8_t *dma_source; +static bool busy, tx_irq_enabled; +static unsigned fifo_budget; +static bool dma_channel_is_busy(int ch) {assert(ch == 1); return busy;} +static void dma_channel_set_read_addr(int ch, const void *p, bool trigger) { + assert(ch == 1 && !busy && !trigger); dma_source = p; +} +static void dma_channel_set_trans_count(int ch, size_t n, bool trigger) { + assert(ch == 1 && !busy && trigger && n > 0); dma_len = n; busy = true; +} +static void uart_set_irq_enables(int uart, bool rx, bool tx) { + assert(uart == 0 && !rx); tx_irq_enabled = tx; +} +static bool uart_is_writable(int uart) {assert(uart == 0); return fifo_budget > 0;} +static void uart_putc_raw(int uart, uint8_t c) { + assert(uart == 0 && fifo_budget > 0); fifo_budget--; output[output_len++] = c; +} +static void tx_service_locked(void); +''' +test = r''' +static void complete_dma(void) { + uint32_t save = spin_lock_blocking(tx_lock); + if (busy) { + assert(output_len + dma_len <= sizeof(output)); + memcpy(output + output_len, dma_source, dma_len); + output_len += dma_len; busy = false; + } + tx_service_locked(); + spin_unlock(tx_lock, save); +} +static void drain(void) { + while (tx_head != tx_tail) complete_dma(); +} +static void reset(void) { + assert(!busy); + tx_head = tx_tail = 0; tx_offset = 0; tx_dma_active = false; + g_uart_tx_dropped = 0; output_len = 0; uart_tx_dma_chan = 1; +} +static unsigned finished; +#define COUNT 1000 +static void *producer(void *arg) { + uint8_t record[8]; memset(record, (int)(uintptr_t)arg, sizeof(record)); + for (unsigned i = 0; i < COUNT; i++) + while (!uart_send_bytes(record, sizeof(record))) sched_yield(); + __atomic_add_fetch(&finished, 1, __ATOMIC_RELEASE); + return NULL; +} +int main(void) { + tx_lock = spin_lock_init((unsigned)spin_lock_claim_unused(true)); + reset(); + kmbox_send_response("HELLO"); + const uint8_t binary[] = {0xFD, 0, 1, 2, 3, 4, 5, 6}; + assert(uart_send_bytes(binary, sizeof(binary))); + kmbox_commands_init(); + kmbox_set_output_callback(command_reply_write); + kmbox_process_serial_line("km.monitor()", 12, "\n", 1, 100); + drain(); + assert(output_len == 7 + 8 + 7); + assert(memcmp(output, "HELLO\r\n", 7) == 0); + assert(memcmp(output + 7, binary, 8) == 0); + assert(memcmp(output + 15, "0\r\n>>> ", 7) == 0); + + reset(); + for (unsigned i = 0; i < UART_TX_RECORD_COUNT - 1; i++) kmbox_send_response("A"); + assert(!uart_send_bytes(binary, sizeof(binary))); + assert(kmbox_get_tx_dropped_bytes() == 8); + drain(); assert(output_len == 7 * 3); + for (unsigned i = 0; i < 7; i++) assert(memcmp(output + i * 3, "A\r\n", 3) == 0); + + reset(); + uint8_t oversized[UART_TX_RECORD_SIZE + 1]; memset(oversized, 'X', sizeof(oversized)); + assert(!uart_send_bytes(oversized, sizeof(oversized))); + assert(tx_head == tx_tail && !busy && output_len == 0); + assert(kmbox_get_tx_dropped_bytes() == sizeof(oversized)); + // Account for the suffix before admitting a text response. + oversized[UART_TX_RECORD_SIZE - 1] = 0; + kmbox_send_response((const char *)oversized); + assert(tx_head == tx_tail && !busy && output_len == 0); + assert(kmbox_get_tx_dropped_bytes() == sizeof(oversized) * 2); + + // IRQ-to-DMA handoff resumes an already partially transmitted record. + reset(); uart_tx_dma_chan = -1; + assert(uart_send_bytes((const uint8_t *)"ABCD", 4)); + assert(tx_irq_enabled); + fifo_budget = 2; on_uart_irq(); assert(tx_offset == 2 && output_len == 2); + uart_tx_dma_chan = 1; tx_service(); drain(); + assert(output_len == 4 && memcmp(output, "ABCD", 4) == 0); + + reset(); + pthread_t a, b; + assert(pthread_create(&a, NULL, producer, (void *)(uintptr_t)0xAA) == 0); + assert(pthread_create(&b, NULL, producer, (void *)(uintptr_t)0xBB) == 0); + while (__atomic_load_n(&finished, __ATOMIC_ACQUIRE) != 2) {complete_dma(); sched_yield();} + assert(pthread_join(a, NULL) == 0 && pthread_join(b, NULL) == 0); + drain(); + assert(output_len == COUNT * 2 * 8); + unsigned a_count = 0, b_count = 0; + for (size_t i = 0; i < output_len; i += 8) { + assert(output[i] == 0xAA || output[i] == 0xBB); + for (unsigned j = 1; j < 8; j++) assert(output[i + j] == output[i]); + if (output[i] == 0xAA) a_count++; else b_count++; + } + assert(a_count == COUNT && b_count == COUNT); +} +''' +with tempfile.TemporaryDirectory() as temp: + temp = Path(temp) + (temp / 'hardware').mkdir() + (temp / 'hardware/sync.h').write_text(''' +#pragma once +#include +#include +#include +#include +typedef pthread_mutex_t spin_lock_t; +static spin_lock_t locks[32]; +static int next_lock = 24; +static int spin_lock_claim_unused(bool required) {assert(required); return next_lock++;} +static spin_lock_t *spin_lock_init(unsigned n) { + assert(n < 32); pthread_mutex_init(&locks[n], NULL); return &locks[n]; +} +static uint32_t spin_lock_blocking(spin_lock_t *p) {pthread_mutex_lock(p); return 0;} +static void spin_unlock(spin_lock_t *p, uint32_t save) {(void)save; pthread_mutex_unlock(p);} +''') + unit = temp / 'test.c' + binary = temp / 'test' + unit.write_text(prefix + queue_state + '\n'.join(function(f) for f in [ + 'tx_service_locked', 'tx_service', 'on_uart_irq', 'uart_enqueue_record', + 'uart_send_bytes', 'command_reply_write', 'kmbox_send_response', 'kmbox_get_tx_dropped_bytes']) + test) + subprocess.run(['cc', '-std=c11', '-pthread', '-fsanitize=address,undefined', f'-I{temp}', + f'-I{ROOT / "lib/kmbox-commands"}', f'-I{ROOT / "lib/hid-defs/include"}', + str(unit), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('Proxy reply routing, record integrity, IRQ/DMA handoff, and concurrent producers: passed') diff --git a/tests/test_uart_timing.py b/tests/test_uart_timing.py new file mode 100644 index 0000000..ae1d29a --- /dev/null +++ b/tests/test_uart_timing.py @@ -0,0 +1,187 @@ +"""Run UART TX logic with deferred DMA and check production clock expressions.""" +from pathlib import Path +import re +import subprocess +import tempfile + +ROOT = Path(__file__).resolve().parents[1] +uart = (ROOT / 'bridge/hw_uart.c').read_text() +serial = (ROOT / 'kmbox_serial_handler.c').read_text() + + +def function(source, name): + match = re.search(r'^(?:bool|void) ' + name + r'\([^;]*?\)\s*\{', source, re.M) + assert match, name + end, depth = match.end(), 1 + while depth: + depth += (source[end] == '{') - (source[end] == '}') + end += 1 + return source[match.start():end] + + +task = serial[serial.index('void kmbox_serial_task(void)'):] +task_clock = re.search(r'uint32_t now_ms = (.*);', task)[1] +click_clock = re.search(r'uint32_t click_now = (.*);', serial)[1] +ppm = re.search(r'int32_t baud_error_ppm = (.*);', uart)[1] +prefix = r''' +#include +#include +#include +#include +#define HW_UART 0 +#define HW_UART_TX_BUFFER_SIZE 512 +static int tx_dma_chan = 1; +static uint8_t tx_buffer[512], output[2048]; +static size_t total_tx_bytes, output_len, dma_len; +static const uint8_t *dma_source; +static bool busy; +static bool dma_channel_is_busy(int ch) {assert(ch == 1); return busy;} +static void dma_channel_set_read_addr(int ch, const void *p, bool trigger) { + assert(ch == 1 && !trigger && !busy); dma_source = p; +} +static void dma_channel_set_trans_count(int ch, size_t n, bool trigger) { + assert(ch == 1 && trigger && !busy); dma_len = n; busy = true; +} +static void tight_loop_contents(void) { + assert(busy); + memcpy(output + output_len, dma_source, dma_len); + output_len += dma_len; busy = false; +} +static void uart_putc_raw(int uart, uint8_t c) { + assert(uart == 0 && !busy); output[output_len++] = c; +} +static void uart_write_blocking(int uart, const uint8_t *data, size_t len) { + for (size_t i = 0; i < len; i++) uart_putc_raw(uart, data[i]); +} +static uint64_t clock_us; +static uint64_t get_absolute_time(void) {return clock_us;} +static uint32_t to_ms_since_boot(uint64_t t) {return (uint32_t)(t / 1000);} +static uint32_t time_us_32(void) {return (uint32_t)clock_us;} +bool hw_uart_tx_ready(void); +void hw_uart_tx_wait(void); +''' +clocks = f''' +static uint32_t task_ms(void) {{ uint32_t now_us = time_us_32(); (void)now_us; return {task_clock}; }} +static uint32_t click_ms(void) {{ return {click_clock}; }} +static int32_t error_ppm(uint32_t actual_baud, uint32_t baud) {{ return {ppm}; }} +''' +test = r''' +int main(void) { + uint8_t first[] = {1, 2, 3}, second[] = {4, 5}; + assert(hw_uart_tx_ready()); + assert(hw_uart_send(first, sizeof(first))); + assert(!hw_uart_tx_ready()); + assert(!hw_uart_send(second, sizeof(second))); + assert(memcmp(dma_source, first, sizeof(first)) == 0); + hw_uart_puts("P"); hw_uart_putc('\n'); + assert(output_len == 5 && memcmp(output, "\1\2\3P\n", 5) == 0); + assert(hw_uart_send(second, sizeof(second))); + hw_uart_tx_wait(); assert(hw_uart_tx_ready()); + assert(memcmp(output + 5, second, sizeof(second)) == 0); + assert(total_tx_bytes == 7); + assert(!hw_uart_send(NULL, 1)); + tx_dma_chan = -1; + assert(hw_uart_send(first, sizeof(first))); + assert(memcmp(output + 7, first, sizeof(first)) == 0); + + // Cross the 32-bit microsecond rollover without premature click expiry. + clock_us = (uint64_t)UINT32_MAX - 20000; + uint32_t start = click_ms(); + clock_us += 40000; + assert(task_ms() - start == 40 && click_ms() == task_ms()); + clock_us += 100000; + assert(task_ms() - start == 140); + // The millisecond clock itself wraps normally after ~49 days. + clock_us = ((uint64_t)UINT32_MAX - 20) * 1000; + start = click_ms(); clock_us += 40000; + assert(task_ms() - start == 40); + assert(error_ppm(2900000, 3000000) == -33333); + assert(error_ppm(3100000, 3000000) == 33333); +} +''' +with tempfile.TemporaryDirectory() as temp: + source = Path(temp) / 'test.c' + binary = Path(temp) / 'test' + source.write_text(prefix + clocks + '\n'.join(function(uart, f) for f in + ['hw_uart_send', 'hw_uart_putc', 'hw_uart_puts', 'hw_uart_tx_ready', 'hw_uart_tx_wait']) + test) + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', str(source), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('UART DMA ordering, baud arithmetic, and timer rollover: passed') + + (Path(temp) / 'hardware').mkdir() + (Path(temp) / 'hardware/sync.h').write_text(''' +#include +typedef int spin_lock_t; +static spin_lock_t lock; +static int spin_lock_claim_unused(bool required) {(void)required; return 24;} +static spin_lock_t *spin_lock_init(unsigned n) {(void)n; return &lock;} +static uint32_t spin_lock_blocking(spin_lock_t *p) {(void)p; return 0;} +static void spin_unlock(spin_lock_t *p, uint32_t v) {(void)p; (void)v;} +''') + source.write_text(r''' +#include +#include "kmbox_commands.c" +int main(void) { + kmbox_commands_init(); + int parsed_x, parsed_y, parsed_button; + assert(kmbox_parse_move_command("km.move( -32768 , +32767 ) ", &parsed_x, &parsed_y)); + assert(parsed_x == -32768 && parsed_y == 32767); + assert(!kmbox_parse_move_command("km.move(999999999999999999999,0)", &parsed_x, &parsed_y)); + assert(!kmbox_parse_move_command("km.move(1,2", &parsed_x, &parsed_y)); + assert(!kmbox_parse_move_command("km.move(1,2)junk", &parsed_x, &parsed_y)); + for (unsigned i = 0; i < KMBOX_BUTTON_COUNT; i++) { + char command[24]; snprintf(command, sizeof(command), "km.click(%u)", i); + assert(kmbox_parse_click_command(command, &parsed_button) && parsed_button == (int)i); + } + assert(!kmbox_parse_click_command("km.click(-1)", &parsed_button)); + assert(!kmbox_parse_click_command("km.click(5)", &parsed_button)); + assert(!kmbox_parse_click_command("km.click(999999999999999999)", &parsed_button)); + assert(!kmbox_parse_click_command("km.click(0", &parsed_button)); + kmbox_update_physical_buttons(1); + kmbox_update_states(100); + kmbox_set_injected_buttons(2); + assert(kmbox_get_current_buttons() == 3 && kmbox_has_forced_buttons()); + uint8_t buttons; + int8_t x8, y8, wheel, pan; + int16_t x16, y16; + kmbox_get_mouse_report(&buttons, &x8, &y8, &wheel, &pan); + assert(buttons == 3); + kmbox_get_mouse_report_16(&buttons, &x16, &y16, &wheel, &pan); + assert(buttons == 3); + kmbox_set_injected_buttons(0); + assert(kmbox_get_current_buttons() == 1 && !kmbox_has_forced_buttons()); + assert(kmbox_try_drain_mouse_16(3, &buttons, &x16, &y16, &wheel, &pan)); + assert(buttons == 1); // Synthetic release must preserve the physical left hold. + kmbox_update_physical_buttons(0); + kmbox_update_states(101); + assert(kmbox_get_current_buttons() == 0); + kmbox_commands_init(); + for (unsigned i = 0; i < KMBOX_BUTTON_COUNT; i++) { + uint32_t start = UINT32_MAX - 30; + kmbox_start_button_click((kmbox_button_t)i, start); + button_state_t *b = &g_kmbox_state.buttons[i]; + uint32_t release = b->click_release_start, end = b->click_end_time; + kmbox_update_states(start + 10); + assert(b->is_clicking && b->is_pressed); + kmbox_update_states(release); + assert(b->is_clicking && !b->is_pressed); + kmbox_update_states(end); + assert(!b->is_clicking && !b->is_forced); + + g_kmbox_state.physical_buttons = (uint8_t)(1u << i); + set_button_state((kmbox_button_t)i, true, start); + set_button_state((kmbox_button_t)i, false, start); + end = b->release_time; + kmbox_update_states(start + 10); + assert(b->is_forced && !b->is_pressed); + kmbox_update_states(end); + assert(!b->is_forced && b->is_pressed); + g_kmbox_state.physical_buttons = 0; + } +} +''') + subprocess.run(['cc', '-std=c11', '-fsanitize=address,undefined', f'-I{temp}', + f'-I{ROOT / "lib/kmbox-commands"}', f'-I{ROOT / "lib/hid-defs/include"}', + str(source), '-o', str(binary)], check=True) + subprocess.run([str(binary)], check=True) + print('All five click/release state machines across millisecond rollover: passed') diff --git a/tests/test_xbox_transfers.py b/tests/test_xbox_transfers.py index c1f897f..6ade16c 100644 --- a/tests/test_xbox_transfers.py +++ b/tests/test_xbox_transfers.py @@ -27,12 +27,16 @@ def function(file, name): gip = re.sub(r'^#include "pico/sync.h"$', '', (ROOT / 'xbox_gip.h').read_text(), flags=re.M) prefix = r''' #include +#include #include #include #include typedef int spin_lock_t; -static spin_lock_t lock; -static spin_lock_t *spin_lock_instance(unsigned n) { (void)n; return &lock; } +static spin_lock_t locks[32]; +static unsigned claim_count; +static int spin_lock_claim_unused(bool required) { assert(required); return 24 + claim_count++; } +static spin_lock_t *spin_lock_init(unsigned n) { assert(n < 32); return &locks[n]; } +static spin_lock_t *spin_lock_instance(unsigned n) {return spin_lock_init(n);} static uint32_t spin_lock_blocking(spin_lock_t *p) { (void)p; return 0; } static void spin_unlock(spin_lock_t *p, uint32_t s) { (void)p; (void)s; } #define __not_in_flash_func(x) x @@ -114,8 +118,27 @@ def function(file, name): assert(pending_in_len == 0 && retained == proxy_in_buf && retained[2] == 9); } ''' +ownership_state = r''' +static uint8_t pending_out_len; +static bool power_on_pending; +static xbox_gamepad_state_t gamepad_state; +static gip_packet_queue_t host_to_device_queue, device_to_host_queue; +static int gamepad_lock_num = -1, h2d_lock_num = -1, d2h_lock_num = -1; +''' +ownership_test = r''' +int main(void) { + assert(xbox_host_driver_init()); + assert(claim_count == 3); + assert(gamepad_state.lock != host_to_device_queue.lock); + assert(host_to_device_queue.lock != device_to_host_queue.lock); + assert(gamepad_state.lock != device_to_host_queue.lock); + assert(xbox_host_driver_init()); + assert(claim_count == 3); +} +''' with tempfile.TemporaryDirectory() as temp: for name, code in { + 'ownership': prefix + gip + ownership_state + function('xbox_host.c', 'xbox_host_driver_init') + ownership_test, 'host': prefix + gip + mock + '\n'.join(function('xbox_host.c', f) for f in ['xbox_host_send_power_on', 'xbox_host_send_keepalive', 'xbox_host_task']) + host_test, 'strings': prefix + device_state + function('xbox_device.c', 'xbox_get_string_descriptor') + device_test, diff --git a/xbox_gip.h b/xbox_gip.h index 044fda6..b6fb31b 100644 --- a/xbox_gip.h +++ b/xbox_gip.h @@ -116,7 +116,7 @@ typedef struct { static inline void gip_queue_init(gip_packet_queue_t *q, unsigned int spin_lock_num) { q->head = 0; q->tail = 0; - q->lock = spin_lock_instance(spin_lock_num); + q->lock = spin_lock_init(spin_lock_num); } static inline bool gip_queue_push(gip_packet_queue_t *q, const uint8_t *data, uint8_t len) { diff --git a/xbox_host.c b/xbox_host.c index 23006bb..4ff6a60 100644 --- a/xbox_host.c +++ b/xbox_host.c @@ -64,9 +64,9 @@ static uint8_t pending_out_len; static bool power_on_pending; // Spinlock numbers for shared state (claim unique IDs from Pico SDK pool) -#define XBOX_SPINLOCK_GAMEPAD 24 -#define XBOX_SPINLOCK_H2D_QUEUE 25 -#define XBOX_SPINLOCK_D2H_QUEUE 26 +static int gamepad_lock_num = -1; +static int h2d_lock_num = -1; +static int d2h_lock_num = -1; //--------------------------------------------------------------------+ // Forward Declarations @@ -84,9 +84,13 @@ static bool xbox_host_driver_init(void) { pending_out_len = 0; power_on_pending = false; memset(&gamepad_state, 0, sizeof(gamepad_state)); - gamepad_state.lock = spin_lock_instance(XBOX_SPINLOCK_GAMEPAD); - gip_queue_init(&host_to_device_queue, XBOX_SPINLOCK_H2D_QUEUE); - gip_queue_init(&device_to_host_queue, XBOX_SPINLOCK_D2H_QUEUE); + // Firmware-lifetime claims survive USB stack reinitialization. + if (gamepad_lock_num < 0) gamepad_lock_num = spin_lock_claim_unused(true); + if (h2d_lock_num < 0) h2d_lock_num = spin_lock_claim_unused(true); + if (d2h_lock_num < 0) d2h_lock_num = spin_lock_claim_unused(true); + gamepad_state.lock = spin_lock_init((unsigned)gamepad_lock_num); + gip_queue_init(&host_to_device_queue, (unsigned)h2d_lock_num); + gip_queue_init(&device_to_host_queue, (unsigned)d2h_lock_num); return true; }