Sends a binary channel datagram: flood by default, or along a given path. Validates data_type (1..0xFFFF), payload length (<= 163) and channel index (one byte), like send_raw_data.
- Use packet type 0x16 and include the four-byte timestamp so the
fixture exercises ADVERT_PATH parsing instead of PRIVATE_KEY handling.
- Preserve the embedded zero byte and existing path assertions.
send_msg_with_retry could return None for a message the recipient received and ACKed:
- The ACK subscription was registered only after send_msg returned. If the ACK was queued right behind MSG_SENT (both frames in one read, or the event loop busy for the round trip), it was dispatched with no listener.
- Each attempt waited only for its own ACK code. A late ACK for an earlier attempt, which the device still reports from its expected-ACK table, was ignored.
The ACK subscription now spans the whole call and accepts the code of any attempt sent so far. All attempts also share one timestamp—the attempt byte already makes each packet unique—so receivers can tell a retry from a new message.
Both commands already existed in commands/device.py but had no entry in
the README's command reference table, so users had no way to discover
them (reported as "missing"). Add rows under Advanced Configuration
explaining that mode controls how many bytes of each hop's node ID are
stored per hop in advertised/logged paths (bytes per hop = mode + 1),
matching the plen >> 6 / hash_mode + 1 logic in reader.py and
commands/base.py.
Fixes#84
logging.basicConfig() at import time in __init__.py silently clobbered the
embedding application's own logging setup. Replace it with a NullHandler so
the library stays silent by default without touching global config.
Also stop MeshCore.__init__ from unconditionally forcing the "meshcore"
logger to INFO when neither debug= nor only_error= is passed - only set a
level when the caller explicitly asks for one, otherwise leave whatever the
app already configured alone.
Document the idiomatic logging setup for consumers in README.md.
Fixes#58
parsePacketPayload copies message/msg_hash/sender_timestamp/attempt/txt_type from a
prior channels_log entry matched by pkt_hash. When that prior entry was logged while
the channel could not be decrypted it has none of those keys, so a later duplicate of
the same transmission raises KeyError: 'message' and aborts handle_rx for the whole
packet, dropping its RX_LOG_DATA. It triggers when a channel-table refresh lands
between two copies of a flooded channel message.
Copy with .get() so the fields are simply absent instead of raising. Adds a regression
test that reproduces the crash (fails on main, passes here).
Follow-up to review of the two preceding commits.
The write lock added in 00135bb prevented concurrent writes from dropping the
link, but bounded nothing. CommandHandler's own timeout does not cover the
write: send() awaits _sender_func() and only afterwards arms
asyncio.wait(futures, timeout=...). So a stalled write -- observed on hardware
running to minutes -- held the lock indefinitely while every other command
queued behind it, with nothing logged, no error raised and no DISCONNECTED
event. Before the lock only the stalled command hung; the others went out and
could trip the error-19 disconnect, which at least recovered. The lock turned
a bounded, self-healing failure into an unbounded silent one, and also blocked
the post-reconnect CMD_APP_START behind the dead connection's holder.
The write (lock acquisition included) is now bounded by
BLEConnection.WRITE_TIMEOUT. On expiry the link is torn down rather than the
lock merely released: the underlying CoreBluetooth write may still be in
flight, and a second write racing it re-creates the overlap the lock exists to
prevent. Tearing down hands over to the reconnect path, which is bounded and
self-healing.
The lock is now a lazily-created property, mirroring _mesh_request_lock in
commands/base.py, so an instance built without __init__ still works. The two
BLE tests previously assigned _write_lock themselves, which meant deleting the
__init__ line left them green; there is now a test that __init__ provides it.
Also in send_anon_req:
- A failed change_contact_path() no longer proceeds. The device still has the
contact as flood, so sendAnonReq() floods the request, and the server gates
REGIONS/OWNER/BASIC behind isRouteDirect() and drops it -- the caller then
waits out a full path-scaled timeout for a reply that cannot arrive. It now
returns ERROR path_reset_failed.
- encode_reply_path() clamps to the server's 64-byte reply_path buffer, which
MyMesh.cpp memcpys into with no length check, and rejects hash mode 3 (the
4-byte hops Packet::isValidPathLen refuses). Not a regression -- the previous
encoder overflowed identically -- but this function is the chokepoint and its
comment claimed to bound the read.
- The hop count saturates at 63 instead of being masked with & 63, which would
wrap a 64-hop path to zero hops, i.e. request a zero-hop reply from a distant
node. Not reachable from a device-sourced contact (the reader caps the field
at 63) but silent if it ever were.
- The suggested_timeout multiplier now scales by the hops actually emitted
rather than the contact's claimed out_path_len, which can differ once the
encoder clamps or truncates.
Correction to 30446ed's message: the claim that mode 0 is "byte-for-byte
identical" is wrong. Differentially, over 30000 randomised contact fields
restricted to what a device can actually emit, mode 0 diverges in 1177 of
10118 cases -- every one of them a path containing a 0x00 byte, and in every
one the old encoder was the wrong one. The accurate claim is "unchanged for
mode-0 paths containing no 0x00 byte".
An anon request tells the server how to route its answer back. The leading
byte of that reply path packs two fields, which the server unpacks as:
reply_path_len = byte & 63
reply_path_hash_size = (byte >> 6) + 1
Three defects in producing it:
1. The hash mode was never written into the top two bits, so the server always
read a hash size of 1 whatever the contact's real mode was.
2. The path was reversed byte-wise (out_path[::-1]) rather than hop-wise. A
return path visits the same hops in reverse order with each hop's
multi-byte hash intact.
3. reader.py built out_path by stripping every NUL from the fixed 64-byte
field. That trims the padding but also eats a legitimate 0x00 inside a hop
hash, shortening the path and shifting every hop after it. It now takes
out_path_len * hash_size bytes, as PATH_DISCOVERY_RESPONSE already did.
Worked example at hash mode 2 (3 bytes per hop), for a contact two hops away
via aabbcc then ddeeff:
before: lenbyte 0x02, path ffeeddccbbaa
-> server reads 2 hops of 1 byte, replies via ['ff', 'ee']
after: lenbyte 0x82, path ddeeffaabbcc
-> server reads 2 hops of 3 bytes, replies via ['ddeeff', 'aabbcc']
The old form routes the response to hops that do not exist, so it is dropped
and the request times out.
At hash mode 0 both encodings are byte-identical -- the mode contributes
nothing to the high bits and byte-wise reversal equals hop-wise reversal for
single-byte hops -- which is why this stayed latent: mode 0 is the default.
Confirmed by the mode-0 and zero-hop tests passing unchanged against the old
code while the mode-1/mode-2 tests fail.
Scope: only anon requests routed direct to a contact with a known multi-hop
path. Flood requests are unaffected (the server answers via createPathReturn
and ignores the supplied reply path), as is login (handleLoginReq never sets
reply_path_len, so its reply always goes out flood). The neighbors zero-hop
probe is unaffected: length 0 makes hash size irrelevant.
Encoding is extracted into encode_reply_path() so it can be tested directly.
Verified on hardware only for the zero-hop case, which still works; the
multi-hop paths are covered by unit tests, as the test radio has no multi-hop
contacts to exercise on air.
Two overlapping write_gatt_char() calls on the same characteristic drop the
BLE link outright. Observed on macOS/CoreBluetooth as "BLE write failed: 19",
after which the connection is gone and the pending command never completes.
Nothing above the transport guaranteed callers were sequential: schedulers,
health checks, periodic status queries and user commands all issue commands
independently, so any unlucky overlap could take the radio down. The existing
_mesh_request_lock only guards a few binary-request helpers, not the transport.
Reproduced on a companion radio over BLE by issuing send_device_query() and
send_node_discover_req() concurrently:
before: BLE write failed: 19, connected=False, command hung >45s
after: both complete in 0.12s, connected=True
Issued sequentially the same two commands take 0.09s each and are fine, so it
is specifically the overlap. Ruled out as causes beforehand: notification load
(three discovers under a 91-packet firehose kept writes at 0.06-0.17s with the
link stable) and the discover command itself.
The lock is created lazily so it binds to the running loop, and is released on
the failure path so one failed write cannot wedge every later command.
send_anon_req() refused to send whenever the destination pubkey was absent
from the client-side contact cache, returning ERROR contact_not_found.
The contact is consulted for one thing only: building the reply-path bytes
appended to the request. The companion firmware needs no contact of its own --
since FIRMWARE_VER_CODE 13 its CMD_SEND_ANON_REQ handler synthesises a
transient anon contact for an unknown pubkey with out_path_len = 0 (zero-hop
direct). Those entries live in a reserved slot ring, are hidden from
CMD_GET_CONTACTS and are never persisted, so nothing is polluted by them.
The client-side refusal therefore blocked a case the device supports, such as
asking a freshly discovered neighbour for its regions before it has ever been
added as a contact. Fall back to a zero-hop reply path instead.
Two smaller fixes in the same function:
- out_path_len is now read once into a local rather than re-read from the
contact dict after the await. That dict is a live reference other commands
mutate in place (send_msg_with_retry's flood fallback, reset_path); if it
flipped to -1 mid-send the suggested_timeout multiplier became 4000 * 0 = 0,
registering the binary request with a zero timeout so the response was
dropped the moment it arrived.
- The value is clamped at 0. update_contact() normally reflects the change
back onto the dict, but if it fails the dict stays -1 and the unsigned
to_bytes raises OverflowError -- which skipped the reset_path at the end of
the method and left the contact pinned to zero-hop on the device.
Verified against a companion radio on fw ver 13: without the change all five
discovered repeaters were refused client-side in 0.0s with no RF sent; with it
all three answered with their region scopes in ~1.1s.
test_send_anon_req_contact_not_found is replaced -- it codified the removed
limitation -- but its original regression (a TypeError on the NoneType
subscript) stays covered.
Why: PR #86 (44b21be) reworked send_raw_data to frame as
0x19 | path_len(1) | path | payload and to reject payloads under 4
bytes, but left test_send_raw_data_wrapper unchanged. The test sent a
2-byte payload (now raising ValueError before any assertion) and
asserted captured_data[1:] == payload, which no longer holds because of
the inserted path_len byte. The firmware drops payloads under 4 bytes,
so the guard is correct; this fixes the test, not the guard.
Bumps the payload to 4 bytes and asserts the zero path_len byte plus the
payload at offset 2, validating the wire format #86 introduced.
Tests: tests/unit/test_protocol_surface_gaps.py -- full suite 156 passed
(test_send_raw_data_wrapper was the lone failure on current main).
Maxwell Burton
fdlamotteandGitHub
John PettittandClaude Opus 5.5
johntericksonandCopilot
Kyle Manna
agessaman
Hipólito Luiz
Joey Leake
ErikBrown2andGitHub
Aluminique
Matthew Wolter