feat: per-zone Whisperer pairing with sensor-over-forecast fallback#55
Merged
Conversation
Design for the per-zone Whisperer pairing feature: new linkedWhispererDeviceId pluginProp + dropdown in zone ConfigUI, new moistureForecast state that always holds Netro's /moistures.json prediction, and a 12h staleness threshold controlling whether zone moisture reflects the paired Whisperer or falls back to the forecast. Single writer: the zone update loop pulls Whisperer state out of Indigo's device DB and resolves moisture source on each cycle. The Whisperer update loop is unchanged. Transition-aware logging avoids log spam. No Netro API changes — pairing lives entirely in plugin-side state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task-by-task TDD plan with 11 bite-sized steps: constant, age-parsing util, Devices.xml changes (new state + ConfigUI dropdown), getWhispererDevices callback, _resolve_zone_moisture pure helper, transition-aware logging, wire-up in _update_zone_devices, doc updates, and final version bump. Uses the existing readingTime state (v1 epoch-millis + v2 ISO-8601) for staleness math rather than the swapped readingLocalTime/ readingLocalDate fields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Use str.removesuffix("Z") instead of rstrip("Z") to reject
multi-Z malformed input rather than silently accepting it.
- Reject bool inputs in the epoch branch (bool is subclass of int).
Two new tests pin the behavior.
…il (#54) - Treat non-numeric soilMoisture as stale rather than raising ValueError (latent crash surface from Whisperer handler contract drift). - Add resolver-level test for v1 epoch-millis readingTime (previously only covered by the util's own tests).
…egration test (#54) - Build new dicts rather than mutating entries from process_zone_moisture to avoid a coupling to that handler's caller-writable implementation detail. - Add an end-to-end test for paired-but-stale → forecast fallback to close the paired/unpaired × fresh/stale matrix at the integration level.
indigo.devices.Device.pluginProps is read-only — the test doubles were plain dicts so the assignment worked in tests but crashed production with "the attribute 'pluginProps' is read-only on this instance". Remove the assignment; rely on replacePluginPropsOnServer to persist, which Indigo then reflects back into pluginProps after the round-trip. Update test fixtures so their replacePluginPropsOnServer stubs mutate the pluginProps dict in place — simulating Indigo's real behavior rather than the previous leak-through.
_update_zone_devices called _log_moisture_source_transition before updateStatesOnServer, and the logger's replacePluginPropsOnServer had no try/except. An IOM hiccup during the prop write would raise, bubble to the outer zone try/except, and silently drop the entire state batch (moisture, moistureForecast, schedules, isIrrigating). Fix: persist state first, then log transitions. Additionally wrap the prop write in try/except so the auxiliary log can never gate the primary state persistence.
The resolver previously collapsed 5 distinct failure modes into
"forecast-stale", so the user-facing log message claimed "stale
(>12h old)" even when the sensor was just uninitialized or its
readingTime was garbage. Split into three tags:
forecast-missing-reading: readingID==0 or soilMoisture absent
or non-numeric (sensor hasn't reported,
or state is uninitialised)
forecast-unparseable-time: readingTime can't be parsed
forecast-stale: genuinely >12h old
Also add a readingID!=0 liveness check because real Indigo returns
integer default 0 for unset Integer states — so an unreported
Whisperer could silently return (0, "whisperer") and look like a
real 0% reading. Tests updated to reflect production defaults.
Adds a debug breadcrumb in the resolver logging the raw readingTime
when parsing fails, so support debugging has the data.
…#54) - Integration test confirms _update_zone_devices actually calls _log_moisture_source_transition (closes wiring gap). - process_zone_moisture returns [] on empty/missing/error paths instead of a fake 0% row, so moistureForecast no longer gets written on API outages. - Boundary test at 11.9/12.0/12.1h locks in the > (not >=) semantics of WHISPERER_STALENESS_HOURS. - Cold-start -> whisperer silence + unchanged-source no-prop-write tests close gaps flagged in review. - Hoist _PluginBase stub + mock_indigo_base fixture into conftest.py to stop the three test modules drifting. - Narrow _update_zone_devices inner except to the exception shapes that can actually leak from process_zone_moisture.
📝 WalkthroughWalkthroughAdds per-zone Whisperer pairing and freshness logic: zones can be paired to a Whisperer device; when paired and the Whisperer reading is ≤12 hours old, zone Changes
Sequence DiagramsequenceDiagram
participant IND as Indigo Plugin
participant ZH as ZoneHandler
participant UTIL as Utils
participant WH as Whisperer Device
participant DB as Indigo State DB
participant LOG as Transition Logger
IND->>ZH: process_zone_moisture(zone_data)
ZH-->>IND: return forecast entries → plugin treats as "moistureForecast"
IND->>IND: read zone.pluginProps.linkedWhispererDeviceId
alt Whisperer paired & enabled
IND->>WH: read soilMoisture, readingTime
IND->>UTIL: parse_reading_age_hours(readingTime)
UTIL-->>IND: age_hours
alt age_hours ≤ WHISPERER_STALENESS_HOURS
IND->>IND: _resolve_zone_moisture(whisperer.soilMoisture) → ("whisperer")
else age_hours > WHISPERER_STALENESS_HOURS
IND->>IND: _resolve_zone_moisture(forecast) → ("forecast-stale")
end
else No Whisperer / disabled
IND->>IND: _resolve_zone_moisture(forecast) → ("forecast")
end
IND->>DB: write "moistureForecast" state
IND->>DB: write "moisture" state (only if resolved value non-null)
IND->>LOG: _log_moisture_source_transition(zone, old, new)
LOG->>DB: attempt to persist lastMoistureSource (best-effort)
LOG-->>IND: suppressed or emitted log based on in-memory cache
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
docs/plans/2026-04-23-whisperer-zone-pairing-plan.md (1)
1-1358: Plan document preserved as-is.Historical TDD plan. Some reference implementations (e.g.,
_resolve_zone_moistureat lines 672-714,_log_moisture_source_transitionat lines 869-919) predate the hardening commits (readingID checks, split failure tags, aux-log protection), so they no longer match the production code verbatim. That's expected for an implementation plan and not a concern. Consider adding a one-line front-matter note like "Final implementation differs in places — see plugin.py for the source of truth" if future readers might be confused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-04-23-whisperer-zone-pairing-plan.md` around lines 1 - 1358, The plan document claims to be preserved but can mislead readers because concrete implementations (e.g., Plugin._resolve_zone_moisture and Plugin._log_moisture_source_transition) evolved; add a single-line front-matter note at the top of docs/plans/2026-04-23-whisperer-zone-pairing-plan.md stating that the plan is historical and that the canonical implementation lives in plugin.py (mentioning the symbols _resolve_zone_moisture and _log_moisture_source_transition as examples), so future readers know to consult plugin.py for the current source of truth.Netro Sprinklers.indigoPlugin/Contents/Server Plugin/utils.py (1)
165-200: LGTM — defensive parsing with correct bool guard.The explicit
not isinstance(reading_time, bool)check correctly handles the bool-as-int subclass issue, andmax(0.0, delta)guards against clock skew.Note: Python 3.11+
datetime.fromisoformatnatively accepts trailing 'Z', while Python 3.10 rejects it. An edge case like "...ZZ" will parse successfully on 3.11+ but fail on 3.10 after the singleremovesuffix("Z")strip. This doesn't affect real Netro timestamps, but if strict cross-version behavior is required, consider validating with regex to reject any remaining 'Z' after the strip.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/utils.py around lines 165 - 200, The current ISO8601 parsing uses candidate = reading_time.removesuffix("Z").strip(), which can yield different behavior across Python versions for inputs like "...ZZ"; fix this by validating the candidate after stripping: if candidate.endswith("Z") (i.e., there were multiple trailing 'Z' chars) then treat it as an invalid timestamp and return None before calling datetime.fromisoformat; update the parsing logic around candidate, datetime.fromisoformat, and the surrounding reading_time handling to perform this extra validation so cross-version behavior is consistent.Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py (3)
1198-1198: Renamefilterparameter todev_filterfor consistency and to avoid shadowing the builtin.Three of the four sibling dropdown callbacks in this file (
availableControllers,sprinklerList,pickController) already usedev_filter. Renaming here lines up with the house style and clears the RuffA002warning. The Indigo callback dispatch is positional, so the rename is safe.♻️ Proposed rename
- def getWhispererDevices(self, filter="", valuesDict=None, typeId="", targetId=0): + def getWhispererDevices(self, dev_filter="", valuesDict=None, typeId="", targetId=0):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py at line 1198, Rename the parameter named filter in the getWhispererDevices function to dev_filter to match the naming used by sibling callback functions (availableControllers, sprinklerList, pickController) and avoid shadowing the built-in name (fixing Ruff A002); update the function signature def getWhispererDevices(self, dev_filter="", valuesDict=None, typeId="", targetId=0) and replace all internal references to filter with dev_filter (no changes to call sites needed because Indigo dispatch is positional).
731-768: Consider initializingsourcebefore theis_enabledbranch for robustness.
sourceis bound only inside the firstif is_enabled:block (line 752) and then used inside a secondif is_enabled:block (line 768). It is functionally correct today because both guards evaluate the same value, but a future refactor that splits the second guard or inlines any of the intermediate states would produce anUnboundLocalError. A trivial pre-binding makes the data-flow obvious and eliminates the implicit coupling between the two guards.♻️ Proposed defensive initialization
if is_enabled: if schedule_response: states.extend( self.zone_handler.process_zone_schedules( schedule_response, zone_num, api_version=api_version ) ) + source = None forecast_val = None if moisture_response:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py around lines 731 - 768, The variable source can be unbound if control paths change; initialize it defensively before the is_enabled block that calls _resolve_zone_moisture and later _log_moisture_source_transition. Locate the block around process_zone_moisture()/ _resolve_zone_moisture(zone_dev, forecast_val) and add a default assignment for source (e.g., None or a sentinel) prior to calling _resolve_zone_moisture so that subsequent use in _log_moisture_source_transition(zone_dev, source) is always safe.
1336-1345: Persisting runtime bookkeeping inpluginPropshas a subtle clobber race — prefer a hidden device state.
lastMoistureSourceis internal runtime bookkeeping, not user configuration, yet it's written back viareplacePluginPropsOnServer(dict(zone_dev.pluginProps)). Because the dict is a snapshot of pluginProps captured inside the polling loop, a concurrent user edit to the zone's ConfigUI (e.g., changinglinkedWhispererDeviceIdthrough the Paired Whisperer dropdown) landing between the snapshot and the write will be silently overwritten. The window is narrow but real, and users pairing/unpairing mid-poll is exactly the workflow being introduced.A clean fix is to expose
lastMoistureSourceas a hidden zone state inDevices.xmland persist viaupdateStateOnServer, which avoids touching pluginProps entirely and also removes the pluginProps write from the hot polling path.♻️ Sketch of the state-based approach
<!-- Devices.xml, inside the zone device's <States> block --> <State id="lastMoistureSource" hidden="true"> <ValueType>String</ValueType> <TriggerLabel>Last Moisture Source</TriggerLabel> <ControlPageLabel>Last Moisture Source</ControlPageLabel> </State>- prev = zone_dev.pluginProps.get("lastMoistureSource") + prev = zone_dev.states.get("lastMoistureSource") or None if prev == new_source: return ... - new_props = dict(zone_dev.pluginProps) - new_props["lastMoistureSource"] = new_source try: - zone_dev.replacePluginPropsOnServer(new_props) + zone_dev.updateStateOnServer("lastMoistureSource", new_source) except Exception as exc: ...Side note: the broad
except Exceptionat line 1340 is the right call here (this auxiliary path must not gate state writes); Ruff BLE001 is a false positive and can be locally silenced if it's noisy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py around lines 1336 - 1345, The current logic persists lastMoistureSource by copying zone_dev.pluginProps and calling replacePluginPropsOnServer, which can clobber concurrent user edits; instead declare a hidden zone state (State id="lastMoistureSource" hidden="true") in Devices.xml and replace the pluginProps write in the polling code: remove the dict snapshot/replacePluginPropsOnServer usage and call zone_dev.updateStateOnServer("lastMoistureSource", new_source) (keeping the surrounding try/except behavior), so runtime bookkeeping stays out of pluginProps and avoids the race.docs/plans/2026-04-23-whisperer-zone-pairing-design.md (1)
178-179: Optional: note design-vs-implementation identifier drift.The implementation uses the Whisperer state key
readingTime(seeplugin.pyline 1259 and the tests intests/test_zone_moisture_resolution.py), while this design still saysreadingLocalTimehere and on line 249. Similarly, the util is publicparse_reading_age_hours(imported atplugin.pyline 74 and again at line 237 of this doc with a leading underscore).Since the doc is marked
Status: Accepted, it's fine to leave as a historical record — but a one-line addendum at the top ("Implementation note: finalized asreadingTimeandparse_reading_age_hours") would help future readers reconcile doc vs code without bisecting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-04-23-whisperer-zone-pairing-design.md` around lines 178 - 179, Add a one-line implementation note at the top of the document stating that the finalized runtime keys and helpers are `readingTime` (not `readingLocalTime`) and `parse_reading_age_hours` (not `_parse_reading_age_hours`); update any mentions in the text (e.g., the line using readingLocalTime and the util reference) to reflect this note so readers can reconcile the design doc with the actual implementation that uses `readingTime` and `parse_reading_age_hours`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/API_NOTES.md`:
- Around line 77-79: The Python example contains a JS-style comment after the
line device = reply_dict["data"]["device"] using // which causes a SyntaxError;
update that snippet by replacing the // comment with a Python comment marker (#)
or removing the inline comment so the line reads as valid Python (locate the
example containing device = reply_dict["data"]["device"] in docs/API_NOTES.md
and edit the comment accordingly).
In `@Netro` Sprinklers.indigoPlugin/Contents/Info.plist:
- Line 6: The PluginVersion in Info.plist is colliding with an existing git tag;
update the <string> value currently set to "2026.5.1" (the PluginVersion) to a
new unused version (for example "2026.5.2") so the release tag can be created;
ensure the updated PluginVersion string in Info.plist matches the intended new
release version.
---
Nitpick comments:
In `@docs/plans/2026-04-23-whisperer-zone-pairing-design.md`:
- Around line 178-179: Add a one-line implementation note at the top of the
document stating that the finalized runtime keys and helpers are `readingTime`
(not `readingLocalTime`) and `parse_reading_age_hours` (not
`_parse_reading_age_hours`); update any mentions in the text (e.g., the line
using readingLocalTime and the util reference) to reflect this note so readers
can reconcile the design doc with the actual implementation that uses
`readingTime` and `parse_reading_age_hours`.
In `@docs/plans/2026-04-23-whisperer-zone-pairing-plan.md`:
- Around line 1-1358: The plan document claims to be preserved but can mislead
readers because concrete implementations (e.g., Plugin._resolve_zone_moisture
and Plugin._log_moisture_source_transition) evolved; add a single-line
front-matter note at the top of
docs/plans/2026-04-23-whisperer-zone-pairing-plan.md stating that the plan is
historical and that the canonical implementation lives in plugin.py (mentioning
the symbols _resolve_zone_moisture and _log_moisture_source_transition as
examples), so future readers know to consult plugin.py for the current source of
truth.
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py:
- Line 1198: Rename the parameter named filter in the getWhispererDevices
function to dev_filter to match the naming used by sibling callback functions
(availableControllers, sprinklerList, pickController) and avoid shadowing the
built-in name (fixing Ruff A002); update the function signature def
getWhispererDevices(self, dev_filter="", valuesDict=None, typeId="", targetId=0)
and replace all internal references to filter with dev_filter (no changes to
call sites needed because Indigo dispatch is positional).
- Around line 731-768: The variable source can be unbound if control paths
change; initialize it defensively before the is_enabled block that calls
_resolve_zone_moisture and later _log_moisture_source_transition. Locate the
block around process_zone_moisture()/ _resolve_zone_moisture(zone_dev,
forecast_val) and add a default assignment for source (e.g., None or a sentinel)
prior to calling _resolve_zone_moisture so that subsequent use in
_log_moisture_source_transition(zone_dev, source) is always safe.
- Around line 1336-1345: The current logic persists lastMoistureSource by
copying zone_dev.pluginProps and calling replacePluginPropsOnServer, which can
clobber concurrent user edits; instead declare a hidden zone state (State
id="lastMoistureSource" hidden="true") in Devices.xml and replace the
pluginProps write in the polling code: remove the dict
snapshot/replacePluginPropsOnServer usage and call
zone_dev.updateStateOnServer("lastMoistureSource", new_source) (keeping the
surrounding try/except behavior), so runtime bookkeeping stays out of
pluginProps and avoids the race.
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/utils.py:
- Around line 165-200: The current ISO8601 parsing uses candidate =
reading_time.removesuffix("Z").strip(), which can yield different behavior
across Python versions for inputs like "...ZZ"; fix this by validating the
candidate after stripping: if candidate.endswith("Z") (i.e., there were multiple
trailing 'Z' chars) then treat it as an invalid timestamp and return None before
calling datetime.fromisoformat; update the parsing logic around candidate,
datetime.fromisoformat, and the surrounding reading_time handling to perform
this extra validation so cross-version behavior is consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d9db4a9-cdf6-4613-9f84-e086c64fd454
📒 Files selected for processing (18)
Netro Sprinklers.indigoPlugin/Contents/Info.plistNetro Sprinklers.indigoPlugin/Contents/Server Plugin/Devices.xmlNetro Sprinklers.indigoPlugin/Contents/Server Plugin/constants.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/device_handlers.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/utils.pyREADME.mddocs/API_NOTES.mddocs/plans/2026-04-23-whisperer-zone-pairing-design.mddocs/plans/2026-04-23-whisperer-zone-pairing-plan.mdtests/conftest.pytests/test_constants_whisperer.pytests/test_moisture_source_logging.pytests/test_reading_age.pytests/test_update_zone_devices_integration.pytests/test_whisperer_pairing_callback.pytests/test_zone_handler.pytests/test_zone_moisture_resolution.py
Comment on lines
+77
to
+79
| ```python | ||
| device = reply_dict["data"]["device"] // Not devices[0] | ||
| ``` |
There was a problem hiding this comment.
JS-style comment in Python example.
// is not a Python comment marker; readers copy-pasting the snippet will get a SyntaxError.
📝 Proposed fix
-device = reply_dict["data"]["device"] // Not devices[0]
+device = reply_dict["data"]["device"] # Not devices[0]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ```python | |
| device = reply_dict["data"]["device"] // Not devices[0] | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/API_NOTES.md` around lines 77 - 79, The Python example contains a
JS-style comment after the line device = reply_dict["data"]["device"] using //
which causes a SyntaxError; update that snippet by replacing the // comment with
a Python comment marker (#) or removing the inline comment so the line reads as
valid Python (locate the example containing device =
reply_dict["data"]["device"] in docs/API_NOTES.md and edit the comment
accordingly).
This was referenced Apr 24, 2026
Two silent-failure surfaces flagged in round-2 review: F2: when replacePluginPropsOnServer fails, the next poll re-read unchanged pluginProps and re-logged the same transition forever. Add an in-memory fallback dict (lazy-init) so a known-logged transition isn't re-emitted even if persistence is broken. Plugin restart re-syncs from pluginProps as before. F6: _resolve_zone_moisture reads whisperer.states.get(...) and .enabled — if Indigo returns something unexpected (corruption, race), AttributeError bubbles to the outer catch and all state writes for the zone are silently lost. Wrap the resolver call with a narrow catch + warning + forecast fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ing (#54) Round-2 review flagged that "non-numeric soilMoisture" was bucketed into forecast-missing-reading alongside "sensor hasn't reported yet" — the operator-actionable signals are different (check sensor firmware vs wait for next poll), but the user-facing log message claimed "no reading yet" for both. Add forecast-invalid-reading tag with a distinct warning message that points operators at the right diagnostic path. Resolver gains a debug breadcrumb logging the raw soilMoisture value when coercion fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ses (#54) - Pin the source-transition wiring test to user-facing message contents ("stale", "Netro forecast") so future log-wording changes are visible. - Hoist test_whisperer_pairing_callback.py to use the shared mock_indigo_base fixture from conftest.py — closes the remaining drift risk on the _PluginBase stub. - Add coverage for the inner except (AttributeError, TypeError, KeyError, IndexError) in _update_zone_devices so a malformed /moistures.json response can never abort other zone writes. - Parametrize the readingID liveness test to also cover the missing-key case (states without "readingID" at all). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- _log_moisture_source_transition: document best-effort persistence and in-memory fallback semantics. - _resolve_zone_moisture: enumerate all 8 source tags accurately (was 5 in pre-round-2 doc, now matches the post-split reality). - process_zone_moisture: document the [] empty-list return on the three no-data paths and explain the contract with its caller. - API_NOTES.md §6: tighten "less than 12 hours" to "≤ 12 hours" to match the strict > boundary in code. - conftest.py: thicker _IndigoPluginBaseStub docstring with pointer to mock_indigo_base. - Devices.xml: cross-ref comment so the help text and the WHISPERER_STALENESS_HOURS constant stay in sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#54) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e pylint W0201 (#54) The lazy-init via getattr in _log_moisture_source_transition is required (tests use Plugin.__new__(Plugin) which skips __init__), but pylint flagged the attribute as defined-outside-init. Add an explicit empty-dict initialiser in __init__ so the attribute is discoverable to static analysis while keeping the lazy-init fallback intact for tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py (1)
736-755: Recommended: simplify the forecast-extraction loop — thefoundflag + else-branch are unreachable.
ZoneHandler.process_zone_moisturereturns either[]or a single-entry list withkey="moisture", so thefound = Falseguard and theelse: states.append(entry)branch never fire under the current handler contract. The over-defensive shape obscures intent.♻️ Proposed simplification
forecast_val = None if moisture_response: try: forecast_states = self.zone_handler.process_zone_moisture( moisture_response, zone_num ) - found = False - for entry in forecast_states: - if not found and entry.get("key") == "moisture": - forecast_val = entry.get("value") - states.append({**entry, "key": "moistureForecast"}) - found = True - else: - states.append(entry) + if forecast_states: + entry = forecast_states[0] + forecast_val = entry.get("value") + states.append({**entry, "key": "moistureForecast"}) except (AttributeError, TypeError, KeyError, IndexError): self.logger.exception( f"Error processing moisture for zone {zone_num} on " f"'{zone_dev.name}' — moisture + moistureForecast " f"states will not update this cycle." )If you want to keep the loop as a hedge against a future handler change that returns extra keys, drop just the
foundflag and inline the else-branch — but at that point the implicit contract is wider than the docstring ofprocess_zone_moistureadvertises.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py around lines 736 - 755, The loop handling forecast_states is over-defensive: remove the unused found flag and the "not found and" guard in the for-loop condition and simply check each entry's key; if entry.get("key") == "moisture" set forecast_val and append the modified entry ({**entry, "key":"moistureForecast"}), otherwise append the entry as-is. This keeps behavior if future process_zone_moisture returns multiple keys but eliminates the unreachable found logic; update the block around zone_handler.process_zone_moisture, forecast_states, and the for entry loop accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py:
- Around line 736-755: The loop handling forecast_states is over-defensive:
remove the unused found flag and the "not found and" guard in the for-loop
condition and simply check each entry's key; if entry.get("key") == "moisture"
set forecast_val and append the modified entry ({**entry,
"key":"moistureForecast"}), otherwise append the entry as-is. This keeps
behavior if future process_zone_moisture returns multiple keys but eliminates
the unreachable found logic; update the block around
zone_handler.process_zone_moisture, forecast_states, and the for entry loop
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d2d51ed-0aa2-485a-9084-900573b871e1
📒 Files selected for processing (10)
Netro Sprinklers.indigoPlugin/Contents/Info.plistNetro Sprinklers.indigoPlugin/Contents/Server Plugin/Devices.xmlNetro Sprinklers.indigoPlugin/Contents/Server Plugin/device_handlers.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.pydocs/API_NOTES.mdtests/conftest.pytests/test_moisture_source_logging.pytests/test_update_zone_devices_integration.pytests/test_whisperer_pairing_callback.pytests/test_zone_moisture_resolution.py
✅ Files skipped from review due to trivial changes (1)
- Netro Sprinklers.indigoPlugin/Contents/Info.plist
🚧 Files skipped from review as they are similar to previous changes (1)
- Netro Sprinklers.indigoPlugin/Contents/Server Plugin/Devices.xml
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.
Closes #54.
Summary
Adds per-zone Whisperer pairing in the plugin so the zone's
moisturestate reflects the actual soil sensor reading (when fresh, ≤ 12h old)
rather than Netro's smart-model daily prediction. Netro's prediction
remains observable on a new
moistureForecaststate.linkedWhispererDeviceIdon zone devices.config, populated by a dynamic
getWhispererDevicescallback.moistureForecast— always holds the raw/moistures.jsonprediction for the zone.moisturesemantics — resolves to the paired Whisperer'ssoilMoisturewhen fresh, else falls back tomoistureForecast.Falls back cleanly when the pairing is missing, disabled, stale,
unparseable, or has no reading yet — each case logged with a
distinct message only on category transitions (no log spam).
Design:
docs/plans/2026-04-23-whisperer-zone-pairing-design.mdImplementation plan:
docs/plans/2026-04-23-whisperer-zone-pairing-plan.mdBuild history
Implemented via TDD with 11 bite-sized commits, then hardened through
a multi-agent code review that caught (a) a production
pluginPropsread-only bug found during live dogfooding on jarvis, (b) a latent
state-write-drop on auxiliary-log failure, (c) five distinct failure
modes collapsing into one misleading
forecast-staletag, and (d)Indigo's integer-default
0sneaking through as a "fresh" sensorreading. All fixed on this branch; no follow-up PR needed.
Test plan
pytest tests/— 488 passed (up from 438 baseline, +50).pylintonplugin.py/utils.py/constants.py/device_handlers.py— 9.48/10 (no regression).2026.5.0 → hotfix → 2026.5.1. Currently running cleanly with
zero errors since last restart.
(value, source)tuple assertions:tests/test_constants_whisperer.pytests/test_reading_age.py(v1 epoch millis + v2 ISO 8601,including Z suffix, garbage input, bool rejection, clock skew)
tests/test_whisperer_pairing_callback.pytests/test_zone_moisture_resolution.py(unpaired, paired+fresh,stale, missing-device, disabled-device, missing-reading,
unparseable-time, readingID liveness, 12h boundary)
tests/test_moisture_source_logging.py(all 5 transitionmessages, cold-start silence, replacePluginPropsOnServer failure
graceful handling)
tests/test_update_zone_devices_integration.py(end-to-endpaired/unpaired × fresh/stale matrix, empty-moistures fallback,
logger-wiring verification)
/moistures.jsonkeeps producing sane predictions when aWhisperer is paired in Netro but stops reporting. Recommended
to dogfood with a disconnected Whisperer for ~1 week before
fully relying on the fallback.
Breaking changes
None. Backward compatible:
linkedWhispererDeviceIdprop →""→unpaired path → identical to pre-PR behaviour (
moistureshowsthe Netro forecast, same as today).
moistureForecaststarts populating on next poll; no databackfill needed.
UiDisplayStateIdstaysmoisture— zone tiles in Indigo keepshowing the "primary" value (now sensor-or-forecast) which is the
right default.
Files changed
Version:
2026.4.4→2026.5.1(minor for the user-visible feature🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores