Repository navigation
fix(web): save unchecked Zap reads as false - #1484
Merged
Merged
Conversation
Setup and Settings left read_pv and read_battery out of a Zap config when the boxes stayed unchecked. Core treats a missing read_pv as PV that may report, so the forecast learner waited for PV the Zap never sends (#1441). Setup now writes both keys as false for a new Zap. Settings fills a missing key with false when it shows a Zap, so the next save repairs an older config. A value the owner set is left as is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Refs #1441. Pairs with #1480.
Problem
The Zap driver reads PV only when
read_pvis true. Setup and Settings → Devices leftread_pvandread_batteryout of the saved config when the boxes stayed unchecked. Settings writes a checkbox only when the owner changes it.forecastMeasurementTopologyskips Zap PV only whenread_pvis explicitly false. With the key missing, it expectssourceful-zap:pv, and the forecast learner waits for PV the Zap never sends. #1480 keeps history running; this change fixes the saved config.Change
read_pv: falseandread_battery: falsewhen it adds a Zap.read_pvorread_batterywithfalsewhen it shows a Zap. A new Zap from "Add new device" gets the same. A value already set, such astrueor"true"in hand-edited YAML, stays as is.No new settings. No Go change: #1480 has right of way on
forecast_measurements.go.Stored configs
A Zap saved before this change still has no
read_pv. Opening Settings → Devices and pressing Save writesfalse. Until then the forecast learner behaves as before.Core could treat a missing
read_pvas false. It has no generic way to know which drivers expose the option, so that would need a Zap check by Lua path in Core. I left it out; say if you want it after #1480 lands.Evidence
setup.js, adds a Zap from a catalog entry and checks the posted config hasread_pv: falseandread_battery: false. It fails onmaster.master.npm test: 645 pass.make verifypasses.host. Opened Settings → Devices, left both boxes unchecked, pressed Save.GET /api/configthen returnedread_pv: false, read_battery: false.🤖 Generated with Claude Code