Add Tomorrow.io weather integration for automatic weather reporting#42
Merged
simons-plugins merged 6 commits intoApr 10, 2026
Merged
Conversation
… reporting Fetches current weather from Tomorrow.io API on a configurable interval (default 30 min) and reports it to all Netro sprinkler devices via the report_weather endpoint, replacing Netro's built-in weather data for more accurate watering schedules. - New tomorrow_client.py module with weather code mapping (Tomorrow.io -> Netro) - Plugin config UI for API key, location, and update interval - Automatic unit conversion: metric for API v2, US for API v1 - Validation for Tomorrow.io settings when enabled - 100% test coverage on new modules (49 new tests) https://claude.ai/code/session_01AdcdU17hU7qs5ecXCCUWJe
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Tomorrow.io weather integration: new plugin prefs and UI labels, a TomorrowClient to fetch/transform realtime weather, metric↔US conversion utilities, pref validation, runtime scheduling/reporting to Netro devices, new device weather states, menu action, and unit tests. Changes
Sequence DiagramsequenceDiagram
participant Plugin as Plugin (runner)
participant TomorrowClient as TomorrowClient (client)
participant TomorrowAPI as Tomorrow.io (external)
participant NetroAPI as Netro API (api_client)
Plugin->>Plugin: check _next_weather_update
alt update due
Plugin->>TomorrowClient: fetch_current_weather()
TomorrowClient->>TomorrowAPI: GET /realtime (apikey, location, units=metric, timeout)
TomorrowAPI-->>TomorrowClient: JSON response / error
TomorrowClient->>TomorrowClient: _transform_response() -> Netro-format dict
TomorrowClient-->>Plugin: weather_dict (metric)
Plugin->>Plugin: convert_weather_metric_to_us() if device api_version == "1"
loop per enabled sprinkler device
Plugin->>NetroAPI: report_weather(device_id, weather_data)
NetroAPI-->>Plugin: success / ThrottleDelayError / failure
end
Plugin->>Plugin: set _next_weather_update
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 3
🧹 Nitpick comments (3)
tests/test_weather_integration.py (1)
160-160: Rename the intentionally unused tuple slots.Ruff is already flagging these unpacked
sanitized/errorsvalues as unused. Switching them to_sanitized/_errorskeeps the test file clean without changing behavior.Also applies to: 175-175, 190-190, 203-203, 214-214, 229-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_weather_integration.py` at line 160, Rename the intentionally unused tuple slots returned from validate_prefs_config from sanitized and errors to _sanitized and _errors at each call site (e.g., the statement currently reading "is_valid, sanitized, errors = validate_prefs_config(values)") so linters stop flagging unused variables; apply the same change for the other occurrences referenced (around the other calls that currently unpack to sanitized/errors). This only renames the local variables and does not change behavior of validate_prefs_config or its return values.tests/test_tomorrow_client.py (2)
12-20: Avoid unconditional module-levelsys.pathmutation.Line 18 globally mutates import state at import time. Prefer an idempotent insert at minimum, or centralize this in shared test bootstrap (for example,
conftest.py) to reduce cross-test coupling.Minimal safe tweak
-sys.path.insert(0, str(SERVER_PLUGIN_DIR)) +if str(SERVER_PLUGIN_DIR) not in sys.path: + sys.path.insert(0, str(SERVER_PLUGIN_DIR))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_tomorrow_client.py` around lines 12 - 20, The test file mutates sys.path unconditionally at import time; change the top-level import hack to an idempotent operation or move it to test bootstrap: either (A) replace sys.path.insert(0, str(SERVER_PLUGIN_DIR)) with a guard that checks if str(SERVER_PLUGIN_DIR) is already in sys.path before inserting, or (B) remove the module-level mutation and put the path adjustment into a shared conftest.py fixture (or setup function) so tests that import TomorrowClient/_TOMORROW_TO_NETRO_CONDITION get the path reliably without global import-time side effects.
287-304: Add explicitNone-value optional-field case.This test covers missing keys, but not keys present with
Nonevalues. Add one case to lock in the “omit optional fields whenNone” behavior described in scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_tomorrow_client.py` around lines 287 - 304, Update the test_optional_fields_omitted_when_none case to also cover keys that are present but set to None: inside the existing data payload add the optional keys ("humidity", "rain", "rain_prob", "wind_speed", "pressure") with value None under data["data"]["values"], call client._transform_response(data) as before, and assert the returned result does not include those keys; this locks in the behavior that _transform_response omits optional fields when they are explicitly None as well as when they are missing.
🤖 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 114-119: The weather update interval fallback currently uses the
wrong default; update the code so pluginPrefs.get("weatherUpdateInterval",
DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES) uses the single shared 30-minute
default constant used by PluginConfig.xml and validate_prefs_config() (i.e.,
ensure DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES is defined as 30 and used
everywhere), or replace the fallback with the same canonical constant/name that
validate_prefs_config() and PluginConfig.xml rely on so upgraded installs get a
30-minute default for _weather_update_interval.
- Around line 1034-1058: If Tomorrow.io stays enabled but its settings changed,
reset the next scheduled push so the new config takes effect immediately: after
creating new_client via _create_tomorrow_client and before assigning
self._tomorrow_client, detect a config change (compare the existing
self._tomorrow_client to new_client or compare a stable identifier on the
client) and if they differ and both are non-None, set self._next_weather_update
= datetime.now(); then assign self._tomorrow_client = new_client and keep the
existing log branches (_next_weather_update is already set when enabling, and
set here when updating settings).
In `@tests/test_tomorrow_client.py`:
- Around line 343-372: Add assertions to ensure the client calls
response.raise_for_status() in the happy-path tests: after invoking
client.fetch_current_weather() in both test_successful_fetch and
test_api_params_correct, assert that the mocked response's raise_for_status was
called (e.g., mock_response.raise_for_status.assert_called_once() or
mock_get.return_value.raise_for_status.assert_called_once()) so the tests fail
if the implementation omits calling raise_for_status().
---
Nitpick comments:
In `@tests/test_tomorrow_client.py`:
- Around line 12-20: The test file mutates sys.path unconditionally at import
time; change the top-level import hack to an idempotent operation or move it to
test bootstrap: either (A) replace sys.path.insert(0, str(SERVER_PLUGIN_DIR))
with a guard that checks if str(SERVER_PLUGIN_DIR) is already in sys.path before
inserting, or (B) remove the module-level mutation and put the path adjustment
into a shared conftest.py fixture (or setup function) so tests that import
TomorrowClient/_TOMORROW_TO_NETRO_CONDITION get the path reliably without global
import-time side effects.
- Around line 287-304: Update the test_optional_fields_omitted_when_none case to
also cover keys that are present but set to None: inside the existing data
payload add the optional keys ("humidity", "rain", "rain_prob", "wind_speed",
"pressure") with value None under data["data"]["values"], call
client._transform_response(data) as before, and assert the returned result does
not include those keys; this locks in the behavior that _transform_response
omits optional fields when they are explicitly None as well as when they are
missing.
In `@tests/test_weather_integration.py`:
- Line 160: Rename the intentionally unused tuple slots returned from
validate_prefs_config from sanitized and errors to _sanitized and _errors at
each call site (e.g., the statement currently reading "is_valid, sanitized,
errors = validate_prefs_config(values)") so linters stop flagging unused
variables; apply the same change for the other occurrences referenced (around
the other calls that currently unpack to sanitized/errors). This only renames
the local variables and does not change behavior of validate_prefs_config or its
return values.
🪄 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: 26d257ea-8ef3-4697-b007-2042cbfeccf0
📒 Files selected for processing (9)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/PluginConfig.xmlNetro Sprinklers.indigoPlugin/Contents/Server Plugin/constants.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/tomorrow_client.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/utils.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/validators.pypytest.initests/test_tomorrow_client.pytests/test_weather_integration.py
- Align DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES to 30 (matching PluginConfig.xml) so upgraded installs get the documented default - Reschedule next weather push when Tomorrow.io settings change (interval, API key, or location) so new config takes effect immediately - Assert raise_for_status() called in happy-path weather client tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add weather_condition, weather_temperature, weather_humidity, weather_rain, weather_rain_prob, weather_wind_speed, weather_pressure, and weather_updated states to sprinkler devices. States update on each successful weather report using metric values from Tomorrow.io. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 116-119: Clamp the loaded and updated interval values to at least
MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES when reading from pluginPrefs and when
applying updates so _weather_update_interval can never be zero or negative;
specifically, replace the direct assignment to self._weather_update_interval
from pluginPrefs (where DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES is used) with a
min/max or max(...) clamp using MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES and do
the same wherever the prefs are reloaded/updated (see the other prefs-handling
block around 1036-1047) so that _next_weather_update (and subsequent
report_weather() polling) is not scheduled in the past.
- Around line 427-430: The per-device except block currently catches all
Exceptions; change it to only catch the documented per-request failures so
programming errors propagate to the outer polling-loop handler. Import
NetroAPIError and requests.exceptions (or the specific exceptions) at the top of
the file, then replace "except Exception as exc:" in the make_request/notify
block with an explicit tuple of expected errors, e.g. "except (NetroAPIError,
requests.exceptions.ConnectionError, requests.exceptions.Timeout,
requests.exceptions.HTTPError) as exc:" (keeping the existing ThrottleDelayError
except branch), so only Netro/requests-related errors are handled per-device and
other exceptions bubble up.
🪄 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: 191ae8b5-470e-4df1-b9b0-1c9f56c97f7f
📒 Files selected for processing (4)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/constants.pyNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.pytests/test_base_modules.pytests/test_tomorrow_client.py
✅ Files skipped from review due to trivial changes (2)
- tests/test_base_modules.py
- tests/test_tomorrow_client.py
🚧 Files skipped from review as they are similar to previous changes (1)
- Netro Sprinklers.indigoPlugin/Contents/Server Plugin/constants.py
Comment on lines
+116
to
+119
| self._weather_update_interval = int( | ||
| pluginPrefs.get("weatherUpdateInterval", DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES) | ||
| ) | ||
| self._tomorrow_client = self._create_tomorrow_client(pluginPrefs) |
There was a problem hiding this comment.
Clamp weatherUpdateInterval at runtime, not just in UI validation.
Both code paths accept any integer from prefs, including 0 or negatives. That makes _next_weather_update land on now/the past every time, so this feature will call Tomorrow.io and report_weather() once per polling cycle per device until the prefs are corrected. Please enforce MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES when loading and updating the interval, even for stale or hand-edited prefs.
Also applies to: 1036-1047
🤖 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
116 - 119, Clamp the loaded and updated interval values to at least
MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES when reading from pluginPrefs and when
applying updates so _weather_update_interval can never be zero or negative;
specifically, replace the direct assignment to self._weather_update_interval
from pluginPrefs (where DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES is used) with a
min/max or max(...) clamp using MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES and do
the same wherever the prefs are reloaded/updated (see the other prefs-handling
block around 1036-1047) so that _next_weather_update (and subsequent
report_weather() polling) is not scheduled in the past.
Comment on lines
+427
to
+430
| except ThrottleDelayError: | ||
| self.logger.debug(f"Skipping weather report for '{dev.name}' - throttled") | ||
| except Exception as exc: | ||
| self.logger.error(f"Could not report weather to '{dev.name}': {exc}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -i '^api_client\.py$' -x sh -c '
echo "== {} ==";
rg -n -C3 "def report_weather|def make_request|ThrottleDelayError|Netro(Connection|Timeout|API)Error|requests\\.exceptions" "{}"
'
fd -i '^plugin\.py$' -x sh -c '
case "{}" in
*"Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py")
echo "== {}: weather handler ==";
sed -n "423,432p" "{}"
;;
esac
'Repository: simons-plugins/netro-indigo
Length of output: 3934
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get full report_weather implementation
echo "=== report_weather() full implementation ==="
sed -n '724,800p' './Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py'
echo ""
echo "=== Context around lines 427-430 in plugin.py (broader scope) ==="
sed -n '410,445p' './Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py'
echo ""
echo "=== Check for outer polling loop handler ==="
rg -n "except.*Error|except Exception" './Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py' -B2 -A1 | head -60Repository: simons-plugins/netro-indigo
Length of output: 5734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify available exception imports in plugin.py
echo "=== Exception imports in plugin.py ==="
head -100 './Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py' | grep -E "^from|^import" | grep -i "exception\|error"
echo ""
echo "=== Verify NetroAPIError is imported in api_client ==="
grep -n "^from exceptions import\|^import.*NetroAPIError" './Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py'
echo ""
echo "=== Check if NetroAPIError is exported/available in exceptions module ==="
grep -n "class NetroAPIError\|NetroAPIError" './Netro Sprinklers.indigoPlugin/Contents/Server Plugin/exceptions.py' | head -5
echo ""
echo "=== Verify requests.exceptions are used elsewhere in the codebase ==="
rg "requests\.exceptions\.(ConnectionError|Timeout|HTTPError)" './Netro Sprinklers.indigoPlugin/Contents/Server Plugin/' -nRepository: simons-plugins/netro-indigo
Length of output: 2072
Narrow the per-device exception handler to expected Netro/requests failures.
Catching bare Exception here swallows programming errors in the weather data preparation and hides bugs that should be logged centrally. Currently, any unexpected error (including typos or logic errors in lines 413–425) will silently skip that device with only the device-level error logged.
The make_request() method documents raising ThrottleDelayError, NetroAPIError, and requests.exceptions.{ConnectionError, Timeout, HTTPError}. Catch only those expected failures for per-device isolation; let unexpected exceptions propagate to the outer polling-loop handler (line 598), which already logs them centrally.
This requires importing NetroAPIError and requests.exceptions at the top of the file and updating the handler accordingly.
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 429-429: 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
427 - 430, The per-device except block currently catches all Exceptions; change
it to only catch the documented per-request failures so programming errors
propagate to the outer polling-loop handler. Import NetroAPIError and
requests.exceptions (or the specific exceptions) at the top of the file, then
replace "except Exception as exc:" in the make_request/notify block with an
explicit tuple of expected errors, e.g. "except (NetroAPIError,
requests.exceptions.ConnectionError, requests.exceptions.Timeout,
requests.exceptions.HTTPError) as exc:" (keeping the existing ThrottleDelayError
except branch), so only Netro/requests-related errors are handled per-device and
other exceptions bubble up.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py (2)
452-454:⚠️ Potential issue | 🟠 MajorNarrow the per-device exception handler in weather reporting.
Line 452 catches
Exception, which hides programming errors in this block and makes regressions harder to detect. Keep per-device isolation, but catch only expected Netro/request failures here.♻️ Proposed fix
-from exceptions import ThrottleDelayError +from exceptions import ThrottleDelayError, NetroAPIError @@ - except Exception as exc: + except ( + NetroAPIError, + requests.exceptions.ConnectionError, + requests.exceptions.Timeout, + requests.exceptions.HTTPError, + ) as exc: self.logger.error(f"Could not report weather to '{dev.name}': {exc}") self.logger.debug(f"Weather report error: \n{traceback.format_exc(10)}")Use this read-only script to verify the exception contract before applying:
#!/bin/bash set -euo pipefail fd -i '^exceptions\.py$' -x rg -n 'class (ThrottleDelayError|NetroAPIError)' fd -i '^api_client\.py$' -x rg -n -C2 'def make_request|raise .*NetroAPIError|raise .*ThrottleDelayError|requests\.exceptions\.(ConnectionError|Timeout|HTTPError)' fd -i '^plugin\.py$' -x rg -n -C2 'except ThrottleDelayError|except Exception as exc'Expected result:
NetroAPIErrorexists andmake_requestraises known request/API exceptions; current weather block still uses a broad catch.🤖 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 452 - 454, The per-device except block currently catches all Exceptions (the lines logging "Could not report weather to '{dev.name}': {exc}" and the traceback.debug call); narrow this to only expected request/API errors by catching the specific exception types raised by the Netro client (e.g. NetroAPIError and ThrottleDelayError) plus requests request-related exceptions (e.g. requests.exceptions.RequestException or ConnectionError/Timeout/HTTPError) and leave the logging the same; replace the broad "except Exception as exc" around the weather reporting for each device with a multi-except for these known exception classes to preserve per-device isolation while letting programming errors propagate.
116-118:⚠️ Potential issue | 🟠 MajorClamp
weatherUpdateIntervalin runtime paths.Line 116 and Line 1060 still trust raw prefs values. If stale/hand-edited prefs contain
0or negative values, weather pushes can run every poll cycle instead of respecting the minimum.♻️ Proposed fix
from constants import ( MAX_ZONE_DURATION_SECONDS, DEFAULT_API_TIMEOUT_SECONDS, DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES, + MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES, MINIMUM_POLLING_INTERVAL_MINUTES, ZONE_START_ENDPOINT, OPERATIONAL_ERROR_EVENTS, COMM_ERROR_EVENTS, DEVICE_EVENT_TYPES, ) @@ - self._weather_update_interval = int( - pluginPrefs.get("weatherUpdateInterval", DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES) - ) + self._weather_update_interval = max( + MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES, + int(pluginPrefs.get( + "weatherUpdateInterval", DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES + )), + ) @@ - new_interval = int(valuesDict.get( + new_interval = max( + MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES, + int(valuesDict.get( "weatherUpdateInterval", DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES - )) + )), + ) if new_interval != self._weather_update_interval: self._weather_update_interval = new_intervalAlso applies to: 1060-1065
🤖 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 116 - 118, The plugin currently trusts raw prefs for weatherUpdateInterval and can accept zero/negative values; change the assignment of self._weather_update_interval (and the other runtime retrieval that uses pluginPrefs.get around the same setting) to clamp to the minimum by converting to int and taking max(..., DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES) so any 0 or negative prefs fall back to the defined minimum, ensuring weather pushes respect the minimum interval.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py:
- Around line 452-454: The per-device except block currently catches all
Exceptions (the lines logging "Could not report weather to '{dev.name}': {exc}"
and the traceback.debug call); narrow this to only expected request/API errors
by catching the specific exception types raised by the Netro client (e.g.
NetroAPIError and ThrottleDelayError) plus requests request-related exceptions
(e.g. requests.exceptions.RequestException or ConnectionError/Timeout/HTTPError)
and leave the logging the same; replace the broad "except Exception as exc"
around the weather reporting for each device with a multi-except for these known
exception classes to preserve per-device isolation while letting programming
errors propagate.
- Around line 116-118: The plugin currently trusts raw prefs for
weatherUpdateInterval and can accept zero/negative values; change the assignment
of self._weather_update_interval (and the other runtime retrieval that uses
pluginPrefs.get around the same setting) to clamp to the minimum by converting
to int and taking max(..., DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES) so any 0 or
negative prefs fall back to the defined minimum, ensuring weather pushes respect
the minimum interval.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: acb72832-4d80-409b-856f-bad83dac69d9
📒 Files selected for processing (2)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/Devices.xmlNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py
✅ Files skipped from review due to trivial changes (1)
- Netro Sprinklers.indigoPlugin/Contents/Server Plugin/Devices.xml
- Hide Tomorrow.io config fields until enable checkbox is ticked (visibleBindingId on textfields, not just help labels) - Move weather update outside Netro token pause block — Tomorrow.io uses its own API and shouldn't be blocked by low Netro tokens - Add "Refresh Weather Now" plugin menu item for manual trigger - Call stateListOrDisplayStateIdChanged() on startup so Indigo picks up new weather states without disabling/re-enabling devices - Also trigger weather update from "Update All Status" menu item Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py (2)
455-458:⚠️ Potential issue | 🟠 MajorNarrow the per-device weather exception handler to expected request/API failures.
Line 457 catches
Exception, which can hide coding bugs in weather preparation/state updates. Catch only expected Netro/request errors here and let unexpected exceptions bubble to the outer polling-loop handler.#!/bin/bash set -euo pipefail # Verify exception types surfaced by report_weather path and available exception classes. fd -i '^api_client\.py$' -x sh -c ' echo "== {} =="; rg -n -C3 "def report_weather|raise .*Error|ThrottleDelayError|NetroAPIError|requests\\.exceptions" "{}" ' fd -i '^exceptions\.py$' -x sh -c ' echo "== {} =="; rg -n "class ThrottleDelayError|class NetroAPIError" "{}" ' fd -i '^plugin\.py$' -x sh -c ' echo "== {} =="; rg -n -C2 "except Exception as exc|_update_weather_from_tomorrow|report_weather\\(" "{}" '🤖 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 455 - 458, The broad except Exception in the per-device report_weather block should be narrowed to only the expected API/request failures so coding bugs can bubble up; replace the generic except Exception as exc with a specific catch (e.g., except NetroAPIError as exc) and include requests exceptions too if the client uses them (e.g., except (NetroAPIError, requests.exceptions.RequestException) as exc), ensuring you import NetroAPIError and requests.exceptions.RequestException at the top and keep the existing ThrottleDelayError handler and logging behavior in report_weather/_update_weather_from_tomorrow so unexpected errors are not swallowed.
116-118:⚠️ Potential issue | 🟠 MajorClamp and sanitize weather interval at runtime in both load/update paths.
Line 116 and Line 1072 still accept unchecked integers from prefs.
0/negative values can schedule weather updates continuously, and malformed persisted values can break initialization. Clamp toMINIMUM_WEATHER_UPDATE_INTERVAL_MINUTESwith a safe parse fallback in both places.♻️ Proposed fix
from constants import ( MAX_ZONE_DURATION_SECONDS, DEFAULT_API_TIMEOUT_SECONDS, DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES, + MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES, MINIMUM_POLLING_INTERVAL_MINUTES, ZONE_START_ENDPOINT, OPERATIONAL_ERROR_EVENTS, COMM_ERROR_EVENTS, DEVICE_EVENT_TYPES, ) @@ - self._weather_update_interval = int( - pluginPrefs.get("weatherUpdateInterval", DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES) - ) + try: + configured_interval = int( + pluginPrefs.get( + "weatherUpdateInterval", + DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES, + ) + ) + except (TypeError, ValueError): + configured_interval = DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES + self._weather_update_interval = max( + MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES, + configured_interval, + ) @@ try: - new_interval = int(valuesDict.get( - "weatherUpdateInterval", DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES - )) + new_interval = max( + MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES, + int( + valuesDict.get( + "weatherUpdateInterval", + DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES, + ) + ), + ) if new_interval != self._weather_update_interval: self._weather_update_interval = new_interval weather_settings_changed = TrueAlso applies to: 1072-1077
🤖 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 116 - 118, The weather update interval is taken directly from pluginPrefs and may be zero, negative, or malformed; change both places that set self._weather_update_interval (the initialization path using pluginPrefs.get("weatherUpdateInterval", DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES) and the prefs-update handler around lines ~1072-1077) to parse the value safely (e.g., int conversion with try/except or safe int coercion), fall back to DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES on parse failure, and then clamp the resulting integer to be at least MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES before assigning to self._weather_update_interval so negative/zero/malformed values cannot be used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Netro` Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py:
- Around line 455-458: The broad except Exception in the per-device
report_weather block should be narrowed to only the expected API/request
failures so coding bugs can bubble up; replace the generic except Exception as
exc with a specific catch (e.g., except NetroAPIError as exc) and include
requests exceptions too if the client uses them (e.g., except (NetroAPIError,
requests.exceptions.RequestException) as exc), ensuring you import NetroAPIError
and requests.exceptions.RequestException at the top and keep the existing
ThrottleDelayError handler and logging behavior in
report_weather/_update_weather_from_tomorrow so unexpected errors are not
swallowed.
- Around line 116-118: The weather update interval is taken directly from
pluginPrefs and may be zero, negative, or malformed; change both places that set
self._weather_update_interval (the initialization path using
pluginPrefs.get("weatherUpdateInterval",
DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES) and the prefs-update handler around
lines ~1072-1077) to parse the value safely (e.g., int conversion with
try/except or safe int coercion), fall back to
DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES on parse failure, and then clamp the
resulting integer to be at least MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTES before
assigning to self._weather_update_interval so negative/zero/malformed values
cannot be used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f9299b6-41ce-459f-9959-f8d1b5f18a7a
📒 Files selected for processing (3)
Netro Sprinklers.indigoPlugin/Contents/Server Plugin/MenuItems.xmlNetro Sprinklers.indigoPlugin/Contents/Server Plugin/PluginConfig.xmlNetro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py
✅ Files skipped from review due to trivial changes (1)
- Netro Sprinklers.indigoPlugin/Contents/Server Plugin/MenuItems.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- Netro Sprinklers.indigoPlugin/Contents/Server Plugin/PluginConfig.xml
Critical: - Log warning on invalid weather interval instead of bare pass - Log warning when Tomorrow.io enabled but API key/location missing Important: - Fix rain unit docstring: mm/hr (intensity) not total mm - Avoid leaking API key in HTTP error logs - Remove unused Tuple import from tomorrow_client - Fix Devices.xml rain state label to say mm/hr Suggestions: - Add traceback logging to broad except Exception handler - Log when weatherCode missing from API response (defaults to Cloudy) - Add wind override boundary tests at exactly 15.0 and 15.1 m/s - Update utils.py module docstring to list conversion functions - Update validate_prefs_config docstring to mention Tomorrow.io - Trigger weather update from RequestStatus action (consistency) - Document Cloudy default rationale and Beaufort scale reference - Update _update_weather_from_tomorrow docstring re device states Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
simons-plugins
deleted the
claude/add-tomorrow-weather-integration-IeqbO
branch
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Tomorrow.io weather API integration to automatically fetch current weather conditions and report them to Netro sprinkler devices. This allows users to override Netro's built-in weather data with more accurate local forecasts for smarter watering schedules.
Key Changes
New Weather Client Module
TomorrowClientclass that fetches real-time weather from Tomorrow.io API and transforms responses into Netro-compatible formatPlugin Integration
_create_tomorrow_client()to initialize client from plugin preferences_update_weather_from_tomorrow()to periodically fetch weather and report to all enabled sprinkler devicesUnit Conversion Utilities
celsius_to_fahrenheit(),mm_to_inches(),ms_to_mph(),hpa_to_inhg()convert_weather_metric_to_us(): Converts complete weather dict from metric to US unitsConfiguration & Validation
MINIMUM_WEATHER_UPDATE_INTERVAL_MINUTESconstantComprehensive Test Coverage
Implementation Details
https://claude.ai/code/session_01AdcdU17hU7qs5ecXCCUWJe
Summary by CodeRabbit
New Features
Tests