Skin temp: fix Fahrenheit (deviation must not get the +32 offset) (#111)#115
Merged
Conversation
…111) The skin-temp metric formatted its Celsius value with the ABSOLUTE C→F formula (×9/5 + 32) even when the value is a signed DEVIATION from baseline. Adding +32 to a deviation is nonsense, and only shows in Fahrenheit (Celsius appends the raw deviation, so it looked fine) — a −4.2 °C deviation rendered as "24.4 °F" (the report), and the graph axis inherited the same corruption. Fix: MetricDescriptor.format picks per value via VitalBands.isAbsoluteSkinTemp (v >= 20 °C) — a DEVIATION uses temperatureDeltaFromCelsius (×9/5, no +32), an ABSOLUTE reading (WHOOP export gives absolute °C) keeps the full C→F. This mirrors the Android HealthScreen per-value branch, so the two platforms now agree. Fixes the hero value, chart readout, and the min/max/avg stat tiles (all route through `format`). Celsius output is unchanged. Test: skin_temp descriptor formats −4.2 → "-7.6 °F", 0.6 → "1.1 °F" (delta), and 34.0 → "93.2 °F" (absolute); Celsius unchanged. NB the reporter's underlying ~−4.2 °C deviation on a live WHOOP MG is itself large (possibly no baseline yet / calibrating) — the reply asks for a debug export to sanity-check the raw MG reading; the +32 corruption is fixed regardless.
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.
Fixes #111 — skin temp shows nonsense in Fahrenheit on a WHOOP MG (reported "24.4 °F"), with a broken graph axis/scale.
Root cause
The skin-temp metric's stored value is either an absolute reading (WHOOP export gives absolute °C) or a signed deviation from your baseline (±°C, the live/computed pipeline). Its Fahrenheit conversion always used the absolute formula
F = C×9/5 + 32. Adding +32 to a deviation is wrong — and only shows in Fahrenheit (Celsius appends the raw deviation, so it looked fine). The math confirms the report:24.4 °F = (−4.2) × 1.8 + 32— a deviation run through the absolute formula. The graph's y-axis inherited the same corruption (the "scale on the side" that didn't make sense).Fix
MetricDescriptor.formatnow picks the formula per value viaVitalBands.isAbsoluteSkinTemp(v)(v >= 20 °C):< 20 °C) →temperatureDeltaFromCelsius(×9/5, no +32)>= 20 °C, e.g. an imported WHOOP reading) → full C→F (×9/5 + 32)This mirrors the Android
HealthScreenper-value branch (which was already correct), so iOS/macOS now agree with Android. Because the hero value, the chart readout, and the min/max/avg stat tiles all format throughformat, one change fixes all of them. Celsius output is unchanged.Re-review note
My first pass just forced skin temp to always-delta — but that would have broken the absolute-import case (34 °C → "61.2 °F" instead of 93.2). Checking Android surfaced the per-value
isAbsoluteSkinTempsplit; the fix now matches it.Testing
Test added:
−4.2 → "-7.6 °F",0.6 → "1.1 °F"(delta) and34.0 → "93.2 °F"(absolute), Celsius unchanged. Can't build the app target / run StrandTests on WSL (Xcode + GRDB), so a testing build verifies the Swift compiles.Caveat (separate from this fix)
The reporter's underlying value works out to ~−4.2 °C deviation on a live WHOOP MG, which is implausibly large — possibly "no personal baseline yet / calibrating," or an MG decode/baseline issue. The +32 corruption is the definite, reproducible bug (fixed here); I'll ask them for a debug export to sanity-check the raw MG reading. Also noted but out of scope:
FullDayChartViewshows skin temp only in °C (ignores the °F preference) — a pre-existing, separate gap.File:
Strand/Data/MetricCatalog.swift(+ test).