Hold pushes until the CMD_GET_CONTACTS response has finished

[l33tdawg]

What happens

CMD_GET_CONTACTS is answered by a ContactsIterator that MyMesh::checkSerialInterface() advances one frame per loop() pass:

} else if (_iter_started              // check if our ContactsIterator is 'running'
           && !_serial->isWriteBusy()) {

Everything else in loop() runs first — BaseChatMesh::loop() and the packet callbacks — and those write push frames straight to the same serial interface (onDiscoveredContact 0x80/0x8A, onContactPathUpdated 0x81, processAck 0x82, queueMessage / onChannelMessageRecv / onChannelDataRecv 0x83, onContactOverwrite 0x8F, onContactsFull 0x90, logRxRaw, onContactResponse, onRawDataRecv, onTraceRecv, …).

So a client that asks for contacts and reads until the response ends can receive this:

> 02 <count LE32>          RESP_CODE_CONTACTS_START
> 03 <contact 148 bytes>   RESP_CODE_CONTACT
> 80 <pubkey>              PUSH_CODE_ADVERT              <-- inside the response
> 03 <contact 148 bytes>   RESP_CODE_CONTACT
...
> 04 <lastmod LE32>        RESP_CODE_END_OF_CONTACTS

Two things are wrong with that. A push code in the middle of a response cannot be told apart from a corrupt reply by a client that is reading a response it asked for, and PUSH_CODE_CONTACT_DELETED (0x8F) / PUSH_CODE_CONTACTS_FULL (0x90) specifically mean the table being read changed while it was being read — which is the one thing a snapshot read must not have to guess about.

For context on how it was found, and to be upfront: a host driver of ours reads the response strictly and fails closed on a frame that belongs to no response, so it was seeing this about 25 times out of 30 reads. That strictness is defensible but you could reasonably call it a client bug. I am opening this because the alternative — every client has to know the full push-code table and decide for itself which codes invalidate a read in progress — seems worse than making the response contiguous, which costs the firmware very little.

The fix

Async pushes now go through a new MyMesh::writePushFrame():

• not iterating → written directly, exactly as before;
• iterating → copied into a small bounded FIFO;
• once the response has ended → flushed one per loop() pass, in order, using the same isWriteBusy() pacing.

Once anything is held, further pushes keep queueing until it drains, so pushes are delivered in the order they were raised.

Responses to commands the client sent itself (handleCmdFrame, writeOKFrame, writeErrFrame) are deliberately untouched — deferring those could delay something a client is waiting on. The FIFO is MAX_DEFERRED_PUSHES (default 8) × MAX_FRAME_SIZE, overridable with -DMAX_DEFERRED_PUSHES=n; a full FIFO drops the push, which is what the BLE serial interface already does when its send queue is full.

Cost

Measured with pio run on the same tree before and after:

environment	RAM before	RAM after	flash before	flash after
RAK_4631_companion_radio_ble	149,912 (63.7%)	151,328 (64.3%)	475,960	477,240
heltec_v4_companion_radio_usb	103,488	104,904	650,237	650,469

+1,416 bytes of RAM in both cases (8 × 176 + 8 length bytes). On the nRF52 that is most of the cost; lowering MAX_DEFERRED_PUSHES lowers it linearly if that matters on the smallest boards.

Testing

Compiled for heltec_v4_companion_radio_usb (ESP32-S3) and RAK_4631_companion_radio_ble (nRF52), both SUCCESS.

Not hardware tested. I have a pair of test radios on this firmware family and can flash them and re-run the read loop that found this, if you would rather see that before merging.