fix(crp): correct the temperature history opcode, and ask the ring what it supports - #41
Merged
Merged
Conversation
…at it supports Two captures from zaggash's COLMI R11 — 2026-07-23 (build 26) and 2026-07-25 (build 30) — read against the vendor decompile. Temperature history was on the wrong opcode. q.b(2,48) is the vendor's querySleepState (d1/b.java:650); real temperature history is i0.b(day, frameIndex) = q.c(2,22,[day,idx]), the same [day, frameIndex] shape as the other timing histories. That is why 2/48 went 23-sent/0-answered. Its sample layout is still unconfirmed by a non-empty capture, so the reply stays an ack rather than inventing values. Adds the vendor's read-backs so the ring can describe itself instead of us inferring from spec sheets: querySupportSpO2Type (2/37, answering NOT_SUPPORT / SLEEP_OXYGEN / TIMING_OXYGEN) and the monitor-state queries 2/6 HR, 2/7 HRV, 2/8 SpO2, 2/45 stress, 2/21 temp, each reporting the configured interval where 0 means off. Those are the evidence base for whether a silent history query means "the monitor is switched off" or "this ring lacks the sensor" — the open question for stress (2/47), temperature and firmware (7/1). They are sent once per connection, not per poll pass. runStartup doubles as the ~30-minute background sync and is also reached from refresh()/querySleep(), and what a ring supports cannot change between syncs; re-asking would add six writes to every pass on the single fdd2 channel a spot SpO2 needs for ~48 s. A fresh CRPSyncEngine is built per connection, so instance state gives that for free. The support decode only accepts the two documented values. CRPBloodOxygenType defines 0/1/2 and getInstance returns null for anything else, and this ring uses 0xFF as a no-reading sentinel elsewhere — treating "any non-zero" as support would let 0xFF read as a capability claim. Deliberately NOT included: SpO2 stays an unconditional CRP capability. The 2026-07-23 capture has a real reading (group 1 / cmd 11 payload 0x61 = 97 %), so the card and Measure button stay. The read-back is diagnostic — refinement is additive-only and cannot take a capability away. AGENTS.md records what the captures actually show: SpO2 works but is slow (~48 s) and contact-sensitive; HR success does not prove contact is adequate for SpO2 (HR returned a reading 3 s before all three attempts, including both failures); and group 3 / cmd 7 [00] is a failure predictor whose contact advice is correct. 776 tests pass.
…tters Two clarity defects found reviewing the branch. No behaviour change. The SpO2 support read-back cannot grant anything today: CRPCoordinator declares no bitmapGatedCapabilities, so refineActiveCapabilities intersects with the empty set and returns early. That is the right outcome — SpO2 is already unconditional because it is hardware-confirmed, and refinement is additive-only so a NOT_SUPPORT answer could not remove it either. But the test was named "grants the SpO2 capabilities back", which claims a wiring that does not exist. Renamed to say it asserts the decode, with a note on why nothing is granted. The read-backs must be sent before applyTimingSettings: they report each all-day monitor's current interval, and applyTimingSettings force-enables everything moments later, so asking afterwards would only describe the state we just imposed — answering nothing about whether stress and temperature were silent because their monitor was off. CRPSyncEngineTest already pinned the ordering, but nothing said why, so the natural response to a failure would have been to reorder the expectation. Both sides now record the constraint. 776 tests pass.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Replaces #40, which was closed because its central premise was falsified. This keeps only what two captures from zaggash's COLMI R11 — 2026-07-23 (build 26) and 2026-07-25 (build 30) — actually support.
SpO2 stays. It works.
The 07-23 capture contains a real reading:
group 1 / cmd 11, payload0x61= 97%.SPO2andMANUAL_SPO2remain unconditional CRP capabilities, so the Vitals SpO2 card and its Measure button stay visible.CRPCoordinatorandRingSyncCoordinatorare untouched by this PR —git diff mainon both is empty.What the captures show is that SpO2 is slow and contact-sensitive, not absent:
3/7 [00]at ~4sTemperature history was on the wrong opcode
q.b(2,48)is the vendor'squerySleepState(d1/b.java:650). Real temperature history isi0.b(day, frameIndex)=q.c(2,22,[day,idx]), the same[day, frameIndex]shape as the other timing histories. That explains 23-sent / 0-answered.Fixed to
2/22. Its sample layout is still unconfirmed by a non-empty capture, so the reply stays an ack rather than inventing values.Ask the ring instead of inferring
querySupportSpO2Type2/37queryTimingHeartRateState2/60= offqueryTimingHrvState2/7queryTimingSpO2State2/8queryTimingStressState2/45queryTimingTempState2/21These are the evidence base for whether a silent history query means "the monitor is off" or "this ring lacks the sensor" — the open question for stress (
2/47), temperature and firmware (7/1).Sent once per connection, not per poll pass.
runStartupdoubles as the ~30-minute background sync and is also reached fromrefresh()/querySleep(); what a ring supports cannot change between syncs. Re-asking would add six writes to every pass on the singlefdd2channel a spot SpO2 needs for ~48s. A freshCRPSyncEngineis built per connection, so instance state gives once-per-connection for free.The support decode accepts only the two documented values.
CRPBloodOxygenTypedefines 0/1/2 andgetInstancereturns null otherwise — and this ring uses0xFFas a no-reading sentinel elsewhere, so "any non-zero" would let0xFFread as a capability claim. (Found in adversarial review of #40.)The read-back is diagnostic: refinement via
refineActiveCapabilitiesis additive-only, so a NOT_SUPPORT answer cannot remove a capability. It puts the ring's own answer in the raw-packet feed where the next capture can confirm or challenge what we assume.What #40 got wrong, and isn't here
WearEvidence, which downgraded the "put the ring on snugly" copy whenever HR had recently succeeded. HR returned a reading 3 seconds before all three SpO2 attempts, including both failures. HR reads fine at contact quality SpO2 cannot use, so it proves nothing about SpO2 contact — the change would have suppressed correct, actionable advice in exactly the cases needing it. The wear fast-fail is left exactly as it shipped.Testing
TZ=UTC ./gradlew testDebugUnitTest— 776 tests, 0 failures. New: unknown/0xFFsupport types grant nothing, types 1 and 2 grant, SpO2 is asserted to remain unconditional, and read-backs are asserted not to repeat within a connection while a new engine re-asks.Reviewer notes
AGENTS.mdrecords what the captures show, including that HR success does not vouch for SpO2 contact — the mistake behind fix(crp): stop claiming SpO2 the R11 has no sensor for, and ask the ring instead #40'sWearEvidence.7/1) is still unanswered, so the device panel keeps showing?. Not addressed here; the group-7 opcode is the vendor's own and this ring simply ignores it.