Fixed three bugs in joining the frames of a multi-frame message. The … - #85
Fixed three bugs in joining the frames of a multi-frame message. The …#85tekstrand wants to merge 3 commits into
Conversation
|
The new assembly logic appears to assume that all data/continuation frames carry bit 0x4, but the PR description says Normal mode produces types 1, 0, 0, 2. In that sequence, the type-0 continuation frames are treated as non-data, so needsSpaceBefore() can insert spaces at arbitrary mid-token boundaries between continuation frames particularly with a word/grid split across frames. I THINK this currently still produces incorrect spacing in Normal mode. Thoughts? |
I think I cycled on this too many times and lost track. You're right. The 0x4 flag is only transmitted in the fast modes. Normal/Slow just mark them inside the payload instead. The first payload bit is 1 for a data frame and 0 for everything else (the 10X/11X frame type header). The desktop app derives it that way. All that said, it's worse than spacing. The inbox gates on the same check before appending continuation frames, so in Normal/Slow modes they're dropped. I'm thinking we fix in the JNI where the payload gets unpacked. if (!is_data_flag) {
auto data = unpack_data_message(frame);
if (!data.empty()) {
type |= 0b100; // normal-mode data frame, marked in the payload
}
}Then every consumer keys off that one type value, so the spacing rule, the first-frame gating, and the inbox all start doing the right thing in without Kotlin changes. Thoughts? |
|
I'm fine with fixing it in JNI, makes sense I think. |
…JNI rendered every unpacked data payload through maybe_insert_callsign_prefix, which rewrites text whose first two tokens look like callsigns into "TOKEN0: rest". Continuation frames of a buffered command are plain payload text, so a MSG body like "KA0XYZ N0CALL QRV" was delivered as "KA0XYZ: N0CALL QRV", with a recipient glued in that was never there; the heuristic now runs only on a frame carrying the first-frame bit, which the transmitter sets only on a line's opening frame. The buffer that collects a multi-frame MSG expired on a flat 60 seconds, but a Slow-mode transmission spaces its frames 30 seconds apart, so one missed frame killed the message mid-flight; each buffer now times out after four frame periods of the submode its command frame arrived in. There is no 60 second floor for the faster submodes: the app strips the MSG checksum without checking it, so a longer window only delivers messages with more frames missing and acknowledges them, and it holds a dead buffer against the frequency longer, where it can swallow data frames of the next transmission on the same offset. The third bug was the decode list guessing where spaces go from the text alone. A token ending in a digit got one, so freetext "N5EKS GRID EM10SM87MJ", split by the packer into two data frames after EM10, displayed as "EM10 SM87MJ"; a buffered command's argument got none, so a GRID reply displayed as GRIDEM13TE. The guess is replaced by the frame type bits the decoder already reports: data frames split at arbitrary byte boundaries and carry their own spaces, so they concatenate untouched, and only a directed header can leave a flush boundary, which gets a space only when neither side already has one. That single rule covers buffered commands, unbuffered commands whose payload keeps its leading space, and freetext, with no command table. The first-frame claim comes from a host build of build_message_frames: a buffered MSG comes out as flags 1, 0, 0, 2 in Normal and 1, 4, 4, 6 in Turbo, so no continuation carries the bit in either submode. The spacing rule was verified on air, desktop app playing over speakers to the tablet: "N5EKS GRID EM10SM87MJ" arrives as two data frames and renders unsplit, read back from the view hierarchy, where the build before this change displayed "EM10 SM87MJ"; a buffered GRID reply still gets its space. The app unit tests cover both directions.
…nto Kotlin. Fast and Turbo carry the flag in the three over-the-air frame type bits, but Normal and Slow have no room for it there and put it in the payload instead, so their data frames arrived in Kotlin with a type holding only the first and last bits. Every consumer keyed on the flag, the multipart reassembly, the relay path and the inbox, took those frames for something else. The JNI already proves a frame is data by unpacking it, so it now sets the flag when the unpack succeeds, which is the same enrichment the desktop does in DecodedText::tryUnpackData, and every consumer downstream sees one convention without changing. A new engine loopback test transmits a forced data frame in Normal mode and asserts the reported type carries the flag; against the previous code it fails with type 0x3.
… was spelled four ways across three files, one of them named isRelayDataFrame where a search for the others would never find it; it now lives on DecodedMessage beside the first and last frame checks, and the copies are gone. The decode list assembly carried a parallel list of frame texts, a DecodeFrame shim and a zip to knit them back together; the frames now flow through as DecodedMessage, the helper rewrite is done by copy, and blanked helper frames are dropped before the space rule runs, which turns that rule into a comparison of two adjacent frames. On the JNI side render_decoded_text reports the enriched type through an out parameter instead of mutating its argument, which removes a copy of every decoded event at both call sites, and the log-only render in js8_engine_create is gone: it unpacked every frame a second time to print a type the enrichment had already been thrown away from. The DECODED log now prints from event_callback, where it shows the type Kotlin actually receives. The first frame guard moved inside maybe_insert_callsign_prefix so the rule is enforced where it is stated, and the two loopback tests share one transmit, place, decode pipeline instead of copying it.
bcc6398 to
d0b63e6
Compare
|
♻️ JNI as discussed, plus an engine test that transmits a forced data frame in Normal mode and checks the type that reaches Kotlin. Rebased onto master, and since this fix makes the type bits the source of truth, the data flag check now lives on DecodedMessage instead of four copies. PR updated. The relay buffer and the decode list buffer still run flat 90s timeouts with the same Slow mode problem item 2 fixes for the inbox buffer. Was thinking of handling this in a followup |
2 participants
What
EM10SM87MJshowing asEM10 SM87MJand a GRID reply showing asGRIDEM13TEWhy
prefixheuristic was running on every frame instead of only the first.EM10 SM87MJsplit) and a buffered command's argument got none (that'sGRIDEM13TE). The decoder already tells us which frames are data frames, and data frames carry their own spaces, so there's nothing to guess.Test
./gradlew :app:testDebugUnitTestEM10SM87MJvsEM10 SM87MJ.SPOTS KA0XYZ EM10 W1ABC FN31 K5DEF EM12as freetext. Master shows colons after EM10 and FN31 that were never sent, the fix shows it as typed.