Fix non-portable ADIF export fields and surface upload failure reasons - #701
Open
patrickrb wants to merge 1 commit into
Open
Fix non-portable ADIF export fields and surface upload failure reasons#701patrickrb wants to merge 1 commit into
patrickrb wants to merge 1 commit into
Conversation
Three problems found while diagnosing a logbook server that had silently
rejected every QSO upload for three days.
**Zero-length fields.** An optional value that was present-but-empty (a QSO
where the other station never sent a grid) exported as `<gridsquare:0> `.
Several ADIF importers treat a length-0 field as malformed and reject the
whole record rather than reading it as "absent" — 39 of 384 records in a real
export carried one. Empty now means omitted, which every parser agrees on.
`mode` had its own null-only guard and so kept emitting `<mode:0>` even after
the shared helper was fixed; the new test caught it. `comment` was emitted
unconditionally because the `<eor>` terminator was glued onto it — they are
separate now.
**QSL_MANUAL is not an ADIF field.** The bare name is non-conformant; ADIF
reserves `APP_<PROGRAMID>_` for program-specific data. We now write
`APP_FT8AF_QSL_MANUAL`, and `QSLRecord` reads both names so files exported by
older builds still round-trip with their confirmation flag intact.
**Upload failures were invisible.** `uploadAdifToCloudlog` returned a bare
boolean and the server's explanation went only to `Log.d`, so `debug.log`
recorded `cloudlog=0 qrz=0 of 113` — indistinguishable from having nothing to
upload. The reason now flows through `SyncResult` into the log line, which
would have read:
QsoAutoSync: done (app-start): cloudlog=0 qrz=0 of 113
cloudlogError=HTTP 400: ... column "tx_pwr" of relation "contacts" does not exist
That is a 30-second diagnosis instead of a three-day silent backlog.
Also collapses `DatabaseOpr.downQSLTable` — the built-in web logbook's ADIF
download — onto `AdifRecord`. It was a line-for-line duplicate of
`AdifRecord.build()` and had drifted: it still carried both bugs above long
after the file-export path was fixed. One builder now, so the next formatting
fix can't land in only one of them (-95 lines).
Tests: zero-length omission across every optional field, the APP_ prefix and
legacy-name import round-trip, and the failure-reason formatting (status +
server body, newline collapsing, truncation, and never echoing the submitted
ADIF into debug.log). 2881 pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #701 +/- ##
============================================
+ Coverage 36.70% 36.72% +0.01%
Complexity 222 222
============================================
Files 227 227
Lines 27933 27938 +5
Branches 3485 3487 +2
============================================
+ Hits 10254 10259 +5
Misses 17426 17426
Partials 253 253
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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
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.
Found while diagnosing why every QSO upload to a Cloudlog-compatible server had
been failing silently for three days. The server-side cause was its own
problem, but getting there surfaced three real app-side ones.
Zero-length fields make records unimportable
An optional value that was present-but-empty exported as
<gridsquare:0>.Several ADIF importers treat a length-0 field as malformed and reject the whole
record instead of reading it as "absent" — 39 of 384 records in a real export
carried one, from QSOs where the other station never sent a grid.
Empty now means omitted, which every parser agrees on.
Two follow-ons the new tests caught:
modehad its own null-only guard (it branches separately for the MFSKsubmode split) and so kept emitting
<mode:0>after the shared helper wasfixed.
commentwas emitted unconditionally because the<eor>terminator wasglued onto it. They are separate now, so a commentless QSO no longer carries
a zero-length comment purely to hold the terminator.
QSL_MANUALis not an ADIF fieldThe bare name is non-conformant — ADIF reserves
APP_<PROGRAMID>_forprogram-specific data, precisely so consumers can skip what they don't know
instead of erroring on it.
We now write
APP_FT8AF_QSL_MANUAL.QSLRecordreads both names, so ADIFfiles written by older FT8AF builds keep importing with their confirmation flag
intact.
Upload failures were invisible
uploadAdifToCloudlogreturned a bare boolean, and the server's ownexplanation went only to
Log.d. Sodebug.logrecorded:…which reads identically whether the network was down, the API key expired, or
the server was rejecting every single record. The backlog grew from 74 to 113
QSOs across three days with nothing to point at.
The reason now flows through
SyncResultinto that line:That is a 30-second diagnosis. The description is collapsed to one line and
truncated (a misconfigured proxy can return a whole HTML error page), and the
QRZ variant deliberately reports only
RESULT/REASON— the rest of QRZ'sresponse echoes the submitted ADIF back, which has no business accumulating in
debug.log.Also: one ADIF builder instead of two
DatabaseOpr.downQSLTable— the built-in web logbook's ADIF download — was aline-for-line duplicate of
AdifRecord.build(), same fields in the same order.The copies had drifted: it still carried both bugs above long after the
file-export path was fixed. It now maps its cursor onto
AdifRecordandnothing else (-95 lines), so the next formatting fix can't land in only one of
them. Its existing tests cover the mapping, plus new ones for the empty-value
and field-name behaviour.
Testing
testDebugUnitTest: 2881 pass. New coverage for zero-length omission acrossevery optional field, the
APP_prefix and legacy-name import round-trip, andthe failure-reason formatting (status + body, newline collapsing, truncation,
and never echoing submitted ADIF).
Built and installed on a Pixel 8. The device disconnected before the
debug.logline could be observed in the wild, so that last confirmation isstill outstanding — the log format itself is unit-tested.
🤖 Generated with Claude Code