diff --git a/docs/binary-record-v1.md b/docs/binary-record-v1.md index 5da056d..c9d2980 100644 --- a/docs/binary-record-v1.md +++ b/docs/binary-record-v1.md @@ -89,11 +89,13 @@ new samples rather than overwriting older ones; sequence gaps and the cumulative lost-sample counter expose that permanent loss. The shared transport state machine distinguishes three nonfatal states. `RETRY` -means zero bytes were accepted and the complete frame may be submitted again. -`PENDING` means the backend owns an in-flight frame, so firmware may only poll -that transfer. `COMPLETE` permits the output task to reuse its packet buffer and -consume more samples. This prevents a timeout after partial progress from -causing an ambiguous whole-frame duplicate. +is valid only from initial submission: it means zero bytes were accepted and the +complete frame may be submitted again. `PENDING` means the backend owns an +in-flight frame, so firmware may only poll that transfer. A `RETRY` returned by +polling fails closed as `FATAL`, because generic code cannot prove whole-frame +resubmission is duplicate-safe. `COMPLETE` permits the output task to reuse its +packet buffer and consume more samples. This prevents a timeout after partial +progress from causing an ambiguous whole-frame duplicate. The direct USB Serial/JTAG backend atomically copies a complete frame into its TX ring, then polls a bounded transmit-drain wait. A timeout remains `PENDING`; it @@ -104,7 +106,9 @@ USB drain is not end-to-end application delivery confirmation. A host process may attach after earlier frames have already left the endpoint, or fail after the endpoint accepts them. CRC and sequence checks make resulting loss visible, but an application acknowledgement and replay window are still required to -guarantee receipt. +guarantee receipt. Accordingly, USB `COMPLETE` means endpoint drain, while the +planned reliable BLE backend will reserve `COMPLETE` for an application ACK of +the exact frame. Receivers report bytes left in an incomplete trailing frame when capture ends. Those bytes cannot pass CRC validation and are not silently admitted as samples. diff --git a/docs/transport-layer-validation-2026-08-17.md b/docs/transport-layer-validation-2026-08-17.md index f0f273d..4b5da1b 100644 --- a/docs/transport-layer-validation-2026-08-17.md +++ b/docs/transport-layer-validation-2026-08-17.md @@ -6,23 +6,38 @@ ESP-IDF's interrupt-driven USB Serial/JTAG driver. ## Transaction contract -- `RETRY`: the backend accepted zero bytes, so whole-frame resubmission is safe. +- `RETRY`: valid only from initial submission; the backend accepted zero bytes, + so whole-frame resubmission is safe. - `PENDING`: the backend owns an in-flight frame. Only completion polling is allowed; the caller must retain and not modify the packet buffer. -- `COMPLETE`: the backend's completion criterion is satisfied and the caller may - reuse the packet buffer. +- `COMPLETE`: the backend's documented completion criterion is satisfied and the + caller may reuse the packet buffer. USB means endpoint drain; reliable BLE + will mean application acknowledgement of the exact frame. - `FATAL`: a programming or backend invariant failed. The output task stops consuming the sample queue rather than silently discarding its in-flight data. + `RETRY` from a pending poll is promoted to `FATAL` because generic code cannot + prove resubmission is duplicate-safe. For USB, submission uses a 512-byte TX ring and a bounded 50 ms write. ESP-IDF's ring-buffer send is all-or-nothing for each `TRK1` frame. Once accepted, bounded 50 ms `usb_serial_jtag_wait_tx_done()` calls continue returning `PENDING` until -the host drains the endpoint. A timeout never resubmits the frame. +the host drains the endpoint. The first completion poll happens immediately +after acceptance; repeated pending polls are scheduler-paced. A timeout never +resubmits the frame. The VFS is switched to driver mode after readable startup output so any unexpected diagnostic output cannot race the driver's ISR by accessing the hardware FIFO directly. Firmware logs are disabled before binary telemetry -tasks start, as before. +tasks start, as before. A terminal transport invariant re-enables error logging, +emits one final diagnostic, and suspends the output task. Because no later binary +frame can follow, that diagnostic cannot corrupt a recoverable stream. + +Queue-allocation and task-creation failure paths switch the VFS back to its +non-driver mode and uninstall the USB driver before emitting their error, so the +only startup diagnostic is not stranded in a TX ring that is immediately freed. +The USB backend's zero-write-to-`RETRY` mapping depends explicitly on its checked +initialized/sole-owner lifecycle; a torn-down backend fails the guard as +`FATAL` instead of masquerading as backpressure. ## Verification @@ -33,12 +48,24 @@ The host C transport fixture compiles the production state machine with - accepted transfers become pending; - repeated pending polls never call submission again; - completion returns the sender to idle; -- backend-confirmed safe retry returns to idle; +- retry returned after acceptance fails closed without resubmission; - invalid arguments and unknown backend states fail closed. -The assembled ESP32-C3 prototype produced a normal 1,728-sample direct-driver -capture with no packet gaps, sample gaps, resets, CRC failures, reported drops, -loop overruns, trailing partial bytes, or timestamp-saturation frames. +The assembled ESP32-C3 prototype first produced a normal 1,728-sample +pre-commit direct-driver capture with no packet gaps, sample gaps, resets, CRC +failures, reported drops, loop overruns, trailing partial bytes, or +timestamp-saturation frames. + +After commit `3c95f3d` was built and flashed exactly, a second capture contained +1,680 contiguous samples, sequences 0 through 1,679, with the same zero-loss +integrity result. Its optional raw wire file contains all 563 startup bytes and +re-decodes to CSV byte-for-byte identical to the live-rendered CSV. Both sides +of that exact capture are tracked as fixtures: + +- `tests/fixtures/direct_usb_3c95f3d.trk`, SHA-256 + `f495486f094a758bb785e145026e3934d60b52dd5083aef7f5d896195d967869` +- `tests/fixtures/direct_usb_3c95f3d.wire`, SHA-256 + `3bdaeadff7962c6eac48c4ebeda285c8eb359e439d5e1728104add2009122c03` For the connected-stall case, the USB endpoint was left enumerated without a serial reader long enough to overflow the 512-sample acquisition queue. When the diff --git a/main/trikke_sensor_main.c b/main/trikke_sensor_main.c index 5bb0bf4..5c0b709 100644 --- a/main/trikke_sensor_main.c +++ b/main/trikke_sensor_main.c @@ -168,6 +168,7 @@ static void write_binary_packet_until_sent( size_t packet_size) { while (true) { + const bool was_pending = sender->pending; const trikke_transport_status_t status = trikke_transport_sender_step( sender, &context->transport, packet, packet_size); @@ -175,12 +176,20 @@ static void write_binary_packet_until_sent( return; } if (status == TRIKKE_TRANSPORT_FATAL) { - // Preserve the in-flight packet and stop consuming the queue. A - // fatal backend invariant is not safely recoverable or retryable. + // The binary stream is terminal at this point, so a final text + // diagnostic cannot corrupt later frames. Preserve the packet and + // stop consuming the sample queue. + esp_log_level_set(TAG, ESP_LOG_ERROR); + ESP_LOGE(TAG, "fatal transport invariant; output task suspended"); while (true) { - vTaskDelay(portMAX_DELAY); + vTaskSuspend(NULL); } } + if (status == TRIKKE_TRANSPORT_PENDING && !was_pending) { + // Poll once immediately after acceptance. Subsequent pending polls + // are paced so a future nonblocking backend cannot busy-spin. + continue; + } vTaskDelay(pdMS_TO_TICKS(TRIKKE_TRANSPORT_RETRY_DELAY_MS)); } } @@ -349,11 +358,11 @@ void app_main(void) s_context.sample_queue = xQueueCreate(TRIKKE_SAMPLE_QUEUE_DEPTH, sizeof(trikke_wire_sample_t)); if (s_context.sample_queue == NULL) { + trikke_usb_transport_deinit(&s_context.usb_transport); ESP_LOGE(TAG, "sample queue allocation failed"); l3g4200d_deinit(&s_context.gyroscope); adxl345_deinit(&s_context.accelerometer); i2c_del_master_bus(bus); - trikke_usb_transport_deinit(&s_context.usb_transport); return; } @@ -370,11 +379,11 @@ void app_main(void) vTaskDelete(acquisition_task_handle); } vQueueDelete(s_context.sample_queue); + trikke_usb_transport_deinit(&s_context.usb_transport); ESP_LOGE(TAG, "telemetry task creation failed"); l3g4200d_deinit(&s_context.gyroscope); adxl345_deinit(&s_context.accelerometer); i2c_del_master_bus(bus); - trikke_usb_transport_deinit(&s_context.usb_transport); return; } diff --git a/main/trikke_transport.c b/main/trikke_transport.c index 29eeac0..7d81889 100644 --- a/main/trikke_transport.c +++ b/main/trikke_transport.c @@ -24,20 +24,29 @@ trikke_transport_status_t trikke_transport_sender_step( return TRIKKE_TRANSPORT_FATAL; } - const trikke_transport_status_t status = sender->pending + const bool was_pending = sender->pending; + const trikke_transport_status_t status = was_pending ? transport->poll(transport->context) : transport->begin(transport->context, packet, packet_size); if (!status_is_valid(status)) { - sender->pending = false; + sender->pending = was_pending; return TRIKKE_TRANSPORT_FATAL; } if (status == TRIKKE_TRANSPORT_PENDING) { sender->pending = true; - } else { - // RETRY from poll is allowed only when the backend has discarded or - // otherwise resolved the old transfer and knows resubmission is safe. + } else if (status == TRIKKE_TRANSPORT_COMPLETE) { sender->pending = false; + } else if (status == TRIKKE_TRANSPORT_RETRY) { + if (was_pending) { + // Once accepted, generic transport code cannot prove that retrying + // the whole frame is duplicate-safe. Fail closed and keep ownership. + sender->pending = true; + return TRIKKE_TRANSPORT_FATAL; + } + sender->pending = false; + } else { + sender->pending = was_pending; } return status; } diff --git a/main/trikke_transport.h b/main/trikke_transport.h index b90fb28..2e295ba 100644 --- a/main/trikke_transport.h +++ b/main/trikke_transport.h @@ -36,7 +36,12 @@ void trikke_transport_sender_init(trikke_transport_sender_t *sender); // Advances one bounded transport operation. The packet storage must remain valid // and unchanged from the first PENDING result through COMPLETE. While pending, -// only poll is called: an ambiguous timeout can never duplicate a frame. +// only poll is called: an ambiguous timeout can never duplicate a frame. RETRY +// is valid only from begin, where it guarantees that no bytes were accepted. +// +// COMPLETE is backend-specific. USB uses endpoint drain, which does not prove +// application receipt. A reliable BLE backend must reserve COMPLETE for an +// application acknowledgement covering this exact frame. trikke_transport_status_t trikke_transport_sender_step( trikke_transport_sender_t *sender, const trikke_transport_t *transport, diff --git a/main/trikke_usb_transport.c b/main/trikke_usb_transport.c index 9d17576..d912808 100644 --- a/main/trikke_usb_transport.c +++ b/main/trikke_usb_transport.c @@ -26,6 +26,9 @@ static trikke_transport_status_t usb_begin_packet( return TRIKKE_TRANSPORT_PENDING; } if (written == 0) { + // Arguments and lifecycle were validated above, and this backend is the + // driver's sole owner. Under that invariant, zero means the all-or-none + // ring submission timed out without accepting this frame. return TRIKKE_TRANSPORT_RETRY; } return TRIKKE_TRANSPORT_FATAL; diff --git a/tests/fixtures/README.md b/tests/fixtures/README.md index dd5b940..dcfa6fe 100644 --- a/tests/fixtures/README.md +++ b/tests/fixtures/README.md @@ -31,6 +31,18 @@ removed before production firmware was built and flashed. capture application opened. That is deliberate evidence of the remaining distinction: USB endpoint drain is observable, but application receipt requires the planned acknowledgement/replay layer. +- `direct_usb_3c95f3d.trk` — SHA-256 + `f495486f094a758bb785e145026e3934d60b52dd5083aef7f5d896195d967869`. + This is the validated-frame output from the final exact-commit smoke capture: + 1,680 contiguous samples, sequences 0 through 1,679, with zero packet/sample + gaps, resets, CRC failures, reported drops, loop overruns, trailing bytes, or + timestamp saturation. +- `direct_usb_3c95f3d.wire` — SHA-256 + `3bdaeadff7962c6eac48c4ebeda285c8eb359e439d5e1728104add2009122c03`. + This is the byte-for-byte wire side of the same capture. It contains 563 bytes + of startup text before the valid frames. That text includes the literal + `TRK1`, producing one rejected candidate header as designed. Extracting all + valid frames reproduces `direct_usb_3c95f3d.trk` byte-for-byte. `tests/test_trikke_protocol.py` verifies the hashes, parses the captures in fragmented chunks, and asserts these signatures so the hardware evidence remains diff --git a/tests/fixtures/direct_usb_3c95f3d.trk b/tests/fixtures/direct_usb_3c95f3d.trk new file mode 100644 index 0000000..039c3b1 Binary files /dev/null and b/tests/fixtures/direct_usb_3c95f3d.trk differ diff --git a/tests/fixtures/direct_usb_3c95f3d.wire b/tests/fixtures/direct_usb_3c95f3d.wire new file mode 100644 index 0000000..84610ff Binary files /dev/null and b/tests/fixtures/direct_usb_3c95f3d.wire differ diff --git a/tests/test_trikke_protocol.py b/tests/test_trikke_protocol.py index a79bbad..148b988 100644 --- a/tests/test_trikke_protocol.py +++ b/tests/test_trikke_protocol.py @@ -185,6 +185,7 @@ class ProtocolContractTest(unittest.TestCase): "first_sequence": 0, "last_sequence": 2143, "max_dropped": 0, + "timing_anomalies": 2, "gaps": [], }, "forced_outage_7s.trk": { @@ -193,6 +194,7 @@ class ProtocolContractTest(unittest.TestCase): "first_sequence": 0, "last_sequence": 1977, "max_dropped": 138, + "timing_anomalies": 3, "gaps": [(511, 650, 1_390_000)], }, "direct_usb_stall.trk": { @@ -201,8 +203,18 @@ class ProtocolContractTest(unittest.TestCase): "first_sequence": 8, "last_sequence": 2065, "max_dropped": 1194, + "timing_anomalies": 1, "gaps": [(511, 1706, 11_950_000)], }, + "direct_usb_3c95f3d.trk": { + "sha256": "f495486f094a758bb785e145026e3934d60b52dd5083aef7f5d896195d967869", + "sample_count": 1680, + "first_sequence": 0, + "last_sequence": 1679, + "max_dropped": 0, + "timing_anomalies": 8, + "gaps": [], + }, } for name, contract in expected.items(): @@ -251,6 +263,10 @@ class ProtocolContractTest(unittest.TestCase): contract["max_dropped"], integrity.sample_gap_count ) self.assertEqual(0, integrity.sample_reset_count) + self.assertEqual( + contract["timing_anomalies"], + integrity.timing_anomaly_count, + ) self.assertEqual( contract["max_dropped"], integrity.final_dropped_sample_count, @@ -268,6 +284,31 @@ class ProtocolContractTest(unittest.TestCase): ] self.assertEqual(contract["gaps"], gaps) + def test_exact_usb_raw_wire_evidence(self) -> None: + wire = ( + ROOT / "tests" / "fixtures" / "direct_usb_3c95f3d.wire" + ).read_bytes() + validated = ( + ROOT / "tests" / "fixtures" / "direct_usb_3c95f3d.trk" + ).read_bytes() + self.assertEqual( + "3bdaeadff7962c6eac48c4ebeda285c8eb359e439d5e1728104add2009122c03", + hashlib.sha256(wire).hexdigest(), + ) + + parser = StreamParser() + frames = [] + for offset in range(0, len(wire), 113): + frames.extend(parser.feed(wire[offset : offset + 113])) + + self.assertEqual(214, len(frames)) + self.assertEqual(validated, b"".join(frame.raw for frame in frames)) + self.assertEqual(0, parser.startup_crc_errors) + self.assertEqual(0, parser.crc_errors) + self.assertEqual(1, parser.header_errors) + self.assertEqual(563, parser.skipped_bytes) + self.assertEqual(0, parser.buffered_bytes) + if __name__ == "__main__": unittest.main() diff --git a/tests/transport_fixture.c b/tests/transport_fixture.c index 32f9c33..af43c04 100644 --- a/tests/transport_fixture.c +++ b/tests/transport_fixture.c @@ -99,11 +99,16 @@ int main(void) TRIKKE_TRANSPORT_PENDING || trikke_transport_sender_step( &sender, &transport, packet, sizeof(packet)) != - TRIKKE_TRANSPORT_RETRY || - sender.pending) { - return fail(6, "backend-confirmed safe retry must return to idle"); + TRIKKE_TRANSPORT_FATAL || + !sender.pending || mock.begin_calls != 4 || mock.poll_calls != 3 || + trikke_transport_sender_step( + &sender, &transport, packet, sizeof(packet)) != + TRIKKE_TRANSPORT_FATAL || + mock.begin_calls != 4 || mock.poll_calls != 4) { + return fail(6, "retry after acceptance must fail closed without resubmit"); } + trikke_transport_sender_init(&sender); if (trikke_transport_sender_step( NULL, &transport, packet, sizeof(packet)) != TRIKKE_TRANSPORT_FATAL ||