Skip to content

fix(takserver): route TAK self-test through the real v1/v2 dispatch path - #6746

Open
jamesarich wants to merge 2 commits into
mainfrom
fix/6583-tak-self-test-v1-path
Open

fix(takserver): route TAK self-test through the real v1/v2 dispatch path#6746
jamesarich wants to merge 2 commits into
mainfrom
fix/6583-tak-self-test-v1-path

Conversation

@jamesarich

Copy link
Copy Markdown
Collaborator

Summary

Relates to #6583 — fixes the self-test's blind spot to firmware-version-gated dispatch; does not itself resolve the reported ATAK marker/chat loss.

The debug "TAK Mesh Test" self-test (Settings → TAK Server, debug builds only) called TakSdkCompressor.compressCoT() directly and always sent fixtures on ATAK_PLUGIN_V2, hardcoding the v2 (zstd/full-CoT) pipeline. It never went through TAKMeshIntegration.useTakV2(), the firmware-version gate (Capabilities(fw).supportsTakV2, requires firmware >= 2.8.0) that real ATAK traffic is routed through. On a radio running firmware < 2.8.0, real traffic silently falls back to the much more limited legacy v1 TAKPacket schema (PLI + chat only, everything else dropped) — but the self-test kept reporting 41/41 regardless of which path a connected radio would actually take. That false-confidence gap is what this PR closes.

Changes

  • TAKMeshIntegration: added sendCoTToMeshForTest(cotMessage, forceV2), an internal test-only seam that calls the same production sendCoTToMeshV1/sendCoTToMeshV2 functions with the protocol forced explicitly instead of read from the connected radio's firmware. This is a deliberate design choice — useTakV2() reads live radio state, which the self-test needs to bypass in order to exercise both paths deterministically in one run regardless of which firmware happens to be connected. It's the only new call site of sendCoTToMeshV1/V2 outside the real dispatch flow, and it's gated the same way the rest of the card is (buildConfig.isDebug).
  • TakSendOutcome.Dropped gained a schemaLimited: Boolean field so callers can tell a real MTU oversize drop apart from an intentional, permanent schema-coverage gap (e.g. v1's TAKPacket having no field for a marker/shape type at all). Previously both were reported as an opaque "dropped".
  • TakMeshTestRunner now runs every fixture through both the v2 and v1 dispatch paths (TakProtocol.V2 then TakProtocol.V1), calling the real TAKMeshIntegration via sendCoTToMeshForTest rather than the SDK compressor directly. A dropped v1 result is labeled expectedDrop only when schemaLimited is true — an oversize drop is a real problem and is surfaced as a failure even on v1, not lumped in with the known "v1 doesn't support markers/shapes" limitation.
  • UI (TAKConfigItemList.kt): the results card now shows a separate section per protocol (v2 / v1) with its own pass / expected-drop / failed counts, so a low v1 pass rate reads as the documented legacy-schema limitation rather than a self-test regression.
  • Added regression tests: TakMeshTestRunnerTest (5 tests) verifies both protocols run for every fixture, that a v1 pass sends on the legacy port and a v2 pass sends on the v2 port regardless of the connected radio's reported firmware, and that v1's expected drops are labeled correctly. TAKMeshIntegrationTest gained 2 tests directly exercising the schemaLimited discrimination: an unsupported CoT type is schema-limited, but an oversize-yet-representable PLI is not (guards against the oversize-drop-mislabeled-as-expected class of bug).

Operator-facing note

The self-test now sends each fixture twice (once per protocol) instead of once, roughly doubling total airtime for a debug user who taps "Run" with a radio connected — paced by the existing 5s inter-send delay on successful sends only (dropped fixtures never reach the radio, so they don't incur the delay).

Test plan

  • spotlessApply spotlessCheck detekt assembleDebug test allTests — full repo-mandated baseline, green (see below)
  • TakMeshTestRunnerTest: 5/5 passing on both jvmTest and testAndroidHostTest targets
  • TAKMeshIntegrationTest: 14/14 passing (12 existing + 2 new) on both targets
  • Manual verification against a real radio on firmware < 2.8.0 (not done here — no such radio available in this environment)

🤖 Generated with Claude Code

The debug "TAK Mesh Test" self-test called TakSdkCompressor.compressCoT()
directly and always sent on ATAK_PLUGIN_V2, bypassing the firmware-version
gate (useTakV2()) that real ATAK traffic goes through. On a radio running
firmware < 2.8.0, real traffic silently falls back to the much more
limited legacy v1 schema (PLI + chat only), but the self-test kept
reporting 41/41 regardless — a false-confidence blind spot surfaced while
investigating #6583.

TakMeshTestRunner now dispatches every fixture through
TAKMeshIntegration.sendCoTToMeshForTest(), a test-only seam that calls the
same production sendCoTToMeshV1/V2 functions with the protocol forced
explicitly, and runs each fixture through both paths so the self-test
reflects what either firmware generation actually does. TakSendOutcome
gained a Dropped.schemaLimited flag so the self-test can tell a real MTU
oversize drop apart from an intentional v1 schema-coverage gap, instead of
inferring "expected" purely from which protocol handled the fixture.
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

An error occurred during the review process. Please try again later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bugfix PR tag label Aug 16, 2026
@github-actions

This comment has been minimized.

…yout

The TakMeshTestCardContent composable was intentionally restructured into
separate V2 and V1 result sections with new headers and dividers; the
reference PNGs for the Results and Running states (Light + Dark) were
never regenerated, causing 4 screenshot test failures in CI.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jamesarich

Copy link
Copy Markdown
Collaborator Author

Follow-up commit ef6f753: regenerated the 4 stale screenshot goldens (ScreenshotTakTestCardResults/Running, Light + Dark). The TAK test card was intentionally restructured into two sections (V2 and V1 results, with new headers and dividers), but the reference PNGs were never updated, which is why CI's screenshot validation failed. Only those 4 PNGs changed; validateDebugScreenshotTest plus spotless/detekt/assembleDebug/test/allTests all pass locally, so CI should now be green.

@jamesarich
jamesarich marked this pull request as ready for review August 17, 2026 13:57
@jamesarich

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor
❌ Action failed

Review failed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@jamesarich

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor
❌ Action failed

Review failed.

@jamesarich

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor
❌ Action failed

Review failed.

@jamesarich

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor
❌ Action failed

Review failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix PR tag

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant