Adversarial review (standards#145) of the storage/migration surface found four
real defects, all data-integrity. All confirmed against the code and fixed; no
false positives.
BLOCKER - concurrent save race. saveChannelMessages / saveMessages are
read-modify-write with an await gap, so two saves to the same key raced and the
second clobbered the first, silently losing messages (e.g. a message and its
delivery ack arriving together). BlobStore now provides synchronized(key, ...),
a per-key operation chain; every RMW - merge-save, remove, and the load-path
legacy migration - runs through it. Different keys stay concurrent. New test
fires two concurrent saves and asserts the union survives.
BLOCKER - non-atomic DB relocation. The documents->support move copied and
deleted each file (.sqlite/-wal/-shm) in turn, so a failure after the main file
moved stranded the DB across two locations and corrupted it. Now copies all
files, verifies each by size, and only then deletes the sources; on any failure
it rolls back the destination and leaves the original intact.
MAJOR - load-path legacy migration raced save. The #194 index->PSK adoption did
a blind write to the PSK key that could clobber a save that landed first. It now
runs under the key lock and MERGES (union) instead of overwriting, so both the
adopted history and any fresh message survive.
MINOR - channel merge key lacked a sender. Two senders posting identical text at
the same timestamp without a messageId would collide and lose one. The key now
includes the sender, matching MessageStore.
flutter analyze clean, dart format clean, 509 tests pass. Full Gemini log in
docs/llm-consultations/.
The persisted store was being overwritten with the windowed in-memory list, so
any channel or DM with more than _messageWindowSize (200) messages lost
everything older than the newest 200 on the first save after load. Observed
live: Public went 232 -> 201 in one session on a build that already had the
#333 fix, so this was not the race - it was windowing truncating the store.
This is the slow-erosion cause behind the whole 566 -> 231 -> 206 -> 201
history.
saveChannelMessages / saveMessages now MERGE into the persisted set instead of
overwriting: upsert by message identity so older persisted messages are kept,
new ones added, and the in-memory copy wins for edits/reactions/status. If the
existing history fails to decode, the save aborts loudly rather than merging
into an empty base and truncating (SAFELANE 6).
Deletion is now an explicit path - removeChannelMessage / removeMessage - since
the app deletes individual messages. Routing delete through the merging save
would resurrect them; the connector's deleteChannelMessage / deleteMessage now
call the explicit remove.
Windowing stays for display and memory; it no longer dictates what is stored.
Tests: 250-message history survives a 200-window save; new message appends;
edit is captured not duplicated; delete does not resurrect. 508 pass, analyze
and format clean.
Cost: a save now re-reads and re-encodes the channel/contact history. Cheap
with drift's per-key writes (#335); a future append-only schema removes even
that.
The guard read pubspec.lock with a newline-sensitive regex, so it passed on
LF (CI) and failed on CRLF (Windows) for the same, correct assets. A guard
that is itself platform-fragile is worse than none. Normalise CRLF->LF before
matching.
Surfaced by the 290+306+335 integration merge, where the lockfile came through
with CRLF endings.
Moves message history, contacts and discovered contacts out of the settings
store. Settings stay in SharedPreferences, which is what it is for.
Ordering is the whole safety argument: WRITE, VERIFY BY READING BACK, and only
then remove the source. #333 was a storage path that chose a key silently and
made 566 real messages read as empty; deleting before verifying would make
that class of mistake permanent instead of cosmetic. On any failure the source
is left intact and the error is logged - never a silent drop (SAFELANE 6).
Idempotent by construction: a key already present in drift is not overwritten,
so re-running is a no-op. If an older build re-writes a migrated key into
prefs, the migrated copy wins and the stale prefs copy is discarded rather
than promoted.
Rehearsed against a COPY of a real 7 MB store, as the plan required before
touching live data:
REHEARSAL: 69 migrated, 0 failed, 5.21 MB, 69 bulk keys expected
Every key checked for exact length, confirmed removed from prefs, and every
settings key confirmed untouched. The live store was never opened.
Two things the tests caught that review would not have:
1. getString THROWS on a non-string value rather than returning null, so a
non-string under a bulk prefix was counted as a migration FAILURE. It now
type-checks with prefs.get() and skips. Alarming falsely is its own bug.
2. The `contacts` prefix was checked against the real store rather than
assumed: it matches only the 6 bulk contact blobs, and correctly does NOT
match contact_unread_count*.
Not yet wired into app startup - that is the switchover, and it is deliberately
a separate commit so this can be reviewed on its own.
flutter analyze clean, dart format clean, 504 tests pass.
Owner asked whether the web assets must be re-downloaded on every drift bump.
They do not - the version can be pinned - but doing that safely needed three
changes, because the failure mode is silent.
1. drift and drift_flutter are pinned EXACTLY (2.34.2 / 0.3.1), no caret. The
committed web assets are built for a specific drift release, so a caret
would let pub resolve a newer drift than the shipped binaries and break the
web build with no signal at compile time.
2. pubspec.lock is now committed (removed from .gitignore). It was ignored,
which for an application means resolution can differ between machines and
CI. That undermines the pin: the assets are matched against the RESOLVED
version, so resolution has to be reproducible. Owner-authorized, and it is
a repo-wide change rather than a storage-only one.
3. New test asserting the shipped assets match the resolved drift version,
plus that both files exist and the wasm really starts with the WebAssembly
magic bytes. Verified in both directions: it passes when matched and fails
loudly with re-download instructions when skewed.
Without (3) a stale asset compiles, deploys, and only fails in the user's
browser - the exact silent-failure class SAFELANE 6 forbids and that produced
#333 earlier.
Upgrading drift is now a deliberate step: bump the pin, re-download both
assets, update web/drift_assets.version. The test fails until you do.
flutter analyze clean, tests pass.
Step 1 of #335, deliberately before any data migration: prove the store works
before trusting it with message history.
Adds drift + drift_flutter, a key/value StoredBlobs table mapping 1:1 onto the
existing SharedPreferences keys, and verifyReadWrite() as a runtime probe.
Key/value rather than relational tables on purpose: it maps directly onto the
current store interfaces, so callers do not change and the migration can be
verified row-for-row against the old keys. Relational schemas come later, once
this is proven and querying is actually wanted.
Verified here:
- 3 tests pass, including a 6 MiB round-trip - larger than the 5 MiB
localStorage cap that makes the web build impossible today
- flutter analyze clean
- Windows release build succeeds
- Web release build compiles
Two findings from the gate that a plan-on-paper would have missed:
1. sqlite3_flutter_libs 0.6.0+eol is a DEPRECATED no-op. From sqlite3 v3.x it
is unnecessary and drift_flutter already covers it, so it is not a direct
dependency here.
2. The web build COMPILES BUT WOULD FAIL AT RUNTIME: drift needs sqlite3.wasm
and drift_worker.dart.js shipped in web/, and neither is present. Per drift
docs they are downloaded from GitHub releases, version-matched to
pubspec.lock (drift 2.34.2, sqlite3 3.5.0), and the server must serve .wasm
as Content-Type: application/wasm. Not done here - downloading files needs
explicit human approval.
Also noted: the drift_dev CLI does not compile at these versions
(allSchemaEntities missing from the drift3_preview GeneratedDatabase), so its
asset tooling is unavailable. build_runner code generation is unaffected.
No user data is touched. No store is switched over. The migration is the next
step and remains gated on the web assets question.
formatNotificationText used GifHelper.parseGif (Giphy only) while the chat
renders via resolveGifUrl (Giphy + allowlisted Tenor CDN). An allowlisted
Tenor GIF therefore rendered inline but still dumped a raw URL into the
notification tray.
Point both at resolveGifUrl so the tray summarises exactly the set the chat
displays. Off-allowlist URLs still pass through as plain text, pinned by test.
This is the same duplicate-detection drift that caused #284; closing it at the
source rather than shipping a known inconsistency.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pasted GIF links rendered as plain text because parseGif only matched the
compact forms. Adds GifHelper.resolveGifUrl, which maps any supported payload
to the URL that renders it, and widens coverage to the links people actually
paste.
Allowlist is deliberately narrow, Giphy and Tenor only. Off-list URLs return
null and stay plain tap-to-open links, never auto-fetched: auto-loading a
stranger's URL leaks the viewer's IP and enables tracking-pixel abuse, and
inline-rendering unmoderated hosts is a malware and inappropriate-content
vector. Tests pin the rejections, including a lookalike host
(giphy.com.evil.example) and a general image host (imgur).
Newly rendered:
- i.giphy.com/<id>.gif and .webp, the form a browser copy produces
- media.tenor.com and c.tenor.com direct assets
A tenor.com/view/<slug>-<id> PAGE url is deliberately NOT matched. The modern
Tenor CDN path uses an opaque hash that cannot be derived from the page id, so
resolving one requires the Tenor API and a key. Such a link stays plain rather
than silently rendering the wrong image. Supporting it is a separate decision.
Also removes the hardcoded 'https://media.giphy.com/media/$gifId/giphy.gif'
literal duplicated across all four render call sites. That duplication is the
same drift that caused the #284 notification regression, where a second
hand-rolled copy of GIF detection went stale.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Notifications summarised a GIF as "Sent a GIF" using their own hand-rolled
`^g:[A-Za-z0-9_-]+$` regex rather than GifHelper. #282 changed the payload to
`https://giphy.com/gifs/<id>`, which that regex does not match, so GIF
notifications regressed to dumping the raw URL into the tray.
Detection now goes through `GifHelper.parseGif`, matching how the adjacent
reaction case already delegates to ReactionHelper. That restores the summary
and covers every form the app can send, including replies (`@[Name] ` +
payload), which the old regex never matched even for legacy `g:<id>`.
Verified no other hand-rolled `^g:`/`^m:`/`^s:`/`^r:` payload regexes remain
outside their owning helpers, so this class of drift is closed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Completes the client half against the firmware as-built (#298):
- Device-info offset 83 carries the FEM LNA state on v16+, appended
unconditionally (reads 0 on non-capable boards). parseFemLnaState is
length-guarded and returns null pre-v16. Byte presence signals firmware
version, not capability — the cap BIT gates the UI.
- 0xC3 reply handler adopts the reported value verbatim: firmware returns
post-apply hardware state, not an echo, so a refused write surfaces as
truth rather than a lie.
- setFemLna / requestFemLnaState no-op unless the capability bit is set,
so a non-capable radio never sees 0xC3 traffic.
- Radio Settings toggle rendered only when the bit is set, driven by
connector state via ListenableBuilder rather than local optimistic
state, so it always shows what the radio reports.
- errCodeUnsupportedCmd documented: a mis-gated request draws [0x01][0x01],
not [0x01][0x06]. Neither is 0xC3-prefixed, so no new error path.
Gating is on the bit alone, never model or version: firmware derives it
from a runtime FEM probe, so two Heltec V4s can legitimately disagree and
rak3401 reports false by design (its SKY66122 gates LNA and PA together).
13 tests. Not hardware-validated — firmware 0xC3 is still on a branch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the client half of the Heltec V4 FEM LNA control surface at the
protocol level: 0xC3 CMD_OFFBAND_FEM_LNA (0x01 SET [value], 0x02 GET),
a 3-byte [0xC3][sub][value] reply parser, and the
OFFBAND_CAP_FEM_LNA = 0x04 capability gate.
Deliberately a fork-private command rather than a sixth byte on the
stock CMD_SET_OTHER_PARAMS (38): that frame is shared with upstream
MeshCore and is sent to every radio regardless of fork, so widening it
would perturb stock firmware. Nothing is emitted unless the capability
bit is set.
The gate checks the bit only, never model or version — firmware derives
it at runtime from the auto-detected FEM chip, so it is a per-unit
answer and two Heltec V4s can legitimately disagree.
PROVISIONAL: firmware owns the caps byte and has not yet confirmed 0x04
is free, and the 0xC3 setter is not built yet. Spec sent to
OffbandMesh/meshcore-firmware#298. Do not merge before firmware
confirms the shape as built.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The firmware path-len byte is packed and self-describing (src/Packet.h:79-84):
high 2 bits = hash size - 1, low 6 bits = hash COUNT (hops), byte length =
count * size. The app treated the low 6 bits as a byte count and ignored the
width, on the strength of a comment asserting the high bits were "not reliably
populated". Both halves of that comment were wrong.
Receive: Contact.fromFrame read `count` bytes where firmware meant `count`
hops, so at 2-byte width it kept HALF of every path and discarded the rest.
DAYTON VA's 2-hop route became a 2-byte fragment, which is why every repeater
login in Bandit's capture timed out.
Send: buildUpdateContactPathFrame wrote the count raw, leaving mode bits 00.
That tells the radio "1-byte hashes" while handing it 2-byte hash data, so it
routed to nodes never on the route. This is the likelier cause of the observed
"routes via c6 and 5c separately" behaviour.
Also fixes a third unit inconsistency: setContactPath wrote customPath.length
(bytes) into pathLength (hops) after every path set.
Contact now carries pathHashWidth per path, so a stored path can never be
re-sliced at a width it was not captured at. realHopCount() is removed rather
than adjusted: it divided an already-correct hop count, halving it at 2-byte
width, and every call site was compensating for a bug that no longer exists.
Migration (Ben's call, option A): records written before this change have no
pathHashWidth and hold truncated bytes that cannot be recovered by
reinterpretation, so their path is dropped and the contact reverts to flood
until the radio re-supplies it. Logged, not silent.
Tests asserted the old semantics (expect(realHopCount(2, 2), 1)) and were
rewritten against firmware truth rather than adjusted to keep passing. Added
send-side encoding coverage, which did not exist. 456 tests pass.
Refs #240, #279, #299
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The picker emitted `g:<id>`, a convention that exists only in this Flutter
client lineage's parseGif (inherited from upstream, not a MeshCore protocol
feature). A recipient on stock or any non-lineage client has no way to expand
it, so a GIF arrived as opaque text (#223).
encodeGif now emits `https://giphy.com/gifs/<id>`, a form parseGif already
matches, so:
- stock clients receive a tappable URL they can actually open
- Offband keeps rendering inline with no render-side change
- legacy `g:<id>` payloads still decode, back-compat unchanged
Cost is ~41B vs ~20B against a ~160B payload. A full message carrying sender
prefix + reply mention + URL measures ~79B, roughly half the cap.
Checked, and did NOT change, the outbound structured-payload guard in
prepareContactOutboundText/prepareChannelOutboundText: Smaz.encodeIfSmaller
declines to compress a URL (base64 overhead exceeds the dictionary gain) and
Cyr2Lat passes unmapped ASCII through, so a GIF URL survives intact without
widening the guard. Both pinned as regression tests instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Blocking yourself silently hid your own traffic with no visible cause,
and because blocks are offloaded to the radio it was pushed there and
could be pulled back by the connect-time union.
BlockService gains a selfKeyHex, injected by the connector when the self
key is learned and cleared on disconnect. Every add path refuses it
(block, importKeys, maybePromote) — importKeys specifically, so a
self-block already on the radio cannot be re-imported. setSelfKey also
heals an existing self-block and pushes REMOVE, covering the case where
the union pull lands before self-info arrives. Contacts and discovery
sheets hide the Block option for your own node.
Per Gemini review: heal on an unchanged key too (an early return would
strand a self-block introduced after the key was known), and order
setSelfKey so all in-memory state settles synchronously, keeping callers
consistent even though the connector fires it unawaited.
Adds test/services/block_service_test.dart — the first coverage for
BlockService (10 tests).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every received DM/channel message now logs arrival wall-clock vs the
sender-claimed payload timestamp (delta + TIME ANOMALY marker beyond
24h), routed directly to AppDebugLogService so the rotating on-disk
trail is unconditional. Models persist a nullable rxTime set at every
live ingest construction site; legacy stored records stay null.
Diagnostic child of #280.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The DTR low→high pulse in the desktop USB open path resets ESP32 USB-Serial/JTAG
chips (VID 303A: C6/H2/S3-HWCDC) into ROM download mode mid-handshake, wedging
the device until a physical reset. The pulse exists for nRF52 (VID 239A)
reconnect-after-unclean-disconnect, so it can't just be removed.
connect() now recovers the port's USB VID from the flserial enumeration
(hardware_id) and gates the pulse: 239A keeps the DTR low→high pulse; 303A and
any unknown/unparseable VID open with DTR asserted and no pulse. VID lookup is
enumeration-only (never opens the port). New usb_vid helper parses both flserial
hardware_id formats (Windows VID_XXXX and macOS/Linux VID:PID=xxxx:xxxx), unit
tested. Build of epic #245.
The autocomplete only completed an unambiguous emoji on Enter; ambiguous
queries (e.g. `:shrug` with multiple matches) sent the literal text instead of
the highlighted glyph, and mentions only selected via Tab. Enter now accepts
the highlighted item (emoji or mention) like Tab, then sends; Tab still selects
without sending, Esc dismisses to send literal text. Removes the now-dead
_emojiQuery field. (#231, emoji + mentions per Ben)
Widget tests: Enter inserts the highlighted emoji (ambiguous `:shr`) and the
highlighted mention (`@Bo` -> @[Bob]).
Part of #231.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A reply prepends `@[Name] `, so a reply-gif renders the gif (prior commit);
this adds the `@Name` reply chip above it so it is clear the gif is a reply.
Extracts TranslatedMessageContent.mentionChip + leadingReplyName (shared by
inline rendering and the reply-gif header); the channel bubble builder renders
the chip above the GifMessage. DMs have no reply feature, so channel-only.
Part of #232.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two shared-root render gaps (message tokens were only matched at fixed
positions):
- TranslatedMessageContent: parse @[Name] anywhere (leading, inline, or
repeated) and render each as a chip; messages without a mention keep full
link support (#233).
- GifHelper.parseGif: strip a leading `@[Name] ` reply prefix before matching
so a reply-with-gif resolves to the gif instead of literal text; arbitrary
text before a gif is still left as text (#232).
Tests: parseGif reply/format cases + widget tests for inline/leading/multiple
mention chips. Full suite 402 green.
Part of #233, #232.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The firmware path-len byte packs a hash-mode hint in the high 2 bits and the
path byte-length in the low 6. Contact.fromFrame stored it raw, so a direct
node at 2-byte mode (0x40) read as pathLength 64: "64 hops" in the UI, a ~195s
ACK timeout, and up to 64 junk bytes pulled into contact.path that broke
repeater status/telemetry/neighbors until the path was cleared (#222).
- contact.dart: decode via pathHopCount (& 0x3F); keep 0xFF as the flood
sentinel; use the decoded length for safePathLen so path bytes stay clean.
- chat_screen _currentPathLabel: show realHopCount(pathLength, pathHashByteWidth)
so 2/3-byte nodes read the correct hop count; 0 -> Direct.
- Regression test for 0x40/0x00/0x03/0x44/0xFF; updated two model_changes tests
that codified the old raw mapping.
Part of #222 (stays open for on-hardware validation by the reporter).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The trace was width-1 end to end (flags=0, a 1-byte path, and a
1-byte-per-hop parse + render), so on a 2-/3-byte path-hash mesh it
returned 1-byte hops (`6A`, not `6Axx`) and failed to route where a
1-byte prefix is ambiguous ("Path trace not available").
- Send: path_sz = floor(log2(width)); build width-byte hops — the
repeater/room "Ping" prefixes (contacts_screen), the round-trip
buildPath (now mirrors by hop, not byte), and the map trace builder.
- Parse: read the flag byte for the hop width, group the hash bytes into
packed width-byte hops, key contacts by the full prefix, 1 SNR per hop.
- Render: list panel + markers + map points iterate packed hops, labelled
at the configured width.
PathHelper gains tracePathSz / traceHopBytes / packPrefix / tracePathHops
/ buildTraceRoundTrip, all unit-tested (incl. width-2 hop mirroring).
Bumps build to +47 for the b47 test binary.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Channel.toShareUri/fromShareUri/isValidShareUri encode the reference
MeshCore app's format: meshcore://channel/add?name=<url-encoded>&secret=<hex32>.
A "Share QR Code" action in the channel menu shows the QR via the existing
QrCodeShareDialog, so it round-trips with the reference app.
Encode/decode unit-tested against both real reference examples (#test, #echo).
Scan-to-add follows.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three routing-match sites keyed/compared nodes by only the first
public-key byte, which collides when two nodes share a first byte on a
2-/3-byte path-hash mesh:
- DirectRepeater.matchesPathStart() replaces the 1-byte
`pubkeyFirstByte == pathBytes.first` direct-repeater check in
chat_screen and path_management_dialog (x3 each).
- map _computeGuessedLocations packs the full configured-width prefix as
the repeaterByHash key and looks up the contact-side hop at the same
width — was keying publicKey[0] but reading pathBytes.last, a silent
miss at width>1 that stopped location-guessing on multi-byte meshes.
- map _filterContactsBySettings overlap detection compares the
configured-width prefix via _samePubkeyPrefix (was publicKey.first).
Adds DirectRepeater.matchesPathStart unit tests. Bumps build to +46 for
the combined path-hash sweep test binary (b46: #151+#154+#155+#156).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "Set Path" dialog parsed, built, and displayed hop prefixes as a fixed
1 byte (substring(0,2)), so on a 2-/3-byte path-hash mesh manual entry produced
wrong-width routing paths and the helper text claimed "1 byte". Make it width-
aware from connector.pathHashByteWidth.
- parsePathPrefixes(text, width, invalid): static + testable; each entry must be
EXACTLY width*2 hex chars (wrong-length / malformed go to invalid, never
silently truncated); width clamped >= 1.
- _updateTextFromContacts / display / maxLength group by the configured width;
the display substring is guarded against short keys.
- show() takes pathHashByteWidth; the chat + path-management callers pass
connector.pathHashByteWidth.
- l10n: reworded the path-entry helper / example / instructions to be width-
agnostic (dropped the "2 hex / 1 byte / first byte" claims). The 17 non-English
locales keep their prior strings, flagged for re-translation (English fixed).
analyze clean; 6 parsePathPrefixes tests (incl. exact-length + clamp). Gemini v1
BLOCKER (unguarded substring) + MAJOR (silent truncation) fixed; v2 MAJORs were
the pre-existing refresh button (out of scope), v2 MINOR (1-byte example) fixed.
Closes#155
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PathHelper.formatPathHex / resolvePathNames assumed 1 byte per hop, so the
direct-chat "full path" and path-management dialogs rendered routing paths
per-byte (84,AB instead of 84AB) and resolved hop names by a single byte on
2-/3-byte path-hash meshes. Group bytes into pathHashByteWidth-sized hops and
match hop names by the full-width prefix.
- formatPathHex(bytes, width) / resolvePathNames(bytes, contacts, width) group
into width-byte hops (trailing partial hop kept); name match uses the full
hop prefix.
- chat_screen + path_management_dialog pass connector.pathHashByteWidth.
analyze clean; 7 PathHelper tests (incl. width-2 collision-avoidance); Gemini: ship.
Closes#154
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DirectRepeater stored only pubkeyFirstByte (1 byte of the last hop), so the SNR
indicator + Nearby Repeaters dialog showed a 1-byte prefix and matched the wrong
repeater on 2-/3-byte path-hash meshes. Capture the last hop's full configured-
width prefix and use it for display, contact-match, and dedup.
- DirectRepeater.pubkeyPrefix (Uint8List) + prefixHex getter; pubkeyFirstByte
retained for the path-management / chat matching sites (those are #156).
- _updateDirectRepeater captures path[len-width..] (or the pubkey prefix when no
path); dedup by listEquals(pubkeyPrefix).
- snr_indicator: status-bar + dialog render prefixHex; matcher keys on the full
width prefix so the correct repeater name resolves.
Bumps build to +45. analyze clean; 6 DirectRepeater tests (2 new); Gemini: ship.
Closes#151
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The fork-only 0xC1 GPS command was sent to any connected radio — both the
60s self-location poll (on gps=1) and the Settings GPS status query. Stock /
non-Offband MeshCore firmware can't answer it, so gate both behind a
capability check.
- firmwareSupportsOffbandGps(offbandCaps): presence of the Offband-fork
offband_caps byte (device-info v14+), which stock MeshCore never emits.
- requestOffbandGps() hard-returns when unsupported — 0xC1 never goes out.
- _reconcileGpsPolling() centralizes the poll decision (support AND gps=1)
across all three triggers + the device-info reply, so frame order is moot.
- Settings hides the GPS status section on unsupported firmware.
Interim presence-gate; a dedicated OFFBAND_CAP_GPS bit can follow once
firmware defines one. Unit-tested (3 new); Gemini review: ship.
Bumps build number to +43 (test build b43).
Closes#144
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Offband 0xC1 GPS query: 1-byte request -> RESP 0xC1 ASCII reply
(enabled/detected/active/fix/baud/lat/lon/alt_cm/sats/time), parsed into
OffbandGpsStatus (lat/lon /1e6). New GPS-status section in the Location dialog
(live-fix vs stored banner + coords/sats/alt/time). Validated against RedCreek's
feat/gps-state-query test firmware.
#140: the per-minute GPS poll now uses the lightweight 0xC1 query instead of
CMD_APP_START (which re-walked channels + re-drained messages = the BLE re-init
flood); a live fix updates self-lat/lon. Validated on b42 (APP_START ~1/min ->
~0, 0xC1 polls clean every 60s, channel re-walk gone).
Gemini (fix-then-ship): atomic completer-claim in _handleOffbandGps to shrink
the stale-reply race; full elimination needs a protocol sequence id, but the
poll shares in-flight via the re-entrancy guard and the query button is disabled
during a request, so concurrent requests don't occur in practice. The hardcoded
snackbars Gemini flagged in _editLocation are pre-existing l10n debt, not
introduced here; all new strings are localized.
Closes#135Closes#140
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Parse the device-info reply's readable strings (build date / model / firmware
version, NUL-terminated after the 8-byte header) and show firmwareVersion +
deviceModel in Settings → Device Info. Raw strings are logged on connect.
End-anchored field mapping (deviceInfoFields) keeps the version correct on
partial frames.
Gemini (fix-then-ship): removed the unused deviceBuildDate field/getter (dead
state) — the build date stays in the connect log via the raw strings, and
deviceInfoFields still parses + tests it.
Closes#134
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Packet Path mis-identified repeaters: a 2-byte path hash like 6A3D was
sliced into two 1-byte hops (6A + 3D), so the first byte collided with an
unrelated repeater (Freemasons 6A..) and the second matched nothing
("Unknown Repeater").
- meshcore_protocol: realHopCount() converts the firmware path BYTE length
to true hops via the device hash width.
- channel_message_path_screen: slice/count/match at the device
pathHashByteWidth (not the unreliable per-message high-bits); bucket and
match on the full w-byte hash (mirrors _pathMatchesContact), not hash[0].
- channel_chat_screen: same width fix on the per-message via-line + hop badge.
- settings: show the build number alongside the version name.
- bump 1.1.2-beta.4+36.
- test: realHopCount regression coverage.
Gemini review (llm-consult): fix-then-ship; 2 findings clarification-only
(_formatHash is canonical hex used identically both sides; pathHashByteWidth
is static per-connection).
Part of epic #112.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The broker-pool GET put a single fixed 6s deadline on the entire paginated dump. A large pool (6+ brokers) streams field-by-field over ~8s and blew the deadline, so the client falsely reported the pool as not finished even though the device sent it all (the END terminator arrives after the client gave up). Re-arm an inactivity Timer on each received frame: a steadily-streaming dump never times out; only a genuine stall (no frame for the timeout window) fails. Adds a TDD test - a slow-but-steady dump, frames under the timeout but total far over it, must complete.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Always-on file logging so users/support can retrieve a log without enabling
anything in the UI. No-op on web (no filesystem).
- FileLogWriter: rotating file writer (size cap + max files), single + batch
writes, unit-tested against a real temp dir.
- FileLogService: singleton initialised in main(); resolves
getApplicationSupportDirectory()/logs, queues lines, periodic 2s batched
flush, bounded queue, never crashes the app on an IO error.
- AppDebugLogService.log + BleDebugLogService.logFrame write every entry to the
file (app logs always, independent of the in-app viewer's enable gate).
Location per the #97 decision: standard support dir on all platforms (Windows
%APPDATA%\app.offband.meshcore\logs). Still TODO before recompile: access UI
("Open logs folder" on desktop / "Share logs" on mobile).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consume the additive OCFG_BROKERS dump fields from firmware #172/#173
(backward-compatible — absent fields read as today's enabled/disabled):
- BrokerRuntimeState (`state`) + BrokerLastError (`last_error`) + a pure
BrokerConfig.status mapping -> Connected / Connecting / Failed (reason) /
Held (no clock|low heap) / Enabled / Disabled; held shown neutral, not error.
- Brokers list shows the real status line + colour instead of just "enabled".
- Editor shows jwt_owner / iata_override resolved defaults (`jwt_owner_resolved`,
`iata_resolved`) as greyed placeholders when the raw key is blank; only writes
the raw key when the user enters a value (blank stays the source of truth).
Sourced from the pool dump (getBrokers); getBroker's per-field GET list is
unchanged pending the firmware GET-ability answer on #173. Unit-tested mapping,
parsing, and backward-compat (9 tests).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A firmware build can ACK a broker enable/disable as success without applying it
(HV4; meshcore-firmware#179). The quick toggle and the editor save previously
trusted the ACK and showed a success snackbar, so an ack-but-no-op looked like
it worked.
Both paths now SET, wait a short settle, re-read the slot, and report what the
device ACTUALLY did:
- applied -> confirm
- notApplied -> warn "possible firmware issue"; the editor stays and re-seeds to
the device's true state instead of popping a false success
- unverified -> re-read failed (e.g. an enable that rebooted the device)
Every device is treated identically: the UI reflects the re-read, never assuming
a change took that the device didn't confirm.
- BrokerConfig.classifyApply pure classifier + BrokerApplyOutcome (unit-tested)
- ObserverConfigService.applySettleDelay
- wired into mqtt_brokers_screen quick toggle + broker_editor_screen save
- widget test for the ACK-but-not-applied path
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The BLE/USB self-info retry was an unbounded 3.5s hammer -- the Web path caps at
3 and skips while syncing; BLE had neither. On a slow device handshake it churned
the connect and starved the contact sync, hanging the contact-load bar. Inherited
from upstream zjs81/meshcore-open (git blame; zero Offband commits).
- Replace the unbounded 3.5s Timer.periodic with a self-rescheduling backoff:
3.5s -> 7 -> 14 -> 28 -> 56s, then a steady ~60s keep-alive. Never hard-stops --
a slow/recovering device still gets prompted.
- Add the Web path's skip-while-syncing guard on BLE (per-tick skip, never
permanent: the timer keeps rescheduling, so a settled sync lets the next tick
send).
- Decisions extracted as pure, tested helpers: nextAppStartRetryDelay(attempt)
and shouldSendAppStartRetry({connected, awaitingSelfInfo, syncing}).
- Disconnect teardown (Gemini's amendment) was already present (:2501 / :2374).
Gemini plan-review: Proceed, no blockers. 6 new helper tests; full suite 299 green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On-hardware feedback: JWT owner defaults to the device pubkey and IATA override
is optional by definition, so gating a save on them is wrong -- they have
under-the-hood firmware defaults.
BrokerConfig.enableError now checks only the structural fields with no default:
URL present, port in range. The JWT field checks are gone; the firmware enforces
JWT completeness with its defaults, and a genuinely-bad enable surfaces the
firmware's reason via the failure messaging added earlier. The editor's pre-save
check and the list's quick Enable both follow.
Tests updated to the new contract (a JWT broker with blank owner/audience now
enables; a structurally-complete quick Enable confirms). Full suite green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On-hardware test feedback:
- Long-press Enable/Disable was silent on success AND failure. It now confirms
on success and, on failure, surfaces the device's reason (lastError) instead
of nothing.
- The client now refuses to quick-Enable a broker that can't work (incomplete
required fields, e.g. JWT without audience/owner) and says why -- so it never
reports "enabled" on a broker that won't run. (Firmware should reject too.)
- The editor blocked a DISABLE on blank/invalid fields. Validation now gates
ENABLING only -- disabling a slot with partial values is fine; it won't be
active.
Validation rules shared via BrokerConfig.enableError (editor pre-save + list
quick-Enable). Full suite green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The broker pool gets its own screen, reached from a "MQTT brokers" row in the
Observer pane (replacing the read-only inline list and its stale "pending
firmware support" note).
- MqttBrokersScreen: a tile per broker (tap -> editor, long-press -> Enable /
Disable / Edit / Clear), and a + FAB that opens the next empty slot (disabled
when all 10 are full) -- matching channels_screen's FAB-add pattern. Each
action re-reads only the affected slot, never the full pool dump.
- Observer pane: the broker section is now a navigational row showing
"N configured - M enabled"; the editor is wired in end-to-end.
3/3 screen tests green; full suite green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The edit UI for one broker slot. Staged-save: controls edit local state; Save
sends only the CHANGED fields through saveBroker (enabled written last), so a
partial save can't leave a live-corrupt broker.
- Fields: url, port, transport, auth_type, username, write-only password,
topic_prefix, iata_override, jwt_* (when auth=jwt), ca_cert (when tls/wss),
plus the enabled toggle.
- Validation before any SET: port 1-65535, jwt audience+owner when auth=jwt.
- Write-only password: sent only when typed; blank keeps the stored secret.
- Refresh: single-slot re-GET (getBroker) + reseed, with a dirty discard guard.
- Back button guards unsaved edits (PopScope -> Discard? dialog) -- Gemini's
state-loss finding.
4/4 widget tests green. Brokers list screen + Observer-pane wiring next.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The write half of the broker editor, against RedCreek's confirmed field-at-a-time
contract. The existing 0xC0 key/value command already carries the broker keys --
no new wire codes -- so the codec comment at observer_config_client.dart:166 is
stale, not a gap.
- saveBroker(): disable a live slot first, write each changed field, write
`enabled` LAST. Stops at the first ERR/timeout and reports the failed field --
a partial save leaves the slot disabled, never live-corrupt (activation guard).
TDD: ordering + partial-failure tests.
- clearBroker(): mqtt.broker.<slot>.clear.
- getBroker(): single-slot re-GET via 14 per-field GETs (recovery / post-save
read), not the 84-frame pool dump. Password is write-only -> best-effort.
Key spellings (mqtt.broker.N.enabled / .clear, scalar field GET) are inferred
from the existing scheme; they're the one thing the joint firmware test confirms.
12/12 service tests green. UI (brokers screen + editor) next.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The firmware answers RESP_CODE_ERR for an empty/invalid channel index, but
_handleErrorFrame never advanced the in-flight channel sync. So every empty slot
waited out the 2s timeout + 3 retries (~8s each): a device with a few populated
channels out of a large capacity took minutes to sync (~40 slots requested, ~29
empty -> ~4 min) before the real channels (which answer CHANNEL_INFO) finally
synced in the last couple seconds.
Now an ERR that lands while a channel GET is in flight (and no generic-ack
command is waiting -- that one is order-correlated to the ERR first) advances to
the next slot immediately, mirroring the CHANNEL_INFO success path minus adding
a channel. The decision is pinned as a pure static predicate and tested,
matching the connector's existing decision-helper test convention.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three observer-delta fixes (per the #80 sync diagnosis / G2):
- A: settings screen gates the Observer category via context.select on the
supported flag, not a connector-wide context.watch -- the watch rebuilt the
whole settings screen on every sync frame, thrashing the UI thread.
- B: a flat Save re-reads only the flat settings (refresh includeBrokers:false),
never the 84-frame broker pool -- no BLE re-flood for a display toggle.
- C: the initial observer read is deferred until the device's channel/contact
sync settles, so it never competes with that sync for BLE.
TDD: B (no OCFG_BROKERS on a flat refresh) + C (refresh deferred while syncing,
fires once idle). Full observer suite green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The G2 BLE log showed the config protocol works (GETs return values; the broker
dump starts) but the firmware's broker dump stalls without BROKERS_END, so
getBrokers() times out. The client was treating that as a TOTAL failure --
discarding the 7 flat settings it read cleanly and blanking the pane.
Now the broker pool loads independently: refresh() builds + shows the flat
config even when getBrokers fails, flags brokersUnavailable, and the pane shows
the broker section as "unavailable" instead of going stale. Flat settings stay
usable. TDD red->green; full observer suite (39) green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes the 2 MINOR findings from the G1 re-review (#76):
- MINOR-A: ObserverConfigService.refresh() no longer wipes lastError / clears
stale on a PARTIAL read. A null from any getFlat is a failed read; the
snapshot is marked stale and the error kept (error-visibility).
- MINOR-B: the settings pane validates status_interval against the firmware
range (10-3600) before sending; an out-of-range value is not put on the wire
and the user is told.
TDD: both tests RED first (current sent '5'; refresh wiped the error), GREEN
after. Full observer suite (37 tests) green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Hardened the staged-save pane (settings_screen capability gate already correct):
- Normalize an unexpected device rotation to a valid SegmentedButton segment
{0,180} so the control always has a valid selection and never echoes an
out-of-range value back on save.
- Disable Save during a refresh (no save/refresh race on the controllers).
- digitsOnly formatter on status-interval (firmware still range-checks).
- Keys on the SSID/password fields for robust widget-test targeting.
4 widget tests pin the security-sensitive behavior: saves send ONLY changed
keys, a blank password keeps the stored secret, an entered password is sent as
wifi.pwd then the field clears, rotation is normalized, stale/error surfaced
(SAFELANE §6). Full observer suite (33 tests) green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
main.dart wiring verified correct (no change): the singleton MeshCoreConnector
(main.dart:38 -> field -> provided by value) is the SAME instance passed to
ObserverConfigService, so the service watches the live transport. Plain
ChangeNotifierProvider (not a proxy) is right -- the connector instance is
stable; only its state changes, which the service reads live. Lazy-safe: the
constructor has no side effects.
This test pins that contract: the service is reachable through Provider and a
mutation to the shared connector's caps flips the Observer gate (supported).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Gemini MAJOR #4 (offset unverified / short-frame robustness):
- Offset VERIFIED against firmware MyMesh.cpp: the device-info tail is three
consecutive out_frame[i++] writes -- client_repeat@80, path_hash_mode@81,
offband_caps@82. Bytes 80/81 are shipping (path-hash), so caps@82 is correct
by adjacency, not a guess.
- Extracted MeshCoreConnector.parseOffbandCaps (pure, bounds-checked) so the
guard + offset are unit-testable; _handleDeviceInfo delegates to it.
- 4 tests pin: pre-v14 short frame -> null, len-83 reads byte 82 verbatim,
observer bit round-trips supportsConfig, hostile truncated frames never OOB.
Live observer node confirms end-to-end at G2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Strycher
