fix: log download truncation and lossy-link stalls, add cancellation and protocol tests - #24
Conversation
…col tests - LogDownloadTracker: only trust a short packet as the log end at the highest offset seen - a stale short retransmit of an earlier block used to set a too-small total and silently truncate the download - GetLog: CancellationToken support; LOG_REQUEST_END and temp-file cleanup already cover the cancel path - LogDownload UI: Cancel button with a per-batch CancellationTokenSource - port the fake-vehicle protocol suite (ordering, loss recovery, stray retransmits, cancel, timeout) with a LogRetryDelayMs test seam
Six toolbar items need ~830px in a single row; the horizontal StackPanel painted "Erase All" (and the new Cancel) past the 540px default width. WrapPanel per the joystick-window fix, MinWidth 420, and a layout guard asserting every toolbar child stays inside the window at 420 and 540.
Ported from the upstream fork: log list, timed full download with oracle byte-compare, cancellation and link-reuse checks, plus a lossy proxy dropping 5% of LOG_DATA. First run found the repair phase serving one gap per 3 s silence window - a lossy 2.3 MB download projected ~65 min.
Track the outstanding request's end and issue the next missing-range request the moment it is satisfied; a new LogRepairDelayMs (500 ms) covers repair responses that were themselves lost. Lossy-link SITL run drops from not finishing to 72.9 s, byte-identical; clean link unchanged.
…end inference - silence tolerance is a time budget (retryLimit x LogRetryDelayMs), so short repair windows no longer shrink it from 30 s to 5 s - repair chaining requires actual coverage progress; stale or duplicated packets cannot multiply requests - only frontier-near packets raise the end-inference bar, so a corrupt far offset cannot poison it; data beyond the known total is ignored - shared IssueNextRequest/SilenceWindowMs helpers; completion checks reuse the computed coverage instead of re-walking the range list per packet - four new discriminating protocol/tracker tests; SITL clean and lossy runs re-verified byte-identical (0.45 s / 73.7 s)
- claim IsBusy before the picker awaits so batches cannot interleave - Cancel enables only while a download runs (new IsDownloading), not during refresh/erase where it had nothing to cancel - a cancel landing during the copy/KML tail now cancels instead of reporting success - toolbar spacing via ItemSpacing/LineSpacing like the sibling views
|
Hi @Rouniy — would you be interested in this change set? It fixes a silent Separately, I've been rewriting the dataflash log core in Rust |
A LOG_DATA packet that is both short and at a corrupt far offset cleared the end-inference bar trivially: only frontier-near packets raise the bar, so end >= bar held and the download ended at a phantom length. Past the true end that stalls the repair phase into a timeout; below it the phantom silently truncated the returned file. Rejecting far end packets outright is not an option: packet loss stalls the frontier at the first dropped block, so the genuine end of a large log always sits far past the trust window, and every recovered gap then forces the vehicle to re-stream the whole log (that variant never finished the lossy SITL run). Instead the tracker records a deferred candidate, discarded when more stream arrives, and GetLog promotes it once a silence window expires: a corrupt packet is followed by more stream, the real end is not. Found via review of the equivalent change on the upstream PR (ArduPilot#3764). SITL: clean unchanged (0.44 s byte-identical), 5% LOG_DATA loss 77.3 s byte-identical in one streaming pass - the earlier 72.9 s reference plus one silence window confirming the end. Harness README reference results and the STATUS.md checkpoint updated to match.
|
Reviewed and merged. Thank you for the careful port and the protocol coverage. Why we accepted it:
Integrated exact contributor head During review we added follow-up
Local verification on the integrated tree:
The contributor PR workflows were held as |
|
Thanks for reviewing and merging the log download PR, @Rouniy! Following up on the offer I mentioned there, in case it got lost in the review
The parser is parity-tested against a multi-vehicle corpus, fuzzed, and Interested? If yes I'd open it as a small stack of reviewable PRs (build |
2 participants
Hardens the MAVLink dataflash log download end to end, ported and extended from
the protocol test work in userepo/MissionPlanner (upstream PR ArduPilot#3764).
Bug fixes
LogDownloadTrackertrusted the first shortpacket as the log end, so a stale short retransmit of an earlier block locked in
a too-small
TotalLengthandSetLengthsilently truncated the file. A shortpacket is now only trusted at the highest offset seen, and only frontier-near
packets can raise that bar — so a corrupt far-offset packet can neither poison
end detection nor lengthen the returned file.
upstream PR). A packet both short and far cleared the highest-offset bar
trivially and ended the download at a phantom length — below the true end it
silently truncated the returned file. Rejecting far end packets outright is
not an option either: packet loss stalls the trusted frontier at the first
dropped block, so the genuine end of a large log always arrives far past it
(that variant passes every unit test and never finishes the lossy SITL run —
each recovered gap forces a full re-stream). A far end packet is now a
deferred candidate, discarded if any more data arrives and accepted once the
stream goes quiet: a corrupt packet is followed by more stream, the real end
is not.
per ~3 s silence window. At 5% packet loss a 2.3 MB log leaves ~1,290 scattered
single-block gaps — roughly 65 minutes projected; the SITL run did not finish in
10. Repair requests now chain the moment the current one is satisfied (gated on
actual coverage progress, so duplicated packets cannot multiply requests), with a
500 ms repair-phase silence fallback. The same scenario now completes in ~77 s
in a single streaming pass, byte-identical (~3 s of that is the silence window
confirming the deferred end above).
retryLimit x LogRetryDelayMs, ~30 s), so the short repair windows no longer shrink thetotal silence a fading radio is allowed before the download aborts. Data
beyond the known log end is ignored.
Cancellation
GetLogtakes aCancellationToken; the download window gets a Cancel button(enabled only while a download actually runs), a per-batch token source, and a
cancel that also covers the copy/KML tail —
LOG_REQUEST_ENDand temp-filecleanup already run on that path.
Download window layout
The six toolbar items need ~830 px in one row; the horizontal
StackPanelpainted "Erase All…" (and the new Cancel) past the 540 px default width. The
toolbar now wraps (
WrapPanelwithItemSpacing/LineSpacing, per the siblingviews) and the window gains
MinWidth/MinHeight. ALayoutOverflowTeststheory guards every toolbar child inside the window at 420 and 540 px.
Testing
15 fake-vehicle protocol tests (
GetLogProtocolTests): ordering, exactblock-multiple logs, loss recovery, stray short retransmits, corrupt short-far
packets (including the silent-truncation case), a far end marker past a
stalled frontier, duplicate storms, the silence time budget, beyond-end data,
oversized counts, empty logs, cancellation, and timeout — plus tracker unit
tests. The truncation, corrupt-packet, duplicate storm, and budget tests all
fail against the previous code.
A manual SITL end-to-end harness with a 5%-loss proxy
(
MissionPlannerTests/Avalonia/MissionPlanner.SitlTests, registered as retainedtooling in
PROJECT_ARTIFACT_AUDIT.tsv; checkpoint recorded inPorting/STATUS.md). Reference results, 2.3 MB log against real ArduCopter SITL:Full suite: 1519/1531 — the failures are the pre-existing platform/environment
set, unchanged from master.