Files
firmware/src/mesh/StreamAPI.h
T
p0ns ca833d944c Fix serial protobuf corruption on short USB CDC writes (#10976)
* Fix serial protobuf corruption on short writes

SerialConsole shared raw debug output and framed protobuf traffic on one
HWCDC stream. Raw text could interleave inside an active frame when API
logging was disabled. Separately, HWCDC deliberately returns a short write
after bounded backpressure; StreamAPI abandoned that frame after PhoneAPI
had already advanced, so the next 0x94c3 header landed inside the previous
declared payload.

Suppress all unframed output after protobuf mode starts and honor the
existing config-replay log pause. For HWCDC, retain short frame tails in the
persistent tx buffers and finish them on later loop passes before dequeuing
another FromRadio packet. Logs remain best-effort and only start when their
complete frame fits; main packets are deferred rather than dropped if a
synchronous log is pending. Do not call HWCDC flush for framed serial output,
since its no-progress path can discard queued bytes. TCP and non-HWCDC
transports keep their existing behavior.

Validated on Cardputer ADV (200-node DB) and Heltec Tracker V2: 400 initial
full-DB sessions plus 140 final sessions across both API-log settings, zero
malformed frames/timeouts/incomplete DBs; 2-20s forced reader stalls resume
with complete DBs; abrupt stalled-client replacement 5/5 per board; 150s
post-close reboot counts stable. Builds pass for Cardputer, Heltec, tbeam,
and rak4631.

* Add serial frame continuation regression tests

Extract the HWCDC pending/deferred frame state machine into a small
transport-independent helper so native tests exercise the same production
logic used by SerialConsole. Keep framing, persistent buffer ownership and
locking in SerialConsole.

Cover short-tail continuation, deferred main-frame ordering, best-effort log
admission, bounded zero-progress calls, generic StreamAPI failure semantics,
PhoneAPI advancement gating, framed-log gating and raw output suppression.
The coverage suite passes 31/31 suites and 582/582 tests.

* Address review: guard flush, assert deferred invariant, dedup framing

- Make SerialConsole::flush() a no-op in protobuf mode: HWCDC::flush()'s
  no-progress path discards queued TX bytes, which would tear a framed
  stream when the sleep path flushes with a stalled host.
- Assert the single required-frame producer invariant in
  StreamFrameWriter::writeFrame() instead of silently dropping a second
  required frame.
- Hoist 0x94C3 header construction into StreamAPI::buildFrameHeader() so
  SerialConsole no longer re-hardcodes the framing constants.

* Test retained serial tail across client replacement

Model a replacement client arriving while an older required frame has an
unwritten tail. Require the old frame to complete before the new frame starts,
so a new 0x94C3 header can never land inside the old declared payload.

* Document serial frame APIs and regression tests

Add concise Doxygen comments for the frame continuation hooks, production
helper, native test doubles, and regression scenarios. Document why retained
frame tails intentionally survive client disconnects: HWCDC may still hold the
accepted prefix, so dropping metadata could insert a new frame header inside
the old declared payload.
2026-07-11 06:27:50 -05:00

133 lines
5.3 KiB
C++

#pragma once
#include "PhoneAPI.h"
#include "Stream.h"
#include "concurrency/Lock.h"
#include "concurrency/OSThread.h"
#include <cstdarg>
// A To/FromRadio packet + our 32 bit header
#define MAX_STREAM_BUF_SIZE (MAX_TO_FROM_RADIO_SIZE + sizeof(uint32_t))
/**
* A version of our 'phone' API that talks over a Stream. So therefore well suited to use with serial links
* or TCP connections.
*
* ## Wire encoding
When sending protobuf packets over serial or TCP each packet is preceded by uint32 sent in network byte order (big endian).
The upper 16 bits must be 0x94C3. The lower 16 bits are packet length (this encoding gives room to eventually allow quite large
packets).
Implementations validate length against the maximum possible size of a BLE packet (our lowest common denominator) of 512 bytes. If
the length provided is larger than that we assume the packet is corrupted and begin again looking for 0x4403 framing.
The packets flowing towards the device are ToRadio protobufs, the packets flowing from the device are FromRadio protobufs.
The 0x94C3 marker can be used as framing to (eventually) resync if packets are corrupted over the wire.
Note: the 0x94C3 framing was chosen to prevent confusion with the 7 bit ascii character set. It also doesn't collide with any
valid utf8 encoding. This makes it a bit easier to start a device outputting regular debug output on its serial port and then only
after it has received a valid packet from the PC, turn off unencoded debug printing and switch to this packet encoding.
*/
class StreamAPI : public PhoneAPI
{
/**
* The stream we read/write from
*/
Stream *stream;
uint8_t rxBuf[MAX_STREAM_BUF_SIZE] = {0};
size_t rxPtr = 0;
/// time of last rx, used, to slow down our polling if we haven't heard from anyone
uint32_t lastRxMsec = 0;
public:
StreamAPI(Stream *_stream) : stream(_stream) {}
/**
* Currently we require frequent invocation from loop() to check for arrived serial packets and to send new packets to the
* phone.
*/
virtual int32_t runOncePart();
virtual int32_t runOncePart(char *buf, uint16_t bufLen);
/// Check the current underlying physical link to see if the client is currently connected
virtual bool checkIsConnected() override = 0;
private:
/**
* Read any rx chars from the link and call handleToRadio
*/
int32_t readStream();
int32_t readStream(const char *buf, uint16_t bufLen);
int32_t handleRecStream(const char *buf, uint16_t bufLen);
/**
* call getFromRadio() and deliver encapsulated packets to the Stream
*/
void writeStream();
protected:
/**
* Send a FromRadio.rebooted = true packet to the phone
*/
void emitRebooted();
virtual void onConnectionChanged(bool connected) override;
/**
* Send the current txBuffer over our stream
*/
bool emitTxBuffer(size_t len);
/// Are we allowed to write packets to our output stream (subclasses can turn this off - i.e. SerialConsole)
bool canWrite = true;
/// Subclasses can use this scratch buffer if they wish
uint8_t txBuf[MAX_STREAM_BUF_SIZE] = {0};
/// Low level function to emit a protobuf encapsulated log record
void emitLogRecord(meshtastic_LogRecord_Level level, const char *src, const char *format, va_list arg);
/// Return whether the transport can accept a frame of the requested size.
virtual bool canWriteFrame(size_t frameLen) { return true; }
/// Let transports recover from or close after an incomplete write.
virtual void onFrameWriteFailed(size_t frameLen, size_t writtenLen) {}
/// Fill in the 4-byte 0x94C3 length header; returns the total frame length.
static size_t buildFrameHeader(uint8_t *buf, size_t payloadLen);
/// Complete retained transport output before dequeuing another PhoneAPI packet.
virtual bool finishPendingFrame() { return true; }
/// Return whether the dedicated log buffer is available for encoding.
virtual bool canEncodeLogRecord() { return true; }
/// Frame and write a payload, optionally using best-effort admission.
virtual bool writeFrame(uint8_t *buf, size_t len, bool bestEffort);
concurrency::Lock streamLock;
private:
/// Dedicated scratch + tx buffer for LogRecord emission.
///
/// The main packet emission path (`writeStream` -> `getFromRadio` ->
/// `emitTxBuffer`) holds `fromRadioScratch` (from PhoneAPI) and `txBuf`
/// from the moment `getFromRadio` starts encoding until `emitTxBuffer`
/// finishes pushing bytes to the stream. If a `LOG_` macro fires during
/// that window and we emit through the API, the old implementation
/// re-used `fromRadioScratch` / `txBuf` and corrupted whatever the main
/// path had already encoded. Symptoms on the host were
/// `google.protobuf.message.DecodeError: Error parsing message with type
/// 'meshtastic.protobuf.FromRadio'` - any tool with
/// `config.security.debug_log_api_enabled=true` under traffic would see
/// torn frames every few messages.
///
/// Giving the log path its own scratch + txBuf means the main path is
/// never clobbered. We still need `streamLock` to serialize the actual
/// `stream->write` call so a log emission and a packet emission don't
/// interleave on the wire.
meshtastic_FromRadio fromRadioScratchLog = {};
uint8_t txBufLog[MAX_STREAM_BUF_SIZE] = {0};
};