fix(ack): firmware-compatible ACK format + multi-ack support#88
Open
agessaman wants to merge 3 commits into
Open
fix(ack): firmware-compatible ACK format + multi-ack support#88agessaman wants to merge 3 commits into
agessaman wants to merge 3 commits into
Conversation
pyMC_core had drifted out of sync with the MeshCore firmware (verified against the companion_radio variant it emulates) on the on-wire ACK format for plain text DMs, in both directions: - Emit: firmware sends a 6-byte ACK for TXT_TYPE_PLAIN — the 4-byte sha256 hash plus an extended-attempt byte and a random byte. The random byte gives each ACK packet a unique packet-hash so mesh dedup never drops a legitimate ACK. pyMC_core emitted only the 4-byte hash, losing that uniqueness. - Accept: process_discrete_ack rejected any payload not exactly 4 bytes, so it silently dropped every 6-byte ACK current firmware emits — causing DM/room-push delivery confirmations from firmware peers to never match. Matching only ever uses the first 4 bytes (firmware does memcmp(..., 4)), so length must not gate acceptance. Changes: - ack.py: accept ACK payloads of length >= 4, match on payload[:4]. - packet_builder.py: add calc_text_ack_hash() (4-byte hash + ext-attempt + random byte) and create_ack_from_bytes() (raw-payload wrapper, mirror of firmware createAck); create_ack now delegates to them and returns the 6-byte form. - text.py: compute the ACK hash once with firmware-faithful C-string semantics (full flags byte; ext-attempt = byte after the text's null terminator) and use it for both DIRECT (discrete ACK) and FLOOD (path-return extra) responses. Tests: round-trip proving receiver-emitted ACK[:4] == sender ack_crc; byte-layout check against an independent sha256; inbound 6-byte match.
ACK format (both directions): - Emit 6-byte ACK for TXT_TYPE_PLAIN (sha256[:4] + ext-attempt + random byte) instead of 4 bytes; the random byte keeps the packet hash unique. - Accept ACKs >= 4 bytes (was: exactly 4), matching on the first 4 — fixes silently dropping firmware's 6-byte ACKs. Multi-ack (PAYLOAD_TYPE_MULTIPART), opt-in via multi_acks pref (default off): - Receive: MultipartAckHandler extracts the embedded ACK into the normal ack-matching path. - Emit: mirror sendAckTo — route the ACK via known out_path, and with multi_acks>0 emit a multi-ack ~300ms earlier. Pref wired in via the CompanionRadio/Bridge set_other_params overrides and at start(). New builders: calc_text_ack_hash, create_ack_from_bytes (+optional path), create_multi_ack. Repeater-side forwarding lives in pyMC_Repeater. Full suite passes.
Modified the TextMessageHandler to flood-route ACKs when the reverse out_path is unknown, ensuring they can reach the sender without a known path. Updated the PacketBuilder to support a new route_type parameter for ACK creation, allowing for both direct and flood routing. Enhanced tests to verify the correct behavior of flood-routed ACKs in scenarios with unknown paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings DM ACK handling in line with the MeshCore companion firmware pyMC_core
emulates, in two parts:
PAYLOAD_TYPE_MULTIPART) support — the optional reliability feature, opt-in via the existingmulti_ackspref.A companion PR in pyMC_Repeater [
fix/ack-compatibility] adds bit-faithful multi-ack forwarding and depends onPacketBuilder.create_multi_ackfrom this branch. The two ship together.1. ACK format
TXT_TYPE_PLAIN—sha256(timestamp ‖ flags ‖ text ‖ pubkey)[:4]+ an extended-attempt byte + a random byte (the random byte gives each ACK packet a unique hash so mesh dedup can't drop it). pyMC_core emitted only the 4-byte hash.process_discrete_ackrejected any payload that wasn't exactly 4 bytes, so pyMC_core silently dropped every 6-byte firmware ACK — DM and room-push confirmations from firmware peers never matched. Only the first 4bytes are ever compared (firmware does
memcmp(..., 4)).2. Multi-ack support (opt-in, default off)
MultipartAckHandlerextracts the embedded ACK from aPAYLOAD_TYPE_MULTIPARTpacket and routes it through the same_register_ack_receivedpath as discrete ACKs.sendAckTo: with a knownout_pathit routes the ACK along it, and whenmulti_acks > 0it emits a multi-ack ~300ms before the normal ACK so repeaters can forward the embeddedACK early. Unknown path keeps today's path-less ACK.
multi_acksis propagated into the text handler fromCompanionRadio/CompanionBridgeset_other_paramsoverrides (matching theset_path_hash_modeconvention) and re-applied atstart().Notable refactors
calc_text_ack_hash,create_ack_from_bytes(raw payload, now with optional path routing),create_multi_ack.TextMessageHandler.__call__into_build_ack_responses/_send_delayed_ack.Out of scope
Repeater multi-ack forwarding (
forwardMultipartDirect/routeDirectRecvAcks) lives in pyMC_Repeater, not here.Testing
ack_crc); ≥4-byte inbound matching; multipart extraction + dispatcher routing; faithful direct/flood/multi-ack emit; pref propagation.