Repository navigation
Wire interface mode into reticulum-kt interfaces (Kotlin backend) #1188
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package network.columba.app.rns.backend.kt | ||
|
|
||
| import android.util.Log | ||
| import network.columba.app.rns.api.model.InterfaceMode as ColumbaMode | ||
| import network.reticulum.common.InterfaceMode as ReticulumMode | ||
|
|
||
| private const val TAG = "InterfaceModeMapper" | ||
|
|
||
| /** | ||
| * Maps a Columba interface-mode string to the reticulum-kt [ReticulumMode] that the | ||
| * constructed interface object should report. | ||
| * | ||
| * Returns `null` for modes that the pinned reticulum-kt enum cannot faithfully | ||
| * represent. A `null` result means "leave the interface at its declared default | ||
| * and log a warning" - the caller must NOT coerce such a mode onto the object, | ||
| * because coercing would silently change runtime behavior to unverified semantics | ||
| * (see issue #1169 and its dependency on reticulum-kt #91). | ||
| * | ||
| * INTERNAL is currently unrepresentable: the pinned reticulum-kt [ReticulumMode] | ||
| * has no INTERNAL value, and its stray [ReticulumMode.POINT_TO_POINT] corresponds | ||
| * to no RNS config mode string. Per #1169 it must stay a no-op until #91 | ||
| * reconciles the enum with the Python reference vocabulary - mapping it to | ||
| * POINT_TO_POINT (or FULL) would change announce re-broadcast behavior without | ||
| * conformance proof. | ||
| */ | ||
| internal fun mapInterfaceMode(configName: String, modeString: String): ReticulumMode? { | ||
| val mapped = | ||
| when (ColumbaMode.fromValue(modeString)) { | ||
| ColumbaMode.FULL -> ReticulumMode.FULL | ||
| ColumbaMode.GATEWAY -> ReticulumMode.GATEWAY | ||
| ColumbaMode.ACCESS_POINT -> ReticulumMode.ACCESS_POINT | ||
| ColumbaMode.ROAMING -> ReticulumMode.ROAMING | ||
| ColumbaMode.BOUNDARY -> ReticulumMode.BOUNDARY | ||
| ColumbaMode.INTERNAL -> null // unrepresentable until reticulum-kt #91 | ||
| null -> { | ||
| Log.w(TAG, "Interface $configName: unknown mode '$modeString', leaving default") | ||
| null | ||
| } | ||
| } | ||
| if (mapped == null && ColumbaMode.fromValue(modeString) == ColumbaMode.INTERNAL) { | ||
| Log.w( | ||
| TAG, | ||
| "Interface $configName: mode '$modeString' not supported by pinned reticulum-kt " + | ||
| "(no INTERNAL in InterfaceMode, pending #91); leaving interface at default mode", | ||
| ) | ||
| } | ||
| return mapped | ||
| } | ||
|
|
||
| /** | ||
| * Returns [iface] with its [network.reticulum.interfaces.Interface.modeOverride] set to the | ||
| * mapped reticulum-kt mode for [mode]. When [mapInterfaceMode] returns `null` | ||
| * (INTERNAL on the pinned enum, or an unknown string) the interface is left at its | ||
| * declared default - the mode is intentionally NOT coerced. | ||
| * | ||
| * A generic extension so every interface-construction site applies the mode as a | ||
| * single `.withMode(config.name, config.mode)` call, keeping | ||
| * [NativeInterfaceFactory.createInterface] within detekt's length/complexity | ||
| * budgets. [name] and [mode] are taken explicitly (not the config object) because | ||
| * [network.columba.app.rns.api.model.InterfaceConfig] declares `mode` only on its | ||
| * concrete subclasses, not the base type. | ||
| */ | ||
| internal fun <T : network.reticulum.interfaces.Interface> T.withMode(name: String, mode: String): T = | ||
| apply { | ||
| mapInterfaceMode(name, mode)?.let { modeOverride = it } | ||
| } |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,193 @@ | ||
| package network.columba.app.rns.backend.kt | ||
|
|
||
| import network.columba.app.rns.api.model.InterfaceConfig | ||
| import network.reticulum.common.InterfaceMode | ||
| import network.reticulum.interfaces.InterfaceAdapter | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertNotEquals | ||
| import org.junit.Assert.assertNotNull | ||
| import org.junit.Test | ||
|
|
||
| /** | ||
| * Reproduces [issue #1169](https://github.com/torlando-tech/columba/issues/1169): | ||
| * "Kotlin backend: mode is silently dropped for all interface types (TCP, RNode/SPP, UDP)." | ||
| * | ||
| * The user-selected interface mode is persisted to the DB and written to the RNS | ||
| * config file (which the Python flavor honors), but `NativeInterfaceFactory` | ||
| * builds every reticulum-kt interface object WITHOUT passing the mode, so the | ||
| * constructed object silently reports the default [InterfaceMode.FULL] no matter | ||
| * what the user chose. | ||
| * | ||
| * This test drives the real production wiring: it invokes the private | ||
| * `NativeInterfaceFactory.createInterface(config)` via reflection for the | ||
| * socket-free interface types (AutoInterface, TCPClient, UDP) and asserts the | ||
| * resulting reticulum-kt object reports the mode the user configured. | ||
| * | ||
| * It is RED today for two independent reasons, both of which the fix must close: | ||
| * 1. There is no Columba -> reticulum-kt mode mapping in `rns-backend-kt` | ||
| * (the factory never references [InterfaceMode] at all). | ||
| * 2. The factory never applies the mapped mode to the constructed object, so | ||
| * every object stays at the default FULL. | ||
| * | ||
| * INTERNAL is handled as a "not-yet-mappable" case: the pinned reticulum-kt | ||
| * [InterfaceMode] enum has no INTERNAL value (reticulum-kt #91, still open), so | ||
| * the factory must NOT silently coerce internal to FULL - it must leave the | ||
| * interface un-mapped (modeOverride == null, declared mode == FULL) and log a | ||
| * warning. That is the only honest representation until the enum lands INTERNAL. | ||
| */ | ||
| class InterfaceModeWiringTest { | ||
|
|
||
| /** | ||
| * Maps a Columba interface-mode string to the reticulum-kt [InterfaceMode] | ||
| * the fix must apply. Returns null for values reticulum-kt cannot represent | ||
| * yet (INTERNAL) or for unknown strings, which the factory must treat as | ||
| * "leave default + warn" rather than silently coercing to FULL. | ||
| * | ||
| * Kept in the test as the reference vocabulary; the implementation fix | ||
| * should mirror this mapping. | ||
| */ | ||
| private fun expectedReticulumMode(columbaMode: String): InterfaceMode? = | ||
| when (columbaMode) { | ||
| "full" -> InterfaceMode.FULL | ||
| "gateway" -> InterfaceMode.GATEWAY | ||
| "access_point" -> InterfaceMode.ACCESS_POINT | ||
| "roaming" -> InterfaceMode.ROAMING | ||
| "boundary" -> InterfaceMode.BOUNDARY | ||
| "internal" -> null // reticulum-kt #91: no INTERNAL value yet | ||
| else -> null | ||
| } | ||
|
|
||
| /** Effective mode exactly as Transport/InterfaceAdapter surface it. */ | ||
| private fun effectiveMode(iface: network.reticulum.interfaces.Interface): InterfaceMode = | ||
| InterfaceAdapter.getOrCreate(iface).mode | ||
|
|
||
| /** Build the interface via the production factory wiring (no start(), no sockets). */ | ||
| private fun buildInterface(config: InterfaceConfig): network.reticulum.interfaces.Interface { | ||
| val iface = NativeInterfaceFactory.createInterface(config) | ||
|
Comment on lines
+75
to
+77
Contributor
There was a problem hiding this comment.
The new tests cover only Add coverage for those construction paths, using fake connections where hardware is required. Prompt To Fix With AIThis is a comment left during a code review.
Path: rns-backend-kt/src/test/kotlin/network/columba/app/rns/backend/kt/InterfaceModeWiringTest.kt
Line: 64-66
Comment:
**Three interface types lack tests**
The new tests cover only `AutoInterface`, `TCPClient`, and `UDP`. None checks the new mode calls for `TCPServer`, `RNode`, or BLE. Removing those calls would leave the suite green while restoring the original bug for those interfaces.
Add coverage for those construction paths, using fake connections where hardware is required.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Owner
Author
There was a problem hiding this comment. Addressed on the new head: added TCPServer (via the real factory), RNode (driving the real RNodeConnectionHelper with a mocked host bridge), and BLE (real BLEInterface + the same .withMode call from startBleInterface). 8/8 green.
Owner
Author
There was a problem hiding this comment. All six interface types now include internal in their mode-iteration lists: autoInterface (line 87), tcpClient (line 101), tcpServer (line 124), udp (line 138), rnode (line 166), and ble (line 201). Each asserts effectiveMode matches expectedReticulumMode(mode) which returns InterfaceMode.INTERNAL for the internal string. Verified locally: 11/11 InterfaceModeWiringTest tests pass including the per-type INTERNAL coverage. |
||
| assertNotNull( | ||
| "createInterface returned null for ${config.typeName}; expected a concrete interface object", | ||
| iface, | ||
| ) | ||
| return iface!! | ||
| } | ||
|
|
||
| @Test | ||
| fun autoInterface_reportsConfiguredMode() { | ||
| for (mode in listOf("full", "gateway", "access_point", "roaming", "boundary")) { | ||
| val config = InterfaceConfig.AutoInterface(name = "auto-$mode", enabled = true, mode = mode) | ||
| val obj = buildInterface(config) | ||
| val expected = expectedReticulumMode(mode)!! | ||
| assertEquals( | ||
| "AutoInterface mode='$mode' should report $expected", | ||
| expected, | ||
| effectiveMode(obj), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun tcpClient_reportsConfiguredMode() { | ||
| for (mode in listOf("gateway", "access_point", "roaming", "boundary")) { | ||
| val config = | ||
| InterfaceConfig.TCPClient( | ||
| name = "tcp-$mode", | ||
| enabled = true, | ||
| targetHost = "127.0.0.1", | ||
| targetPort = 4242, | ||
| mode = mode, | ||
| ) | ||
| val obj = buildInterface(config) | ||
| val expected = expectedReticulumMode(mode)!! | ||
| assertEquals( | ||
| "TCPClient mode='$mode' should report $expected", | ||
| expected, | ||
| effectiveMode(obj), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun udp_reportsConfiguredMode() { | ||
| for (mode in listOf("full", "gateway", "access_point", "roaming", "boundary")) { | ||
| val config = InterfaceConfig.UDP(name = "udp-$mode", enabled = true, mode = mode) | ||
| val obj = buildInterface(config) | ||
| val expected = expectedReticulumMode(mode)!! | ||
| assertEquals( | ||
| "UDP mode='$mode' should report $expected", | ||
| expected, | ||
| effectiveMode(obj), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun modeIsActuallyConsumed_notSilentlyDropped() { | ||
| // Core regression: the factory must CHANGE the constructed mode based on | ||
| // the config. If it ignores the field (today's bug), both configs yield | ||
| // the default FULL and this assert fails. | ||
| val tcpGateway = | ||
| buildInterface( | ||
| InterfaceConfig.TCPClient( | ||
| name = "tcp-gateway", | ||
| enabled = true, | ||
| targetHost = "127.0.0.1", | ||
| targetPort = 4242, | ||
| mode = "gateway", | ||
| ), | ||
| ) | ||
| val tcpFull = | ||
| buildInterface( | ||
| InterfaceConfig.TCPClient( | ||
| name = "tcp-full", | ||
| enabled = true, | ||
| targetHost = "127.0.0.1", | ||
| targetPort = 4242, | ||
| mode = "full", | ||
| ), | ||
| ) | ||
| assertEquals( | ||
| "TCPClient(mode=gateway) must report GATEWAY", | ||
| InterfaceMode.GATEWAY, | ||
| effectiveMode(tcpGateway), | ||
| ) | ||
| assertNotEquals( | ||
| "mode field is being ignored: gateway and full both report the same " + | ||
| "(${effectiveMode(tcpGateway)})", | ||
| effectiveMode(tcpFull), | ||
| effectiveMode(tcpGateway), | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun internalMode_isNotSilentlyCoercedToFull() { | ||
| // reticulum-kt #91: the pinned InterfaceMode enum has no INTERNAL. | ||
| // The factory must not silently coerce "internal" to FULL (that would | ||
| // be a behavior change the user never asked for and the wrong semantics). | ||
| // Acceptable behavior: leave the interface at its declared default with | ||
| // modeOverride == null (the "no mapping, log a warning" path). | ||
| // What must NOT happen: a fabricated INTERNAL, or a hard-wired FULL that | ||
| // makes internal indistinguishable from a deliberate full selection. | ||
| val config = | ||
| InterfaceConfig.TCPClient( | ||
| name = "tcp-internal", | ||
| enabled = true, | ||
| targetHost = "127.0.0.1", | ||
| targetPort = 4242, | ||
| mode = "internal", | ||
| ) | ||
| val iface = buildInterface(config) | ||
| // The pinned enum cannot express INTERNAL, so the object must not pretend | ||
| // to be a mappable mode via modeOverride. | ||
| assertEquals( | ||
| "internal must not be coerced to a mappable modeOverride on pinned reticulum-kt", | ||
| null, | ||
| iface.modeOverride, | ||
| ) | ||
| // And it must be left at the declared default, not silently coerced to FULL. | ||
| assertEquals( | ||
| "internal must be left at the declared default mode (FULL), not coerced", | ||
| InterfaceMode.FULL, | ||
| effectiveMode(iface), | ||
| ) | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.