From c5de905a3f5aa7c4f68d91e3e04f03e019f93219 Mon Sep 17 00:00:00 2001 From: tekstrand Date: Thu, 27 Aug 2026 19:05:50 -0500 Subject: [PATCH 1/3] Fixed three bugs in joining the frames of a multi-frame message. The 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. --- adapters/android/jni/js8_engine_jni.cpp | 6 +- .../example/service/JS8EngineService.kt | 16 +++-- .../com/js8call/example/ui/DecodeViewModel.kt | 59 +++++----------- .../DecodeViewModelMultipartAssemblyTest.kt | 70 ++++++++++++++----- 4 files changed, 81 insertions(+), 70 deletions(-) diff --git a/adapters/android/jni/js8_engine_jni.cpp b/adapters/android/jni/js8_engine_jni.cpp index 65efb138..87142c7f 100644 --- a/adapters/android/jni/js8_engine_jni.cpp +++ b/adapters/android/jni/js8_engine_jni.cpp @@ -230,6 +230,7 @@ static bool is_callsign_like(std::string const& token) { return has_digit; } +// First frame of a line only; a continuation's payload is plain text. static std::string maybe_insert_callsign_prefix(std::string const& text) { std::size_t first_sep = std::string::npos; for (std::size_t i = 0; i < text.size(); ++i) { @@ -276,6 +277,7 @@ static std::string render_decoded_text(JS8Engine_Native* native, js8core::events } bool is_data_flag = (decoded.type & 0b100) == 0b100; + bool is_first_frame = (decoded.type & 0b1) == 0b1; // Try data payloads first (mirrors desktop unpack order). __android_log_print(ANDROID_LOG_DEBUG, "JS8FrameDebug", "Trying data unpacker: frame='%s', decoded.type=0x%02x", @@ -284,7 +286,7 @@ static std::string render_decoded_text(JS8Engine_Native* native, js8core::events auto data = unpack_fast_data_message(frame); __android_log_print(ANDROID_LOG_DEBUG, "JS8FrameDebug", "unpack_fast_data returned: '%s'", data.c_str()); - if (!data.empty()) return maybe_insert_callsign_prefix(data); + if (!data.empty()) return is_first_frame ? maybe_insert_callsign_prefix(data) : data; // Fast-data frames should not be treated as heartbeat/compound/directed. __android_log_print(ANDROID_LOG_WARN, "JS8FrameDebug", "Fast data unpack failed, returning raw frame: '%s'", frame.c_str()); @@ -293,7 +295,7 @@ static std::string render_decoded_text(JS8Engine_Native* native, js8core::events auto data = unpack_data_message(frame); __android_log_print(ANDROID_LOG_DEBUG, "JS8FrameDebug", "unpack_data returned: '%s'", data.c_str()); - if (!data.empty()) return maybe_insert_callsign_prefix(data); + if (!data.empty()) return is_first_frame ? maybe_insert_callsign_prefix(data) : data; } // Heartbeat (most common for status beacons) diff --git a/android/app/src/main/java/com/js8call/example/service/JS8EngineService.kt b/android/app/src/main/java/com/js8call/example/service/JS8EngineService.kt index 620afd93..e2b09102 100644 --- a/android/app/src/main/java/com/js8call/example/service/JS8EngineService.kt +++ b/android/app/src/main/java/com/js8call/example/service/JS8EngineService.kt @@ -536,7 +536,7 @@ class JS8EngineService : Service() { updateHeardCallsign(text) broadcastDecode(utc, snr, dt, freq, text, type, quality, mode, driftMs) handleRelayFrame(text, snr, mode, freq, type) - maybeHandleIncomingMessage(text, snr, freq, type) + maybeHandleIncomingMessage(text, snr, freq, type, mode) maybeHandleAutoReply(text, snr, mode) maybeReportToPskReporter(utc, snr, freq, text) } @@ -2369,7 +2369,7 @@ class JS8EngineService : Service() { // Drifted timeline, so slots match the engine's cycle boundaries. val now = System.currentTimeMillis() + (engine?.timeDriftMs() ?: 0L) - val frameDuration = getFrameDurationMs() + val frameDuration = framePeriodMs(getPreferredTxSubmode()) // Base delay var delay = if (first) { @@ -2414,8 +2414,8 @@ class JS8EngineService : Service() { heartbeatHandler.postDelayed(heartbeatRunnable, waitMs) } - private fun getFrameDurationMs(): Long { - return when (getPreferredTxSubmode()) { + private fun framePeriodMs(submode: Int): Long { + return when (submode) { SUBMODE_SLOW -> 30000L SUBMODE_NORMAL -> 15000L SUBMODE_FAST -> 10000L @@ -2918,7 +2918,7 @@ class JS8EngineService : Service() { * FROM: TO MSG (multi-frame: command frame) * payload... (multi-frame: data frames follow) */ - private fun maybeHandleIncomingMessage(text: String, snr: Int, freq: Float, type: Int) { + private fun maybeHandleIncomingMessage(text: String, snr: Int, freq: Float, type: Int, submode: Int) { val callsign = getConfiguredCallsign() Log.d(TAG, "maybeHandleIncomingMessage: text='$text' type=$type callsign=$callsign") if (callsign == null) { @@ -2973,6 +2973,8 @@ class JS8EngineService : Service() { snr = snr, frequency = freq, lastUpdated = now, + // Rides out three lost frames in a row before giving up. + timeoutMs = 4 * framePeriodMs(submode), parts = if (initialPayload.isNotBlank()) mutableListOf(initialPayload) else mutableListOf() ) synchronized(msgLock) { @@ -3020,17 +3022,17 @@ class JS8EngineService : Service() { val snr: Int, val frequency: Float, var lastUpdated: Long, + val timeoutMs: Long, val parts: MutableList = mutableListOf() ) private val msgBuffers = mutableMapOf() private val msgLock = Any() - private val MSG_BUFFER_TIMEOUT_MS = 60_000L private fun cleanupMsgBuffers(now: Long) { synchronized(msgLock) { msgBuffers.entries.removeIf { (_, buffer) -> - now - buffer.lastUpdated > MSG_BUFFER_TIMEOUT_MS + now - buffer.lastUpdated > buffer.timeoutMs } } } diff --git a/android/app/src/main/java/com/js8call/example/ui/DecodeViewModel.kt b/android/app/src/main/java/com/js8call/example/ui/DecodeViewModel.kt index 1c8bf36e..aef1e554 100644 --- a/android/app/src/main/java/com/js8call/example/ui/DecodeViewModel.kt +++ b/android/app/src/main/java/com/js8call/example/ui/DecodeViewModel.kt @@ -14,50 +14,26 @@ import kotlin.math.roundToInt import org.json.JSONArray import org.json.JSONObject -internal fun assembleMultipartDecodeText(frameTexts: List): String { - return buildString { - frameTexts.forEachIndexed { index, frameText -> - if (index > 0 && shouldInsertMultipartSpace(this, frameText)) { - append(' ') - } - append(frameText) - } - } -} +/** A decoded frame reaching the list: its rendered text and JS8 type bits. */ +internal data class DecodeFrame(val text: String, val type: Int) -internal fun shouldInsertMultipartSpace(builder: StringBuilder, nextText: String): Boolean { - if (builder.isEmpty()) return false - if (nextText.isEmpty()) return false - var prevIndex = builder.length - 1 - while (prevIndex >= 0 && builder[prevIndex].isWhitespace()) { - prevIndex-- - } - if (prevIndex < 0) return false +private fun isDataFrame(type: Int): Boolean = (type and 0x4) != 0 - var nextIndex = 0 - while (nextIndex < nextText.length && nextText[nextIndex].isWhitespace()) { - nextIndex++ - } - if (nextIndex >= nextText.length) return false - - val prevChar = builder[prevIndex] - val nextChar = nextText[nextIndex] - if ((!prevChar.isLetterOrDigit() && prevChar != ':') || !nextChar.isLetterOrDigit()) { - return false - } - - var tokenStart = prevIndex - while (tokenStart >= 0 && !builder[tokenStart].isWhitespace()) { - tokenStart-- +internal fun assembleMultipartDecodeText(frames: List): String = buildString { + frames.forEachIndexed { index, frame -> + if (index > 0 && needsSpaceBefore(this, frames[index - 1].type, frame.text)) { + append(' ') + } + append(frame.text) } - val prevToken = builder.substring(tokenStart + 1, prevIndex + 1) - val hasDigit = prevToken.any { it.isDigit() } - return hasDigit || prevChar == ':' || isGroupToken(prevToken) } -private fun isGroupToken(token: String): Boolean { - if (!token.startsWith("@") || token.length < 2) return false - return token.drop(1).all { it.isLetterOrDigit() || it == '/' } +// Data frames split mid-word and carry their own spaces; only a directed header +// can leave a flush boundary that needs one. +private fun needsSpaceBefore(soFar: CharSequence, prevType: Int, next: String): Boolean { + if (isDataFrame(prevType)) return false + if (soFar.isEmpty() || next.isEmpty()) return false + return !soFar.last().isWhitespace() && !next.first().isWhitespace() } /** @@ -251,7 +227,8 @@ class DecodeViewModel(application: Application) : AndroidViewModel(application) val frameTexts = buffer.frames.map { it.text }.toMutableList() normalizeCompoundDirectedHelpers(buffer.frames, frameTexts) - val assembledText = assembleMultipartDecodeText(frameTexts) + val frames = buffer.frames.zip(frameTexts) { frame, text -> DecodeFrame(text, frame.type) } + val assembledText = assembleMultipartDecodeText(frames) // Use the last frame's metadata (most recent) val lastFrame = buffer.frames.last() @@ -335,8 +312,6 @@ class DecodeViewModel(application: Application) : AndroidViewModel(application) return directedCommandTailRegex.matches(tail) } - private fun isDataFrame(type: Int): Boolean = (type and 0x4) != 0 - /** * Find a buffer key that matches the given frequency within tolerance. * Returns null if no matching buffer is found. diff --git a/android/app/src/test/java/com/js8call/example/ui/DecodeViewModelMultipartAssemblyTest.kt b/android/app/src/test/java/com/js8call/example/ui/DecodeViewModelMultipartAssemblyTest.kt index 5dc74051..f255bee3 100644 --- a/android/app/src/test/java/com/js8call/example/ui/DecodeViewModelMultipartAssemblyTest.kt +++ b/android/app/src/test/java/com/js8call/example/ui/DecodeViewModelMultipartAssemblyTest.kt @@ -1,45 +1,77 @@ package com.js8call.example.ui import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue import org.junit.Test class DecodeViewModelMultipartAssemblyTest { + // JS8 frame type bits: 0x1 first, 0x2 last, 0x4 data. + private fun directed(text: String) = DecodeFrame(text, 0x1) + private fun data(text: String) = DecodeFrame(text, 0x4) + @Test - fun insertsSpaceAfterGroupCallsignAcrossFrames() { - val assembled = assembleMultipartDecodeText( - listOf("2W0OXE: @RAYNET", "TEST") + fun dataFramesConcatenateWithoutASpace() { + // On air: freetext "N5EKS GRID EM10SM87MJ" split mid-token across two frames. + assertEquals( + "N5EKS GRID EM10SM87MJ", + assembleMultipartDecodeText(listOf(data("N5EKS GRID EM10"), data("SM87MJ"))) ) - - assertEquals("2W0OXE: @RAYNET TEST", assembled) } @Test - fun insertsSpaceAfterAllcallAcrossFrames() { - val assembled = assembleMultipartDecodeText( - listOf("@ALLCALL", "QUERY") + fun bufferedCommandArgumentGetsASpace() { + // GRID strips the separator before packing its argument, so the frames meet flush. + assertEquals( + "NT5DF: N5EKS GRID EM13TE", + assembleMultipartDecodeText(listOf(directed("NT5DF: N5EKS GRID"), data("EM13TE"))) ) + } - assertEquals("@ALLCALL QUERY", assembled) + @Test + fun unbufferedCommandKeepsItsOwnSpace() { + // STATUS is not buffered, so its data frame arrives with a leading space. + assertEquals( + "NT5DF: N5EKS STATUS IDLE AND MONITORING", + assembleMultipartDecodeText(listOf(directed("NT5DF: N5EKS STATUS"), data(" IDLE AND MONITORING"))) + ) } @Test - fun stillInsertsSpaceAfterDirectedCallsign() { - val assembled = assembleMultipartDecodeText( - listOf("2W0OXE", "SNR?") + fun groupTargetHeaderGetsASpace() { + assertEquals( + "2W0OXE: @RAYNET TEST", + assembleMultipartDecodeText(listOf(directed("2W0OXE: @RAYNET"), data("TEST"))) ) + } - assertEquals("2W0OXE SNR?", assembled) + @Test + fun directedHeaderThenManyDataFramesReassembles() { + assertEquals( + "NT5DF: N5EKS MSG HELLO WORLD HOW ARE YOU", + assembleMultipartDecodeText( + listOf( + directed("NT5DF: N5EKS MSG"), + data("HELLO WORL"), + data("D HOW ARE Y"), + data("OU") + ) + ) + ) } @Test - fun doesNotInsertSpaceAfterNonAddressWord() { - assertFalse(shouldInsertMultipartSpace(StringBuilder("HELLO"), "WORLD")) + fun aBlankedHelperFrameLeavesNoLeadingSpace() { + // normalizeCompoundDirectedHelpers blanks a helper frame but leaves it in the list. + assertEquals( + "NT5DF: N5EKS GRID EM13", + assembleMultipartDecodeText(listOf(directed(""), data("NT5DF: N5EKS GRID EM13"))) + ) } @Test - fun detectsGroupTokensAsMultipartBoundaries() { - assertTrue(shouldInsertMultipartSpace(StringBuilder("@RAYNET"), "TEST")) + fun singleFramePassesThrough() { + assertEquals( + "K0OG: KN4CRD SNR +2", + assembleMultipartDecodeText(listOf(directed("K0OG: KN4CRD SNR +2"))) + ) } } From a4f27c0ada3336edb2bb7b05d9837cb3e2080ee7 Mon Sep 17 00:00:00 2001 From: tekstrand Date: Sat, 5 Sep 2026 18:47:53 -0500 Subject: [PATCH 2/3] Set the data flag on Normal and Slow mode data frames as they cross into 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. --- adapters/android/jni/js8_engine_jni.cpp | 16 +++++++++----- .../com/js8call/core/JS8EngineLoopbackTest.kt | 21 +++++++++++++++++++ .../java/com/js8call/core/TestEngine.kt | 11 +++++----- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/adapters/android/jni/js8_engine_jni.cpp b/adapters/android/jni/js8_engine_jni.cpp index 87142c7f..8ae7720c 100644 --- a/adapters/android/jni/js8_engine_jni.cpp +++ b/adapters/android/jni/js8_engine_jni.cpp @@ -268,7 +268,7 @@ static std::string maybe_insert_callsign_prefix(std::string const& text) { return rebuilt; } -static std::string render_decoded_text(JS8Engine_Native* native, js8core::events::Decoded const& decoded) { +static std::string render_decoded_text(JS8Engine_Native* native, js8core::events::Decoded& decoded) { using namespace js8core::protocol::varicode; auto const& frame = decoded.data; @@ -295,7 +295,12 @@ static std::string render_decoded_text(JS8Engine_Native* native, js8core::events auto data = unpack_data_message(frame); __android_log_print(ANDROID_LOG_DEBUG, "JS8FrameDebug", "unpack_data returned: '%s'", data.c_str()); - if (!data.empty()) return is_first_frame ? maybe_insert_callsign_prefix(data) : data; + if (!data.empty()) { + // Normal and Slow carry the data flag in the payload, not in the + // frame type; set it so Kotlin sees one convention for all modes. + decoded.type |= 0b100; + return is_first_frame ? maybe_insert_callsign_prefix(data) : data; + } } // Heartbeat (most common for status beacons) @@ -431,7 +436,8 @@ static void event_callback(JS8Engine_Native* native, js8core::events::Variant co // Handle different event types if (auto* decoded = std::get_if(&event)) { - auto rendered = render_decoded_text(native, *decoded); + auto enriched = *decoded; + auto rendered = render_decoded_text(native, enriched); auto emit_decoded = [&](js8core::events::Decoded const& d, std::string const& text_str) { jmethodID method = env->GetMethodID(handler_class, "onDecoded", "(IIFFLjava/lang/String;IFII)V"); @@ -443,7 +449,7 @@ static void event_callback(JS8Engine_Native* native, js8core::events::Variant co env->DeleteLocalRef(text); } }; - emit_decoded(*decoded, rendered); + emit_decoded(enriched, rendered); } else if (auto* spectrum = std::get_if(&event)) { // Call onSpectrum(float[] bins, float binHz, float powerDb, float peakDb) jmethodID method = env->GetMethodID(handler_class, "onSpectrum", "([FFFF)V"); @@ -594,7 +600,7 @@ JS8Engine_Native* js8_engine_create(JNIEnv* env, jobject callback_handler, __android_log_print(ANDROID_LOG_DEBUG, "JS8Engine_Native", "DecodeFinished: count=%zu", e.decoded); } else if (std::holds_alternative(event)) { - auto const& e = std::get(event); + auto e = std::get(event); auto rendered = render_decoded_text(native, e); __android_log_print(ANDROID_LOG_INFO, "JS8Engine_Native", "DECODED: SNR=%d dB, freq=%.1f Hz, text='%s', raw='%s', type=%d, mode=%d", diff --git a/android/js8core-lib/src/androidTest/java/com/js8call/core/JS8EngineLoopbackTest.kt b/android/js8core-lib/src/androidTest/java/com/js8call/core/JS8EngineLoopbackTest.kt index e9c3bae4..58681efe 100644 --- a/android/js8core-lib/src/androidTest/java/com/js8call/core/JS8EngineLoopbackTest.kt +++ b/android/js8core-lib/src/androidTest/java/com/js8call/core/JS8EngineLoopbackTest.kt @@ -2,6 +2,7 @@ package com.js8call.core import android.util.Log import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith @@ -37,6 +38,26 @@ class JS8EngineLoopbackTest { assertTrue("own transmission did not decode", decoded.any { "CQ" in it.text }) } + @Test + fun dataFrameReportsTheDataType() { + val tx = TestEngine().use { it.transmitFromMidPeriod("HELLO", forceData = true) } + assertTrue("captured ${tx.seconds}s, expected a whole frame", tx.seconds > 10) + + val period = tx.placedInPeriod(periodMs = 15_000, startDelayMs = 500) + .withNoise(rms = 300.0, seed = 1) + + val decoded = TestEngine().use { it.decode(period) } + Log.i(TAG, "decoded ${decoded.size}: $decoded") + val frame = decoded.firstOrNull { "HELLO" in it.text } + assertNotNull("data frame did not decode", frame) + + // Normal mode leaves the data flag out of the transmitted frame type; + // the JNI proves it by unpacking and must report it set, or every + // consumer keyed on the flag drops the frame. + assertTrue("type 0x${"%X".format(frame!!.type)} lacks the data flag", + frame.type and 0x4 != 0) + } + private companion object { const val TAG = "LoopbackTest" } diff --git a/android/js8core-lib/src/androidTest/java/com/js8call/core/TestEngine.kt b/android/js8core-lib/src/androidTest/java/com/js8call/core/TestEngine.kt index c0892e27..7cf38852 100644 --- a/android/js8core-lib/src/androidTest/java/com/js8call/core/TestEngine.kt +++ b/android/js8core-lib/src/androidTest/java/com/js8call/core/TestEngine.kt @@ -5,8 +5,8 @@ import org.junit.Assert.assertTrue import java.util.concurrent.CopyOnWriteArrayList /** One decode as the engine reported it. */ -internal data class Decode(val text: String, val snr: Int, val dt: Float, val freq: Float) { - override fun toString() = String.format("'%s' dt=%+.2f f=%.0f snr=%d", text, dt, freq, snr) +internal data class Decode(val text: String, val snr: Int, val dt: Float, val freq: Float, val type: Int) { + override fun toString() = String.format("'%s' dt=%+.2f f=%.0f snr=%d type=0x%X", text, dt, freq, snr, type) } /** @@ -29,7 +29,7 @@ internal class TestEngine : AutoCloseable { utc: Int, snr: Int, dt: Float, freq: Float, text: String, type: Int, quality: Float, mode: Int, driftMs: Int ) { - decodes.add(Decode(text, snr, dt, freq)) + decodes.add(Decode(text, snr, dt, freq, type)) } override fun onDecodeFinished(count: Int) { @@ -85,7 +85,7 @@ internal class TestEngine : AutoCloseable { * to defer to the next boundary on its own, and returns what came off the * TX tap: at the engine rate, leading silence trimmed. */ - fun transmitFromMidPeriod(text: String): ShortArray { + fun transmitFromMidPeriod(text: String, forceData: Boolean = false): ShortArray { engine.setTransmitReady(true) val offset = System.currentTimeMillis() % PERIOD_MS engine.setTimeDriftMs((ASK_AT_MS - offset + PERIOD_MS) % PERIOD_MS) @@ -95,7 +95,8 @@ internal class TestEngine : AutoCloseable { myGrid = "EM12", submode = 0, audioFrequencyHz = 1500.0, - txDelaySec = 0.0 + txDelaySec = 0.0, + forceData = forceData ) assertTrue("transmit refused", accepted) From d0b63e6dfe62bced5c9ecdf686069ce47018959f Mon Sep 17 00:00:00 2001 From: tekstrand Date: Sat, 5 Sep 2026 19:26:19 -0500 Subject: [PATCH 3/3] Cleaned up after a review pass over the two fixes. The data flag test 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. --- adapters/android/jni/js8_engine_jni.cpp | 51 ++++++++--------- .../js8call/example/model/DecodedMessage.kt | 11 ++++ .../example/service/JS8EngineService.kt | 7 +-- .../com/js8call/example/ui/DecodeViewModel.kt | 55 ++++++++----------- .../DecodeViewModelMultipartAssemblyTest.kt | 10 +++- .../com/js8call/core/JS8EngineLoopbackTest.kt | 45 ++++++++------- 6 files changed, 92 insertions(+), 87 deletions(-) diff --git a/adapters/android/jni/js8_engine_jni.cpp b/adapters/android/jni/js8_engine_jni.cpp index 8ae7720c..0383c87d 100644 --- a/adapters/android/jni/js8_engine_jni.cpp +++ b/adapters/android/jni/js8_engine_jni.cpp @@ -231,7 +231,8 @@ static bool is_callsign_like(std::string const& token) { } // First frame of a line only; a continuation's payload is plain text. -static std::string maybe_insert_callsign_prefix(std::string const& text) { +static std::string maybe_insert_callsign_prefix(std::string const& text, bool first_frame) { + if (!first_frame) return text; std::size_t first_sep = std::string::npos; for (std::size_t i = 0; i < text.size(); ++i) { if (std::isspace(static_cast(text[i]))) { @@ -268,9 +269,14 @@ static std::string maybe_insert_callsign_prefix(std::string const& text) { return rebuilt; } -static std::string render_decoded_text(JS8Engine_Native* native, js8core::events::Decoded& decoded) { +// Renders the display text. *type gets the frame type, with the data flag +// added when only the payload carried it. +static std::string render_decoded_text(JS8Engine_Native* native, + js8core::events::Decoded const& decoded, + int* type) { using namespace js8core::protocol::varicode; + *type = decoded.type; auto const& frame = decoded.data; if (frame.size() < 12 || frame.find(' ') != std::string::npos) { return frame; @@ -286,7 +292,7 @@ static std::string render_decoded_text(JS8Engine_Native* native, js8core::events auto data = unpack_fast_data_message(frame); __android_log_print(ANDROID_LOG_DEBUG, "JS8FrameDebug", "unpack_fast_data returned: '%s'", data.c_str()); - if (!data.empty()) return is_first_frame ? maybe_insert_callsign_prefix(data) : data; + if (!data.empty()) return maybe_insert_callsign_prefix(data, is_first_frame); // Fast-data frames should not be treated as heartbeat/compound/directed. __android_log_print(ANDROID_LOG_WARN, "JS8FrameDebug", "Fast data unpack failed, returning raw frame: '%s'", frame.c_str()); @@ -298,8 +304,8 @@ static std::string render_decoded_text(JS8Engine_Native* native, js8core::events if (!data.empty()) { // Normal and Slow carry the data flag in the payload, not in the // frame type; set it so Kotlin sees one convention for all modes. - decoded.type |= 0b100; - return is_first_frame ? maybe_insert_callsign_prefix(data) : data; + *type |= 0b100; + return maybe_insert_callsign_prefix(data, is_first_frame); } } @@ -436,20 +442,21 @@ static void event_callback(JS8Engine_Native* native, js8core::events::Variant co // Handle different event types if (auto* decoded = std::get_if(&event)) { - auto enriched = *decoded; - auto rendered = render_decoded_text(native, enriched); - - auto emit_decoded = [&](js8core::events::Decoded const& d, std::string const& text_str) { - jmethodID method = env->GetMethodID(handler_class, "onDecoded", "(IIFFLjava/lang/String;IFII)V"); - if (method) { - jstring text = env->NewStringUTF(text_str.c_str()); - env->CallVoidMethod(native->callback_handler, method, - d.utc, d.snr, d.xdt, d.frequency, - text, d.type, d.quality, d.mode, d.drift_ms); - env->DeleteLocalRef(text); - } - }; - emit_decoded(enriched, rendered); + int type = 0; + auto rendered = render_decoded_text(native, *decoded, &type); + __android_log_print(ANDROID_LOG_INFO, "JS8Engine_Native", + "DECODED: SNR=%d dB, freq=%.1f Hz, text='%s', raw='%s', type=%d, mode=%d", + decoded->snr, decoded->frequency, rendered.c_str(), + decoded->data.c_str(), type, decoded->mode); + + jmethodID method = env->GetMethodID(handler_class, "onDecoded", "(IIFFLjava/lang/String;IFII)V"); + if (method) { + jstring text = env->NewStringUTF(rendered.c_str()); + env->CallVoidMethod(native->callback_handler, method, + decoded->utc, decoded->snr, decoded->xdt, decoded->frequency, + text, type, decoded->quality, decoded->mode, decoded->drift_ms); + env->DeleteLocalRef(text); + } } else if (auto* spectrum = std::get_if(&event)) { // Call onSpectrum(float[] bins, float binHz, float powerDb, float peakDb) jmethodID method = env->GetMethodID(handler_class, "onSpectrum", "([FFFF)V"); @@ -599,12 +606,6 @@ JS8Engine_Native* js8_engine_create(JNIEnv* env, jobject callback_handler, auto const& e = std::get(event); __android_log_print(ANDROID_LOG_DEBUG, "JS8Engine_Native", "DecodeFinished: count=%zu", e.decoded); - } else if (std::holds_alternative(event)) { - auto e = std::get(event); - auto rendered = render_decoded_text(native, e); - __android_log_print(ANDROID_LOG_INFO, "JS8Engine_Native", - "DECODED: SNR=%d dB, freq=%.1f Hz, text='%s', raw='%s', type=%d, mode=%d", - e.snr, e.frequency, rendered.c_str(), e.data.c_str(), e.type, e.mode); } event_callback(native, event); }; diff --git a/android/app/src/main/java/com/js8call/example/model/DecodedMessage.kt b/android/app/src/main/java/com/js8call/example/model/DecodedMessage.kt index a2f7ca8f..3588d60b 100644 --- a/android/app/src/main/java/com/js8call/example/model/DecodedMessage.kt +++ b/android/app/src/main/java/com/js8call/example/model/DecodedMessage.kt @@ -36,6 +36,12 @@ data class DecodedMessage( */ fun isSingleFrame(): Boolean = isFirstFrame() && isLastFrame() + /** + * Check if this frame carries a data payload. + * JS8CallData flag is bit 2 (type & 4). + */ + fun isDataFrame(): Boolean = isDataFrame(type) + /** * Get color resource ID based on SNR level. */ @@ -82,6 +88,11 @@ data class DecodedMessage( localCal.get(java.util.Calendar.SECOND) ) } + + companion object { + /** The same data-flag test for callers holding only the type bits. */ + fun isDataFrame(type: Int): Boolean = (type and 4) != 0 + } } /** diff --git a/android/app/src/main/java/com/js8call/example/service/JS8EngineService.kt b/android/app/src/main/java/com/js8call/example/service/JS8EngineService.kt index e2b09102..d56ee9bc 100644 --- a/android/app/src/main/java/com/js8call/example/service/JS8EngineService.kt +++ b/android/app/src/main/java/com/js8call/example/service/JS8EngineService.kt @@ -31,6 +31,7 @@ import com.js8call.core.TruSdxDirectSerial import com.js8call.core.UsbSerialBridge import com.js8call.core.UsbSerialPortCatalog import com.js8call.example.MainActivity +import com.js8call.example.model.DecodedMessage import com.js8call.example.ui.AudioDevices import com.js8call.example.MessageLogWriter import com.js8call.example.R @@ -2993,7 +2994,7 @@ class JS8EngineService : Service() { } // Not a directed command - check if it's a data frame for a buffered MSG - if (!isDataFrame(type)) { + if (!DecodedMessage.isDataFrame(type)) { return } @@ -3071,7 +3072,6 @@ class JS8EngineService : Service() { } } - private fun isDataFrame(type: Int): Boolean = (type and 0x4) != 0 private fun isSubscribedGroup(target: String): Boolean { if (!target.startsWith("@")) return false @@ -3196,7 +3196,7 @@ class JS8EngineService : Service() { return } - if (!isRelayDataFrame(type)) return + if (!DecodedMessage.isDataFrame(type)) return val result = synchronized(relayLock) { val key = findMatchingRelayBufferKey(freq) ?: return@synchronized null @@ -3443,7 +3443,6 @@ class JS8EngineService : Service() { return target.contains("@") } - private fun isRelayDataFrame(type: Int): Boolean = (type and 0x4) != 0 private fun isLastFrame(type: Int): Boolean = (type and 0x2) != 0 diff --git a/android/app/src/main/java/com/js8call/example/ui/DecodeViewModel.kt b/android/app/src/main/java/com/js8call/example/ui/DecodeViewModel.kt index aef1e554..93c4deae 100644 --- a/android/app/src/main/java/com/js8call/example/ui/DecodeViewModel.kt +++ b/android/app/src/main/java/com/js8call/example/ui/DecodeViewModel.kt @@ -14,27 +14,21 @@ import kotlin.math.roundToInt import org.json.JSONArray import org.json.JSONObject -/** A decoded frame reaching the list: its rendered text and JS8 type bits. */ -internal data class DecodeFrame(val text: String, val type: Int) - -private fun isDataFrame(type: Int): Boolean = (type and 0x4) != 0 - -internal fun assembleMultipartDecodeText(frames: List): String = buildString { - frames.forEachIndexed { index, frame -> - if (index > 0 && needsSpaceBefore(this, frames[index - 1].type, frame.text)) { - append(' ') +internal fun assembleMultipartDecodeText(frames: List): String { + val parts = frames.filter { it.text.isNotEmpty() } + return buildString { + parts.forEachIndexed { index, frame -> + if (index > 0 && needsSpaceBefore(parts[index - 1], frame)) append(' ') + append(frame.text) } - append(frame.text) } } -// Data frames split mid-word and carry their own spaces; only a directed header -// can leave a flush boundary that needs one. -private fun needsSpaceBefore(soFar: CharSequence, prevType: Int, next: String): Boolean { - if (isDataFrame(prevType)) return false - if (soFar.isEmpty() || next.isEmpty()) return false - return !soFar.last().isWhitespace() && !next.first().isWhitespace() -} +// Data frames split mid-word and carry their own spaces. A directed header +// can meet its payload flush: a buffered command strips the separator +// before packing, so the boundary needs the space put back. +private fun needsSpaceBefore(prev: DecodedMessage, next: DecodedMessage): Boolean = + !prev.isDataFrame() && !prev.text.last().isWhitespace() && !next.text.first().isWhitespace() /** * ViewModel for the Decodes screen. @@ -224,10 +218,8 @@ class DecodeViewModel(application: Application) : AndroidViewModel(application) * Assemble a complete message from buffered frames. */ private fun assembleMessage(buffer: MessageBuffer): DecodedMessage { - val frameTexts = buffer.frames.map { it.text }.toMutableList() - normalizeCompoundDirectedHelpers(buffer.frames, frameTexts) - - val frames = buffer.frames.zip(frameTexts) { frame, text -> DecodeFrame(text, frame.type) } + val frames = buffer.frames.toMutableList() + normalizeCompoundDirectedHelpers(frames) val assembledText = assembleMultipartDecodeText(frames) // Use the last frame's metadata (most recent) @@ -247,20 +239,17 @@ class DecodeViewModel(application: Application) : AndroidViewModel(application) ) } - private fun normalizeCompoundDirectedHelpers( - frames: List, - frameTexts: MutableList - ) { - if (frameTexts.size < 2 || frames.size < 2) return + private fun normalizeCompoundDirectedHelpers(frames: MutableList) { + if (frames.size < 2) return val firstFrame = frames[0] val secondFrame = frames[1] - if (isDataFrame(firstFrame.type) || isDataFrame(secondFrame.type)) return + if (firstFrame.isDataFrame() || secondFrame.isDataFrame()) return if (!firstFrame.isFirstFrame() || firstFrame.isLastFrame()) return if (secondFrame.isFirstFrame()) return - val first = frameTexts[0].trim() - val second = frameTexts[1].trim() + val first = firstFrame.text.trim() + val second = secondFrame.text.trim() if (!isCompoundDeHelperFrame(first)) return val fromCall = first.substringBefore(' ').trim() @@ -268,14 +257,14 @@ class DecodeViewModel(application: Application) : AndroidViewModel(application) val rewrittenDirected = rewriteDirectedPlaceholder(second, fromCall) if (rewrittenDirected != null) { - frameTexts[0] = "" - frameTexts[1] = rewrittenDirected + frames[0] = firstFrame.copy(text = "") + frames[1] = secondFrame.copy(text = rewrittenDirected) return } if (isDirectedCompoundHeader(second)) { - frameTexts[0] = "" - frameTexts[1] = "$fromCall: $second" + frames[0] = firstFrame.copy(text = "") + frames[1] = secondFrame.copy(text = "$fromCall: $second") } } diff --git a/android/app/src/test/java/com/js8call/example/ui/DecodeViewModelMultipartAssemblyTest.kt b/android/app/src/test/java/com/js8call/example/ui/DecodeViewModelMultipartAssemblyTest.kt index f255bee3..773bb834 100644 --- a/android/app/src/test/java/com/js8call/example/ui/DecodeViewModelMultipartAssemblyTest.kt +++ b/android/app/src/test/java/com/js8call/example/ui/DecodeViewModelMultipartAssemblyTest.kt @@ -1,12 +1,18 @@ package com.js8call.example.ui +import com.js8call.example.model.DecodedMessage import org.junit.Assert.assertEquals import org.junit.Test class DecodeViewModelMultipartAssemblyTest { // JS8 frame type bits: 0x1 first, 0x2 last, 0x4 data. - private fun directed(text: String) = DecodeFrame(text, 0x1) - private fun data(text: String) = DecodeFrame(text, 0x4) + private fun frame(text: String, type: Int) = DecodedMessage( + utc = 0, snr = 0, dt = 0f, frequency = 0f, text = text, + type = type, quality = 0f, mode = 0 + ) + + private fun directed(text: String) = frame(text, 0x1) + private fun data(text: String) = frame(text, 0x4) @Test fun dataFramesConcatenateWithoutASpace() { diff --git a/android/js8core-lib/src/androidTest/java/com/js8call/core/JS8EngineLoopbackTest.kt b/android/js8core-lib/src/androidTest/java/com/js8call/core/JS8EngineLoopbackTest.kt index 58681efe..554a8c6c 100644 --- a/android/js8core-lib/src/androidTest/java/com/js8call/core/JS8EngineLoopbackTest.kt +++ b/android/js8core-lib/src/androidTest/java/com/js8call/core/JS8EngineLoopbackTest.kt @@ -2,7 +2,6 @@ package com.js8call.core import android.util.Log import androidx.test.ext.junit.runners.AndroidJUnit4 -import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith @@ -22,40 +21,40 @@ class JS8EngineLoopbackTest { @Test fun ownTransmissionDecodes() { - val tx = TestEngine().use { it.transmitFromMidPeriod("CQ CQ CQ") } - - // A whole frame is 12.6 s. Short means the modulator joined one in - // progress instead of waiting for the boundary. - assertTrue("captured ${tx.seconds}s, expected a whole frame", tx.seconds > 10) - - // Submode A starts 500 ms into its 15 s period. The noise gives the - // decoder a floor to measure against instead of digital silence. - val period = tx.placedInPeriod(periodMs = 15_000, startDelayMs = 500) - .withNoise(rms = 300.0, seed = 1) - - val decoded = TestEngine().use { it.decode(period) } - Log.i(TAG, "decoded ${decoded.size}: $decoded") + val decoded = loopback("CQ CQ CQ") assertTrue("own transmission did not decode", decoded.any { "CQ" in it.text }) } @Test fun dataFrameReportsTheDataType() { - val tx = TestEngine().use { it.transmitFromMidPeriod("HELLO", forceData = true) } + val decoded = loopback("HELLO", forceData = true) + val frame = requireNotNull(decoded.firstOrNull { "HELLO" in it.text }) { + "data frame did not decode" + } + + // Normal mode leaves the data flag out of the transmitted frame type; + // the JNI proves it by unpacking and must report it set, or every + // consumer keyed on the flag drops the frame. + assertTrue("type 0x${"%X".format(frame.type)} lacks the data flag", + frame.type and 0x4 != 0) + } + + /** Transmits [text], places the capture in a noisy period and decodes it. */ + private fun loopback(text: String, forceData: Boolean = false): List { + val tx = TestEngine().use { it.transmitFromMidPeriod(text, forceData) } + + // A whole frame is 12.6 s. Short means the modulator joined one in + // progress instead of waiting for the boundary. assertTrue("captured ${tx.seconds}s, expected a whole frame", tx.seconds > 10) + // Submode A starts 500 ms into its 15 s period. The noise gives the + // decoder a floor to measure against instead of digital silence. val period = tx.placedInPeriod(periodMs = 15_000, startDelayMs = 500) .withNoise(rms = 300.0, seed = 1) val decoded = TestEngine().use { it.decode(period) } Log.i(TAG, "decoded ${decoded.size}: $decoded") - val frame = decoded.firstOrNull { "HELLO" in it.text } - assertNotNull("data frame did not decode", frame) - - // Normal mode leaves the data flag out of the transmitted frame type; - // the JNI proves it by unpacking and must report it set, or every - // consumer keyed on the flag drops the frame. - assertTrue("type 0x${"%X".format(frame!!.type)} lacks the data flag", - frame.type and 0x4 != 0) + return decoded } private companion object {