Keep USB line coding diagnostic only. Fix cdc connect reconfiguring UART

1
This commit is contained in:
2026-08-31 03:40:55 +02:00
parent 6ad6c00d68
commit 2aafff25be
8 changed files with 16 additions and 153 deletions
+1 -1
View File
@@ -92,7 +92,7 @@ The broker currently enters nonblocking serial read/write APIs while holding its
`usb_cdc_transport` has a permanent transport task and TinyUSB callbacks. Attached plus host DTR asserted is treated as open. Opening starts the serial service if necessary, creates the `usb-cdc` broker client, and opportunistically requests writer ownership; otherwise USB observes. `usb_cdc_transport` has a permanent transport task and TinyUSB callbacks. Attached plus host DTR asserted is treated as open. Opening starts the serial service if necessary, creates the `usb-cdc` broker client, and opportunistically requests writer ownership; otherwise USB observes.
TinyUSB callbacks enqueue/copy data and state; the transport task owns broker lifecycle and forwarding. The callback caches the latest supported host line coding regardless of current writer state; the task applies it only while USB is writer, the serial service is running, and TX is empty. The pending value is discarded when the CDC session closes and is not automatically persisted. TinyUSB callbacks enqueue/copy data and state; the transport task owns broker lifecycle and forwarding. The line-coding callback records the latest host setting for diagnostics only. It never reconfigures UART1: physical framing and speed remain controlled by the explicit serial configuration, regardless of USB writer ownership.
### HTTPS, WebSocket, and web serial ### HTTPS, WebSocket, and web serial
+1 -1
View File
@@ -54,7 +54,7 @@ This is a semantic map, not a complete file inventory. Start here, then read the
- Dependencies: TinyUSB, broker, serial service - Dependencies: TinyUSB, broker, serial service
- Flow: `USB host <-> USB task <-> broker` - Flow: `USB host <-> USB task <-> broker`
- Lifecycle: permanent owner task; broker client exists only while attached with host DTR asserted. - Lifecycle: permanent owner task; broker client exists only while attached with host DTR asserted.
- Constraint: callbacks cache the latest host line coding; the task applies it only while USB is writer, UART is running, and TX is empty. It is RAM-only and is discarded when the CDC session closes. - Constraint: callbacks retain the latest host line coding only for diagnostics; it never reconfigures UART1. UART1 framing and speed remain controlled by the serial configuration and its explicit persistence commands.
## Web and WebSocket serial ## Web and WebSocket serial
+7 -1
View File
@@ -46,7 +46,13 @@ These observations should be checked when touching the relevant area; they are n
## Active Task ## Active Task
No active task recorded. - **Objective:** Prevent USB CDC host line coding from implicitly reconfiguring UART1 when USB becomes writer.
- **Relevant files:** `src/usb_cdc_transport.{c,h}`, `src/usb_console.c`, USB serial documentation.
- **Findings:** The transport converted cached CDC line coding into `serial_config_t` and called `serial_service_apply_config()` after writer acquisition. That deliberately stopped/restarted UART1 and changed its RAM configuration.
- **Decision:** Treat CDC line coding as diagnostic metadata only. UART1 configuration remains exclusively controlled by explicit `serial` commands and their NVS persistence.
- **Changes completed:** Removed the pending line-coding apply path and its counters; retained the latest host setting for `usb status`; updated the contract documentation.
- **Remaining work:** Target-hardware verification: acquire/release USB writer ownership after changing a host terminal's line coding and confirm UART1 remains at the configured framing.
- **Risks / things to remember:** Host terminal line-coding selectors no longer configure the physical RS-232 port; use `serial set`/`serial save` instead. `pio run` passed after the change.
### Handoff template ### Handoff template
+1 -1
View File
@@ -85,7 +85,7 @@ Each client has a generation-safe ID. There can be one writer and multiple obser
| `usb counters` / `usb clear-counters` | Show or clear USB counters. | | `usb counters` / `usb clear-counters` | Show or clear USB counters. |
| `usb request-writer` / `usb release-writer` | Request or release USB writer ownership. | | `usb request-writer` / `usb release-writer` | Request or release USB writer ownership. |
Opening `/dev/ttyACM*` with DTR asserted creates the `usb-cdc` broker client, starts UART1 if needed, and requests writer ownership. It becomes an observer if another client is writer. USB data is binary-transparent. Supported host line coding is 1101000000 baud, 7/8 data bits, none/odd/even parity, and 1/2 stop bits; settings apply only while USB owns the writer lease and are not saved automatically. Opening `/dev/ttyACM*` with DTR asserted creates the `usb-cdc` broker client, starts UART1 if needed, and requests writer ownership. It becomes an observer if another client is writer. USB data is binary-transparent. The host's CDC line coding is shown by `usb status` for diagnostics only; it does not alter UART1. Configure physical baud rate, framing, flow control, and DTR explicitly with `serial` commands and persist them with `serial save`.
## Wi-Fi ## Wi-Fi
+1 -1
View File
@@ -93,7 +93,7 @@ Implemented and hardware-validated:
- Automatic broker connection when the host opens the port with DTR asserted. - Automatic broker connection when the host opens the port with DTR asserted.
- Opportunistic writer acquisition with observer fallback. - Opportunistic writer acquisition with observer fallback.
- Binary-transparent bidirectional serial data. - Binary-transparent bidirectional serial data.
- Supported host line-coding application while USB owns the writer lease. - Host line-coding visibility for diagnostics; UART1 remains under explicit serial configuration control.
- Disconnect cleanup, bounded buffering, counters, and UART0 lifecycle controls. - Disconnect cleanup, bounded buffering, counters, and UART0 lifecycle controls.
### Phase 4 — Wi-Fi foundation ### Phase 4 — Wi-Fi foundation
+3 -139
View File
@@ -9,7 +9,6 @@
#include "freertos/queue.h" #include "freertos/queue.h"
#include "freertos/stream_buffer.h" #include "freertos/stream_buffer.h"
#include "freertos/task.h" #include "freertos/task.h"
#include "serial_config.h"
#include "serial_service.h" #include "serial_service.h"
#include "tinyusb.h" #include "tinyusb.h"
#include "tinyusb_cdc_acm.h" #include "tinyusb_cdc_acm.h"
@@ -54,8 +53,8 @@ static atomic_uint s_connection_generation;
static session_broker_client_id_t s_broker_client_id; static session_broker_client_id_t s_broker_client_id;
static bool s_writer; static bool s_writer;
/* Host-provided CDC line coding is diagnostic information only. */
static usb_cdc_transport_line_coding_t s_line_coding; static usb_cdc_transport_line_coding_t s_line_coding;
static bool s_line_coding_pending;
static usb_cdc_transport_counters_t s_counters; static usb_cdc_transport_counters_t s_counters;
static const char s_language_descriptor[] = {0x09, 0x04}; static const char s_language_descriptor[] = {0x09, 0x04};
@@ -134,14 +133,11 @@ static void device_event_callback(tinyusb_event_t *event, void *arg)
notify_transport_task(); notify_transport_task();
break; break;
case TINYUSB_EVENT_DETACHED: { case TINYUSB_EVENT_DETACHED: {
/* A new attachment must receive fresh control state and line coding. */ /* A new attachment must receive fresh control state. */
unsigned int old_state = atomic_exchange(&s_usb_state, 0U); unsigned int old_state = atomic_exchange(&s_usb_state, 0U);
if (usb_state_is_open(old_state)) { if (usb_state_is_open(old_state)) {
atomic_fetch_add(&s_connection_generation, 1U); atomic_fetch_add(&s_connection_generation, 1U);
} }
taskENTER_CRITICAL(&s_state_lock);
s_line_coding_pending = false;
taskEXIT_CRITICAL(&s_state_lock);
notify_transport_task(); notify_transport_task();
break; break;
} }
@@ -226,13 +222,6 @@ static void cdc_line_state_callback(int itf, cdcacm_event_t *event)
} }
} }
if (!event->line_state_changed_data.dtr) {
/* Do not apply a closed host session's deferred line coding after reopen. */
taskENTER_CRITICAL(&s_state_lock);
s_line_coding_pending = false;
taskEXIT_CRITICAL(&s_state_lock);
}
notify_transport_task(); notify_transport_task();
} }
@@ -252,137 +241,15 @@ static void cdc_line_coding_callback(int itf, cdcacm_event_t *event)
sizeof(coding)); sizeof(coding));
taskENTER_CRITICAL(&s_state_lock); taskENTER_CRITICAL(&s_state_lock);
if (s_line_coding_pending) {
/* Preserve the latest complete setting and account for the superseded one. */
++s_counters.callback_drops;
}
s_line_coding = (usb_cdc_transport_line_coding_t) { s_line_coding = (usb_cdc_transport_line_coding_t) {
.baud_rate = coding.bit_rate, .baud_rate = coding.bit_rate,
.stop_bits = coding.stop_bits, .stop_bits = coding.stop_bits,
.parity = coding.parity, .parity = coding.parity,
.data_bits = coding.data_bits, .data_bits = coding.data_bits,
}; };
s_line_coding_pending = true;
taskEXIT_CRITICAL(&s_state_lock); taskEXIT_CRITICAL(&s_state_lock);
notify_transport_task(); /* CDC line coding must not reconfigure the independently configured UART1. */
}
static bool take_pending_line_coding(usb_cdc_transport_line_coding_t *coding)
{
bool pending;
taskENTER_CRITICAL(&s_state_lock);
pending = s_line_coding_pending;
if (pending) {
*coding = s_line_coding;
s_line_coding_pending = false;
}
taskEXIT_CRITICAL(&s_state_lock);
return pending;
}
static bool serial_configs_equal(const serial_config_t *left,
const serial_config_t *right)
{
return left->version == right->version &&
left->baud_rate == right->baud_rate &&
left->data_bits == right->data_bits &&
left->parity == right->parity &&
left->stop_bits == right->stop_bits &&
left->flow_control == right->flow_control &&
left->dtr_behavior == right->dtr_behavior &&
left->rts_threshold == right->rts_threshold;
}
static bool map_line_coding(const usb_cdc_transport_line_coding_t *coding,
serial_config_t *config)
{
if (coding->baud_rate < SERIAL_CONFIG_MIN_BAUD_RATE ||
coding->baud_rate > SERIAL_CONFIG_MAX_BAUD_RATE) {
return false;
}
config->baud_rate = coding->baud_rate;
switch (coding->data_bits) {
case 7U:
config->data_bits = SERIAL_CONFIG_DATA_BITS_7;
break;
case 8U:
config->data_bits = SERIAL_CONFIG_DATA_BITS_8;
break;
default:
return false;
}
switch (coding->parity) {
case CDC_LINE_CODING_PARITY_NONE:
config->parity = SERIAL_CONFIG_PARITY_NONE;
break;
case CDC_LINE_CODING_PARITY_ODD:
config->parity = SERIAL_CONFIG_PARITY_ODD;
break;
case CDC_LINE_CODING_PARITY_EVEN:
config->parity = SERIAL_CONFIG_PARITY_EVEN;
break;
default:
/* Mark and space parity are intentionally not representable by UART policy. */
return false;
}
switch (coding->stop_bits) {
case CDC_LINE_CODING_STOP_BITS_1:
config->stop_bits = SERIAL_CONFIG_STOP_BITS_1;
break;
case CDC_LINE_CODING_STOP_BITS_2:
config->stop_bits = SERIAL_CONFIG_STOP_BITS_2;
break;
default:
/* This also rejects USB's 1.5-stop-bit encoding. */
return false;
}
return true;
}
static void apply_pending_line_coding(bool writer)
{
if (!writer || !serial_service_is_running()) {
return;
}
/* Restarting UART1 discards queued TX, so defer framing changes until idle. */
if (serial_service_tx_pending() > 0U) {
return;
}
usb_cdc_transport_line_coding_t coding;
if (!take_pending_line_coding(&coding)) {
return;
}
serial_config_t current;
if (serial_service_get_config(&current) != ESP_OK) {
add_counter(&s_counters.line_coding_failed, 1U);
return;
}
serial_config_t desired = current;
if (!map_line_coding(&coding, &desired)) {
add_counter(&s_counters.line_coding_rejected, 1U);
return;
}
/* Flow control, DTR policy, and RTS threshold remain from current RAM state. */
if (serial_configs_equal(&current, &desired)) {
return;
}
if (serial_service_apply_config(&desired) == ESP_OK) {
add_counter(&s_counters.line_coding_applied, 1U);
} else {
add_counter(&s_counters.line_coding_failed, 1U);
}
} }
static bool writer_event_type(session_broker_event_type_t type) static bool writer_event_type(session_broker_event_type_t type)
@@ -762,8 +629,6 @@ static void transport_task(void *context)
continue; continue;
} }
apply_pending_line_coding(writer);
if (atomic_load(&s_connection_generation) != observed_generation) { if (atomic_load(&s_connection_generation) != observed_generation) {
continue; continue;
} }
@@ -812,7 +677,6 @@ static void reset_uninitialized_state(void)
.parity = USB_CDC_TRANSPORT_PARITY_NONE, .parity = USB_CDC_TRANSPORT_PARITY_NONE,
.data_bits = 8U, .data_bits = 8U,
}; };
s_line_coding_pending = false;
memset(&s_counters, 0, sizeof(s_counters)); memset(&s_counters, 0, sizeof(s_counters));
taskEXIT_CRITICAL(&s_state_lock); taskEXIT_CRITICAL(&s_state_lock);
} }
-3
View File
@@ -44,9 +44,6 @@ typedef struct {
uint64_t writer_revocations; uint64_t writer_revocations;
uint64_t writer_events; uint64_t writer_events;
uint64_t service_start_failures; uint64_t service_start_failures;
uint64_t line_coding_applied;
uint64_t line_coding_rejected;
uint64_t line_coding_failed;
uint64_t callback_drops; uint64_t callback_drops;
uint64_t control_drops; uint64_t control_drops;
} usb_cdc_transport_counters_t; } usb_cdc_transport_counters_t;
+2 -6
View File
@@ -76,7 +76,7 @@ static int show_status(void)
(unsigned int)snapshot.line_coding.data_bits, (unsigned int)snapshot.line_coding.data_bits,
parity_name(snapshot.line_coding.parity), parity_name(snapshot.line_coding.parity),
stop_bits_name(snapshot.line_coding.stop_bits)); stop_bits_name(snapshot.line_coding.stop_bits));
printf("USB line coding changes RAM only; use 'serial save' to persist it.\n"); printf("Host line coding is reported only; UART1 uses the serial configuration.\n");
return 0; return 0;
} }
@@ -110,11 +110,7 @@ static int show_counters(void)
counter->writer_denials, counter->writer_denials,
counter->writer_revocations, counter->writer_revocations,
counter->writer_events); counter->writer_events);
printf("Line coding: applied=%" PRIu64 " rejected=%" PRIu64 printf("Service start failures=%" PRIu64 "\n",
" failed=%" PRIu64 " service-start-failures=%" PRIu64 "\n",
counter->line_coding_applied,
counter->line_coding_rejected,
counter->line_coding_failed,
counter->service_start_failures); counter->service_start_failures);
printf("Control/callback drops: control=%" PRIu64 " callback=%" PRIu64 "\n", printf("Control/callback drops: control=%" PRIu64 " callback=%" PRIu64 "\n",
counter->control_drops, counter->control_drops,