Per-endpoint polling intervals to reduce API token usage#49
Merged
Conversation
Each Netro device has its own independent 2000/day token limit. The plugin was using a single global counter that got overwritten by every API response, causing incorrect polling pauses. - Add DeviceTokenState dataclass for per-device token tracking - Replace scalar _token_remaining with _device_tokens dict - Add device_key param to make_request(), threaded through all 11 convenience methods - Add should_pause_polling_for() and token_remaining_for() methods - Move pause check from runConcurrentThread into per-device update methods (_update_sprinkler_device, _update_whisperer_device) - Versioned state persistence (v2 format) with v1 migration - Updated all existing token tests + new per-device isolation tests Closes #43 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
V1 throttle was often triggered by incorrect global token counting. Restoring it after migration blocks all API calls until midnight. Only restore throttle_until from v2 format (where it's legitimate). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Set near-future reset time on parse failure to prevent permanent device self-lock (was None → no auto-reset possible) - Cast token_remaining to int() on restore + add TypeError to except to handle corrupted prefs - Per-device try/except in restore loop so one bad timestamp doesn't abort all subsequent devices - Log info when discarding v1 throttle state during migration - Remove unused `field` import from dataclasses - Add boundary test at exactly TOKEN_PAUSE_THRESHOLD - Add v2 throttle restore tests (future and expired) - Add test for malformed device timestamp skipping Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…anged The reading deduplication was skipping ALL state updates when the readingID hadn't changed, including token_remaining and time which change every poll. Now updates metadata states (token_remaining, token_reset, time, last_active) on every poll while still skipping sensor data updates to preserve Indigo's lastChanged timestamp. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Netro's 2000 calls/day budget is shared across all devices on an account. With everything polling at 5-min intervals, the plugin exhausted tokens by mid-afternoon. Now each endpoint has its own configurable interval: - Events: 5 min (fast — online/offline status) - Device info: 10 min (status, firmware, tokens) - Moistures: 10 min (zone moisture levels) - Schedules: 30 min (watering times, rarely change) - Sensors: 30 min (Whisperer readings) - Weather realtime: 30 min (existing, unchanged) - Forecast: 4 hrs (was hardcoded 1 hr, now configurable) Expected daily usage: ~1,752 calls (down from ~4,000+) Also fixes whisperer token_remaining state not updating when sensor reading unchanged (reading dedup was skipping all states). Closes #48 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughReplaced a single global polling interval with independent per-endpoint intervals and migrated API token throttling from a global counter to per-device token budgets. Plugin UI, validators, constants, polling loop, and API client persistence/logic were updated to support per-endpoint timers and per-device rate-limit handling. PluginVersion bumped to 2026.4.1. Changes
Sequence DiagramsequenceDiagram
participant PL as Plugin (poll loop)
participant AC as NetroAPIClient
participant NET as Netro API
participant TB as TokenBudget (prefs / per-device map)
loop Each _loop_interval
PL->>PL: check _next_events_update
alt events due
PL->>AC: get_events(device_key)
AC->>NET: HTTP request
NET-->>AC: response + meta (token_remaining, reset)
AC->>TB: update per-device token state(device_key, meta)
AC->>AC: should_pause_polling_for(device_key)?
alt paused for device
AC-->>PL: skip device updates
else
AC-->>PL: return events
end
end
PL->>PL: check other endpoint timers (deviceInfo, moistures, schedules, sensors, forecast)
PL->>PL: advance/reset next-update timers
PL->>PL: sleep(_loop_interval)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 8
🧹 Nitpick comments (3)
tests/test_base_modules.py (1)
145-147: Also assert the primary forecast default here.This only locks the compatibility alias. If
DEFAULT_FORECAST_INTERVAL_MINUTESdrifts fromFORECAST_UPDATE_INTERVAL_MINUTES, the scheduler can change while this test stays green.Possible test update
- """FORECAST_UPDATE_INTERVAL_MINUTES should be 4 hours (legacy constant).""" + """Forecast defaults should stay aligned at 4 hours.""" + assert DEFAULT_FORECAST_INTERVAL_MINUTES == 240 assert FORECAST_UPDATE_INTERVAL_MINUTES == 240Also add
DEFAULT_FORECAST_INTERVAL_MINUTESto the constants import list at the top of the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_base_modules.py` around lines 145 - 147, Update the test_forecast_update_interval test to also assert that DEFAULT_FORECAST_INTERVAL_MINUTES equals 240 (the same legacy value) so the primary default cannot drift from the compatibility alias, and add DEFAULT_FORECAST_INTERVAL_MINUTES to the constants import list at the top of tests/test_base_modules.py; modify only the test_forecast_update_interval function and the imports to include the new constant.tests/test_validators.py (1)
443-470: CoverweatherUpdateIntervalandforecastIntervalin the prefs happy-path tests.
validate_prefs_config()now sanitizes both fields, but these assertions never touch them. A field-id/default mismatch betweenPluginConfig.xmlandvalidators.pywould still pass.Possible test update
values = { "eventsInterval": "5", "deviceInfoInterval": "10", "moisturesInterval": "10", "schedulesInterval": "30", "sensorInterval": "30", + "weatherUpdateInterval": "30", + "forecastInterval": "240", "apiTimeout": "30", "maxZoneRunTime": "3600", } @@ assert sanitized["eventsInterval"] == 5 assert sanitized["deviceInfoInterval"] == 10 + assert sanitized["weatherUpdateInterval"] == 30 + assert sanitized["forecastInterval"] == 240 assert sanitized["apiTimeout"] == 30 assert sanitized["maxZoneRunTime"] == 3600 @@ assert sanitized["eventsInterval"] == 5 assert sanitized["deviceInfoInterval"] == 10 assert sanitized["schedulesInterval"] == 30 + assert sanitized["weatherUpdateInterval"] == 30 + assert sanitized["forecastInterval"] == 240 assert sanitized["apiTimeout"] == 5 assert sanitized["maxZoneRunTime"] == 3600🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_validators.py` around lines 443 - 470, The tests for validate_prefs_config are missing assertions for the newly-sanitized fields weatherUpdateInterval and forecastInterval; update test_valid_prefs to assert sanitized["weatherUpdateInterval"] and sanitized["forecastInterval"] equal their expected numeric values (converted from the input strings), and update test_valid_prefs_defaults to assert those keys equal their default numeric values so the happy-path covers both fields and catches any field-id/default mismatches between PluginConfig.xml and validators.py.Netro Sprinklers.indigoPlugin/Contents/Server Plugin/validators.py (1)
590-635: Pull the interval defaults fromconstants.pyinstead of repeating literals.
_PREFS_FIELDSnow duplicates the same5/10/10/30/30/30/240values that already exist asDEFAULT_*_INTERVAL_MINUTES. Keeping one source of truth will prevent the UI, validation, and call-budget math from drifting on the next tuning pass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/validators.py around lines 590 - 635, _PREFERENCES default interval literals in _PREFS_FIELDS should be replaced with the corresponding DEFAULT_*_INTERVAL_MINUTES constants from constants.py to avoid duplicating the single source of truth; update the fourth argument of each PrefsFieldSpec call (for "eventsInterval", "deviceInfoInterval", "moisturesInterval", "schedulesInterval", "sensorInterval", "apiTimeout", "maxZoneRunTime", "weatherUpdateInterval", and "forecastInterval") to use the matching DEFAULT_EVENTS_INTERVAL_MINUTES, DEFAULT_DEVICE_INFO_INTERVAL_MINUTES, DEFAULT_MOISTURES_INTERVAL_MINUTES, DEFAULT_SCHEDULES_INTERVAL_MINUTES, DEFAULT_SENSOR_INTERVAL_MINUTES, DEFAULT_API_TIMEOUT_SECONDS (or equivalent DEFAULT_API_TIMEOUT), DEFAULT_MAX_ZONE_RUN_TIME_SECONDS (or equivalent), DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES, and DEFAULT_FORECAST_INTERVAL_MINUTES constants from constants.py and import them if not already imported so UI, validation, and call-budget math all reference the same canonical defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py:
- Around line 498-509: The code serializes raw API keys (device_key) as map keys
when writing self._device_tokens into the saved state; change the persistence to
use a non-secret stable identifier by transforming keys with the existing helper
_state_key(device_key) (or introduce one) when building device_tokens and when
reading it back, and update all reads/writes of self._device_tokens (and any
warning/logging that currently prints device_key) to use that same
_state_key(...) identifier so secrets are never written to throttle_state or
logs and token history remains decoupled from key rotation.
- Around line 58-65: Polling is being gated per-device using
should_pause_polling_for(key) which checks DeviceTokenState, but the quota is
account-wide; replace those per-device checks in the polling loops with the API
client's account-wide gate by calling the api client's should_pause_polling
property (e.g., use api_client.should_pause_polling) and pause the loop when
that property is True; keep DeviceTokenState as-is for tracking per-device usage
but ensure all poll entry points consult api_client.should_pause_polling rather
than should_pause_polling_for.
- Around line 455-466: The rate-limit reset parsing in _update_token_state and
_handle_http_error uses datetime.fromisoformat(...).replace(tzinfo=timezone.utc)
which corrupts offset-aware timestamps; change the logic to parse the ISO string
with datetime.fromisoformat(...) then if the resulting dt.tzinfo is not None
call dt = dt.astimezone(timezone.utc) to convert the instant to UTC, otherwise
set dt = dt.replace(tzinfo=timezone.utc) (or treat naive values as UTC) and use
that dt for reset_time/ calculations; update both _update_token_state and
_handle_http_error to follow this pattern so offset-aware timestamps are
correctly converted instead of having their instant shifted.
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py:
- Around line 779-783: The current code replaces self.person["devices"] with a
single device on each poll, breaking multi-controller lookups in
_get_device_dict() and _get_zone_dict(); instead, find the device by serial
(netro_serial from device_data.get("serial", dev.address")) in
self.person["devices"] and update that entry in-place (or append if not
present), then set self.netro_devices = self.person["devices"] so the full
cached list is preserved across polls. Ensure you update the existing device
dict (merge/update keys) rather than reassigning the whole list.
- Around line 791-837: The schedule and moisture refresh logic is nested inside
the device-info branch so their intervals (self._next_schedules_update,
self._next_moistures_update) never fire independently; pull the schedule and
moisture fetch/processing out of the device-info-only path so they run based
solely on their own timers: call self.api_client.get_schedules and
sprinkler_handler.process_schedules when now >= self._next_schedules_update
regardless of device-info, and call self.api_client.get_moistures when now >=
self._next_moistures_update regardless of device-info; keep existing error
handling/logging and ensure the extracted zones_data is available to
_ensure_zone_devices and _update_zone_devices (or guard those calls if
zones_data is not present).
- Around line 773-774: The scheduled-timer guard (the if now >=
self._next_device_info_update check around api_client.get_device_info) prevents
manual refreshes from reaching Netro; modify _update_from_netro to accept a
force=False parameter and make the timer gate conditional on not force (i.e.,
only skip Netro calls when not force and the deadline hasn't elapsed), then
update manual entry points RequestStatus and updateAllStatus to call
_update_from_netro(force=True) so manual refreshes bypass the timer; also apply
the same change to the other guarded call locations around the
api_client.get_device_info checks (the instance noted at the 893-894 region).
- Around line 113-118: The _loop_interval calculation currently takes the min of
Netro endpoint timers but omits the weather and forecast cadences; update the
min(...) used when setting _loop_interval to also include the configured
weatherUpdateInterval and forecastInterval (or their internal names, e.g.
_weather_interval and _forecast_interval) so those updates can fire on time, and
apply the same change to every place where _loop_interval is computed/recomputed
(initial assignment and the other recalculation sites that reference
_loop_interval). Also guard against None/0 by treating missing intervals as a
large value (or validate them before taking min) so they don't incorrectly
shorten the interval.
- Around line 1198-1220: The code updates interval attributes (interval_fields
-> _events_interval, _device_info_interval, etc.) but doesn’t re-arm the
corresponding next-run timestamps (the _next_*_update attributes), so shortening
an interval won't trigger sooner; when you detect a change in setattr inside the
loop over interval_fields, also set the matching _next_{field_name}_update
attribute to now + new_interval_in_seconds (use time.time() or equivalent) so
the scheduler recognizes the new deadline; map
"eventsInterval"->"_next_events_update",
"deviceInfoInterval"->"_next_device_info_update",
"moisturesInterval"->"_next_moistures_update",
"schedulesInterval"->"_next_schedules_update",
"sensorInterval"->"_next_sensor_update" and then recompute self._loop_interval
as you already do.
---
Nitpick comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/validators.py:
- Around line 590-635: _PREFERENCES default interval literals in _PREFS_FIELDS
should be replaced with the corresponding DEFAULT_*_INTERVAL_MINUTES constants
from constants.py to avoid duplicating the single source of truth; update the
fourth argument of each PrefsFieldSpec call (for "eventsInterval",
"deviceInfoInterval", "moisturesInterval", "schedulesInterval",
"sensorInterval", "apiTimeout", "maxZoneRunTime", "weatherUpdateInterval", and
"forecastInterval") to use the matching DEFAULT_EVENTS_INTERVAL_MINUTES,
DEFAULT_DEVICE_INFO_INTERVAL_MINUTES, DEFAULT_MOISTURES_INTERVAL_MINUTES,
DEFAULT_SCHEDULES_INTERVAL_MINUTES, DEFAULT_SENSOR_INTERVAL_MINUTES,
DEFAULT_API_TIMEOUT_SECONDS (or equivalent DEFAULT_API_TIMEOUT),
DEFAULT_MAX_ZONE_RUN_TIME_SECONDS (or equivalent),
DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES, and DEFAULT_FORECAST_INTERVAL_MINUTES
constants from constants.py and import them if not already imported so UI,
validation, and call-budget math all reference the same canonical defaults.
In `@tests/test_base_modules.py`:
- Around line 145-147: Update the test_forecast_update_interval test to also
assert that DEFAULT_FORECAST_INTERVAL_MINUTES equals 240 (the same legacy value)
so the primary default cannot drift from the compatibility alias, and add
DEFAULT_FORECAST_INTERVAL_MINUTES to the constants import list at the top of
tests/test_base_modules.py; modify only the test_forecast_update_interval
function and the imports to include the new constant.
In `@tests/test_validators.py`:
- Around line 443-470: The tests for validate_prefs_config are missing
assertions for the newly-sanitized fields weatherUpdateInterval and
forecastInterval; update test_valid_prefs to assert
sanitized["weatherUpdateInterval"] and sanitized["forecastInterval"] equal their
expected numeric values (converted from the input strings), and update
test_valid_prefs_defaults to assert those keys equal their default numeric
values so the happy-path covers both fields and catches any field-id/default
mismatches between PluginConfig.xml and validators.py.
🪄 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: 02ccc461-3758-4abf-a73d-ba5718f5d86a
📒 Files selected for processing (9)
Netro Sprinklers.indigoPlugin/Contents/Info.plistNetro Sprinklers.indigoPlugin/Contents/Server Plugin/PluginConfig.xmlNetro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/constants.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/validators.pytests/test_api_client.pytests/test_base_modules.pytests/test_validators.py
Comment on lines
+58
to
+65
| @dataclass | ||
| class DeviceTokenState: | ||
| """Per-device API token budget tracking. | ||
|
|
||
| Each Netro device has its own independent 2000/day token limit. | ||
| """ | ||
| token_remaining: int = 2000 | ||
| token_reset: Optional[datetime] = None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 '\bshould_pause_polling_for\s*\(' --type=py
rg -n -C2 '\btoken_remaining_for\s*\(' --type=py
rg -n -C2 '\bshould_pause_polling\b' --type=pyRepository: simons-plugins/netro-indigo
Length of output: 10053
Polling gates on per-device token snapshots instead of account-wide budget minimum.
The polling paths at plugin.py lines 763 and 901 call should_pause_polling_for(key) to gate individual device polls, but the API token quota is shared account-wide (~2000 calls/day across all devices). This allows one device to keep polling on a stale snapshot after another device has consumed the shared budget below the pause threshold.
The API client provides should_pause_polling property (which returns True if any device is below threshold) for account-wide gating, but the polling loops use the per-device variant instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py around
lines 58 - 65, Polling is being gated per-device using
should_pause_polling_for(key) which checks DeviceTokenState, but the quota is
account-wide; replace those per-device checks in the polling loops with the API
client's account-wide gate by calling the api client's should_pause_polling
property (e.g., use api_client.should_pause_polling) and pause the loop when
that property is True; keep DeviceTokenState as-is for tracking per-device usage
but ensure all poll entry points consult api_client.should_pause_polling rather
than should_pause_polling_for.
Comment on lines
455
to
+466
| try: | ||
| self._token_remaining = int(meta.get("token_remaining", 2000)) | ||
| remaining = int(meta.get("token_remaining", 2000)) | ||
| reset_time = None | ||
| reset_str = meta.get("token_reset", "") | ||
| if reset_str: | ||
| self._token_reset = datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc) | ||
| reset_time = datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc) | ||
| except (ValueError, TypeError) as exc: | ||
| self.logger.warning(f"Could not parse token info from response: {exc}") | ||
| # Set safe default to trigger proactive pause and prevent exhausting token budget | ||
| self._token_remaining = TOKEN_PAUSE_THRESHOLD - 1 | ||
| remaining = TOKEN_PAUSE_THRESHOLD - 1 | ||
| # Set near-future reset so auto-reset can unlock this device | ||
| reset_time = datetime.now(timezone.utc) + timedelta(hours=1) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from datetime import datetime, timezone
s = "2026-04-11T00:00:00+02:00"
dt = datetime.fromisoformat(s)
print("original:", dt.isoformat())
print("replace :", dt.replace(tzinfo=timezone.utc).isoformat())
print("convert :", dt.astimezone(timezone.utc).isoformat())
PYRepository: simons-plugins/netro-indigo
Length of output: 178
🏁 Script executed:
cd "Netro Sprinklers.indigoPlugin/Contents/Server Plugin" && wc -l api_client.pyRepository: simons-plugins/netro-indigo
Length of output: 88
🏁 Script executed:
cd "Netro Sprinklers.indigoPlugin/Contents/Server Plugin" && sed -n '450,470p' api_client.pyRepository: simons-plugins/netro-indigo
Length of output: 996
🏁 Script executed:
cd "Netro Sprinklers.indigoPlugin/Contents/Server Plugin" && rg "_handle_http_error" api_client.py -A 30Repository: simons-plugins/netro-indigo
Length of output: 1534
🏁 Script executed:
cd "Netro Sprinklers.indigoPlugin/Contents/Server Plugin" && rg "fromisoformat.*replace.*tzinfo" api_client.pyRepository: simons-plugins/netro-indigo
Length of output: 268
🏁 Script executed:
cd "Netro Sprinklers.indigoPlugin/Contents/Server Plugin" && rg "_handle_http_error" api_client.py -B 5 -A 50 | head -100Repository: simons-plugins/netro-indigo
Length of output: 2892
🏁 Script executed:
cd "Netro Sprinklers.indigoPlugin/Contents/Server Plugin" && rg "fromisoformat" api_client.py -nRepository: simons-plugins/netro-indigo
Length of output: 457
🏁 Script executed:
cd "Netro Sprinklers.indigoPlugin/Contents/Server Plugin" && sed -n '540,565p' api_client.pyRepository: simons-plugins/netro-indigo
Length of output: 1672
Fix timezone handling in rate limit reset time parsing.
datetime.fromisoformat(...).replace(tzinfo=timezone.utc) incorrectly handles offset-aware timestamps. When the API returns a timezone-aware value, replace() changes the instant instead of converting it, causing reset time calculations to drift by the offset amount. This pattern occurs in both _update_token_state() (line 460) and _handle_http_error() (line 408).
Correct approach
-reset_time = datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc)
+reset_time = datetime.fromisoformat(reset_str)
+if reset_time.tzinfo is None:
+ reset_time = reset_time.replace(tzinfo=timezone.utc)
+else:
+ reset_time = reset_time.astimezone(timezone.utc)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py around
lines 455 - 466, The rate-limit reset parsing in _update_token_state and
_handle_http_error uses datetime.fromisoformat(...).replace(tzinfo=timezone.utc)
which corrupts offset-aware timestamps; change the logic to parse the ISO string
with datetime.fromisoformat(...) then if the resulting dt.tzinfo is not None
call dt = dt.astimezone(timezone.utc) to convert the instant to UTC, otherwise
set dt = dt.replace(tzinfo=timezone.utc) (or treat naive values as UTC) and use
that dt for reset_time/ calculations; update both _update_token_state and
_handle_http_error to follow this pattern so offset-aware timestamps are
correctly converted instead of having their instant shifted.
Comment on lines
+498
to
509
| device_tokens = {} | ||
| for key, state in self._device_tokens.items(): | ||
| device_tokens[key] = { | ||
| "token_remaining": state.token_remaining, | ||
| "token_reset": state.token_reset.isoformat() if state.token_reset else None, | ||
| } | ||
|
|
||
| save_state = { | ||
| "version": 2, | ||
| "throttle_until": self._throttle_until.isoformat() if self._throttle_until else None, | ||
| "token_remaining": self._token_remaining, | ||
| "token_reset": self._token_reset.isoformat() if self._token_reset else None, | ||
| "device_tokens": device_tokens, | ||
| "last_saved": datetime.now(timezone.utc).isoformat() |
There was a problem hiding this comment.
Don't persist raw API keys inside throttle_state.
device_key is the API key or serial number, and this JSON uses it verbatim as the map key. That writes v2 credentials into pluginPrefs/backups and also couples token history to key rotation. Persist a non-secret stable identifier instead, and use that same identifier in warning logs.
Possible direction
+import hashlib
@@
+ `@staticmethod`
+ def _state_key(device_key: str) -> str:
+ return hashlib.sha256(device_key.encode("utf-8")).hexdigest()Then use _state_key(device_key) everywhere _device_tokens is read or written, or pass a non-secret Indigo device identifier into this layer instead of the credential.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py around
lines 498 - 509, The code serializes raw API keys (device_key) as map keys when
writing self._device_tokens into the saved state; change the persistence to use
a non-secret stable identifier by transforming keys with the existing helper
_state_key(device_key) (or introduce one) when building device_tokens and when
reading it back, and update all reads/writes of self._device_tokens (and any
warning/logging that currently prints device_key) to use that same
_state_key(...) identifier so secrets are never written to throttle_state or
logs and token history remains decoupled from key rotation.
Comment on lines
+113
to
+118
| # Main loop sleep uses the shortest interval so fast endpoints fire on time | ||
| self._loop_interval = min( | ||
| self._events_interval, self._device_info_interval, | ||
| self._moistures_interval, self._schedules_interval, | ||
| self._sensor_interval | ||
| ) |
There was a problem hiding this comment.
_loop_interval still ignores weather and forecast cadence.
The main sleep interval is computed from the Netro endpoint timers only. If weatherUpdateInterval or forecastInterval is configured below those values, the loop still sleeps too long and those updates run later than requested.
Also applies to: 141-149, 1215-1220
🤖 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
113 - 118, The _loop_interval calculation currently takes the min of Netro
endpoint timers but omits the weather and forecast cadences; update the min(...)
used when setting _loop_interval to also include the configured
weatherUpdateInterval and forecastInterval (or their internal names, e.g.
_weather_interval and _forecast_interval) so those updates can fire on time, and
apply the same change to every place where _loop_interval is computed/recomputed
(initial assignment and the other recalculation sites that reference
_loop_interval). Also guard against None/0 by treating missing intervals as a
large value (or validate them before taking min) so they don't incorrectly
shorten the interval.
Comment on lines
+773
to
+774
| if now >= self._next_device_info_update: | ||
| reply_dict = self.api_client.get_device_info(key, api_version=api_version) |
There was a problem hiding this comment.
Timer gates now block the manual refresh paths.
RequestStatus and updateAllStatus() still call _update_from_netro() directly, but these new guards skip the Netro calls unless the scheduled deadline has already elapsed. That turns an on-demand refresh into a no-op most of the time.
Suggested direction
-def _update_from_netro(self):
+def _update_from_netro(self, force=False):
...
- self._update_sprinkler_device(dev)
+ self._update_sprinkler_device(dev, force=force)
...
- self._update_whisperer_device(dev)
+ self._update_whisperer_device(dev, force=force)
-def _update_sprinkler_device(self, dev):
+def _update_sprinkler_device(self, dev, force=False):
...
- if now >= self._next_device_info_update:
+ if force or now >= self._next_device_info_update:
-def _update_whisperer_device(self, dev):
- if datetime.now() < self._next_sensor_update:
+def _update_whisperer_device(self, dev, force=False):
+ if not force and datetime.now() < self._next_sensor_update:
returnThen call _update_from_netro(force=True) from the manual refresh entry points.
Also applies to: 893-894
🤖 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
773 - 774, The scheduled-timer guard (the if now >=
self._next_device_info_update check around api_client.get_device_info) prevents
manual refreshes from reaching Netro; modify _update_from_netro to accept a
force=False parameter and make the timer gate conditional on not force (i.e.,
only skip Netro calls when not force and the deadline hasn't elapsed), then
update manual entry points RequestStatus and updateAllStatus to call
_update_from_netro(force=True) so manual refreshes bypass the timer; also apply
the same change to the other guarded call locations around the
api_client.get_device_info checks (the instance noted at the 893-894 region).
Comment on lines
+779
to
+783
| # Update person/netro_devices for legacy compatibility | ||
| netro_serial = device_data.get("serial", dev.address) | ||
| device_data["id"] = netro_serial | ||
| self.person = {"id": netro_serial, "devices": [device_data]} | ||
| self.netro_devices = self.person["devices"] |
There was a problem hiding this comment.
Don't collapse the cached device list to a single controller.
This rewrites self.person["devices"] on every sprinkler poll, so _get_device_dict() / _get_zone_dict() can only see the last-updated controller. In a multi-sprinkler install, zone actions and other cached-device lookups will start failing for the earlier devices after the next poll.
🤖 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
779 - 783, The current code replaces self.person["devices"] with a single device
on each poll, breaking multi-controller lookups in _get_device_dict() and
_get_zone_dict(); instead, find the device by serial (netro_serial from
device_data.get("serial", dev.address")) in self.person["devices"] and update
that entry in-place (or append if not present), then set self.netro_devices =
self.person["devices"] so the full cached list is preserved across polls. Ensure
you update the existing device dict (merge/update keys) rather than reassigning
the whole list.
Comment on lines
+791
to
+837
| # --- Schedules (only when info also fires) --- | ||
| schedule_dict = None | ||
| active_schedule_name = None | ||
| if now >= self._next_schedules_update: | ||
| try: | ||
| schedule_dict = self.api_client.get_schedules(key, api_version=api_version) | ||
| schedule_states, active_schedule_name = self.sprinkler_handler.process_schedules( | ||
| schedule_dict, api_version=api_version | ||
| ) | ||
| update_list.extend(schedule_states) | ||
| except Exception: | ||
| update_list.append( | ||
| {"key": "activeSchedule", "value": "Error getting current schedule"}) | ||
| self.logger.debug(f"API error: \n{traceback.format_exc(10)}") | ||
| self._fireTrigger("getScheduleCall") | ||
|
|
||
| # Update zone information (properties, not states) | ||
| zone_names, max_durations, zones_data = self.sprinkler_handler.extract_zone_info( | ||
| device_data, self.maxZoneRunTime | ||
| ) | ||
| props = copy.deepcopy(dev.pluginProps) | ||
| props["NumZones"] = len(device_data.get("zones", [])) | ||
| props["ZoneNames"] = zone_names | ||
| props["MaxZoneDurations"] = ", ".join(max_durations) | ||
| props["zones"] = json.dumps(zones_data) | ||
| if active_schedule_name: | ||
| props["ScheduledZoneDurations"] = active_schedule_name | ||
| dev.replacePluginPropsOnServer(props) | ||
|
|
||
| # Fetch moisture levels (used by zone devices below) | ||
| try: | ||
| moisture_dict = self.api_client.get_moistures(key, api_version=api_version) | ||
| except Exception: | ||
| self.logger.warning(f"Moisture API unavailable for '{dev.name}' - zone moisture states may be stale") | ||
| self.logger.debug(f"Moisture API error: \n{traceback.format_exc(10)}") | ||
| # Send state updates | ||
| if update_list: | ||
| dev.updateStatesOnServer(update_list) | ||
|
|
||
| # Auto-create and update zone devices | ||
| self._ensure_zone_devices(dev, zones_data) | ||
| self._update_zone_devices( | ||
| dev, device_data, schedule_dict, moisture_dict, api_version | ||
| ) | ||
| # Update zone information (properties, not states) | ||
| zone_names, max_durations, zones_data = self.sprinkler_handler.extract_zone_info( | ||
| device_data, self.maxZoneRunTime | ||
| ) | ||
| props = copy.deepcopy(dev.pluginProps) | ||
| props["NumZones"] = len(device_data.get("zones", [])) | ||
| props["ZoneNames"] = zone_names | ||
| props["MaxZoneDurations"] = ", ".join(max_durations) | ||
| props["zones"] = json.dumps(zones_data) | ||
| if active_schedule_name: | ||
| props["ScheduledZoneDurations"] = active_schedule_name | ||
| dev.replacePluginPropsOnServer(props) | ||
|
|
||
| # Ensure Indigo variables exist for each zone (for variable substitution) | ||
| # Must be after replacePluginPropsOnServer to avoid props overwrite race | ||
| self._ensure_zone_variables(dev, zones_data) | ||
| # --- Moistures (only when info also fires) --- | ||
| moisture_dict = None | ||
| if now >= self._next_moistures_update: | ||
| try: | ||
| moisture_dict = self.api_client.get_moistures(key, api_version=api_version) | ||
| except Exception: | ||
| self.logger.warning(f"Moisture API unavailable for '{dev.name}' - zone moisture states may be stale") | ||
| self.logger.debug(f"Moisture API error: \n{traceback.format_exc(10)}") | ||
|
|
||
| # Update zone devices (uses info + schedule + moisture data) | ||
| self._ensure_zone_devices(dev, zones_data) | ||
| self._update_zone_devices( | ||
| dev, device_data, schedule_dict, moisture_dict, api_version | ||
| ) |
There was a problem hiding this comment.
Schedules and moistures are still effectively tied to deviceInfoInterval.
These calls only run inside the device-info branch, so a shorter schedulesInterval or moisturesInterval is ignored. For example, deviceInfoInterval=30 and schedulesInterval=5 still yields 30-minute schedule refreshes, which defeats the new per-endpoint polling behavior.
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 801-801: Do not catch blind exception: Exception
(BLE001)
[warning] 829-829: Do not catch blind exception: Exception
(BLE001)
🤖 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
791 - 837, The schedule and moisture refresh logic is nested inside the
device-info branch so their intervals (self._next_schedules_update,
self._next_moistures_update) never fire independently; pull the schedule and
moisture fetch/processing out of the device-info-only path so they run based
solely on their own timers: call self.api_client.get_schedules and
sprinkler_handler.process_schedules when now >= self._next_schedules_update
regardless of device-info, and call self.api_client.get_moistures when now >=
self._next_moistures_update regardless of device-info; keep existing error
handling/logging and ensure the extracted zones_data is available to
_ensure_zone_devices and _update_zone_devices (or guard those calls if
zones_data is not present).
Comment on lines
+1198
to
+1220
| # Update per-endpoint polling intervals | ||
| interval_fields = { | ||
| "eventsInterval": ("_events_interval", DEFAULT_EVENTS_INTERVAL_MINUTES), | ||
| "deviceInfoInterval": ("_device_info_interval", DEFAULT_DEVICE_INFO_INTERVAL_MINUTES), | ||
| "moisturesInterval": ("_moistures_interval", DEFAULT_MOISTURES_INTERVAL_MINUTES), | ||
| "schedulesInterval": ("_schedules_interval", DEFAULT_SCHEDULES_INTERVAL_MINUTES), | ||
| "sensorInterval": ("_sensor_interval", DEFAULT_SENSOR_INTERVAL_MINUTES), | ||
| } | ||
| for field_id, (attr_name, default) in interval_fields.items(): | ||
| try: | ||
| new_val = int(valuesDict.get(field_id, default)) | ||
| if new_val != getattr(self, attr_name): | ||
| setattr(self, attr_name, new_val) | ||
| self.logger.info(f"{field_id} updated to {new_val} minutes") | ||
| except (ValueError, TypeError): | ||
| self.logger.warning(f"Invalid {field_id} value, keeping existing setting") | ||
|
|
||
| # Recalculate main loop sleep interval | ||
| self._loop_interval = min( | ||
| self._events_interval, self._device_info_interval, | ||
| self._moistures_interval, self._schedules_interval, | ||
| self._sensor_interval | ||
| ) |
There was a problem hiding this comment.
Updated intervals won't take effect until the old deadlines expire.
This changes the numeric settings, but it never re-arms the corresponding _next_*_update timestamps. Shortening a poll interval from something large to something small can still wait nearly the entire old interval before the first poll under the new setting.
Suggested fix
- interval_fields = {
- "eventsInterval": ("_events_interval", DEFAULT_EVENTS_INTERVAL_MINUTES),
- "deviceInfoInterval": ("_device_info_interval", DEFAULT_DEVICE_INFO_INTERVAL_MINUTES),
- "moisturesInterval": ("_moistures_interval", DEFAULT_MOISTURES_INTERVAL_MINUTES),
- "schedulesInterval": ("_schedules_interval", DEFAULT_SCHEDULES_INTERVAL_MINUTES),
- "sensorInterval": ("_sensor_interval", DEFAULT_SENSOR_INTERVAL_MINUTES),
+ interval_fields = {
+ "eventsInterval": ("_events_interval", "_next_events_update", DEFAULT_EVENTS_INTERVAL_MINUTES),
+ "deviceInfoInterval": ("_device_info_interval", "_next_device_info_update", DEFAULT_DEVICE_INFO_INTERVAL_MINUTES),
+ "moisturesInterval": ("_moistures_interval", "_next_moistures_update", DEFAULT_MOISTURES_INTERVAL_MINUTES),
+ "schedulesInterval": ("_schedules_interval", "_next_schedules_update", DEFAULT_SCHEDULES_INTERVAL_MINUTES),
+ "sensorInterval": ("_sensor_interval", "_next_sensor_update", DEFAULT_SENSOR_INTERVAL_MINUTES),
}
- for field_id, (attr_name, default) in interval_fields.items():
+ for field_id, (attr_name, next_attr, default) in interval_fields.items():
try:
new_val = int(valuesDict.get(field_id, default))
if new_val != getattr(self, attr_name):
setattr(self, attr_name, new_val)
+ setattr(self, next_attr, datetime.now())
self.logger.info(f"{field_id} updated to {new_val} minutes")🤖 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
1198 - 1220, The code updates interval attributes (interval_fields ->
_events_interval, _device_info_interval, etc.) but doesn’t re-arm the
corresponding next-run timestamps (the _next_*_update attributes), so shortening
an interval won't trigger sooner; when you detect a change in setattr inside the
loop over interval_fields, also set the matching _next_{field_name}_update
attribute to now + new_interval_in_seconds (use time.time() or equivalent) so
the scheduler recognizes the new deadline; map
"eventsInterval"->"_next_events_update",
"deviceInfoInterval"->"_next_device_info_update",
"moisturesInterval"->"_next_moistures_update",
"schedulesInterval"->"_next_schedules_update",
"sensorInterval"->"_next_sensor_update" and then recompute self._loop_interval
as you already do.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove dead MINIMUM_POLLING_INTERVAL_MINUTES imports (plugin.py, validators.py) and update stale docstring referencing removed pollingInterval attribute - Seed new per-endpoint fields from legacy pollingInterval on first load so upgrading users don't silently jump to more aggressive defaults (uses max(seed, default) to respect minimum intervals) - Document schedules/moistures coupling to device_info interval in PluginConfig.xml help text (they nest inside the device_info guard since zone updates need fresh device_data) - Promote schedule/moisture fetch exceptions from DEBUG-only traceback to WARNING with exception type + message so operators can diagnose without flipping debug on - Expand token parse-failure warning to state the pause consequence and include device key + reset time — was silent about pausing - Add device-scoped debug log when ThrottleDelayError aborts a sprinkler update cycle mid-flight (replaces bare `pass`) - Add tests for parse-failure near-future reset time and the new warning message content Bump PluginVersion to 2026.4.1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py (2)
455-460:⚠️ Potential issue | 🟠 MajorConvert offset-aware
token_resetvalues instead of replacingtzinfo.
datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc)changes the instant when Netro returns an offset-aware timestamp. Parse first, thenastimezone(timezone.utc)for aware values and onlyreplace()naive ones;_handle_http_error()still needs the same fix.🩹 Safer UTC parsing
reset_str = meta.get("token_reset", "") if reset_str: - reset_time = datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc) + reset_time = datetime.fromisoformat(reset_str) + if reset_time.tzinfo is None: + reset_time = reset_time.replace(tzinfo=timezone.utc) + else: + reset_time = reset_time.astimezone(timezone.utc)#!/bin/bash set -euo pipefail # Find every site still using replace(tzinfo=timezone.utc) after fromisoformat(). rg -n 'fromisoformat\(.*\)\.replace\(tzinfo=timezone\.utc\)' --type=py # Demonstrate why replace() is wrong for offset-aware timestamps. python - <<'PY' from datetime import datetime, timezone s = "2026-04-11T00:00:00+02:00" dt = datetime.fromisoformat(s) print("replace :", dt.replace(tzinfo=timezone.utc).isoformat()) print("convert :", dt.astimezone(timezone.utc).isoformat()) PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py around lines 455 - 460, The code incorrectly forces offset-aware timestamps to a different instant by calling datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc); instead parse into dt = datetime.fromisoformat(reset_str) and then if dt.tzinfo is None do reset_time = dt.replace(tzinfo=timezone.utc) else do reset_time = dt.astimezone(timezone.utc) so aware values are converted properly; apply the same change in the _handle_http_error() path as well (look for the token_reset parsing/replace usage).
503-514:⚠️ Potential issue | 🟠 MajorDon't serialize raw device credentials into
throttle_state.
device_tokensuses the API key / serial as the persisted JSON key, sopluginPrefs, backups, and the related warning paths now retain credential material. Persist a non-secret stable identifier instead and use that same identifier in logs.Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py (6)
812-816:⚠️ Potential issue | 🔴 CriticalDon't collapse the cached controller list to a single device.
This overwrites
self.person["devices"]on every sprinkler poll, so_get_device_dict()/_get_zone_dict()lose earlier controllers after the next update. Multi-controller installs will start failing zone lookups for the controllers that were polled earlier.🩹 Preserve the cached list
- netro_serial = device_data.get("serial", dev.address) - device_data["id"] = netro_serial - self.person = {"id": netro_serial, "devices": [device_data]} - self.netro_devices = self.person["devices"] + netro_serial = device_data.get("serial", dev.address) + current_device = {**device_data, "id": netro_serial} + devices = list(self.person.get("devices", [])) + + for idx, existing in enumerate(devices): + if existing.get("id") == netro_serial: + devices[idx] = {**existing, **current_device} + break + else: + devices.append(current_device) + + self.person["devices"] = devices + self.person.setdefault("id", netro_serial) + self.netro_devices = devices🤖 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 812 - 816, The code currently overwrites self.person["devices"] and self.netro_devices with a single device each poll (in the block that sets netro_serial, device_data["id"], self.person and self.netro_devices), which loses previously cached controllers and breaks _get_device_dict()/_get_zone_dict(); instead, preserve and merge into the cached devices list: locate the block that assigns self.person = {"id": ..., "devices": [device_data]} and change it to update or append device_data into the existing self.person["devices"] (match by id/serial), replacing the existing entry if present or appending if new, then set self.netro_devices = self.person["devices"] so multi-controller installs retain previously discovered controllers.
146-151:⚠️ Potential issue | 🟠 Major
_loop_intervalstill ignores weather and forecast cadence.The main sleep interval is still derived from Netro timers only. If
weatherUpdateIntervalorforecastIntervalis set below those values, the loop sleeps too long and Tomorrow.io updates run late.Also applies to: 1257-1262
🤖 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 146 - 151, The loop interval calculation for self._loop_interval currently takes the min of Netro-only timers (_events_interval, _device_info_interval, _moistures_interval, _schedules_interval, _sensor_interval) but omits the weather/forecast cadence, causing Tomorrow.io updates to run late when weatherUpdateInterval or forecastInterval is shorter; update the min(...) call to also include the weather/forecast intervals (the attributes or internal variables representing them—e.g. weatherUpdateInterval/forecastInterval or their internal counterparts like _weather_interval and _forecast_interval) so the main loop sleeps for the shortest of all timers and triggers those updates on time.
824-877:⚠️ Potential issue | 🟠 Major
schedulesIntervalandmoisturesIntervalstill aren't independent.Both fetches are nested under the device-info timer, while the main loop advances
_next_schedules_update/_next_moistures_updateunconditionally. A shorter schedules/moistures cadence is therefore skipped until the next device-info cycle instead of firing on time.Also applies to: 1099-1104
🤖 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 824 - 877, The schedule and moisture fetches are incorrectly gated by the device-info timing block so shorter schedulesInterval/moisturesInterval values get skipped; update the logic so each uses its own timer check (compare now to self._next_schedules_update and self._next_moistures_update) and calls self.api_client.get_schedules / self.api_client.get_moistures independently of the device-info block, processing results with sprinkler_handler.process_schedules and updating props/states as needed; ensure exceptions still set warnings/debug logs and that schedule_dict/moisture_dict remain available for the subsequent calls to _ensure_zone_devices and _update_zone_devices so those functions receive the latest data.
795-801:⚠️ Potential issue | 🟠 MajorPause polling on the shared account budget, not the current device snapshot.
This PR is trying to protect a shared Netro call budget. Gating each device with
should_pause_polling_for(key)lets another device keep polling on a stale higher token snapshot after a different device has already driven the shared budget below the pause threshold.Also applies to: 942-948
🤖 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 795 - 801, The per-device pause currently calls self.api_client.should_pause_polling_for(key) and logs token_remaining_for(key), which uses a stale per-device snapshot; change these checks to use the shared/account-level token snapshot instead (e.g., call should_pause_polling_for with the API client's shared account key or account identifier such as self.api_client.account_key or self.api_client.shared_key) and log token_remaining_for against that same shared account key; apply the same replacement where the duplicate occurs (the other block around lines 942-948) so polling is gated by the shared budget rather than each device's snapshot.
805-807:⚠️ Potential issue | 🟠 MajorThese timer guards also block manual refreshes.
RequestStatusandupdateAllStatus()still call the scheduled polling path, so most on-demand refreshes return immediately unless the deadline has already elapsed. Thread aforceflag through these guards so manual refresh bypasses the scheduler.Also applies to: 935-936
🤖 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 805 - 807, The polling guards (e.g., "if now >= self._next_device_info_update" and the analogous guard at lines 935-936) are preventing manual refreshes; modify RequestStatus and updateAllStatus to accept a force=False parameter, thread that force flag down to the code that calls get_device_info (and the analogous zone/status refresh path), and change the guard to bypass the timer when force is True (i.e., proceed if force or now >= self._next_device_info_update). Ensure the force flag is passed through to wherever the guarded checks run so manual refreshes skip the schedule check.
1240-1255:⚠️ Potential issue | 🟠 MajorConfig changes don't re-arm the next poll deadlines.
Changing
_events_interval/_device_info_interval/ etc. without resetting the matching_next_*_updatetimestamp means a newly shorter interval can still wait nearly the full old delay before the first poll at the new cadence.🤖 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 1240 - 1255, When an interval field (eventsInterval/deviceInfoInterval/moisturesInterval/schedulesInterval/sensorInterval) is changed in the loop that updates _events_interval/_device_info_interval/_moistures_interval/_schedules_interval/_sensor_interval, also re-arm the corresponding next-poll timestamp (e.g. _next_events_update, _next_device_info_update, _next_moistures_update, _next_schedules_update, _next_sensor_update) so the new cadence takes effect immediately; after setting setattr(self, attr_name, new_val) set the matching _next_*_update to time.time() + new_val * 60 (ensure time is imported) so the next poll is scheduled relative to now at the new interval, and log that the deadline was re-armed.
🧹 Nitpick comments (3)
.planning/codebase/STRUCTURE.md (3)
43-43: Hardcoded version in documentation may become stale.The specific version "requests==2.32.5" in this comment could become outdated. Consider referencing "see requirements.txt" without repeating the version number.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/codebase/STRUCTURE.md at line 43, The doc comment currently hardcodes a specific dependency version ("requests==2.32.5") which can drift; update the line referencing requirements.txt to remove the explicit version and instead say something like "see requirements.txt" or "pinned in requirements.txt" so the source of truth is the requirements file; locate the literal "requests==2.32.5" in STRUCTURE.md and replace it with a reference to requirements.txt (keep the file name requirements.txt as the canonical identifier).
92-92: Enumerate the actual per-endpoint polling interval preference keys.The generic phrase "Per-endpoint polling intervals (all configurable, each with a minimum)" is inconsistent with the specificity shown elsewhere in this documentation (see lines 90-91, 93-94 which list actual key names, and the tables in other sections). Based on the AI summary, the actual keys are:
eventsInterval,deviceInfoInterval,moisturesInterval,schedulesInterval,sensorInterval, andforecastInterval.Consider replacing the generic description with an explicit list for consistency and developer reference.
📝 Suggested improvement
- `apiTimeout` — request timeout seconds (default 5, range 1-60) - `maxZoneRunTime` — max zone duration seconds (default 10800/3hr) -- Per-endpoint polling intervals (all configurable, each with a minimum) +- Per-endpoint polling intervals: `eventsInterval`, `deviceInfoInterval`, `moisturesInterval`, `schedulesInterval`, `sensorInterval`, `forecastInterval` (each configurable with a minimum) - `tomorrowEnabled` / `tomorrowApiKey` / `tomorrowLocation` — Tomorrow.io🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/codebase/STRUCTURE.md at line 92, Replace the generic line "Per-endpoint polling intervals (all configurable, each with a minimum)" with an explicit enumeration of the polling interval preference keys so it matches surrounding specificity; list the keys exactly as eventsInterval, deviceInfoInterval, moisturesInterval, schedulesInterval, sensorInterval, and forecastInterval and note that each is configurable with a minimum value (keeping the existing minimum wording if present).
35-35: Consider removing or generalizing the line count.The "~1900 lines" estimate will become stale as the code evolves. Either remove it or replace with a more general descriptor like "main plugin class" to reduce maintenance burden.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/codebase/STRUCTURE.md at line 35, Remove the brittle "~1900 lines" size annotation from the STRUCTURE entry for plugin.py and replace it with a stable descriptor such as "main plugin class" (or simply omit the line count); update the entry "plugin.py ← Plugin(indigo.PluginBase)" so it reads e.g. "plugin.py ← Plugin(indigo.PluginBase), main plugin class" to avoid stale line-count metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.planning/codebase/INTEGRATIONS.md:
- Around line 52-58: Update the quota text to state the 2,000 calls/day is
account-wide (shared across all devices) rather than per-device; edit the
sentence mentioning "per device (shared between v1 and v2 keys for the same
device)" to clarify the 2,000-call budget is shared across the account and
applies to all devices and keys, while retaining references to
THROTTLE_LIMIT_MINUTES, TOKEN_PAUSE_THRESHOLD, and TOKEN_WARNING_THRESHOLD so
the document still explains the 61-minute lockout and the proactive
pause/warning thresholds.
In @.planning/codebase/STACK.md:
- Around line 17-19: Update the documented PluginVersion value in
.planning/codebase/STACK.md: change the line with the PluginVersion symbol from
`2026.4.0` to `2026.4.1` so the CFBundleIdentifier/CFBundleDisplayName block
reflects the PR's version bump and keeps release/tag guidance accurate.
---
Duplicate comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py:
- Around line 455-460: The code incorrectly forces offset-aware timestamps to a
different instant by calling
datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc); instead parse
into dt = datetime.fromisoformat(reset_str) and then if dt.tzinfo is None do
reset_time = dt.replace(tzinfo=timezone.utc) else do reset_time =
dt.astimezone(timezone.utc) so aware values are converted properly; apply the
same change in the _handle_http_error() path as well (look for the token_reset
parsing/replace usage).
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py:
- Around line 812-816: The code currently overwrites self.person["devices"] and
self.netro_devices with a single device each poll (in the block that sets
netro_serial, device_data["id"], self.person and self.netro_devices), which
loses previously cached controllers and breaks
_get_device_dict()/_get_zone_dict(); instead, preserve and merge into the cached
devices list: locate the block that assigns self.person = {"id": ..., "devices":
[device_data]} and change it to update or append device_data into the existing
self.person["devices"] (match by id/serial), replacing the existing entry if
present or appending if new, then set self.netro_devices =
self.person["devices"] so multi-controller installs retain previously discovered
controllers.
- Around line 146-151: The loop interval calculation for self._loop_interval
currently takes the min of Netro-only timers (_events_interval,
_device_info_interval, _moistures_interval, _schedules_interval,
_sensor_interval) but omits the weather/forecast cadence, causing Tomorrow.io
updates to run late when weatherUpdateInterval or forecastInterval is shorter;
update the min(...) call to also include the weather/forecast intervals (the
attributes or internal variables representing them—e.g.
weatherUpdateInterval/forecastInterval or their internal counterparts like
_weather_interval and _forecast_interval) so the main loop sleeps for the
shortest of all timers and triggers those updates on time.
- Around line 824-877: The schedule and moisture fetches are incorrectly gated
by the device-info timing block so shorter schedulesInterval/moisturesInterval
values get skipped; update the logic so each uses its own timer check (compare
now to self._next_schedules_update and self._next_moistures_update) and calls
self.api_client.get_schedules / self.api_client.get_moistures independently of
the device-info block, processing results with
sprinkler_handler.process_schedules and updating props/states as needed; ensure
exceptions still set warnings/debug logs and that schedule_dict/moisture_dict
remain available for the subsequent calls to _ensure_zone_devices and
_update_zone_devices so those functions receive the latest data.
- Around line 795-801: The per-device pause currently calls
self.api_client.should_pause_polling_for(key) and logs token_remaining_for(key),
which uses a stale per-device snapshot; change these checks to use the
shared/account-level token snapshot instead (e.g., call should_pause_polling_for
with the API client's shared account key or account identifier such as
self.api_client.account_key or self.api_client.shared_key) and log
token_remaining_for against that same shared account key; apply the same
replacement where the duplicate occurs (the other block around lines 942-948) so
polling is gated by the shared budget rather than each device's snapshot.
- Around line 805-807: The polling guards (e.g., "if now >=
self._next_device_info_update" and the analogous guard at lines 935-936) are
preventing manual refreshes; modify RequestStatus and updateAllStatus to accept
a force=False parameter, thread that force flag down to the code that calls
get_device_info (and the analogous zone/status refresh path), and change the
guard to bypass the timer when force is True (i.e., proceed if force or now >=
self._next_device_info_update). Ensure the force flag is passed through to
wherever the guarded checks run so manual refreshes skip the schedule check.
- Around line 1240-1255: When an interval field
(eventsInterval/deviceInfoInterval/moisturesInterval/schedulesInterval/sensorInterval)
is changed in the loop that updates
_events_interval/_device_info_interval/_moistures_interval/_schedules_interval/_sensor_interval,
also re-arm the corresponding next-poll timestamp (e.g. _next_events_update,
_next_device_info_update, _next_moistures_update, _next_schedules_update,
_next_sensor_update) so the new cadence takes effect immediately; after setting
setattr(self, attr_name, new_val) set the matching _next_*_update to time.time()
+ new_val * 60 (ensure time is imported) so the next poll is scheduled relative
to now at the new interval, and log that the deadline was re-armed.
---
Nitpick comments:
In @.planning/codebase/STRUCTURE.md:
- Line 43: The doc comment currently hardcodes a specific dependency version
("requests==2.32.5") which can drift; update the line referencing
requirements.txt to remove the explicit version and instead say something like
"see requirements.txt" or "pinned in requirements.txt" so the source of truth is
the requirements file; locate the literal "requests==2.32.5" in STRUCTURE.md and
replace it with a reference to requirements.txt (keep the file name
requirements.txt as the canonical identifier).
- Line 92: Replace the generic line "Per-endpoint polling intervals (all
configurable, each with a minimum)" with an explicit enumeration of the polling
interval preference keys so it matches surrounding specificity; list the keys
exactly as eventsInterval, deviceInfoInterval, moisturesInterval,
schedulesInterval, sensorInterval, and forecastInterval and note that each is
configurable with a minimum value (keeping the existing minimum wording if
present).
- Line 35: Remove the brittle "~1900 lines" size annotation from the STRUCTURE
entry for plugin.py and replace it with a stable descriptor such as "main plugin
class" (or simply omit the line count); update the entry "plugin.py ←
Plugin(indigo.PluginBase)" so it reads e.g. "plugin.py ←
Plugin(indigo.PluginBase), main plugin class" to avoid stale line-count
metadata.
🪄 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: 2aac25e9-f914-478f-9562-ce39d20f1d94
📒 Files selected for processing (14)
.planning/codebase/ARCHITECTURE.md.planning/codebase/CONCERNS.md.planning/codebase/CONVENTIONS.md.planning/codebase/INTEGRATIONS.md.planning/codebase/STACK.md.planning/codebase/STRUCTURE.md.planning/codebase/TESTING.mdCLAUDE.mdNetro Sprinklers.indigoPlugin/Contents/Info.plistNetro Sprinklers.indigoPlugin/Contents/Server Plugin/PluginConfig.xmlNetro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/validators.pytests/test_api_client.py
✅ Files skipped from review due to trivial changes (4)
- Netro Sprinklers.indigoPlugin/Contents/Info.plist
- .planning/codebase/TESTING.md
- CLAUDE.md
- .planning/codebase/ARCHITECTURE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Netro Sprinklers.indigoPlugin/Contents/Server Plugin/validators.py
Comment on lines
+52
to
+58
| - **Daily quota**: 2,000 calls/day per device (shared between v1 and v2 keys for the same device) | ||
| - **Reset**: Midnight UTC | ||
| - **HTTP 429**: Rate limit exceeded — plugin enforces a 61-minute lockout | ||
| (`THROTTLE_LIMIT_MINUTES = 61` in `constants.py`) | ||
| - **Proactive pause**: When `token_remaining < TOKEN_PAUSE_THRESHOLD` (100), | ||
| polling is suspended before hitting the limit | ||
| - **Warning threshold**: `TOKEN_WARNING_THRESHOLD = 200` — logs a warning |
There was a problem hiding this comment.
Document the quota as account-wide, not per-device.
The linked issue/PR objective for this work is a shared 2,000-call/day Netro budget across all devices. Keeping "per device" here will lead users to size intervals incorrectly and makes the polling-budget guidance misleading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.planning/codebase/INTEGRATIONS.md around lines 52 - 58, Update the quota
text to state the 2,000 calls/day is account-wide (shared across all devices)
rather than per-device; edit the sentence mentioning "per device (shared between
v1 and v2 keys for the same device)" to clarify the 2,000-call budget is shared
across the account and applies to all devices and keys, while retaining
references to THROTTLE_LIMIT_MINUTES, TOKEN_PAUSE_THRESHOLD, and
TOKEN_WARNING_THRESHOLD so the document still explains the 61-minute lockout and
the proactive pause/warning thresholds.
Comment on lines
+17
to
+19
| - **CFBundleIdentifier**: `com.simons-plugins.netro` | ||
| - **PluginVersion**: `2026.4.0` (format: `YYYY.R.patch`) | ||
| - **CFBundleDisplayName**: Netro Smart Sprinklers |
There was a problem hiding this comment.
Update the documented PluginVersion.
This planning doc still says 2026.4.0, but this PR's version bump is 2026.4.1. Leaving the old value here will make release/tag guidance inaccurate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.planning/codebase/STACK.md around lines 17 - 19, Update the documented
PluginVersion value in .planning/codebase/STACK.md: change the line with the
PluginVersion symbol from `2026.4.0` to `2026.4.1` so the
CFBundleIdentifier/CFBundleDisplayName block reflects the PR's version bump and
keeps release/tag guidance accurate.
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.
Summary
Netro's 2000 calls/day budget is shared across all devices on an account. The old single polling interval (5 min for everything) burned ~4,000+ calls/day. Now each endpoint has its own configurable interval.
Changes
constants.py— Per-endpoint default and minimum interval constantsPluginConfig.xml— Replaced single polling field with per-endpoint config fields + configurable forecast intervalplugin.py— Per-endpoint timers in_update_sprinkler_deviceand_update_whisperer_device, main loop sleeps on shortest interval, configurable forecast intervalvalidators.py— Validation specs for all new interval fieldstoken_remainingstate now updates even when sensor reading unchangedTest plan
Closes #48
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Migration
Other