tighten transport failure semantics
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
+14
-5
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
Vendored
+12
@@ -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
|
||||
|
||||
BIN
Binary file not shown.
BIN
Binary file not shown.
@@ -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()
|
||||
|
||||
@@ -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 ||
|
||||
|
||||
Reference in New Issue
Block a user