Phase 03: ESP32-C5 serial link hardening + bench diagnostics
Enumeration: - Merge the library's stock probe table instead of replacing it, so adding Espressif 0x303A/0x1001 doesn't drop every other supported device - Select the ESP32-C5 by VID/PID rather than list position - Log USB interface descriptors to distinguish CDC data from the JTAG interface Lifecycle: - Don't close the shared port in MqttViewModel.onCleared() - the foreground recording service outlives the ViewModel and would beacon into a dead port - Handle ACTION_USB_DEVICE_DETACHED so the UI stops reporting a stale link - Implement the STATUS heartbeat on both sides (1 Hz) plus a phone-side watchdog - Surface write failures and firmware drop counters on the CAM Pinger card Protocol: - Assert DTR/RTS on open (unverified on hardware - see FLASHING.md step 5) - Raise SERIAL_LINK_MAX_PAYLOAD 160 -> 512 on both sides; real third-party CAMs exceed 160 and were being silently dropped at the resync branch - Move the enlarged buffers off task stacks; serialize send_frame with a mutex Firmware and app must be updated together - a 512/160 mismatch fails silently.
This commit is contained in:
+111
-41
@@ -1,8 +1,10 @@
|
||||
#include "serial_link.h"
|
||||
#include <string.h>
|
||||
#include <stdlib.h>
|
||||
#include "freertos/FreeRTOS.h"
|
||||
#include "freertos/task.h"
|
||||
#include "driver/uart.h"
|
||||
#include "freertos/semphr.h"
|
||||
#include "driver/usb_serial_jtag.h"
|
||||
#include "esp_log.h"
|
||||
|
||||
static const char *TAG = "serial_link";
|
||||
@@ -12,6 +14,29 @@ static const char *TAG = "serial_link";
|
||||
|
||||
static serial_link_cam_tx_cb_t s_on_cam_tx;
|
||||
|
||||
// ---- Counters reported to the phone in every heartbeat (see SERIAL_MSG_STATUS in the header).
|
||||
// Saturating rather than wrapping: "65535 drops" reads as "lots and still going", whereas a wrap
|
||||
// back to a small number during a long bench run reads as "the problem went away".
|
||||
static uint16_t s_oversize_drops;
|
||||
static uint16_t s_tx_failures;
|
||||
static uint16_t s_rx_crc_errors;
|
||||
|
||||
// Serializes send_frame(): it writes a frame as four separate usb_serial_jtag_write_bytes() calls
|
||||
// and shares one static CRC scratch buffer, and it's now called from three tasks (rx_forward for
|
||||
// CAM_RX, the heartbeat task for STATUS, and potentially others). Without this, two frames could
|
||||
// interleave on the wire and both would be discarded by the phone's framer.
|
||||
static SemaphoreHandle_t s_tx_mutex;
|
||||
|
||||
static inline void bump(uint16_t *counter)
|
||||
{
|
||||
if (*counter < 0xFFFF) (*counter)++;
|
||||
}
|
||||
|
||||
void serial_link_note_tx_failure(void)
|
||||
{
|
||||
bump(&s_tx_failures);
|
||||
}
|
||||
|
||||
// ---- CRC-16/CCITT-FALSE (poly 0x1021, init 0xFFFF, no reflect, no xorout) ----
|
||||
// Bytewise (no table) - frames here are at most SERIAL_LINK_MAX_PAYLOAD + 3 bytes, so table
|
||||
// lookup isn't worth the flash/RAM tradeoff. MUST match the Kotlin-side implementation exactly
|
||||
@@ -41,28 +66,37 @@ static bool send_frame(uint8_t type, const uint8_t *payload, int len)
|
||||
head[1] = (uint8_t)(len & 0xFF);
|
||||
head[2] = (uint8_t)((len >> 8) & 0xFF);
|
||||
|
||||
uint16_t crc;
|
||||
{
|
||||
// Compute CRC over head+payload without a combined buffer copy: CRC is a running
|
||||
// state, so feed it in two calls worth of bytes by concatenating into a small stack
|
||||
// buffer (payload is capped at SERIAL_LINK_MAX_PAYLOAD, so head+payload comfortably
|
||||
// fits on the stack).
|
||||
uint8_t crc_buf[3 + SERIAL_LINK_MAX_PAYLOAD];
|
||||
memcpy(crc_buf, head, 3);
|
||||
if (len > 0) memcpy(crc_buf + 3, payload, (size_t)len);
|
||||
crc = crc16_ccitt_false(crc_buf, (size_t)(3 + len));
|
||||
// Concatenate head+payload to CRC them in one pass. static, not stack: at
|
||||
// SERIAL_LINK_MAX_PAYLOAD = 512 this buffer is 515 bytes, which is a meaningful bite out of a
|
||||
// 4 KB task stack, and send_frame() is called from several tasks. Guarded by s_tx_mutex below
|
||||
// so the shared buffer (and the four-part write) can't interleave between callers.
|
||||
static uint8_t s_crc_buf[3 + SERIAL_LINK_MAX_PAYLOAD];
|
||||
|
||||
if (s_tx_mutex && xSemaphoreTake(s_tx_mutex, pdMS_TO_TICKS(200)) != pdTRUE) {
|
||||
ESP_LOGW(TAG, "send_frame: tx mutex timeout, dropping frame");
|
||||
return false;
|
||||
}
|
||||
|
||||
memcpy(s_crc_buf, head, 3);
|
||||
if (len > 0) memcpy(s_crc_buf + 3, payload, (size_t)len);
|
||||
uint16_t crc = crc16_ccitt_false(s_crc_buf, (size_t)(3 + len));
|
||||
|
||||
uint8_t sync[2] = {SYNC0, SYNC1};
|
||||
uint8_t crc_bytes[2] = {(uint8_t)(crc & 0xFF), (uint8_t)((crc >> 8) & 0xFF)};
|
||||
|
||||
// Four separate writes rather than one assembled buffer - simplest given payload is
|
||||
// already wherever the caller has it (avoids a second copy of up to 160 bytes).
|
||||
// usb_serial_jtag_write_bytes() blocks up to the given tick timeout if the host isn't
|
||||
// reading fast enough; 100ms is generous for a ~160-byte frame at USB full-speed and keeps
|
||||
// a wedged/disconnected host from hanging the radio TX/RX tasks indefinitely.
|
||||
const TickType_t write_timeout = pdMS_TO_TICKS(100);
|
||||
int wrote = 0;
|
||||
wrote += uart_write_bytes(SERIAL_LINK_UART_NUM, sync, sizeof(sync));
|
||||
wrote += uart_write_bytes(SERIAL_LINK_UART_NUM, head, sizeof(head));
|
||||
if (len > 0) wrote += uart_write_bytes(SERIAL_LINK_UART_NUM, payload, (size_t)len);
|
||||
wrote += uart_write_bytes(SERIAL_LINK_UART_NUM, crc_bytes, sizeof(crc_bytes));
|
||||
wrote += usb_serial_jtag_write_bytes(sync, sizeof(sync), write_timeout);
|
||||
wrote += usb_serial_jtag_write_bytes(head, sizeof(head), write_timeout);
|
||||
if (len > 0) wrote += usb_serial_jtag_write_bytes(payload, (size_t)len, write_timeout);
|
||||
wrote += usb_serial_jtag_write_bytes(crc_bytes, sizeof(crc_bytes), write_timeout);
|
||||
|
||||
if (s_tx_mutex) xSemaphoreGive(s_tx_mutex);
|
||||
|
||||
return wrote == (int)(sizeof(sync) + sizeof(head) + len + sizeof(crc_bytes));
|
||||
}
|
||||
@@ -70,24 +104,53 @@ static bool send_frame(uint8_t type, const uint8_t *payload, int len)
|
||||
bool serial_link_send_cam_rx(int8_t rssi, const uint8_t *cam_uper, int cam_len)
|
||||
{
|
||||
if (cam_len < 0 || cam_len > SERIAL_LINK_MAX_PAYLOAD - 1) {
|
||||
ESP_LOGW(TAG, "send_cam_rx: cam_len too large (%d)", cam_len);
|
||||
// Counted, not just logged: this log line goes to the flashing port, which nobody is
|
||||
// watching during a phone bench session - so the symptom would be "that station just
|
||||
// never shows up in the app" with no visible cause.
|
||||
bump(&s_oversize_drops);
|
||||
ESP_LOGW(TAG, "send_cam_rx: cam_len too large (%d), total oversize drops %u",
|
||||
cam_len, s_oversize_drops);
|
||||
return false;
|
||||
}
|
||||
uint8_t payload[SERIAL_LINK_MAX_PAYLOAD];
|
||||
payload[0] = (uint8_t)rssi;
|
||||
memcpy(payload + 1, cam_uper, (size_t)cam_len);
|
||||
return send_frame(SERIAL_MSG_CAM_RX, payload, 1 + cam_len);
|
||||
// static, not stack (515 bytes at MAX_PAYLOAD 512); only rx_forward_task calls this, and
|
||||
// send_frame's mutex covers the handoff onto the wire.
|
||||
static uint8_t s_cam_rx_payload[SERIAL_LINK_MAX_PAYLOAD];
|
||||
s_cam_rx_payload[0] = (uint8_t)rssi;
|
||||
memcpy(s_cam_rx_payload + 1, cam_uper, (size_t)cam_len);
|
||||
return send_frame(SERIAL_MSG_CAM_RX, s_cam_rx_payload, 1 + cam_len);
|
||||
}
|
||||
|
||||
bool serial_link_send_status(uint8_t status)
|
||||
{
|
||||
return send_frame(SERIAL_MSG_STATUS, &status, 1);
|
||||
// [status:1][oversize_drops:2 LE][tx_failures:2 LE][rx_crc_errors:2 LE] - keep in lockstep
|
||||
// with EspLinkStatus.parse() in the app's SerialFrame.kt.
|
||||
uint8_t payload[7];
|
||||
payload[0] = status;
|
||||
payload[1] = (uint8_t)(s_oversize_drops & 0xFF);
|
||||
payload[2] = (uint8_t)((s_oversize_drops >> 8) & 0xFF);
|
||||
payload[3] = (uint8_t)(s_tx_failures & 0xFF);
|
||||
payload[4] = (uint8_t)((s_tx_failures >> 8) & 0xFF);
|
||||
payload[5] = (uint8_t)(s_rx_crc_errors & 0xFF);
|
||||
payload[6] = (uint8_t)((s_rx_crc_errors >> 8) & 0xFF);
|
||||
return send_frame(SERIAL_MSG_STATUS, payload, sizeof(payload));
|
||||
}
|
||||
|
||||
// 1 Hz heartbeat. This is what lets the phone tell "link alive, radio quiet" from "link dead" -
|
||||
// the app's watchdog (UsbSerialTransport.kt) marks the link ERROR after 3 missed beats. Keep the
|
||||
// period in step with LINK_TIMEOUT_MS over there.
|
||||
static void status_task(void *arg)
|
||||
{
|
||||
(void)arg;
|
||||
while (1) {
|
||||
serial_link_send_status(0);
|
||||
vTaskDelay(pdMS_TO_TICKS(1000));
|
||||
}
|
||||
}
|
||||
|
||||
// ---- RX framing state machine ----
|
||||
// Runs in its own task, byte-at-a-time off the UART driver's RX ring buffer (via
|
||||
// uart_read_bytes with a short timeout, not raw ISR access - simplest correct option for a
|
||||
// link this slow/small; revisit if CAM traffic volume ever makes this a bottleneck).
|
||||
// Runs in its own task, byte-at-a-time off the USB Serial/JTAG driver's RX ring buffer (via
|
||||
// usb_serial_jtag_read_bytes with a short timeout, not raw ISR access - simplest correct option
|
||||
// for a link this slow/small; revisit if CAM traffic volume ever makes this a bottleneck).
|
||||
typedef enum {
|
||||
WAIT_SYNC0,
|
||||
WAIT_SYNC1,
|
||||
@@ -106,12 +169,16 @@ static void rx_task(void *arg)
|
||||
uint8_t type = 0;
|
||||
uint16_t len = 0;
|
||||
uint16_t payload_idx = 0;
|
||||
uint8_t payload[SERIAL_LINK_MAX_PAYLOAD];
|
||||
uint16_t crc_recv = 0;
|
||||
// static, not stack: at SERIAL_LINK_MAX_PAYLOAD = 512 these two are >1 KB together, a quarter
|
||||
// of this task's 4 KB stack. Safe as statics because rx_task is a singleton - one instance,
|
||||
// created once in serial_link_init().
|
||||
static uint8_t payload[SERIAL_LINK_MAX_PAYLOAD];
|
||||
static uint8_t crc_buf[3 + SERIAL_LINK_MAX_PAYLOAD];
|
||||
|
||||
uint8_t byte;
|
||||
while (1) {
|
||||
int n = uart_read_bytes(SERIAL_LINK_UART_NUM, &byte, 1, pdMS_TO_TICKS(50));
|
||||
int n = usb_serial_jtag_read_bytes(&byte, 1, pdMS_TO_TICKS(50));
|
||||
if (n <= 0) continue;
|
||||
|
||||
switch (state) {
|
||||
@@ -153,7 +220,6 @@ static void rx_task(void *arg)
|
||||
case WAIT_CRC_HI: {
|
||||
crc_recv |= (uint16_t)byte << 8;
|
||||
|
||||
uint8_t crc_buf[3 + SERIAL_LINK_MAX_PAYLOAD];
|
||||
crc_buf[0] = type;
|
||||
crc_buf[1] = (uint8_t)(len & 0xFF);
|
||||
crc_buf[2] = (uint8_t)((len >> 8) & 0xFF);
|
||||
@@ -167,7 +233,9 @@ static void rx_task(void *arg)
|
||||
ESP_LOGW(TAG, "rx: unexpected frame type 0x%02x from phone, ignoring", type);
|
||||
}
|
||||
} else {
|
||||
ESP_LOGW(TAG, "rx: CRC mismatch (got %04x want %04x), dropping frame", crc_recv, crc_calc);
|
||||
bump(&s_rx_crc_errors);
|
||||
ESP_LOGW(TAG, "rx: CRC mismatch (got %04x want %04x), dropping frame (total %u)",
|
||||
crc_recv, crc_calc, s_rx_crc_errors);
|
||||
}
|
||||
state = WAIT_SYNC0;
|
||||
break;
|
||||
@@ -180,20 +248,22 @@ void serial_link_init(serial_link_cam_tx_cb_t on_cam_tx)
|
||||
{
|
||||
s_on_cam_tx = on_cam_tx;
|
||||
|
||||
uart_config_t cfg = {
|
||||
.baud_rate = SERIAL_LINK_BAUD,
|
||||
.data_bits = UART_DATA_8_BITS,
|
||||
.parity = UART_PARITY_DISABLE,
|
||||
.stop_bits = UART_STOP_BITS_1,
|
||||
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
|
||||
.source_clk = UART_SCLK_DEFAULT,
|
||||
s_tx_mutex = xSemaphoreCreateMutex();
|
||||
if (!s_tx_mutex) {
|
||||
// Fail loudly rather than silently running unserialized: interleaved frames would look
|
||||
// like random CRC errors on the phone, which is a miserable thing to debug.
|
||||
ESP_LOGE(TAG, "failed to create tx mutex");
|
||||
abort();
|
||||
}
|
||||
|
||||
usb_serial_jtag_driver_config_t cfg = {
|
||||
.tx_buffer_size = SERIAL_LINK_USB_BUF_SIZE,
|
||||
.rx_buffer_size = SERIAL_LINK_USB_BUF_SIZE,
|
||||
};
|
||||
ESP_ERROR_CHECK(uart_driver_install(SERIAL_LINK_UART_NUM, 1024, 1024, 0, NULL, 0));
|
||||
ESP_ERROR_CHECK(uart_param_config(SERIAL_LINK_UART_NUM, &cfg));
|
||||
ESP_ERROR_CHECK(uart_set_pin(SERIAL_LINK_UART_NUM, SERIAL_LINK_TX_GPIO, SERIAL_LINK_RX_GPIO,
|
||||
UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE));
|
||||
ESP_ERROR_CHECK(usb_serial_jtag_driver_install(&cfg));
|
||||
|
||||
xTaskCreate(rx_task, "serial_link_rx", 4096, NULL, 6, NULL);
|
||||
ESP_LOGI(TAG, "serial_link up on UART%d, TX=GPIO%d RX=GPIO%d @ %d baud",
|
||||
SERIAL_LINK_UART_NUM, SERIAL_LINK_TX_GPIO, SERIAL_LINK_RX_GPIO, SERIAL_LINK_BAUD);
|
||||
xTaskCreate(status_task, "serial_link_status", 2560, NULL, 4, NULL);
|
||||
ESP_LOGI(TAG, "serial_link up on native USB Serial/JTAG (VID 0x303A / PID 0x1001), "
|
||||
"max payload %d, 1 Hz heartbeat", SERIAL_LINK_MAX_PAYLOAD);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user