Track API tokens per device instead of globally#47
Merged
4 commits merged intoApr 15, 2026
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>
📝 WalkthroughWalkthroughReplaces global token tracking with per-device token state: adds Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as Plugin.runConcurrentThread / DeviceUpdater
participant APIClient as NetroAPIClient
participant External as Netro API
participant Prefs as Persistence
Plugin->>APIClient: should_pause_polling_for(device_key)
alt paused for device
APIClient-->>Plugin: True (token_remaining_for(device_key))
Plugin-->>Plugin: skip device update
else not paused
Plugin->>APIClient: make_request(..., device_key)
APIClient->>External: HTTP request
External-->>APIClient: response + meta(token_remaining, token_reset)
APIClient->>APIClient: _update_token_budget(meta, device_key)
APIClient->>Prefs: _save_throttle_state({version:2, device_tokens})
APIClient-->>Plugin: response payload
Plugin->>Plugin: process device update
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 1
🧹 Nitpick comments (1)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py (1)
58-69: LGTM — clean dataclass for per-device state.The
DeviceTokenStatedataclass appropriately encapsulates the per-device token budget tracking with sensible defaults.Nit: Static analysis indicates
__all__is not sorted. Consider alphabetizing for consistency:-__all__ = ["NetroAPIClient", "DeviceTokenState", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"] +__all__ = ["DeviceTokenState", "NetroAPIClient", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"]🤖 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 - 69, Alphabetize the __all__ export list to keep consistency: update the __all__ declaration that currently lists ["NetroAPIClient", "DeviceTokenState", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"] so the symbols are sorted (e.g., "DeviceTokenState" before "NetroAPIClient" and the TOKEN constants in alphabetical order) while preserving the same names and order within each quoted string.
🤖 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/Info.plist:
- Around line 5-6: The PluginVersion value in the Info.plist is a duplicate
(currently "2026.3.1"); update the <key>PluginVersion</key> string to a new,
unique version (e.g., "2026.3.2" or higher) so the pipeline version check
passes—edit the string value associated with PluginVersion in Netro
Sprinklers.indigoPlugin/Contents/Info.plist (the PluginVersion entry) and commit
the change.
---
Nitpick comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py:
- Around line 58-69: Alphabetize the __all__ export list to keep consistency:
update the __all__ declaration that currently lists ["NetroAPIClient",
"DeviceTokenState", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"] so the
symbols are sorted (e.g., "DeviceTokenState" before "NetroAPIClient" and the
TOKEN constants in alphabetical order) while preserving the same names and order
within each quoted string.
🪄 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: 8a3b2dd4-c779-4076-b315-e7d22df5d9d6
📒 Files selected for processing (4)
Netro Sprinklers.indigoPlugin/Contents/Info.plistNetro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.pytests/test_api_client.py
Comment on lines
5
to
+6
| <key>PluginVersion</key> | ||
| <string>2026.3.0</string> | ||
| <string>2026.3.1</string> |
There was a problem hiding this comment.
Version 2026.3.1 already exists — increment to a new version.
The pipeline check failed because this version tag already exists in the repository. Bump to 2026.3.2 or higher to pass the version check.
<key>PluginVersion</key>
- <string>2026.3.1</string>
+ <string>2026.3.2</string>📝 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
| <key>PluginVersion</key> | |
| <string>2026.3.0</string> | |
| <string>2026.3.1</string> | |
| <key>PluginVersion</key> | |
| <string>2026.3.2</string> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Netro` Sprinklers.indigoPlugin/Contents/Info.plist around lines 5 - 6, The
PluginVersion value in the Info.plist is a duplicate (currently "2026.3.1");
update the <key>PluginVersion</key> string to a new, unique version (e.g.,
"2026.3.2" or higher) so the pipeline version check passes—edit the string value
associated with PluginVersion in Netro
Sprinklers.indigoPlugin/Contents/Info.plist (the PluginVersion entry) and commit
the change.
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>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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:
- Line 237: Replace any exposure of raw device_key (including device_key[:8])
with the safe identifier helper _device_id(device_key) when writing logs and
when using keys in persisted pluginPrefs device_tokens; specifically update the
logger.info call that currently uses device_key[:8], the logging around lines
474-477, and the persistence that writes pluginPrefs['device_tokens'] (lines
~497-507) to use _device_id(device_key) as the label/key while continuing to
accept raw device_key as the method argument. Ensure all references to
device_tokens keys and log labels consistently use _device_id(...) so raw API
keys are never persisted or logged.
- Line 69: The __all__ export list is not sorted which triggers Ruff RUF022;
update the module-level __all__ variable to contain the exported names in
lexical order (e.g., reorder the entries for NetroAPIClient, DeviceTokenState,
TOKEN_PAUSE_THRESHOLD, TOKEN_WARNING_THRESHOLD into a sorted sequence) so the
list is alphabetized while preserving the same symbols (__all__, NetroAPIClient,
DeviceTokenState, TOKEN_PAUSE_THRESHOLD, TOKEN_WARNING_THRESHOLD).
- Around line 455-465: The except block that handles token parse failures
currently forces remaining below TOKEN_PAUSE_THRESHOLD and sets reset_time=None
which can permanently pause the device; change the error handling in
api_client.py so the except keeps the device active by setting remaining to a
safe value above the pause threshold (e.g. TOKEN_PAUSE_THRESHOLD + 1) instead of
TOKEN_PAUSE_THRESHOLD - 1, preserve/reset reset_time as None, and keep the
logger.warning call (use the existing variables remaining, reset_time,
TOKEN_PAUSE_THRESHOLD, meta, and self.logger to locate the code).
- Line 460: The code is incorrectly using .replace(tzinfo=timezone.utc) which
relabels offsets instead of converting absolute times; update the logic where
you parse the API timestamps—specifically the assignment creating reset_time
(fromisoformat(...).replace(tzinfo=timezone.utc)) and the code that sets
self._throttle_until (currently using .replace) to convert to UTC with
.astimezone(timezone.utc) after parsing the offset-aware datetime; ensure you
call fromisoformat(...) first and then .astimezone(timezone.utc) so token_reset
/ reset_time and self._throttle_until hold the correct UTC moments.
🪄 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: fcfbd10b-6329-4fe6-be50-a24ce69e3ebf
📒 Files selected for processing (2)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.pytests/test_api_client.py
✅ Files skipped from review due to trivial changes (1)
- tests/test_api_client.py
|
|
||
| # Re-export threshold constants for convenient access | ||
| __all__ = ["NetroAPIClient", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"] | ||
| __all__ = ["NetroAPIClient", "DeviceTokenState", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"] |
There was a problem hiding this comment.
Sort __all__ to satisfy Ruff RUF022.
Static analysis flagged this line; sorting avoids lint noise in CI.
Proposed fix
-__all__ = ["NetroAPIClient", "DeviceTokenState", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"]
+__all__ = ["DeviceTokenState", "NetroAPIClient", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"]📝 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
| __all__ = ["NetroAPIClient", "DeviceTokenState", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"] | |
| __all__ = ["DeviceTokenState", "NetroAPIClient", "TOKEN_PAUSE_THRESHOLD", "TOKEN_WARNING_THRESHOLD"] |
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 69-69: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🤖 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 at line
69, The __all__ export list is not sorted which triggers Ruff RUF022; update the
module-level __all__ variable to contain the exported names in lexical order
(e.g., reorder the entries for NetroAPIClient, DeviceTokenState,
TOKEN_PAUSE_THRESHOLD, TOKEN_WARNING_THRESHOLD into a sorted sequence) so the
list is alphabetized while preserving the same symbols (__all__, NetroAPIClient,
DeviceTokenState, TOKEN_PAUSE_THRESHOLD, TOKEN_WARNING_THRESHOLD).
| state.token_reset = None | ||
| self._save_throttle_state() | ||
| self.logger.info("API token budget has reset to daily limit (2000)") | ||
| self.logger.info(f"API token budget reset for device {device_key[:8]}...") |
There was a problem hiding this comment.
Do not persist or log raw device keys (v2 API keys).
Line 497+ stores raw keys in pluginPrefs, and Lines 237/474 log key prefixes. This expands API-key exposure surface.
Suggested hardening direction
+import hashlib
+
+def _device_id(self, device_key: str) -> str:
+ return hashlib.sha256(device_key.encode("utf-8")).hexdigest()[:16]Use _device_id(device_key) for log labels and persisted device_tokens keys, while continuing to accept raw device_key at method boundaries.
Also applies to: 474-477, 497-507
🤖 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 at line
237, Replace any exposure of raw device_key (including device_key[:8]) with the
safe identifier helper _device_id(device_key) when writing logs and when using
keys in persisted pluginPrefs device_tokens; specifically update the logger.info
call that currently uses device_key[:8], the logging around lines 474-477, and
the persistence that writes pluginPrefs['device_tokens'] (lines ~497-507) to use
_device_id(device_key) as the label/key while continuing to accept raw
device_key as the method argument. Ensure all references to device_tokens keys
and log labels consistently use _device_id(...) so raw API keys are never
persisted or logged.
Comment on lines
455
to
+465
| 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 | ||
| reset_time = None | ||
|
|
There was a problem hiding this comment.
Avoid self-locking a device on token-parse failures.
Line 463 forces remaining below pause threshold on parse errors, and with token_reset=None this can permanently pause that device before any recovery call is made.
Proposed fix
except (ValueError, TypeError) as exc:
self.logger.warning(f"Could not parse token info from response: {exc}")
- remaining = TOKEN_PAUSE_THRESHOLD - 1
- reset_time = None
+ # Keep prior state (or assume full budget for new devices) to avoid false hard-pause.
+ prior = self._device_tokens.get(device_key)
+ remaining = prior.token_remaining if prior else 2000
+ reset_time = prior.token_reset if prior else None🤖 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 - 465, The except block that handles token parse failures currently
forces remaining below TOKEN_PAUSE_THRESHOLD and sets reset_time=None which can
permanently pause the device; change the error handling in api_client.py so the
except keeps the device active by setting remaining to a safe value above the
pause threshold (e.g. TOKEN_PAUSE_THRESHOLD + 1) instead of
TOKEN_PAUSE_THRESHOLD - 1, preserve/reset reset_time as None, and keep the
logger.warning call (use the existing variables remaining, reset_time,
TOKEN_PAUSE_THRESHOLD, meta, and self.logger to locate the code).
| 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) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify both the API misuse pattern and semantic difference.
set -euo pipefail
rg -nP 'fromisoformat\(.*\)\.replace\(tzinfo=timezone\.utc\)' "Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py"
python - <<'PY'
from datetime import datetime, timezone
s = "2026-04-10T01:00:00+02:00"
d = datetime.fromisoformat(s)
print("replace:", d.replace(tzinfo=timezone.utc).isoformat())
print("convert:", d.astimezone(timezone.utc).isoformat())
PYRepository: simons-plugins/netro-indigo
Length of output: 346
Use timezone conversion, not timezone replacement, for token_reset and _throttle_until.
Lines 408 and 460 both use .replace(tzinfo=timezone.utc), which re-labels wall time instead of converting absolute time if the API returns an offset-aware timestamp. For example, "2026-04-10T01:00:00+02:00" becomes "2026-04-10T01:00:00+00:00" (wrong) instead of "2026-04-09T23:00:00+00:00" (correct).
Proposed fix
- reset_time = datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc)
+ parsed = datetime.fromisoformat(reset_str)
+ reset_time = (
+ parsed.astimezone(timezone.utc)
+ if parsed.tzinfo is not None
+ else parsed.replace(tzinfo=timezone.utc)
+ )Apply the same fix to line 408 (self._throttle_until).
🤖 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 at line
460, The code is incorrectly using .replace(tzinfo=timezone.utc) which relabels
offsets instead of converting absolute times; update the logic where you parse
the API timestamps—specifically the assignment creating reset_time
(fromisoformat(...).replace(tzinfo=timezone.utc)) and the code that sets
self._throttle_until (currently using .replace) to convert to UTC with
.astimezone(timezone.utc) after parsing the offset-aware datetime; ensure you
call fromisoformat(...) first and then .astimezone(timezone.utc) so token_reset
/ reset_time and self._throttle_until hold the correct UTC moments.
- 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>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py (3)
458-460:⚠️ Potential issue | 🟠 MajorConvert offset-aware timestamps to UTC instead of relabeling them.
Line 460 still uses
.replace(tzinfo=timezone.utc), which is wrong for offset-aware API timestamps. If Netro returns something like...+02:00, this shifts the actual reset moment and can pause/unpause a device at the wrong time. The same fix is still needed in_handle_http_error()around Line 408.Suggested fix
reset_str = meta.get("token_reset", "") if reset_str: - reset_time = datetime.fromisoformat(reset_str).replace(tzinfo=timezone.utc) + parsed = datetime.fromisoformat(reset_str) + reset_time = ( + parsed.astimezone(timezone.utc) + if parsed.tzinfo is not None + else parsed.replace(tzinfo=timezone.utc) + )Apply the same pattern where
_throttle_untilis populated frommeta["token_reset"].#!/bin/bash set -euo pipefail file='Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py' echo 'Occurrences of the relabeling pattern:' rg -n 'fromisoformat\(.*\)\.replace\(tzinfo=timezone\.utc\)' "$file" echo python - <<'PY' from datetime import datetime, timezone s = "2026-04-10T01:00:00+02:00" parsed = datetime.fromisoformat(s) print("replace :", parsed.replace(tzinfo=timezone.utc).isoformat()) print("astimezone:", parsed.astimezone(timezone.utc).isoformat()) PYExpected result: the two printed timestamps differ, proving
replace(...)is not a UTC conversion.🤖 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 458 - 460, The code currently uses datetime.fromisoformat(...).replace(tzinfo=timezone.utc) which incorrectly relabels offset-aware timestamps; change both occurrences (the block that sets reset_time from meta.get("token_reset") and the similar logic in _handle_http_error where _throttle_until is set from meta["token_reset"]) to parse with datetime.fromisoformat(...) and then call .astimezone(timezone.utc) to convert the timestamp to UTC (e.g., reset_time = datetime.fromisoformat(reset_str).astimezone(timezone.utc)); update any variable names (_throttle_until, reset_time) accordingly and keep timezone imported.
474-478:⚠️ Potential issue | 🟠 MajorAvoid persisting or logging raw device keys.
For v2 devices,
device_keyis the API key. Writing it intopluginPrefs["throttle_state"]["device_tokens"]and loggingdevice_key[:8]here still expands the secret-exposure surface. Please switch persistence and log labels to a stable derived identifier instead; the same issue also remains in the reset log at Line 237.Also applies to: 498-508
🤖 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 474 - 478, The code logs and persists raw device_key (the v2 API key) via variables like device_key and pluginPrefs["throttle_state"]["device_tokens"] and in the token-low warning (uses key_display) and reset log; replace any use of device_key for logging or persistence with a stable derived identifier (e.g., SHA256 or HMAC-SHA256 hex of device_key using a plugin-scoped salt) so the raw key is never stored or printed; update the token-warning block (variables device_key, key_display, reset_time, TOKEN_WARNING_THRESHOLD) and the reset logging/persistence sites (including the reset log referenced at Line 237 and the range 498-508) to compute and use derived_id for keys in pluginPrefs and log messages, and ensure only the derived_id and necessary token counters/timestamps are persisted, never the plaintext device_key.
461-465:⚠️ Potential issue | 🟠 MajorDon't turn a token-parse glitch into a one-hour device pause.
Lines 463-465 still force
remainingbelowTOKEN_PAUSE_THRESHOLD, so one malformedmetapayload on a successful response can stop polling that device until the synthetic reset expires. A safer fallback here is to keep the previous state, or default above the pause threshold for first-seen devices.Suggested fix
except (ValueError, TypeError) as exc: self.logger.warning(f"Could not parse token info from response: {exc}") - 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) + prior = self._device_tokens.get(device_key) + remaining = prior.token_remaining if prior else 2000 + reset_time = prior.token_reset if prior else None🤖 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 461 - 465, The except block that catches ValueError/TypeError should not force remaining below TOKEN_PAUSE_THRESHOLD or set a one-hour reset_time; instead, when parsing token meta fails in the except after logger.warning, preserve the previous remaining value if one exists (e.g. a stored token_remaining or previously parsed "remaining" in the surrounding scope) or, for first-seen devices, set remaining to a safe default above TOKEN_PAUSE_THRESHOLD (for example TOKEN_PAUSE_THRESHOLD + 1), and do not assign a synthetic reset_time; update the handler around the parse (the except handling for token meta) to use the preserved/default remaining and remove/reset_time modification so a single malformed meta payload won’t pause polling.
🤖 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 553-565: The restore logic assumes state["device_tokens"] is a
mapping and calls .items(); guard this by verifying state.get("device_tokens")
is a dict (or at least has .items()) before iterating, and if it's not, log a
warning via self.logger.warning and skip restoring device tokens to avoid an
AttributeError; keep the existing parsing into DeviceTokenState and the except
(ValueError, TypeError) block for token parsing, and update references to
_device_tokens, DeviceTokenState, state, key, and token_state accordingly.
---
Duplicate comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py:
- Around line 458-460: The code currently uses
datetime.fromisoformat(...).replace(tzinfo=timezone.utc) which incorrectly
relabels offset-aware timestamps; change both occurrences (the block that sets
reset_time from meta.get("token_reset") and the similar logic in
_handle_http_error where _throttle_until is set from meta["token_reset"]) to
parse with datetime.fromisoformat(...) and then call .astimezone(timezone.utc)
to convert the timestamp to UTC (e.g., reset_time =
datetime.fromisoformat(reset_str).astimezone(timezone.utc)); update any variable
names (_throttle_until, reset_time) accordingly and keep timezone imported.
- Around line 474-478: The code logs and persists raw device_key (the v2 API
key) via variables like device_key and
pluginPrefs["throttle_state"]["device_tokens"] and in the token-low warning
(uses key_display) and reset log; replace any use of device_key for logging or
persistence with a stable derived identifier (e.g., SHA256 or HMAC-SHA256 hex of
device_key using a plugin-scoped salt) so the raw key is never stored or
printed; update the token-warning block (variables device_key, key_display,
reset_time, TOKEN_WARNING_THRESHOLD) and the reset logging/persistence sites
(including the reset log referenced at Line 237 and the range 498-508) to
compute and use derived_id for keys in pluginPrefs and log messages, and ensure
only the derived_id and necessary token counters/timestamps are persisted, never
the plaintext device_key.
- Around line 461-465: The except block that catches ValueError/TypeError should
not force remaining below TOKEN_PAUSE_THRESHOLD or set a one-hour reset_time;
instead, when parsing token meta fails in the except after logger.warning,
preserve the previous remaining value if one exists (e.g. a stored
token_remaining or previously parsed "remaining" in the surrounding scope) or,
for first-seen devices, set remaining to a safe default above
TOKEN_PAUSE_THRESHOLD (for example TOKEN_PAUSE_THRESHOLD + 1), and do not assign
a synthetic reset_time; update the handler around the parse (the except handling
for token meta) to use the preserved/default remaining and remove/reset_time
modification so a single malformed meta payload won’t pause polling.
🪄 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: f2c1fe6d-547d-4f36-9f3c-8aae544906f7
📒 Files selected for processing (2)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.pytests/test_api_client.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_api_client.py
Comment on lines
+553
to
+565
| if is_v2 and "device_tokens" in state: | ||
| for key, token_state in state["device_tokens"].items(): | ||
| try: | ||
| reset_time = None | ||
| if token_state.get("token_reset"): | ||
| reset_time = datetime.fromisoformat(token_state["token_reset"]) | ||
| self._device_tokens[key] = DeviceTokenState( | ||
| token_remaining=int(token_state.get("token_remaining", 2000)), | ||
| token_reset=reset_time, | ||
| ) | ||
| except (ValueError, TypeError) as exc: | ||
| key_display = key[:8] + "..." if len(key) > 8 else key | ||
| self.logger.warning(f"Could not restore token state for device {key_display}: {exc}") |
There was a problem hiding this comment.
Guard device_tokens before iterating .items().
This restore path assumes state["device_tokens"] is a mapping. If the persisted JSON is corrupted to a list/string, .items() raises AttributeError, which is not caught by the outer handler and can break client initialization.
Suggested fix
# V2 format: restore per-device token budgets
if is_v2 and "device_tokens" in state:
- for key, token_state in state["device_tokens"].items():
+ device_tokens = state["device_tokens"]
+ if not isinstance(device_tokens, dict):
+ raise TypeError("device_tokens must be an object")
+ for key, token_state in device_tokens.items():
try:
reset_time = None
if token_state.get("token_reset"):
reset_time = datetime.fromisoformat(token_state["token_reset"])🤖 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 553 - 565, The restore logic assumes state["device_tokens"] is a mapping
and calls .items(); guard this by verifying state.get("device_tokens") is a dict
(or at least has .items()) before iterating, and if it's not, log a warning via
self.logger.warning and skip restoring device tokens to avoid an AttributeError;
keep the existing parsing into DeviceTokenState and the except (ValueError,
TypeError) block for token parsing, and update references to _device_tokens,
DeviceTokenState, state, key, and token_state accordingly.
…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>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py (1)
1014-1025: Stale docstring: "proactive pause" no longer applies globally.Lines 1015-1016 mention "proactive pause when API tokens drop below threshold" but global pause logic was removed. The per-device pause now happens inside each device update method, not here.
📝 Suggested docstring update
- Includes proactive pause when API tokens drop below threshold to - prevent exhausting the daily limit. + Per-device token budget checks are handled inside individual device + update methods to prevent exhausting each device's daily limit.🤖 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 1014 - 1025, Stale docstring: remove or reword the phrase about a global "proactive pause" and clarify that per-device token pauses occur inside each device update method; edit the docstring above the concurrent thread loop (the block that logs "Starting concurrent thread" and calls self._update_from_netro()) to remove the global pause statement and instead state that token-based pauses are handled per-device inside each device update method.
🤖 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/plugin.py:
- Around line 901-907: meta_keys currently includes "last_active" which is not
produced by WhispererHandler.process_sensor_data and causes mismatched metadata
handling; update the meta_keys set in the sensor polling block (the variable
named meta_keys used before dev.updateStatesOnServer and the debug log
referencing new_reading_id) to remove "last_active" and include only the keys
actually returned by process_sensor_data (e.g., "token_remaining",
"token_reset", "time", "api_last_active"), so metadata-only updates use the
correct keys and SprinklerHandler-produced "last_active" stays separate.
---
Nitpick comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py:
- Around line 1014-1025: Stale docstring: remove or reword the phrase about a
global "proactive pause" and clarify that per-device token pauses occur inside
each device update method; edit the docstring above the concurrent thread loop
(the block that logs "Starting concurrent thread" and calls
self._update_from_netro()) to remove the global pause statement and instead
state that token-based pauses are handled per-device inside each device update
method.
🪄 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: 912459d6-bc03-46f2-99ef-913202dc398d
📒 Files selected for processing (1)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py
Comment on lines
+901
to
+907
| # Update only metadata states that change every poll | ||
| meta_keys = {"token_remaining", "token_reset", "time", "last_active", "api_last_active"} | ||
| meta_states = [s for s in states if s["key"] in meta_keys] | ||
| if meta_states: | ||
| dev.updateStatesOnServer(meta_states) | ||
| self.logger.debug( | ||
| f"Sensor '{dev.name}' reading unchanged (ID {new_reading_id}), skipping update" | ||
| f"Sensor '{dev.name}' reading unchanged (ID {new_reading_id}), updated metadata only" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the state keys produced by WhispererHandler.process_sensor_data()
# to verify they include: token_remaining, token_reset, time, last_active, api_last_active
# Search for process_sensor_data implementation and state key assignments
ast-grep --pattern $'def process_sensor_data($$$) {
$$$
}'
# Also search for state key definitions in whisperer handler
rg -n "token_remaining|token_reset|last_active|api_last_active" --type py -C2Repository: simons-plugins/netro-indigo
Length of output: 46917
Fix hardcoded metadata keys: remove "last_active" for sensor polling.
The meta_keys set includes "last_active", but WhispererHandler.process_sensor_data() returns "api_last_active", not "last_active". The "last_active" key is only produced by SprinklerHandler for device info. Remove it from the set:
meta_keys = {"token_remaining", "token_reset", "time", "api_last_active"}🤖 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
901 - 907, meta_keys currently includes "last_active" which is not produced by
WhispererHandler.process_sensor_data and causes mismatched metadata handling;
update the meta_keys set in the sensor polling block (the variable named
meta_keys used before dev.updateStatesOnServer and the debug log referencing
new_reading_id) to remove "last_active" and include only the keys actually
returned by process_sensor_data (e.g., "token_remaining", "token_reset", "time",
"api_last_active"), so metadata-only updates use the correct keys and
SprinklerHandler-produced "last_active" stays separate.
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
Changes
api_client.py:DeviceTokenStatedataclass,_device_tokensdict replacing scalar_token_remaining,device_keyparam threaded throughmake_request()and all 11 convenience methods,should_pause_polling_for()andtoken_remaining_for()methods, v2 state formatplugin.py: Removed global pause fromrunConcurrentThread(), added per-device pause checks in_update_sprinkler_device()and_update_whisperer_device()tests/test_api_client.py: Updated all token tests + new per-device isolation testsTest plan
Closes #43
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores