Skip to content

Test Garmin recognition through the production catalog bridge - #2771

Merged
ryanbr merged 1 commit into
ryanbr:mainfrom
kavemang:agent/garmin-production-recognition-tests-2708
Oct 15, 2026
Merged

ryanbr merged 1 commit into
ryanbr:mainfrom
kavemang:agent/garmin-production-recognition-tests-2708

Conversation

@kavemang

@kavemang kavemang commented Oct 11, 2026 •

Copy link
Copy Markdown

What this PR does

Garmin recognition assertions currently call GarminBroadcast.isGarmin, a wrapper with no production callers on either platform. Device discovery uses ExperimentalBrand.recognise and its DeviceBrandCatalog bridge instead.

Remove both unused wrappers and test the production recognizer directly. Matching Swift/Kotlin cases cover all nine Garmin name tokens, uppercase and accented model names, standard-HR routing, registry prefix, live-HR capability, and generic/unknown-name boundaries. Retain the wizard's broadcast guidance and its test. Runtime recognition, catalog values and BLE behavior are unchanged.

Type of change

  • Refactor / cleanup
  • CI / tooling (regression tests)

How it was tested

  • Linux Swift harness: 13 committed pure ExperimentalDriversTests methods passed against the actual catalog, ExperimentalBrand, GarminBroadcast and Huami parser. Compiled the production SourceKind declaration separately; excluded framework/store-dependent source-coordinator methods. This does not validate Combine, CoreBluetooth, the full store or Apple app targets.
  • Kotlin/JUnit harness: 14 committed pure methods passed using the actual catalog, recognizer and parser, the production SourceKind declaration and pinned Kotlin 1.9.24/JDK 17. Source-coordinator methods were excluded; this is not a full Android app build or unit suite.
  • Private wrong-brand mutations failed the recognition assertions on both platforms; no mutations or harness artifacts are committed.
  • Core Tools suite: 153 tests passed. Source-comment hygiene, whitespace and diff-scoped i18n checks passed.
  • Native CI at unchanged head 0cf667961c18a4342cc3832c975907a4209882e3: all six expected checks passed, with a stable complete roster confirmed twice.
  • Android CI: full app/KSP/Kotlin/APK build passed (2m 58s), followed by full testFullDebugUnitTest (1m 9s).
  • Apple CI: universal macOS Strand build and iOS NOOPiOS build passed. macOS StrandTests: 2,280 tests, 2 skipped, zero failures; all 16 ExperimentalDriversTests passed, including the retargeted Garmin recognition, new other-brand boundaries and broadcast-guidance test. iOS is compile-only; its test step was skipped by workflow design.
  • Tools CI: 234 capture, 50 parity acceptance, 2 R-R preservation and 153 core tests passed. Hygiene and i18n checks also passed.
  • No warnings naming either changed production/test file in Android/macOS/iOS logs. Apple merge-current-main steps were skipped, so this validates the actual PR head rather than a hypothetical merge with later main.
  • No radio/strap hardware validation performed. This removes unused helpers and changes tests; no BLE connection path, commands, payloads or writes change.

Checklist

  • Native Android full unit suite and Apple app tests verified at PR head
  • No new warnings naming changed files in native build logs
  • No UI styling changes
  • No hardcoded hex frame bytes added
  • Follows docs/CONTRIBUTING.md; both platforms changed together
  • No generated output, private notes, identifiers or secrets committed

Related issues

Refs #2708

@kavemang
kavemang marked this pull request as ready for review October 11, 2026 04:54
@ryanbr

ryanbr commented Oct 15, 2026

Copy link
Copy Markdown
Owner

Reviewed and verified locally. Green at 0cf6679, 6/6.

GarminBroadcast.isGarmin had no production caller on either platform (the isGarmin at AddDeviceWizard.kt:280 is an unrelated local val), so both sides of the one-line delegate come out together cleanly, and moving the recognition note into the ExperimentalBrand sentence keeps the doc honest about where it now lives.

The replacement tests are a good deal stronger than what they replace. Three names became eleven, and the model-only ones carry the weight: Venu 3, Instinct 2, epix Pro, vivosmart 5 and HRM-Pro all have to be recognised without the Garmin token in front of them, which the old three-name test never asked for. Asserting sourceKind, idPrefix and canStreamLiveHR alongside the brand is the part that matters most, since recognition without the liveBLE routing would leave the watch discovered and unreadable.

garminRecognitionDoesNotCaptureOtherBrands is the half that was missing entirely. Amazfit still resolving to its own brand is what stops a widened Garmin matcher quietly eating it, and the blank and whitespace cases pin the advertised-name path against a device that advertises no name at all.

No parity ledger update is needed here, since Strand/BLE sits outside Swift ledger scope, and that matches the files touched.

Good to merge.

@ryanbr
ryanbr merged commit 509ad80 into ryanbr:main Oct 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants