Fix dashboard showing "n/a" for a legitimate 0.0 °C reading - #92
Merged
Conversation
app.js gated the temperature display on `snap.temperature?.celsius` truthiness, so an exact 0 °C reading (plausible for a Pi in an unheated/outdoor enclosure) rendered as "n/a" indistinguishably from a failed sensor read. A successful reading always carries a non-empty zone, while a failed collection yields zone: "", so key off `snap.temperature?.zone` instead. Closes #64
|
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
2 participants
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.
📖 Description
app.jsgated the temperature display onsnap.temperature?.celsiustruthiness (internal/web/assets/app.js:221, formerlyif (snap.temperature && snap.temperature.celsius)). Since0is falsy in JavaScript, a legitimate exact 0.0 °C reading (plausible for a Pi in an unheated/outdoor enclosure) was indistinguishable from a failed sensor read and rendered as "n/a".The root cause is that the API always marshals
Snapshot.Temperature, and a failed collection yields{"zone": "", "celsius": 0}— so a successful reading is actually distinguished by a non-emptyzone, not by thecelsiusvalue. This fix keys the frontend check offsnap.temperature?.zoneinstead, per the fix suggested in the issue for a non-breaking, within-v1 change.The longer-term fix (making
Temperaturea pointer, omitted on failure likegpu_temperature) is a breaking API change and is left for/api/v2/, as noted in the issue.This is a frontend-only change; no Go API types were touched.
🎫 Issues
Closes #64
👩💻 Reviewer Notes
Single-line fix in
internal/web/assets/app.js. No build step for the frontend (plain JS embedded viago:embed), so the fix is the diff itself — no bundler artifacts to check.📑 Test Plan
There is no JS test runner in this repository (see
docs/TESTS.md);internal/web/xss_test.goestablishes the pattern of a Go test scanning the embeddedapp.jssource for a banned/required construct, so I followed that pattern:internal/web/temperature_test.go(TestAppJS_TemperatureNAUsesZoneNotCelsius), which fails ifapp.jsreintroduces acelsius-truthiness gate on the "n/a" fallback, and asserts thezone-based check is present.go build ./...,go vet ./...,go test ./... -race -cover, andgolangci-lint runall pass locally.✅ Checklist
General
go test ./... -race -coverpasses locally).go vet ./...andgolangci-lint runare clean.ARCHITECTURE.mdif this changes a documented design decision. (not applicable — no documented design decision changed)REST API / configuration / packaging
Not applicable — this is a frontend-only fix; the
/api/v1/...response shape (Temperature.Celsiusalways marshaled) is unchanged.⏭ Next Steps
The longer-term API fix noted in the issue — making
Temperaturea pointer so a failed reading isnull/omitted instead of{"zone": "", "celsius": 0}— is a breaking change and would need to land under/api/v2/.... Not addressed here.