diff --git a/Netro Sprinklers.indigoPlugin/Contents/Info.plist b/Netro Sprinklers.indigoPlugin/Contents/Info.plist index e286d3d..22e459d 100644 --- a/Netro Sprinklers.indigoPlugin/Contents/Info.plist +++ b/Netro Sprinklers.indigoPlugin/Contents/Info.plist @@ -3,7 +3,7 @@ PluginVersion - 2026.4.1 + 2026.4.4 ServerApiVersion 3.6 IwsApiVersion diff --git a/Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py b/Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py index 10d128e..63b4327 100644 --- a/Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py +++ b/Netro Sprinklers.indigoPlugin/Contents/Server Plugin/api_client.py @@ -736,15 +736,57 @@ def start_watering( Args: key: Device serial number (v1) or API key (v2) - zones: List of zone dicts with id and duration + zones: Non-empty list of zone dicts with ``id`` (1-based zone + index, int) and ``duration`` (minutes, int — the Netro API + takes minutes on the wire for both v1 and v2). On v1 the + whole list is forwarded so each zone may have its own + duration. On v2 the API accepts only a flat ``zones`` array + of zone index integers plus a single top-level ``duration`` + that applies to every zone, so passing mixed durations to v2 + is a caller error and raises ``ValueError``. delay: Delay in minutes before starting (default 0) start_time: Optional epoch timestamp for scheduled start api_version: API version to use ("1" or "2") + Raises: + ValueError: ``zones`` is empty, a zone dict is missing ``id`` + or ``duration``, or v2 is called with per-zone durations + that aren't all identical. + Returns: API response confirming watering started """ - data: Dict[str, Any] = {"key": key, "zones": zones} + if not zones: + raise ValueError("start_watering requires at least one zone") + + # Validate shape once up front so callers get a single clear error + # rather than a bare KeyError/IndexError from deeper in the method. + parsed_zones: List[Dict[str, int]] = [] + for idx, z in enumerate(zones): + try: + parsed_zones.append({ + "id": int(z["id"]), + "duration": int(z["duration"]), + }) + except (KeyError, TypeError, ValueError) as exc: + raise ValueError( + f"start_watering: zones[{idx}] must be a dict with int " + f"'id' and 'duration' (got {z!r})" + ) from exc + + data: Dict[str, Any] = {"key": key} + if str(api_version) == "2": + durations = {z["duration"] for z in parsed_zones} + if len(durations) > 1: + raise ValueError( + "Netro v2 water.json only supports a single duration for " + "all zones; per-zone durations are not expressible on v2 " + f"(got {[(z['id'], z['duration']) for z in parsed_zones]})" + ) + data["zones"] = [z["id"] for z in parsed_zones] + data["duration"] = parsed_zones[0]["duration"] + else: + data["zones"] = parsed_zones if delay > 0: data["delay"] = delay if start_time: diff --git a/Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py b/Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py index 4aff24c..a6aef44 100644 --- a/Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py +++ b/Netro Sprinklers.indigoPlugin/Contents/Server Plugin/plugin.py @@ -54,7 +54,6 @@ DEFAULT_SENSOR_INTERVAL_MINUTES, DEFAULT_WEATHER_UPDATE_INTERVAL_MINUTES, DEFAULT_FORECAST_INTERVAL_MINUTES, - ZONE_START_ENDPOINT, OPERATIONAL_ERROR_EVENTS, COMM_ERROR_EVENTS, DEVICE_EVENT_TYPES, @@ -1559,30 +1558,59 @@ def actionControlSprinkler(self, action, dev): # ZONE ON # if action.sprinklerAction == indigo.kSprinklerAction.ZoneOn: - zone_dict = self._get_zone_dict(dev.states["id"], action.zoneIndex) - self.logger.debug(f"zone_dict: {zone_dict}") - if zone_dict: - zoneName = zone_dict["name"] - data = { - "id": zone_dict["id"], - "duration": (zone_dict["maxRuntime"] if zone_dict["maxRuntime"] <= self.maxZoneRunTime - else self.maxZoneRunTime), - } - try: - self.api_client.make_request(ZONE_START_ENDPOINT, method="put", data=data) - self.logger.info(f'sent "{dev.name} - {zoneName}" on') - dev.updateStateOnServer("activeZone", action.zoneIndex) - except requests.exceptions.RequestException: - # Network/HTTP error - log with traceback and fire trigger - self.logger.exception(f'send "{dev.name} - {zoneName}" on failed') - self._fireTrigger("startZoneFailed", dev.id) - except ThrottleDelayError: - self.logger.warning(f'send "{dev.name} - {zoneName}" throttled - in rate limit period') - self._fireTrigger("startZoneFailed", dev.id) - else: + zone_index = action.zoneIndex + if zone_index < 1 or zone_index > len(dev.zoneMaxDurations): self.logger.error( - f"Zone number {action.zoneIndex} doesn't exist in this controller " - f"and can't be enabled.") + f"Zone number {zone_index} doesn't exist on '{dev.name}' " + f"(has {len(dev.zoneMaxDurations)} zones)") + self._fireTrigger("startZoneFailed", dev.id) + return + + # A zone with max duration 0 is configured as disabled in the plugin + # prefs — refuse rather than silently clamping to 1 minute, otherwise + # users running a supposedly-off zone would get no feedback. + zone_max_seconds = dev.zoneMaxDurations[zone_index - 1] + if zone_max_seconds <= 0: + self.logger.error( + f"Zone {zone_index} on '{dev.name}' is disabled " + f"(max duration is 0) — cannot start") + self._fireTrigger("startZoneFailed", dev.id) + return + + # dev.zoneMaxDurations is seconds, Netro API expects minutes. + duration_seconds = min(zone_max_seconds, self.maxZoneRunTime) + duration_minutes = max(1, int(round(duration_seconds / 60))) + + try: + zone_name = dev.zoneNames[zone_index - 1] + except IndexError: + zone_name = f"Zone {zone_index}" + + try: + key, api_version = self._get_device_auth(dev) + zones = [{"id": zone_index, "duration": duration_minutes}] + response = self.api_client.start_watering(key, zones, api_version=api_version) + response_status = response.get("status") if isinstance(response, dict) else None + if response_status != "OK": + self.logger.error( + f'send "{dev.name} - {zone_name}" on rejected by Netro: {response}') + self._fireTrigger("startZoneFailed", dev.id) + return + self.logger.info( + f'sent "{dev.name} - {zone_name}" on for {duration_minutes}min') + dev.updateStateOnServer("activeZone", zone_index) + except requests.exceptions.RequestException: + self.logger.exception(f'send "{dev.name} - {zone_name}" on failed') + self._fireTrigger("startZoneFailed", dev.id) + except ThrottleDelayError: + self.logger.warning( + f'send "{dev.name} - {zone_name}" throttled - in rate limit period') + self._fireTrigger("startZoneFailed", dev.id) + except Exception: # pylint: disable=broad-exception-caught + # Anything else — auth lookup failure, stale dev props, malformed + # zone payload from api_client — surface loudly and mark failed so + # the user sees something actionable instead of a silent miss. + self.logger.exception(f'send "{dev.name} - {zone_name}" on errored') self._fireTrigger("startZoneFailed", dev.id) # ALL ZONES OFF # diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 5da2fda..8a4c730 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -1076,6 +1076,79 @@ def test_start_watering_v2_uses_v2_endpoint(self, client): called_data = json.loads(mock_post.call_args[1]["data"]) assert called_data["key"] == "API_KEY_V2" + def test_start_watering_v2_flattens_zones_payload(self, client): + """V2 water.json requires zones as flat ints + top-level duration.""" + mock_response = Mock() + mock_response.status_code = 200 + mock_response.json.return_value = {"status": "OK", "data": {}, "meta": {"token_remaining": 1900}} + + with patch("api_client.requests.post", return_value=mock_response) as mock_post: + client.start_watering( + "API_KEY_V2", + [{"id": 3, "duration": 15}, {"id": 5, "duration": 15}], + api_version="2", + ) + called_data = json.loads(mock_post.call_args[1]["data"]) + assert called_data["zones"] == [3, 5] + assert called_data["duration"] == 15 + # Should NOT carry the v1 nested shape on v2. + assert not any(isinstance(z, dict) for z in called_data["zones"]) + + def test_start_watering_v1_preserves_nested_zones(self, client): + """V1 water should preserve per-zone durations in the zones list.""" + mock_response = Mock() + mock_response.status_code = 200 + mock_response.json.return_value = {"status": "OK", "data": {}, "meta": {"token_remaining": 1900}} + + with patch("api_client.requests.post", return_value=mock_response) as mock_post: + client.start_watering( + "SERIAL123", + [{"id": 1, "duration": 10}, {"id": 2, "duration": 20}], + ) + called_data = json.loads(mock_post.call_args[1]["data"]) + assert called_data["zones"] == [{"id": 1, "duration": 10}, {"id": 2, "duration": 20}] + assert "duration" not in called_data + + def test_start_watering_empty_zones_raises(self, client): + """Empty zones list is a caller error, not a silent no-op.""" + with pytest.raises(ValueError, match="at least one zone"): + client.start_watering("SERIAL123", []) + + def test_start_watering_missing_id_raises(self, client): + """Zone dict missing 'id' raises a clear ValueError instead of bare KeyError.""" + with pytest.raises(ValueError, match="zones\\[0\\]"): + client.start_watering("SERIAL123", [{"duration": 10}]) + + def test_start_watering_missing_duration_raises(self, client): + """Zone dict missing 'duration' raises a clear ValueError.""" + with pytest.raises(ValueError, match="zones\\[0\\]"): + client.start_watering("SERIAL123", [{"id": 1}]) + + def test_start_watering_v2_mixed_durations_raises(self, client): + """V2 cannot express per-zone durations — mixed input must raise.""" + with pytest.raises(ValueError, match="single duration"): + client.start_watering( + "API_KEY_V2", + [{"id": 3, "duration": 15}, {"id": 5, "duration": 20}], + api_version="2", + ) + + def test_start_watering_v2_same_durations_succeeds(self, client): + """V2 with uniform durations should succeed and flatten the payload.""" + mock_response = Mock() + mock_response.status_code = 200 + mock_response.json.return_value = {"status": "OK", "data": {}, "meta": {"token_remaining": 1900}} + + with patch("api_client.requests.post", return_value=mock_response) as mock_post: + client.start_watering( + "API_KEY_V2", + [{"id": 3, "duration": 15}, {"id": 5, "duration": 15}], + api_version="2", + ) + called_data = json.loads(mock_post.call_args[1]["data"]) + assert called_data["zones"] == [3, 5] + assert called_data["duration"] == 15 + def test_get_events_uses_v2_endpoint(self, client): """get_events should always use v2 endpoint.""" mock_response = Mock()