fix(ring): restore the Android manufacturer-data match; fall back off 0x1E on RT-series firmware

AGENTS.md

@@ -12,6 +12,28 @@ root) and match its actual behavior. Do not port iOS's CoreBluetooth sequencing
 Bluetooth stack behaves differently (pairing/bonding flow, MTU negotiation, background
 restrictions), and a straight iOS port has caused real pairing/data-collection bugs before.
 
+## Manufacturer data: Android splits the field, iOS doesn't — put the company ID back
+
+`ScanRecord.getManufacturerSpecificData()` returns a `SparseArray` **keyed by company ID, with the
+ID removed from the value**. CoreBluetooth hands iOS the raw block, company ID and all. Every
+coordinator was ported from Swift and therefore matches a little-endian company-ID *prefix*
+(`TK5Coordinator` `10786501`, `ColmiSmartHealthCoordinator` `1078`, `LuckRingCoordinator` `64ff`,
+`RWfitProtocol.MANUFACTURER_HEX_PREFIXES` `d605…`/`d606…`). `RingBLEClient` used to pass
+`valueAt(0)` through untouched, so those prefixes could never appear and **every manufacturer-data
+fallback in the registry was dead code on Android** — a ring whose name the catalog didn't
+recognise matched nothing at all (issue #56, an `Ale-Hop2211 E1C7` YCBT ring). It also read only
+entry 0 despite a comment claiming otherwise.
+
+Build `AdvertisementInfo` through `AdvertisementMatcher` (`WearableDriver.kt`), which restores the
+company ID and offers **every** entry to each coordinator, registry order still outermost. If you
+add a coordinator that matches manufacturer bytes, write the prefix in on-air layout (company ID
+first) and cover it in `AdvertisementMatcherTest`.
+
+Same lesson, different field: a name pattern is one convention, so keep it in one constant.
+`WearableModel.SMARTHEALTH_NAME_PATTERN` gates two decisions in series — whether
+`modelForAdvertisedName` returns a card at all, and whether `ColmiSmartHealthCoordinator` accepts
+the name — and the two copies that used to exist drifted into the same bug.
+
 ## Colmi/Yawell OS-bonding is a hand-curated allowlist — not "match QRing exactly"
 
 **This is the one rule in this file most likely to get silently reverted by a future "generalize

app/build.gradle.kts

@@ -21,7 +21,7 @@ android {
         // versionCode/versionName are overridable from Gradle properties so the release CI
         // can drive them straight from the git tag (e.g. -PappVersionCode=5 -PappVersionName=1.0.0).
         // Local builds fall back to the literals below.
-        versionCode = (project.findProperty("appVersionCode") as String?)?.toIntOrNull() ?: 39
+        versionCode = (project.findProperty("appVersionCode") as String?)?.toIntOrNull() ?: 40
         versionName = (project.findProperty("appVersionName") as String?) ?: "2.7.0"
         testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
 

app/src/main/java/com/pulseloop/ring/ColmiSyncEngine.kt

@@ -79,12 +79,30 @@ class ColmiSyncEngine(
     private val watchdogTimeoutMs = 10_000L
     private val activityWatchdogTimeoutMs = 20_000L
 
-    // Realtime HR keepalive
-    private var realtimeHRActive = false
+    // Realtime HR keepalive.
+    // Volatile: written on Main (start/stop actions), read on the notify thread —
+    // [handleRawNotify] → [onRealtimeHeartRateRejected] decides whether a `0x9E` is the reply to
+    // a live `0x1E` session or an unsolicited push, and a stale read there strands the workout
+    // with no bpm for its whole duration.
+    @Volatile private var realtimeHRActive = false
     private var realtimeKeepaliveJob: Job? = null
-    private var manualHRActive = false
+    @Volatile private var manualHRActive = false
     private var manualSpO2Active = false
 
+    /**
+     * This ring answered the `0x1E` realtime-HR request with a `0x9E` error frame, so the session
+     * runs on the `0x69` spot-measure stream instead (see [onRealtimeHeartRateRejected]).
+     *
+     * Sticky for the life of the engine: once a ring has refused `0x1E` it refuses every one of
+     * them, so later workouts skip the probe and start on `0x69` directly. A reconnect builds a
+     * new driver and a new engine, so this never outlives the firmware it was measured against.
+     */
+    @Volatile private var realtimeRejected = false
+
+    /** Wall-clock of the last `0x69` frame seen while the fallback stream runs — the fallback
+     *  keepalive re-issues the start only when the ring has actually gone quiet. */
+    @Volatile private var lastFallbackFrameAt = 0L
+
     /** Last bpm seen while a manual spot measurement runs: QRing reports it in the 0x6A stop
      *  frame so the ring's own measurement log records the reading (0 = cancelled). */
     private var lastManualBpm = 0
@@ -98,6 +116,11 @@ class ColmiSyncEngine(
 
         /** 20 s wall-clock re-arm, matching QRing's HeartActivity timer. */
         private const val REALTIME_KEEPALIVE_MS = 20_000L
+
+        /** How long the `0x69` fallback stream may stay silent before the start is re-issued.
+         *  Generous on purpose: the R09's SpO2 twin streamed warm-up frames for ~25 s before its
+         *  first reading, so a shorter window would restart a measurement that was working. */
+        private const val FALLBACK_STREAM_IDLE_MS = 30_000L
     }
 
     override fun runStartup() {
@@ -176,6 +199,21 @@ class ColmiSyncEngine(
      * the ring's reported interval.
      */
     override fun handleRawNotify(data: ByteArray) {
+        // Realtime-HR rejection / fallback bookkeeping runs ahead of the seeding guard: both are
+        // live-measurement concerns, unrelated to whether a config seed is in progress.
+        val frame = ColmiPacket.validating(data)?.bytes
+        if (frame?.get(0)?.toUByte() == ColmiCommandID.REALTIME_HEART_RATE_ERROR) {
+            onRealtimeHeartRateRejected()
+            return
+        }
+        // Any 0x69 *heart-rate* frame is proof the fallback stream is alive. Reading type matters:
+        // a concurrent 0x69/3 SpO2 measure shares the opcode and must not stand in for HR traffic.
+        if (frame?.get(0)?.toUByte() == ColmiCommandID.MANUAL_HEART_RATE &&
+            frame[1].toUByte() == ColmiCommandID.RT_HEART_RATE
+        ) {
+            lastFallbackFrameAt = System.currentTimeMillis()
+        }
+
         // Device-support reply is independent of config seeding: remember the temperature-path
         // capability and, if the ring wants a bond, ask the client to create one. Return early —
         // a 0x3C frame carries nothing else we consume.
@@ -537,6 +575,12 @@ class ColmiSyncEngine(
     // MARK: Measurement actions
 
     override fun startHeartRate() {
+        // A ring that has already refused 0x1E on this connection never gets asked again — go
+        // straight to the 0x69 stream the vendor app itself uses (see onRealtimeHeartRateRejected).
+        if (realtimeRejected) {
+            startFallbackHeartRateStream()
+            return
+        }
         realtimeHRActive = true
         writer?.enqueue(encoder.realtimeHeartRate(enable = true))
         // Re-arm the stream on a wall-clock timer like QRing (20 s), not per received frame:
@@ -551,6 +595,67 @@ class ColmiSyncEngine(
         }
     }
 
+    /**
+     * The ring answered `0x1E` with a `0x9E` error frame — it does not implement the continuous
+     * realtime-HR request. Move the live session onto the `0x69` stream instead.
+     *
+     * **Why `0x69` is the right fallback, not a guess.** No `BaseReqCmd` in the decompiled QRing
+     * app is built with opcode 30 — the SDK only ever *receives* `0x1E` (`BeanFactory` case 30 →
+     * `RealTimeHeartRateRsp`, a bare bpm push). `0x1E` as a *request* comes from GadgetBridge
+     * (`YawellRingDeviceSupport.onEnableRealtimeHeartRateMeasurement`), which is where PulseLoop
+     * took it from, and RT-series firmware rejects it. Every live reading QRing itself takes goes
+     * through `StartHeartRateReq.getSimpleReq(TYPE_HEARTRATE=1)` = `0x69 01 00`, whose reply
+     * (`StartHeartRateRsp`: `[type, errCode, value]`) the decoder already maps to
+     * `HeartRateSample`, and which the ring keeps streaming until a `0x6A` stops it.
+     *
+     * Issue #55, `R09_9D07` / firmware `RT09_3.10.22_260420`: every one of the eleven `0x1E`
+     * frames in the user's capture — start, stop and continue alike — came back `9e ee`, so the
+     * workout screen never showed a bpm, while `0x69 03` (SpO2, same command family) streamed
+     * warm-up frames for ~25 s and then returned real readings.
+     */
+    private fun onRealtimeHeartRateRejected() {
+        if (!realtimeHRActive && !realtimeRejected) {
+            // Unsolicited 0x9E with no session running (the ring volunteering a failed reading):
+            // remember the refusal so the next startHeartRate skips the probe, but start nothing.
+            realtimeRejected = true
+            return
+        }
+        realtimeRejected = true
+        if (!realtimeHRActive) return
+        realtimeHRActive = false
+        realtimeKeepaliveJob?.cancel(); realtimeKeepaliveJob = null
+        startFallbackHeartRateStream()
+    }
+
+    /**
+     * Run the live-HR session on `0x69` and keep it alive.
+     *
+     * The re-arm is idle-gated rather than unconditional: the ring streams on its own once
+     * started, and re-issuing `0x69 01` mid-measurement restarts the reading. It fires only after
+     * [FALLBACK_STREAM_IDLE_MS] of silence, which is what covers a ring-side measurement window
+     * expiring partway through a workout. Repeat starts are safe — the capture in issue #55 shows
+     * the ring accepting three `0x69 03` starts in ninety seconds.
+     */
+    private fun startFallbackHeartRateStream() {
+        if (manualHRActive) return  // already streaming on 0x69
+        manualHRActive = true
+        lastManualBpm = 0
+        lastFallbackFrameAt = System.currentTimeMillis()
+        writer?.enqueue(encoder.manualHeartRate(enable = true))
+        realtimeKeepaliveJob?.cancel()
+        realtimeKeepaliveJob = scope.launch {
+            while (isActive) {
+                delay(REALTIME_KEEPALIVE_MS)
+                if (!manualHRActive) continue
+                val silentFor = System.currentTimeMillis() - lastFallbackFrameAt
+                if (silentFor >= FALLBACK_STREAM_IDLE_MS) {
+                    lastFallbackFrameAt = System.currentTimeMillis()
+                    writer?.enqueue(encoder.manualHeartRate(enable = true))
+                }
+            }
+        }
+    }
+
     override fun stopHeartRate() {
         realtimeKeepaliveJob?.cancel(); realtimeKeepaliveJob = null
         if (manualHRActive) {
@@ -579,6 +684,11 @@ class ColmiSyncEngine(
     override fun stopSpO2() {
         if (!manualSpO2Active) return
         manualSpO2Active = false
+        // `0x6A` tears down the ring's whole realtime engine, HR included — that is exactly why
+        // RingSyncCoordinator follows every spot stop with restartWorkoutHeartRateIfActive(). Drop
+        // the HR bookkeeping with it, or that restart short-circuits on a stream the ring has
+        // already stopped and the workout shows no bpm until the idle keepalive re-arms.
+        manualHRActive = false
         writer?.enqueue(encoder.manualSpO2(enable = false))
     }
 

app/src/main/java/com/pulseloop/ring/RingBLEClient.kt

@@ -1159,13 +1159,18 @@ class RingBLEClient(
 
     private fun matchDeviceType(name: String?, scanRecord: ScanRecord?): RingDeviceType? {
         val serviceUUIDs = scanRecord?.serviceUuids?.map { it.uuid.toString() } ?: emptyList()
-        // Iterate all manufacturer-specific data entries to find a match
-        var mfg: ByteArray? = null
+        // Flatten the SparseArray to (companyId, value) pairs — every entry, not just index 0.
+        // [AdvertisementMatcher] puts the company ID back in front of each value: Android strips
+        // it into the key, the coordinators match it as a prefix, and without it every
+        // manufacturer-data fallback in the registry was unreachable (issue #56).
+        val manufacturerEntries = mutableListOf<Pair<Int, ByteArray>>()
         scanRecord?.manufacturerSpecificData?.let { data ->
-            if (data.size() > 0) mfg = data.valueAt(0)
+            for (i in 0 until data.size()) {
+                val value = data.valueAt(i) ?: continue
+                manufacturerEntries.add(data.keyAt(i) to value)
+            }
         }
-        val info = AdvertisementInfo(serviceUUIDs, mfg)
-        return coordinators.firstOrNull { it.matches(name, info) }?.deviceType
+        return AdvertisementMatcher.match(coordinators, name, serviceUUIDs, manufacturerEntries)
     }
 
     private inline fun updateState(crossinline update: BLEState.() -> BLEState) {

app/src/main/java/com/pulseloop/ring/WearableDriver.kt

@@ -206,6 +206,12 @@ interface RingSyncEngine {
 
 /**
  * Ported from [AdvertisementInfo] in WearableCoordinator.swift.
+ *
+ * [manufacturerData] is one manufacturer-specific data block in its **on-air layout**: the
+ * little-endian company ID first, then the vendor bytes. That is what CoreBluetooth hands iOS,
+ * and it is what every coordinator's prefix matches (`1078…`, `64ff…`, `d605…`). Android's
+ * `ScanRecord` splits the same field the other way, so build this through
+ * [AdvertisementMatcher.manufacturerBlocks] rather than from `valueAt()` directly.
  */
 data class AdvertisementInfo(
     val serviceUUIDs: List<String>,
@@ -222,6 +228,63 @@ data class AdvertisementInfo(
         31 * serviceUUIDs.hashCode() + manufacturerData.contentHashCode()
 }
 
+/**
+ * Turns Android's parsed advertisement fields into the [AdvertisementInfo]s the coordinators
+ * expect, and walks the registry against them.
+ *
+ * Exists because of a platform mismatch that made every manufacturer-data fallback dead code on
+ * Android (issue #56). `ScanRecord.getManufacturerSpecificData()` returns a `SparseArray` **keyed
+ * by company ID with the ID stripped from the value**; the coordinators were ported from Swift,
+ * where CoreBluetooth includes the company ID in the bytes, so they all match a little-endian
+ * company-ID prefix (`TK5Coordinator` `10786501`, `ColmiSmartHealthCoordinator` `1078`,
+ * `LuckRingCoordinator` `64ff`, `RWfitProtocol.MANUFACTURER_HEX_PREFIXES` `d605…`/`d606…`). Those
+ * prefixes can never appear in a `valueAt()` payload, so the fallback never fired for any YCBT,
+ * LuckRing or RWfit ring — a ring whose name the catalog doesn't recognise was simply invisible.
+ */
+object AdvertisementMatcher {
+
+    /**
+     * Re-attach the company ID to each entry, little-endian, restoring the on-air layout.
+     *
+     * Returns one block per entry — **all** of them, not just index 0: a device may advertise
+     * several company blocks and nothing guarantees the family marker is the first. A device with
+     * no manufacturer data yields a single `null` block, so the service/name matches still run.
+     */
+    fun manufacturerBlocks(entries: List<Pair<Int, ByteArray>>): List<ByteArray?> {
+        if (entries.isEmpty()) return listOf(null)
+        return entries.map { (companyId, value) ->
+            byteArrayOf(
+                (companyId and 0xFF).toByte(),
+                ((companyId shr 8) and 0xFF).toByte(),
+            ) + value
+        }
+    }
+
+    /**
+     * First coordinator in registry order that claims the advertisement, or `null`.
+     *
+     * Registry order stays the outer loop — it is load-bearing (see `RingBLEClient.coordinators`) —
+     * so a coordinator listed earlier still wins even when a later one matches a different
+     * manufacturer block of the same device.
+     */
+    fun match(
+        coordinators: List<WearableCoordinator>,
+        name: String?,
+        serviceUUIDs: List<String>,
+        manufacturerEntries: List<Pair<Int, ByteArray>>,
+    ): RingDeviceType? {
+        val blocks = manufacturerBlocks(manufacturerEntries)
+        for (coordinator in coordinators) {
+            for (block in blocks) {
+                if (coordinator.matches(name, AdvertisementInfo(serviceUUIDs, block))) {
+                    return coordinator.deviceType
+                }
+            }
+        }
+        return null
+    }
+}
+
 /**
  * Ported from [WearableCoordinator] in WearableCoordinator.swift.
  * Capability + metadata descriptor for a wearable family.

app/src/main/java/com/pulseloop/ring/YCBTCoordinators.kt

@@ -94,19 +94,30 @@ object TK5Coordinator : WearableCoordinator {
  *
  * Both confirmed YCBT rings (`TK5 24AA`, `R99 54DC`) name themselves `<MODEL><SPACE><4 hex>`, while
  * every QRing-Colmi in the catalog uses an underscore (`R02_A1B2`, `COLMI R10_9C3F`). That
- * space-versus-underscore split is the primary signal — not the manufacturer data, which is
- * unconfirmed for this family (the only capture in it, the TK5, isn't even this coordinator).
+ * space-versus-underscore split is the primary signal; the manufacturer marker is the fallback for
+ * a unit the catalog doesn't name, and issue #56 confirmed one (`Ale-Hop2211 E1C7`, company 0x7810)
+ * — the first capture that actually belongs to this coordinator.
  */
 @OptIn(ExperimentalStdlibApi::class)
 object ColmiSmartHealthCoordinator : WearableCoordinator {
     override val deviceType: RingDeviceType = RingDeviceType.COLMI_SMART_HEALTH
 
-    /** The SmartHealth naming convention: model, one space, four hex digits. Anchored end to end. */
-    private val namePattern = Regex("^[A-Za-z0-9]+( [A-Za-z0-9]+)* [0-9A-Fa-f]{4}$")
+    /** The SmartHealth naming convention: model, one space, four hex digits. Anchored end to end.
+     *  The literal lives on [WearableModel.SMARTHEALTH_NAME_PATTERN] — this coordinator and the
+     *  catalog card gate the *same* decision in series (the card decides whether
+     *  `modelForAdvertisedName` returns non-null at all), so a second copy here can only drift.
+     *  It did: both rejected the hyphen in `Ale-Hop2211 E1C7` (issue #56). */
+    private val namePattern = Regex(WearableModel.SMARTHEALTH_NAME_PATTERN)
 
     /** The Yucheng SDK's company ID (0x7810, little-endian => `1078`), matched as a manufacturer-
      *  data prefix. Demoted to corroborating evidence only — never overrides a name match, since a
-     *  QRing-Colmi may carry the same company ID. */
+     *  QRing-Colmi may carry the same company ID.
+     *
+     *  Confirmed on hardware by issue #56: an `Ale-Hop2211 E1C7` (JieLi, model string `TRINITY`,
+     *  ships with SmartHealth) advertises `1078 d408 7700 …` alongside Heart Rate + `FEE7` — the
+     *  same shape the TK5 capture shows, but without TK5's `6501` second word, which is why that
+     *  coordinator stands aside for this one. Until the Android fix in [AdvertisementMatcher] this
+     *  branch was unreachable, so the ring matched nothing at all. */
     private const val MANUFACTURER_HEX_MARKER = "1078"
 
     private fun isSmartHealthName(name: String?): Boolean = name != null && namePattern.matches(name)