mirror of
https://github.com/MarekZegare4/MeshCore-Solo.git
synced 2026-07-26 14:58:12 +00:00
fix: bound Serial.write() to prevent indefinite hang on stalled USB host
Adafruit_USBD_CDC::write() blocks as long as DTR is asserted, even if the host stopped draining the FIFO. Every writeFrame() call and the GPX trail export wrote directly to Serial, so a serial monitor left open but idle could stall the main loop until the 30s watchdog reset the device. streamWriteUntil() caps the wait with an absolute deadline shared across chained writes, and only enters the wait loop when availableForWrite() is actually exhausted, so a healthy host sees no behavior change. The GPX export latches after the first stall to avoid paying the timeout once per write call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -11,8 +11,41 @@
|
||||
#include "icons.h" // scalable mini-icons (map markers, north arrow, grid dots)
|
||||
#include "NavView.h"
|
||||
#include "WaypointsView.h"
|
||||
#include <helpers/StreamUtils.h>
|
||||
#include <math.h>
|
||||
|
||||
// Bounds every Serial write the GPX export makes. A full trail dump is tens
|
||||
// of KB across hundreds of individual write() calls (one or more per point) —
|
||||
// without this, a serial monitor that's open but not actively reading (DTR
|
||||
// asserted, nothing draining the USB-CDC FIFO) would block each one, and the
|
||||
// main loop with it, indefinitely (see StreamUtils.h). Duck-typed to the
|
||||
// handful of Print methods Trail.h's GPX writers actually call, so it slots
|
||||
// into their `S& out` template parameter without any change there.
|
||||
// __FlashStringHelper content is plain flash on nRF52/ARM (no
|
||||
// Harvard-architecture PROGMEM split), so it's safe to read like any other
|
||||
// char*.
|
||||
//
|
||||
// The first stalled write gives up for the rest of the export (_gave_up),
|
||||
// rather than re-arming a fresh timeout on every one of those hundreds of
|
||||
// calls — a host that's truly stuck would otherwise cost calls × timeout_ms
|
||||
// in total instead of one bounded stall.
|
||||
struct BoundedSerialPrint {
|
||||
uint32_t timeout_ms;
|
||||
bool _gave_up;
|
||||
BoundedSerialPrint(uint32_t t) : timeout_ms(t), _gave_up(false) {}
|
||||
size_t write(const uint8_t* buf, size_t len) {
|
||||
if (_gave_up) return 0;
|
||||
size_t sent = streamWriteUntil(Serial, buf, len, millis() + timeout_ms);
|
||||
if (sent < len) _gave_up = true;
|
||||
return sent;
|
||||
}
|
||||
size_t print(const char* s) { return write((const uint8_t*)s, strlen(s)); }
|
||||
size_t print(const __FlashStringHelper* s) {
|
||||
const char* p = reinterpret_cast<const char*>(s);
|
||||
return write((const uint8_t*)p, strlen(p));
|
||||
}
|
||||
};
|
||||
|
||||
class TrailScreen : public UIScreen {
|
||||
UITask* _task;
|
||||
TrailStore* _store;
|
||||
@@ -219,7 +252,8 @@ private:
|
||||
}
|
||||
void handleExport() {
|
||||
if (!Serial) { _task->showAlert("Connect USB first", 1200); return; }
|
||||
size_t n = _store->exportGpx(Serial, _task->waypoints());
|
||||
BoundedSerialPrint out{300};
|
||||
size_t n = _store->exportGpx(out, _task->waypoints());
|
||||
showExportAlert(n);
|
||||
}
|
||||
|
||||
@@ -229,7 +263,8 @@ private:
|
||||
if (!ds) { _task->showAlert("FS unavailable", 800); return; }
|
||||
File f = ds->openRead(TRAIL_FILE);
|
||||
if (!f) { _task->showAlert("No saved trail", 800); return; }
|
||||
size_t n = TrailStore::exportGpxFromFile(f, Serial, _task->waypoints());
|
||||
BoundedSerialPrint out{300};
|
||||
size_t n = TrailStore::exportGpxFromFile(f, out, _task->waypoints());
|
||||
f.close();
|
||||
if (n == 0) { _task->showAlert("Bad saved file", 1000); return; }
|
||||
showExportAlert(n);
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
#include "ArduinoSerialInterface.h"
|
||||
#include "StreamUtils.h"
|
||||
|
||||
// Worst-case time we'll let writeFrame() block waiting for the host to drain
|
||||
// the link, before giving up on the rest of the frame. See StreamUtils.h.
|
||||
#define SERIAL_WRITE_TIMEOUT_MS 300
|
||||
|
||||
#define RECV_STATE_IDLE 0
|
||||
#define RECV_STATE_HDR_FOUND 1
|
||||
@@ -32,8 +37,11 @@ size_t ArduinoSerialInterface::writeFrame(const uint8_t src[], size_t len) {
|
||||
hdr[1] = (len & 0xFF); // LSB
|
||||
hdr[2] = (len >> 8); // MSB
|
||||
|
||||
_serial->write(hdr, 3);
|
||||
return _serial->write(src, len);
|
||||
// Single deadline shared across header + payload, so a stalled host costs
|
||||
// at most SERIAL_WRITE_TIMEOUT_MS total, not that much per write() call.
|
||||
uint32_t deadline = millis() + SERIAL_WRITE_TIMEOUT_MS;
|
||||
if (streamWriteUntil(*_serial, hdr, 3, deadline) < 3) return 0;
|
||||
return streamWriteUntil(*_serial, src, len, deadline);
|
||||
}
|
||||
|
||||
size_t ArduinoSerialInterface::checkRecvFrame(uint8_t dest[]) {
|
||||
|
||||
40
src/helpers/StreamUtils.h
Normal file
40
src/helpers/StreamUtils.h
Normal file
@@ -0,0 +1,40 @@
|
||||
#pragma once
|
||||
// A USB-CDC serial monitor or companion app can leave the port open (DTR
|
||||
// asserted) without ever draining it. Stream::write() then blocks inside the
|
||||
// platform's "TX FIFO full" wait loop for as long as that holds true — which
|
||||
// is forever in practice, since the main loop()'s hardware watchdog is only
|
||||
// re-armed once per iteration. streamWriteUntil() caps that wait instead of
|
||||
// trusting the platform to give up.
|
||||
//
|
||||
// Under normal conditions (host actually reading) this is a no-op compared to
|
||||
// a plain write(): writes are never asked for more than availableForWrite()
|
||||
// already reports free, so they return immediately without entering the
|
||||
// platform's internal wait/yield loop at all. Only a genuinely stalled host
|
||||
// (zero throughput) hits the deadline; a slow-but-alive one just takes longer,
|
||||
// same as a plain write() would.
|
||||
#include <Arduino.h>
|
||||
|
||||
// Returns the number of bytes actually written; less than `len` only when the
|
||||
// host stopped draining the link before `deadline_ms` (an absolute millis()
|
||||
// value — share one deadline across multiple calls that make up one logical
|
||||
// message, so a stall costs a fixed total budget, not one per call).
|
||||
static inline size_t streamWriteUntil(Stream& s, const uint8_t* buf, size_t len, uint32_t deadline_ms) {
|
||||
size_t sent = 0;
|
||||
while (sent < len) {
|
||||
int avail = s.availableForWrite();
|
||||
if (avail <= 0) {
|
||||
if ((int32_t)(millis() - deadline_ms) >= 0) break; // host stalled — give up
|
||||
yield(); // let the USB/BLE background task run so it can drain the FIFO
|
||||
continue;
|
||||
}
|
||||
size_t chunk = (size_t)avail < (len - sent) ? (size_t)avail : (len - sent);
|
||||
size_t w = s.write(buf + sent, chunk);
|
||||
if (w == 0) {
|
||||
if ((int32_t)(millis() - deadline_ms) >= 0) break;
|
||||
yield();
|
||||
continue;
|
||||
}
|
||||
sent += w;
|
||||
}
|
||||
return sent;
|
||||
}
|
||||
Reference in New Issue
Block a user