From e2ada1973aa8c40ee8f71d764aaf0d2e9213deb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 02:55:26 +0000 Subject: [PATCH 1/4] feat(scene): add a dedicated weather scene trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the community request "Scène : nouveau déclencheur sur la météo" (topic 10523): run a scene from the weather itself — close the blinds when the wind picks up, stop the watering when it rains, alert on frost or on a storm. Per the project lead's answer in that thread, the weather is exposed as a *dedicated trigger*, not as a device whose features would be usable everywhere. A weather *condition* (evaluated inside a running scene) is a separate request and is deliberately out of scope here. Server: - new `weather.matched` trigger type, configured with a house, a watched pivot property (temperature, wind_speed, humidity, condition) and the `operator` / `value` couple already shared by the threshold triggers - new gated job `check-weather-triggers` (every 15 min) reusing the core weather provider loop: it only polls when an active scene carries the trigger, guards against overlapping runs, and is also relaunched by the external-integration freshness nudge - the poll sends the current *and* the previous payload, so the matcher is stateless and edge-triggered: a scene runs when the rule starts matching, not on every poll while it keeps matching. The first poll after a start is a baseline, like the weather-alert trigger - wind speed is compared in km/h (the pivot carries m/s), the unit the dashboard widget displays and the one users write their rules with Front: new WeatherTrigger editor component, trigger type registered in the trigger list, i18n keys added in en/fr/de. Spec: docs/specs/external-integrations.md B.18 gains point 7. --- docs/specs/external-integrations.md | 10 +- front/src/config/i18n/de.json | 19 +- front/src/config/i18n/en.json | 19 +- front/src/config/i18n/fr.json | 19 +- .../routes/scene/edit-scene/TriggerCard.jsx | 11 +- .../triggers/ChooseTriggerTypeCard.jsx | 3 +- .../edit-scene/triggers/WeatherTrigger.jsx | 214 ++++++++++++++++++ server/config/scheduler-jobs.js | 5 + ...xternalIntegration.handleWeatherRefresh.js | 10 +- server/lib/scene/scene.triggers.js | 53 ++++- server/lib/weather/index.js | 9 + server/lib/weather/weather.checkTriggers.js | 92 ++++++++ server/models/scene.js | 11 +- ...externalIntegration.weatherRefresh.test.js | 8 +- .../triggers/scene.trigger.weather.test.js | 205 +++++++++++++++++ .../lib/weather/weather.checkTriggers.test.js | 152 +++++++++++++ server/utils/constants.js | 16 ++ 17 files changed, 841 insertions(+), 15 deletions(-) create mode 100644 front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx create mode 100644 server/lib/weather/weather.checkTriggers.js create mode 100644 server/test/lib/scene/triggers/scene.trigger.weather.test.js create mode 100644 server/test/lib/weather/weather.checkTriggers.test.js diff --git a/docs/specs/external-integrations.md b/docs/specs/external-integrations.md index 06916bd221..f58de25721 100644 --- a/docs/specs/external-integrations.md +++ b/docs/specs/external-integrations.md @@ -397,10 +397,18 @@ The core widget renders the base fields as today and the extensions **when prese **4. The weather-alert scene trigger (core-owned — works with every provider, zero integration contract)**. The field need (the MF pilot again): "run a scene when the vigilance level raises". The trigger is fed by the **core**, not by a push channel: a scheduled job (`check-weather-alerts`, every 30 min) — **gated**: it runs only when at least one active scene carries a weather-alert trigger, so users without such scenes cost their provider zero extra API calls — pulls the weather of every house with coordinates through the normal provider loop, and **diffs the normalized alerts** against the previous poll. Diff identity: the alert `type` when present, the trimmed lowercased `event` text otherwise — one more reason `type` exists. Severity is ranked `minor < moderate < severe < extreme`; a new alert **or a severity increase** fires `weather.alert-raised`, a disappeared alert fires `weather.alert-ended`, a de-escalation that does not clear the alert fires nothing (the alert is still on — re-announcing it adds noise, its end will fire). The first poll after a core start is a **baseline**: no events (a restart during an ongoing storm must not re-fire every scene). Trigger configuration: house, optional phenomenon type filter (`any` by default), minimal severity (`minor` by default). Because the diff runs on **normalized pivot alerts**, the trigger works identically for every provider — OpenWeather One Call alerts included — and the integration has nothing to implement. -**5. The freshness nudge — "trigger, not data" (B.17 doctrine, transposed)**. 30 min is the floor without integration cooperation; a provider that *knows* something changed can do better, but never by pushing data: **`external-integration.weather.refresh`** (integration → core, empty payload, no `message_id`, no ack — fire-and-forget) means only "re-pull me now". The core then runs the exact same gated check as the scheduled job: the data re-enters through the audited pull path (`weather.get` → `normalizeWeather` → diff), the push channel carries **zero state**. Accepted from "weather" integrations only; rate-limited to 1 per minute per integration, silently dropped beyond (fire-and-forget has no error path, and a dropped nudge costs at most the 30-min floor). The checks are **serialized**: a nudge (or the scheduled job) landing while a check is still in flight is dropped — two overlapping runs would diff the same baseline and fire every transition twice. The MF integration polls vigilance upstream, nudges on change: the scene fires seconds later. +**5. The freshness nudge — "trigger, not data" (B.17 doctrine, transposed)**. 30 min is the floor without integration cooperation; a provider that *knows* something changed can do better, but never by pushing data: **`external-integration.weather.refresh`** (integration → core, empty payload, no `message_id`, no ack — fire-and-forget) means only "re-pull me now". The core then runs the exact same gated checks as the scheduled jobs — the alert diff (point 4) and the weather scene trigger (point 7): the data re-enters through the audited pull path (`weather.get` → `normalizeWeather` → diff), the push channel carries **zero state**. Accepted from "weather" integrations only; rate-limited to 1 per minute per integration, silently dropped beyond (fire-and-forget has no error path, and a dropped nudge costs at most the 30-min floor). The checks are **serialized**: a nudge (or the scheduled job) landing while a check is still in flight is dropped — two overlapping runs would diff the same baseline and fire every transition twice. The MF integration polls vigilance upstream, nudges on change: the scene fires seconds later. **6. Provider images (vigilance map, rain radar, satellite view)**. The pivot format is deliberately numeric/textual — images do not belong in a weather payload, but weather providers genuinely have them (MF vigilance map, Buienradar/DWD/MeteoSwiss rain radars). The generic channel keeps the trust boundary intact: the payload only ever declares **metadata** — optional `images` (≤ 3 entries): `key` (`^[a-z0-9][a-z0-9-]{0,31}$`) and optional multi-language `label` (values ≤ 50 chars) — and the bytes travel **on demand** over `external-integration.weather.get-image` `{ message_id, key }` → `command-result` `data.image` (raw base64, no data-URI prefix; 15 s ack, the camera exception). The core validates the **decoded bytes** (PNG or JPEG magic numbers only, ≤ 500 KB), caches them 10 min per (integration, key), and serves them to the frontend from its own origin (`GET /api/v1/house/:house_selector/weather/image/:key`, standard user auth) as a data URI — the browser **never loads a third-party URL** (no user-IP leak on every dashboard render, the same doctrine as the catalog cover re-hosting, C.1). The widget gains one "provider images" toggle and renders every declared image with its label; a provider without images loses nothing. The request path is **allowlisted end to end**: the core shape-checks the requested key against the declaration regex before consulting any provider, and the proxy only relays a key **declared in the last normalized payload** of that integration — an undeclared key 404s without a single byte sent to the integration, so an authenticated caller can neither probe the integration with arbitrary keys nor grow the 10-min cache past the declared set (≤ 3 entries per integration, the images cap). Provider images are **instance-global, not per-house**: neither the route (whose house selector only scopes authentication and 404) nor `weather.get-image` carries coordinates — the provider serves the images it knows (a national vigilance map, or the area of the last `weather.get` it answered). If a per-house need materializes, threading the house coordinates into the command is a purely additive extension. +**7. The weather scene trigger (core-owned, same doctrine as point 4)**. The field need (community request, topic 10523): "close the blinds when the wind goes above 20 km/h, stop the watering when it rains, alert me when it freezes". Scope note from the request thread: the weather is deliberately **not** exposed as a device whose features would be usable everywhere — it is a **dedicated trigger**. A weather *condition* (evaluated inside a running scene) is a separate request and is not part of this point. + +Same shape as the alert trigger, so it works with every provider and costs nothing to integrations: a scheduled job (`check-weather-triggers`, every 15 min — weather thresholds are more time-sensitive than vigilance bulletins) — **gated**: it runs only when at least one active scene carries a `weather.matched` trigger — pulls the weather of every house with coordinates through the normal provider loop (`units: metric`), and emits the trigger check with **both the current and the previous payload**. The first poll after a core start is a **baseline**: no event, for the same reason as point 4. + +Trigger configuration: house, watched pivot property (`temperature`, `wind_speed`, `humidity`, `condition`), and the **shared `operator` / `value` couple of the threshold triggers** (`=`, `!=`, `>`, `>=`, `<`, `<=` — no new operator vocabulary). Compared values: °C for the temperature, % for the humidity, the pivot **condition enum** as a string for the condition, and **km/h** for the wind speed — the pivot carries m/s, the trigger converts it exactly like the dashboard widget, because km/h is the unit users write their rules in. A property the provider does not expose never matches, and a numeric rule whose value is empty or unparseable never matches. + +**Edge-triggered, never repeating**: the scene runs when the rule matches on the current payload and did **not** match on the previous one. So "wind > 20 km/h → close the blinds" runs once when the wind picks up, not every 15 minutes for as long as it blows. Because both payloads travel in the event, the matcher is stateless: editing a scene, adding a trigger or changing a threshold resets nothing. + **What v1 anticipated (and it held)**: extensible manifest `type`, catalog filter by type, WS `.` convention, proxy service extensible by capability (`device.*`, `message.*`, now `weather.*`). Phase 3: deprecation of the core `openweather` service in favor of a community-maintained external equivalent (same path as the communication services in B.15). ### B.20 Docker image cleanup diff --git a/front/src/config/i18n/de.json b/front/src/config/i18n/de.json index 4703487ea2..5afc183a50 100644 --- a/front/src/config/i18n/de.json +++ b/front/src/config/i18n/de.json @@ -3512,7 +3512,8 @@ }, "weather": { "alert-raised": "Wetterwarnung ausgelöst", - "alert-ended": "Wetterwarnung beendet" + "alert-ended": "Wetterwarnung beendet", + "matched": "Wetter" } }, "triggersCard": { @@ -3645,6 +3646,22 @@ "severe": "Schwer (orange)", "extreme": "Extrem (rot)" } + }, + "weather": { + "description": "Dieser Auslöser wird ausgelöst, sobald das Wetter des ausgewählten Hauses die untenstehende Regel erfüllt. Das Wetter wird alle 15 Minuten geprüft: Die Szene wird nur beim Wechsel ausgeführt und startet daher nicht erneut, solange die Regel weiterhin zutrifft.", + "houseLabel": "Haus", + "fieldLabel": "Wetterwert", + "fields": { + "temperature": "Temperatur", + "wind_speed": "Windgeschwindigkeit", + "humidity": "Luftfeuchtigkeit", + "condition": "Wetterlage" + }, + "operatorLabel": "Operator", + "valueLabel": "Wert", + "unitTemperature": "°C", + "unitWindSpeed": "km/h", + "unitHumidity": "%" } } }, diff --git a/front/src/config/i18n/en.json b/front/src/config/i18n/en.json index 3080417573..3b64c83ac1 100644 --- a/front/src/config/i18n/en.json +++ b/front/src/config/i18n/en.json @@ -3512,7 +3512,8 @@ }, "weather": { "alert-raised": "Weather alert raised", - "alert-ended": "Weather alert ended" + "alert-ended": "Weather alert ended", + "matched": "Weather" } }, "triggersCard": { @@ -3645,6 +3646,22 @@ "severe": "Severe (orange)", "extreme": "Extreme (red)" } + }, + "weather": { + "description": "This will trigger when the weather of the selected house starts matching the rule below. The weather is checked every 15 minutes: the scene runs on the transition only, so it does not run again for as long as the rule stays true.", + "houseLabel": "House", + "fieldLabel": "Weather property", + "fields": { + "temperature": "Temperature", + "wind_speed": "Wind speed", + "humidity": "Humidity", + "condition": "Weather condition" + }, + "operatorLabel": "Operator", + "valueLabel": "Value", + "unitTemperature": "°C", + "unitWindSpeed": "km/h", + "unitHumidity": "%" } } }, diff --git a/front/src/config/i18n/fr.json b/front/src/config/i18n/fr.json index 152327809a..d100cda013 100644 --- a/front/src/config/i18n/fr.json +++ b/front/src/config/i18n/fr.json @@ -3512,7 +3512,8 @@ }, "weather": { "alert-raised": "Alerte météo déclenchée", - "alert-ended": "Alerte météo terminée" + "alert-ended": "Alerte météo terminée", + "matched": "Météo" } }, "triggersCard": { @@ -3645,6 +3646,22 @@ "severe": "Sévère (orange)", "extreme": "Extrême (rouge)" } + }, + "weather": { + "description": "Ce déclencheur se lance quand la météo de la maison sélectionnée commence à vérifier la règle ci-dessous. La météo est vérifiée toutes les 15 minutes : la scène se lance uniquement au changement, elle ne se relance donc pas tant que la règle reste vraie.", + "houseLabel": "Maison", + "fieldLabel": "Donnée météo", + "fields": { + "temperature": "Température", + "wind_speed": "Vitesse du vent", + "humidity": "Humidité", + "condition": "Condition météo" + }, + "operatorLabel": "Opérateur", + "valueLabel": "Valeur", + "unitTemperature": "°C", + "unitWindSpeed": "km/h", + "unitHumidity": "%" } } }, diff --git a/front/src/routes/scene/edit-scene/TriggerCard.jsx b/front/src/routes/scene/edit-scene/TriggerCard.jsx index 6b1e076495..3f16fdaf26 100644 --- a/front/src/routes/scene/edit-scene/TriggerCard.jsx +++ b/front/src/routes/scene/edit-scene/TriggerCard.jsx @@ -12,6 +12,7 @@ import CalendarEventIsComing from './triggers/CalendarEventIsComing'; import AlarmModeTrigger from './triggers/AlarmModeTrigger'; import MQTTReceivedTrigger from './triggers/MQTTReceivedTrigger'; import WeatherAlert from './triggers/WeatherAlert'; +import WeatherTrigger from './triggers/WeatherTrigger'; import { EVENTS } from '../../../../../server/utils/constants'; import GladysStartTrigger from './triggers/GladysStartTrigger'; @@ -36,7 +37,8 @@ const TRIGGER_ICON = { [EVENTS.SYSTEM.START]: 'fe-activity', [EVENTS.MQTT.RECEIVED]: 'fe-hash', [EVENTS.WEATHER.ALERT_RAISED]: 'fe-alert-triangle', - [EVENTS.WEATHER.ALERT_ENDED]: 'fe-check-circle' + [EVENTS.WEATHER.ALERT_ENDED]: 'fe-check-circle', + [EVENTS.WEATHER.MATCHED]: 'fe-cloud' }; const WEATHER_ALERT_TRIGGERS = [EVENTS.WEATHER.ALERT_RAISED, EVENTS.WEATHER.ALERT_ENDED]; @@ -182,6 +184,13 @@ const TriggerCard = ({ children, ...props }) => ( {WEATHER_ALERT_TRIGGERS.includes(props.trigger.type) && ( )} + {props.trigger.type === EVENTS.WEATHER.MATCHED && ( + + )} ); diff --git a/front/src/routes/scene/edit-scene/triggers/ChooseTriggerTypeCard.jsx b/front/src/routes/scene/edit-scene/triggers/ChooseTriggerTypeCard.jsx index daedbb7a7d..8f87461efc 100644 --- a/front/src/routes/scene/edit-scene/triggers/ChooseTriggerTypeCard.jsx +++ b/front/src/routes/scene/edit-scene/triggers/ChooseTriggerTypeCard.jsx @@ -28,7 +28,8 @@ const TRIGGER_LIST = [ EVENTS.SYSTEM.START, EVENTS.MQTT.RECEIVED, EVENTS.WEATHER.ALERT_RAISED, - EVENTS.WEATHER.ALERT_ENDED + EVENTS.WEATHER.ALERT_ENDED, + EVENTS.WEATHER.MATCHED ]; class ChooseTriggerType extends Component { diff --git a/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx b/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx new file mode 100644 index 0000000000..76d0dadb47 --- /dev/null +++ b/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx @@ -0,0 +1,214 @@ +import { Component } from 'preact'; +import { connect } from 'unistore/preact'; +import { Text } from 'preact-i18n'; + +import { RequestStatus } from '../../../../utils/consts'; +import { WEATHER_TRIGGER_FIELDS } from '../../../../../../server/utils/constants'; + +// the generic condition enum of the weather pivot format (B.18). 'night' +// and 'unknown' are left out on purpose: they are not weather situations +// a user would build a scene on. +const WEATHER_CONDITIONS = [ + 'clear', + 'partly-cloudy', + 'cloud', + 'fog', + 'drizzle', + 'rain', + 'pouring', + 'sleet', + 'hail', + 'snow', + 'thunderstorm', + 'wind' +]; + +const NUMERIC_OPERATORS = ['>', '>=', '<', '<=', '=', '!=']; +const CONDITION_OPERATORS = ['=', '!=']; + +const OPERATOR_LABELS = { + '=': 'editScene.triggersCard.newState.equal', + '>': 'editScene.triggersCard.newState.superior', + '>=': 'editScene.triggersCard.newState.superiorOrEqual', + '<': 'editScene.triggersCard.newState.less', + '<=': 'editScene.triggersCard.newState.lessOrEqual', + '!=': 'editScene.triggersCard.newState.different' +}; + +const UNIT_LABELS = { + [WEATHER_TRIGGER_FIELDS.TEMPERATURE]: 'editScene.triggersCard.weather.unitTemperature', + [WEATHER_TRIGGER_FIELDS.WIND_SPEED]: 'editScene.triggersCard.weather.unitWindSpeed', + [WEATHER_TRIGGER_FIELDS.HUMIDITY]: 'editScene.triggersCard.weather.unitHumidity' +}; + +// the wind speed rule of the original request, so a freshly added trigger +// is already meaningful +const DEFAULT_FIELD = WEATHER_TRIGGER_FIELDS.WIND_SPEED; +const DEFAULT_OPERATOR = '>'; +const DEFAULT_VALUE = 20; + +class WeatherTrigger extends Component { + getHouses = async () => { + this.setState({ + SceneGetHouses: RequestStatus.Getting + }); + try { + const houses = await this.props.httpClient.get('/api/v1/house'); + this.setState({ + houses, + SceneGetHouses: RequestStatus.Success + }); + } catch (e) { + this.setState({ + SceneGetHouses: RequestStatus.Error + }); + } + }; + + onHouseChange = e => { + this.props.updateTriggerProperty(this.props.index, 'house', e.target.value); + }; + + onFieldChange = e => { + const field = e.target.value; + this.props.updateTriggerProperty(this.props.index, 'weather_field', field); + // the operator and the value vocabularies differ between a condition + // (a string of the pivot enum) and a measure (a number): switching + // field resets both, so the trigger is never left in a mixed state + if (field === WEATHER_TRIGGER_FIELDS.CONDITION) { + this.props.updateTriggerProperty(this.props.index, 'operator', '='); + this.props.updateTriggerProperty(this.props.index, 'value', WEATHER_CONDITIONS[0]); + } else { + this.setState({ valueInput: String(DEFAULT_VALUE) }); + this.props.updateTriggerProperty(this.props.index, 'operator', DEFAULT_OPERATOR); + this.props.updateTriggerProperty(this.props.index, 'value', DEFAULT_VALUE); + } + }; + + onOperatorChange = e => { + this.props.updateTriggerProperty(this.props.index, 'operator', e.target.value); + }; + + onConditionChange = e => { + this.props.updateTriggerProperty(this.props.index, 'value', e.target.value); + }; + + onValueChange = e => { + const raw = e.target.value; + this.setState({ valueInput: raw }); + const value = parseFloat(raw.replace(',', '.')); + // an empty or unparseable input leaves no `value` at all on the trigger: + // the scene stays saveable, and a rule without a value never matches + this.props.updateTriggerProperty(this.props.index, 'value', Number.isNaN(value) ? undefined : value); + }; + + constructor(props) { + super(props); + const isFreshTrigger = !props.trigger.weather_field; + const isCondition = props.trigger.weather_field === WEATHER_TRIGGER_FIELDS.CONDITION; + let valueInput = ''; + if (isFreshTrigger) { + valueInput = String(DEFAULT_VALUE); + } else if (!isCondition && props.trigger.value !== undefined) { + valueInput = String(props.trigger.value); + } + this.state = { + houses: [], + valueInput + }; + } + + componentDidMount() { + this.getHouses(); + // defaults, so a freshly added trigger is valid without touching the selects + if (!this.props.trigger.weather_field) { + this.props.updateTriggerProperty(this.props.index, 'weather_field', DEFAULT_FIELD); + this.props.updateTriggerProperty(this.props.index, 'operator', DEFAULT_OPERATOR); + this.props.updateTriggerProperty(this.props.index, 'value', DEFAULT_VALUE); + } + } + + render({ trigger }, { houses, valueInput }) { + const field = trigger.weather_field || DEFAULT_FIELD; + const isCondition = field === WEATHER_TRIGGER_FIELDS.CONDITION; + const operators = isCondition ? CONDITION_OPERATORS : NUMERIC_OPERATORS; + return ( +
+

+ +

+
+
+ +
+ +
+
+
+ +
+ +
+
+
+
+
+ +
+ +
+
+
+
+
+ +
+ {isCondition ? ( + + ) : ( +
+ + + + + + +
+ )} +
+
+
+
+ ); + } +} + +export default connect('httpClient,user', {})(WeatherTrigger); diff --git a/server/config/scheduler-jobs.js b/server/config/scheduler-jobs.js index e83a3fcd5a..877fcbd063 100644 --- a/server/config/scheduler-jobs.js +++ b/server/config/scheduler-jobs.js @@ -36,6 +36,11 @@ const jobs = [ rule: '0 */30 * * * *', // every 30 minutes event: EVENTS.WEATHER.CHECK_ALERTS, }, + { + name: 'check-weather-triggers', + rule: '0 */15 * * * *', // every 15 minutes + event: EVENTS.WEATHER.CHECK_TRIGGERS, + }, ]; module.exports = jobs; diff --git a/server/lib/external-integration/externalIntegration.handleWeatherRefresh.js b/server/lib/external-integration/externalIntegration.handleWeatherRefresh.js index 0d77e58627..be9b03fc21 100644 --- a/server/lib/external-integration/externalIntegration.handleWeatherRefresh.js +++ b/server/lib/external-integration/externalIntegration.handleWeatherRefresh.js @@ -5,10 +5,11 @@ const { WEATHER_REFRESH_MIN_INTERVAL_MS } = require('./constants'); /** * @description Handle the freshness nudge of a "weather" integration * (B.18 point 5, "trigger, not data"): re-run the same gated alert check - * as the 30-min scheduled poll — the data re-enters through the audited - * pull path, the nudge itself carries nothing. Fire-and-forget: a nudge - * from a non-weather integration or beyond the rate limit is silently - * dropped (no error path, the 30-min floor catches up). + * as the 30-min scheduled poll, and the same gated weather-trigger check + * as the 15-min one — the data re-enters through the audited pull path, + * the nudge itself carries nothing. Fire-and-forget: a nudge from a + * non-weather integration or beyond the rate limit is silently dropped + * (no error path, the scheduled floors catch up). * @param {object} service - The sending integration service. * @example * externalIntegration.handleWeatherRefresh(service); @@ -27,6 +28,7 @@ function handleWeatherRefresh(service) { } this.weatherRefreshTimes.set(service.id, now); this.event.emit(EVENTS.WEATHER.CHECK_ALERTS); + this.event.emit(EVENTS.WEATHER.CHECK_TRIGGERS); } module.exports = { diff --git a/server/lib/scene/scene.triggers.js b/server/lib/scene/scene.triggers.js index ff722b3418..101ff46ec6 100644 --- a/server/lib/scene/scene.triggers.js +++ b/server/lib/scene/scene.triggers.js @@ -1,7 +1,7 @@ const cloneDeep = require('lodash.clonedeep'); const logger = require('../../utils/logger'); -const { EVENTS } = require('../../utils/constants'); +const { EVENTS, WEATHER_TRIGGER_FIELDS } = require('../../utils/constants'); const { compare } = require('../../utils/compare'); const matchSunEvent = (self, sceneSelector, event, trigger) => @@ -25,6 +25,56 @@ const matchWeatherAlert = (self, sceneSelector, event, trigger) => WEATHER_ALERT_SEVERITY_RANK[event.alert.severity] >= (WEATHER_ALERT_SEVERITY_RANK[trigger.weather_alert_severity] || 1); +// How each watched property of a weather trigger is read in a pivot +// weather payload. The core always polls in metric units, so the compared +// values are °C, % and the pivot condition enum as-is. The pivot wind +// speed is in m/s: it is converted to km/h, the unit the dashboard widget +// displays and the one users configure their scenes with. +const WEATHER_TRIGGER_VALUE_GETTERS = { + [WEATHER_TRIGGER_FIELDS.TEMPERATURE]: (weather) => weather.temperature, + [WEATHER_TRIGGER_FIELDS.HUMIDITY]: (weather) => weather.humidity, + [WEATHER_TRIGGER_FIELDS.WIND_SPEED]: (weather) => + typeof weather.wind_speed === 'number' ? weather.wind_speed * 3.6 : undefined, + [WEATHER_TRIGGER_FIELDS.CONDITION]: (weather) => weather.weather, +}; + +// undefined when there is no payload (first poll of the house) or when the +// provider does not expose the watched property (everything but the +// temperature and the condition is optional in the pivot format) +const getWeatherTriggerValue = (weather, field) => { + const getter = WEATHER_TRIGGER_VALUE_GETTERS[field]; + if (weather === undefined || weather === null || getter === undefined) { + return undefined; + } + return getter(weather); +}; + +// Same house, and the watched property matches the rule *now* while it did +// not at the previous poll: the trigger is a transition, so a scene does +// not re-run every poll for as long as it keeps raining. The event carries +// both payloads, so the matcher stays stateless and editing a scene never +// resets anything. +const matchWeather = (self, sceneSelector, event, trigger) => { + if (event.house !== trigger.house) { + return false; + } + // the condition is compared as a string of the pivot enum, everything + // else as a number — a value left empty in the UI never matches + const isCondition = trigger.weather_field === WEATHER_TRIGGER_FIELDS.CONDITION; + const expectedValue = isCondition ? trigger.value : Number(trigger.value); + if (!isCondition && Number.isNaN(expectedValue)) { + return false; + } + const currentValue = getWeatherTriggerValue(event.weather, trigger.weather_field); + if (currentValue === undefined || currentValue === null) { + return false; + } + const previousValue = getWeatherTriggerValue(event.previous_weather, trigger.weather_field); + const previousValueValidateRule = + previousValue !== undefined && previousValue !== null && compare(trigger.operator, previousValue, expectedValue); + return compare(trigger.operator, currentValue, expectedValue) && !previousValueValidateRule; +}; + const triggersFunc = { [EVENTS.DEVICE.NEW_STATE]: (self, sceneSelector, event, trigger) => { // Multi-select triggers store their features in `device_features`, legacy triggers @@ -134,6 +184,7 @@ const triggersFunc = { event.topic === trigger.topic && (!trigger.message || trigger.message === event.message), [EVENTS.WEATHER.ALERT_RAISED]: matchWeatherAlert, [EVENTS.WEATHER.ALERT_ENDED]: matchWeatherAlert, + [EVENTS.WEATHER.MATCHED]: matchWeather, }; module.exports = { diff --git a/server/lib/weather/index.js b/server/lib/weather/index.js index ee3e941561..5a434caaa9 100644 --- a/server/lib/weather/index.js +++ b/server/lib/weather/index.js @@ -3,6 +3,7 @@ const { getImage } = require('./weather.getImage'); const { getProviders } = require('./weather.getProviders'); const { command } = require('./weather.command'); const { checkAlerts } = require('./weather.checkAlerts'); +const { checkTriggers } = require('./weather.checkTriggers'); const { INTENTS, EVENTS } = require('../../utils/constants'); const { eventFunctionWrapper } = require('../../utils/functionsWrapper'); @@ -17,11 +18,18 @@ const Weather = function Weather(service, event, messageManager, house) { // in-flight guard of checkAlerts: the scheduled job and the freshness // nudge must never diff the same baseline concurrently this.checkAlertsRunning = false; + // last weather payload per house selector, compared by checkTriggers so + // the weather scene trigger only fires on a transition; in-memory on + // purpose: a restart re-baselines without firing + this.houseWeather = new Map(); + // in-flight guard of checkTriggers, same reason as checkAlerts + this.checkTriggersRunning = false; this.event.on(INTENTS.WEATHER.GET, this.command.bind(this)); this.event.on(INTENTS.WEATHER.TOMORROW, this.command.bind(this)); this.event.on(INTENTS.WEATHER.AFTER_TOMORROW, this.command.bind(this)); this.event.on(INTENTS.WEATHER.DAY, this.command.bind(this)); this.event.on(EVENTS.WEATHER.CHECK_ALERTS, eventFunctionWrapper(this.checkAlerts.bind(this))); + this.event.on(EVENTS.WEATHER.CHECK_TRIGGERS, eventFunctionWrapper(this.checkTriggers.bind(this))); }; Weather.prototype.get = get; @@ -29,5 +37,6 @@ Weather.prototype.getImage = getImage; Weather.prototype.getProviders = getProviders; Weather.prototype.command = command; Weather.prototype.checkAlerts = checkAlerts; +Weather.prototype.checkTriggers = checkTriggers; module.exports = Weather; diff --git a/server/lib/weather/weather.checkTriggers.js b/server/lib/weather/weather.checkTriggers.js new file mode 100644 index 0000000000..3eac491e33 --- /dev/null +++ b/server/lib/weather/weather.checkTriggers.js @@ -0,0 +1,92 @@ +const Promise = require('bluebird'); +const db = require('../../models'); +const logger = require('../../utils/logger'); +const { EVENTS, WEATHER_UNITS } = require('../../utils/constants'); + +/** + * @description The actual check, always called under the in-flight guard + * of checkTriggers. + * @returns {Promise} Resolves when every house has been checked. + * @example + * await runCheck.call(this); + */ +async function runCheck() { + // same gate as the alert check: a LIKE probe on the JSON column is + // dialect-dependent, so the trigger types are checked in JS on the + // active scenes — a light query, ran at most every 15 min + const activeScenes = await db.Scene.findAll({ + where: { active: true }, + attributes: ['triggers'], + }); + // the triggers column is NOT NULL: every scene carries an array + const someSceneListens = activeScenes.some((scene) => + scene.triggers.some((trigger) => trigger.type === EVENTS.WEATHER.MATCHED), + ); + if (!someSceneListens) { + return; + } + const houses = await this.house.get(); + const locatedHouses = houses.filter((house) => house.latitude !== null && house.longitude !== null); + await Promise.each(locatedHouses, async (house) => { + let weather; + try { + weather = await this.get({ + latitude: house.latitude, + longitude: house.longitude, + language: 'en', + units: WEATHER_UNITS.METRIC, + }); + } catch (e) { + // no provider configured or provider down: nothing to compare, the + // previous payload is kept so recovery does not fire a scene on a + // transition the user never lived + logger.debug(`weather.checkTriggers: no weather for house ${house.selector}: ${e.message}`); + return; + } + const previousWeather = this.houseWeather.get(house.selector); + this.houseWeather.set(house.selector, weather); + if (previousWeather === undefined) { + // first poll of the house: baseline only, a core restart while it is + // already raining must not re-run every weather scene + return; + } + this.event.emit(EVENTS.TRIGGERS.CHECK, { + type: EVENTS.WEATHER.MATCHED, + house: house.selector, + weather, + previous_weather: previousWeather, + }); + }); +} + +/** + * @description Poll the weather of every located house and feed the + * dedicated weather scene trigger with the current and the previous + * payload, so the scene engine only fires on a transition. Runs every 15 + * minutes (scheduler job check-weather-triggers) and on an integration + * freshness nudge. Gated: no active scene with a weather trigger means + * zero third-party calls. + * @returns {Promise} Resolves when every house has been checked. + * @example + * await weather.checkTriggers(); + */ +async function checkTriggers() { + // the scheduled job and the freshness nudge both land here: two + // overlapping runs would compare against the same previous payload and + // could fire the same transition twice, so a run already in flight wins + // and the new one is dropped (a dropped nudge costs at most the 15-min + // floor) + if (this.checkTriggersRunning) { + return; + } + this.checkTriggersRunning = true; + try { + await runCheck.call(this); + } finally { + this.checkTriggersRunning = false; + } +} + +module.exports = { + checkTriggers, +}; diff --git a/server/models/scene.js b/server/models/scene.js index 9142fa90ff..67b8cabe3e 100644 --- a/server/models/scene.js +++ b/server/models/scene.js @@ -1,5 +1,11 @@ const Joi = require('@hapi/joi').extend(require('@hapi/joi-date')); -const { ACTION_LIST, ACTIONS, EVENT_LIST, ALARM_MODES_LIST } = require('../utils/constants'); +const { + ACTION_LIST, + ACTIONS, + EVENT_LIST, + ALARM_MODES_LIST, + WEATHER_TRIGGER_FIELDS_LIST, +} = require('../utils/constants'); const { WEATHER_ALERT_TYPES, WEATHER_ALERT_SEVERITIES } = require('../lib/external-integration/constants'); const { addSelectorBeforeValidateHook } = require('../utils/addSelector'); const iconList = require('../config/icons.json'); @@ -154,6 +160,9 @@ const triggersSchema = Joi.array().items( // weather-alert triggers (B.18): phenomenon type filter and minimal severity weather_alert_type: Joi.string().valid(...WEATHER_ALERT_TYPES, 'any'), weather_alert_severity: Joi.string().valid(...WEATHER_ALERT_SEVERITIES), + // weather trigger: the watched pivot property, compared with the shared + // `operator` / `value` couple of the threshold triggers + weather_field: Joi.string().valid(...WEATHER_TRIGGER_FIELDS_LIST), }), ); diff --git a/server/test/lib/external-integration/externalIntegration.weatherRefresh.test.js b/server/test/lib/external-integration/externalIntegration.weatherRefresh.test.js index cc912f255c..8ae9b2b2f1 100644 --- a/server/test/lib/external-integration/externalIntegration.weatherRefresh.test.js +++ b/server/test/lib/external-integration/externalIntegration.weatherRefresh.test.js @@ -7,23 +7,25 @@ const { EVENTS } = require('../../../utils/constants'); const { buildSupervisor, seedExternalService, TEST_WEATHER_MANIFEST } = require('./testUtils.test'); describe('externalIntegration.handleWeatherRefresh', () => { - it('should relaunch the alert check on a nudge from a weather integration', async () => { + it('should relaunch the alert and the weather trigger checks on a nudge from a weather integration', async () => { const { externalIntegration, event } = buildSupervisor(); const service = await seedExternalService({ manifest: TEST_WEATHER_MANIFEST }); externalIntegration.handleWeatherRefresh(service); sinonAssert.calledWith(event.emit, EVENTS.WEATHER.CHECK_ALERTS); + sinonAssert.calledWith(event.emit, EVENTS.WEATHER.CHECK_TRIGGERS); }); it('should rate-limit nudges to one per minute per integration', async () => { const { externalIntegration, event } = buildSupervisor(); const service = await seedExternalService({ manifest: TEST_WEATHER_MANIFEST }); + // an accepted nudge relaunches both gated checks externalIntegration.handleWeatherRefresh(service); externalIntegration.handleWeatherRefresh(service); - expect(event.emit.callCount).to.equal(1); + expect(event.emit.callCount).to.equal(2); // a nudge older than the window is accepted again externalIntegration.weatherRefreshTimes.set(service.id, Date.now() - 61 * 1000); externalIntegration.handleWeatherRefresh(service); - expect(event.emit.callCount).to.equal(2); + expect(event.emit.callCount).to.equal(4); }); it('should silently ignore a nudge from a non-weather integration', async () => { diff --git a/server/test/lib/scene/triggers/scene.trigger.weather.test.js b/server/test/lib/scene/triggers/scene.trigger.weather.test.js new file mode 100644 index 0000000000..dee6681622 --- /dev/null +++ b/server/test/lib/scene/triggers/scene.trigger.weather.test.js @@ -0,0 +1,205 @@ +const sinon = require('sinon').createSandbox(); +const { expect } = require('chai'); + +const { assert, fake } = sinon; + +const EventEmitter = require('events'); +const StateManager = require('../../../../lib/state'); +const SceneManager = require('../../../../lib/scene'); +const { triggersFunc } = require('../../../../lib/scene/scene.triggers'); +const { ACTIONS, EVENTS, WEATHER_TRIGGER_FIELDS } = require('../../../../utils/constants'); + +const event = new EventEmitter(); + +const matcher = triggersFunc[EVENTS.WEATHER.MATCHED]; + +const buildEvent = (weather, previousWeather, house = 'my-house') => ({ + type: EVENTS.WEATHER.MATCHED, + house, + weather, + previous_weather: previousWeather, +}); + +describe('Scene.triggers.weather', () => { + let sceneManager; + + const device = { + setValue: fake.resolves(null), + }; + + const brain = {}; + + const service = { + getService: fake.returns(null), + }; + + beforeEach(() => { + const house = { + get: fake.resolves([]), + }; + + const scheduler = { + scheduleJob: (date, callback) => { + return { + callback, + date, + cancel: () => {}, + }; + }, + }; + + brain.addNamedEntity = fake.returns(null); + brain.removeNamedEntity = fake.returns(null); + + const stateManager = new StateManager(); + + sceneManager = new SceneManager(stateManager, event, device, {}, {}, house, {}, {}, {}, scheduler, brain, service); + }); + + afterEach(() => { + sinon.reset(); + }); + + it('should execute the scene when the wind speed crosses the threshold', async () => { + await sceneManager.addScene({ + selector: 'weather-scene', + active: true, + actions: [ + [ + { + type: ACTIONS.LIGHT.TURN_OFF, + devices: ['light-1'], + }, + ], + ], + triggers: [ + { + type: EVENTS.WEATHER.MATCHED, + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.WIND_SPEED, + operator: '>', + value: 20, + }, + ], + }); + // 8 m/s = 28.8 km/h, previously 2 m/s = 7.2 km/h + sceneManager.checkTrigger(buildEvent({ wind_speed: 8 }, { wind_speed: 2 })); + return new Promise((resolve, reject) => { + sceneManager.queue.start(() => { + try { + assert.calledOnce(device.setValue); + resolve(); + } catch (e) { + reject(e); + } + }); + }); + }); + + it('should not match another house', () => { + const trigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.TEMPERATURE, + operator: '<', + value: 0, + }; + expect(matcher(null, 'scene', buildEvent({ temperature: -3 }, { temperature: 5 }, 'other'), trigger)).to.equal( + false, + ); + }); + + it('should compare the wind speed in km/h', () => { + const trigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.WIND_SPEED, + operator: '>', + value: 20, + }; + // 6 m/s = 21.6 km/h, above the threshold expressed in km/h + expect(matcher(null, 'scene', buildEvent({ wind_speed: 6 }, { wind_speed: 1 }), trigger)).to.equal(true); + // 5 m/s = 18 km/h, below it — the raw m/s value would have matched + expect(matcher(null, 'scene', buildEvent({ wind_speed: 5 }, { wind_speed: 1 }), trigger)).to.equal(false); + // a provider that does not expose the wind speed never matches + expect(matcher(null, 'scene', buildEvent({ temperature: 12 }, { wind_speed: 1 }), trigger)).to.equal(false); + }); + + it('should only fire on the transition, not while the rule stays true', () => { + const trigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.TEMPERATURE, + operator: '<=', + value: 0, + }; + // the frost starts + expect(matcher(null, 'scene', buildEvent({ temperature: -1 }, { temperature: 3 }), trigger)).to.equal(true); + // it lasts: the scene must not run again at the next poll + expect(matcher(null, 'scene', buildEvent({ temperature: -4 }, { temperature: -1 }), trigger)).to.equal(false); + // it ends: nothing either + expect(matcher(null, 'scene', buildEvent({ temperature: 4 }, { temperature: -4 }), trigger)).to.equal(false); + }); + + it('should fire when the previous payload does not carry the watched property', () => { + const trigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.HUMIDITY, + operator: '>=', + value: 90, + }; + // no previous payload at all (event built without one) + expect(matcher(null, 'scene', buildEvent({ humidity: 95 }), trigger)).to.equal(true); + // a previous payload that does not expose the property + expect(matcher(null, 'scene', buildEvent({ humidity: 95 }, { temperature: 3 }), trigger)).to.equal(true); + // an explicitly null previous payload + expect(matcher(null, 'scene', buildEvent({ humidity: 95 }, null), trigger)).to.equal(true); + // a null current value never matches + expect(matcher(null, 'scene', buildEvent({ humidity: null }, { humidity: 10 }), trigger)).to.equal(false); + // a null previous value is not a previous match: the trigger fires + expect(matcher(null, 'scene', buildEvent({ humidity: 95 }, { humidity: null }), trigger)).to.equal(true); + }); + + it('should compare the weather condition as a string of the pivot enum', () => { + const trigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.CONDITION, + operator: '=', + value: 'thunderstorm', + }; + expect(matcher(null, 'scene', buildEvent({ weather: 'thunderstorm' }, { weather: 'cloud' }), trigger)).to.equal( + true, + ); + expect( + matcher(null, 'scene', buildEvent({ weather: 'thunderstorm' }, { weather: 'thunderstorm' }), trigger), + ).to.equal(false); + expect(matcher(null, 'scene', buildEvent({ weather: 'rain' }, { weather: 'cloud' }), trigger)).to.equal(false); + // "the weather is no longer clear" + const notClear = { ...trigger, operator: '!=', value: 'clear' }; + expect(matcher(null, 'scene', buildEvent({ weather: 'rain' }, { weather: 'clear' }), notClear)).to.equal(true); + }); + + it('should never match a numeric rule without a usable value', () => { + const trigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.TEMPERATURE, + operator: '<', + }; + // value left empty in the UI + expect(matcher(null, 'scene', buildEvent({ temperature: -3 }, { temperature: 5 }), trigger)).to.equal(false); + expect( + matcher(null, 'scene', buildEvent({ temperature: -3 }, { temperature: 5 }), { ...trigger, value: 'abc' }), + ).to.equal(false); + // a numeric value stored as a string stays comparable + expect( + matcher(null, 'scene', buildEvent({ temperature: -3 }, { temperature: 5 }), { ...trigger, value: '0' }), + ).to.equal(true); + }); + + it('should never match an unknown watched property', () => { + const trigger = { + house: 'my-house', + weather_field: 'pressure', + operator: '>', + value: 1000, + }; + expect(matcher(null, 'scene', buildEvent({ pressure: 1020 }, { pressure: 990 }), trigger)).to.equal(false); + }); +}); diff --git a/server/test/lib/weather/weather.checkTriggers.test.js b/server/test/lib/weather/weather.checkTriggers.test.js new file mode 100644 index 0000000000..4342c3a669 --- /dev/null +++ b/server/test/lib/weather/weather.checkTriggers.test.js @@ -0,0 +1,152 @@ +const { expect } = require('chai'); +const sinon = require('sinon').createSandbox(); + +const { fake } = sinon; + +const db = require('../../../models'); +const Weather = require('../../../lib/weather'); +const { EVENTS, WEATHER_TRIGGER_FIELDS } = require('../../../utils/constants'); + +const HOUSE = { selector: 'weather-trigger-house', latitude: 48.85, longitude: 2.35 }; + +const buildWeather = (getResults) => { + // getResults: array of payloads (or Error) returned by successive polls + let call = -1; + const provider = { + weather: { + get: fake(async () => { + call += 1; + const result = getResults[Math.min(call, getResults.length - 1)]; + if (result instanceof Error) { + throw result; + } + return result; + }), + }, + }; + const service = { + getService: () => provider, + stateManager: { + getAllKeys: () => ['ext-fake-weather'], + }, + }; + const event = { on: fake.returns(null), emit: fake.returns(null) }; + const house = { get: fake.resolves([HOUSE, { selector: 'no-gps-house', latitude: null, longitude: null }]) }; + const weather = new Weather(service, event, {}, house); + return { weather, event, provider, house }; +}; + +const triggerCheckCalls = (event) => + event.emit.getCalls().filter((callObject) => callObject.args[0] === EVENTS.TRIGGERS.CHECK); + +describe('weather.checkTriggers', () => { + let scene; + // the bootstrap cleans and re-seeds the database between every test: + // the listening scene must be re-created each time. The create also + // exercises the new Joi `weather_field` of the scene model. + beforeEach(async () => { + scene = await db.Scene.create({ + name: 'Weather trigger scene test', + icon: 'fe-cloud', + active: true, + triggers: [ + { + type: EVENTS.WEATHER.MATCHED, + house: HOUSE.selector, + weather_field: WEATHER_TRIGGER_FIELDS.WIND_SPEED, + operator: '>', + value: 20, + }, + ], + actions: [[]], + }); + }); + afterEach(() => { + sinon.reset(); + }); + + it('should not call any provider when no active scene listens to the weather trigger', async () => { + await db.Scene.update({ active: false }, { where: { id: scene.id } }); + const { weather, provider } = buildWeather([{ temperature: 12 }]); + await weather.checkTriggers(); + expect(provider.weather.get.callCount).to.equal(0); + }); + + it('should baseline on the first poll and send both payloads on the next ones', async () => { + const first = { temperature: 12, wind_speed: 2 }; + const second = { temperature: 14, wind_speed: 8 }; + const { weather, event, provider } = buildWeather([first, second]); + + // poll 1: baseline, no event — a restart while it is already windy + // must not re-run every weather scene + await weather.checkTriggers(); + expect(triggerCheckCalls(event)).to.have.lengthOf(0); + + // poll 2: the trigger check gets the current and the previous payload + await weather.checkTriggers(); + const calls = triggerCheckCalls(event); + expect(calls).to.have.lengthOf(1); + expect(calls[0].args[1]).to.deep.equal({ + type: EVENTS.WEATHER.MATCHED, + house: HOUSE.selector, + weather: second, + previous_weather: first, + }); + // the house without coordinates is skipped: one call per poll + expect(provider.weather.get.callCount).to.equal(2); + }); + + it('should drop a check landing while another one is still in flight', async () => { + let resolveInFlight; + const inFlight = new Promise((resolve) => { + resolveInFlight = resolve; + }); + let call = 0; + const provider = { + weather: { + get: fake(() => { + call += 1; + return call === 1 ? Promise.resolve({ temperature: 2 }) : inFlight; + }), + }, + }; + const service = { getService: () => provider, stateManager: { getAllKeys: () => ['ext-fake-weather'] } }; + const event = { on: fake.returns(null), emit: fake.returns(null) }; + const house = { get: fake.resolves([HOUSE]) }; + const weather = new Weather(service, event, {}, house); + + // poll 1: baseline + await weather.checkTriggers(); + + // poll 2 hangs on the provider; poll 3 lands while it is in flight + // and must be dropped immediately, without waiting for the provider + const second = weather.checkTriggers(); + const third = weather.checkTriggers(); + await third; + resolveInFlight({ temperature: 5 }); + await second; + + expect(provider.weather.get.callCount).to.equal(2); + expect(triggerCheckCalls(event)).to.have.lengthOf(1); + + // the guard is released: a later check polls and compares again + await weather.checkTriggers(); + expect(provider.weather.get.callCount).to.equal(3); + expect(triggerCheckCalls(event)).to.have.lengthOf(2); + }); + + it('should keep the previous payload when the provider fails', async () => { + const payload = { temperature: 12, wind_speed: 2 }; + const { weather, event, provider } = buildWeather([payload, new Error('provider down'), payload]); + await weather.checkTriggers(); + await weather.checkTriggers(); + // the failing poll emitted nothing at all + expect(triggerCheckCalls(event)).to.have.lengthOf(0); + await weather.checkTriggers(); + const calls = triggerCheckCalls(event); + expect(provider.weather.get.callCount).to.equal(3); + // the baseline of the first poll survived the failure + expect(calls).to.have.lengthOf(1); + expect(calls[0].args[1].previous_weather).to.deep.equal(payload); + }); +}); diff --git a/server/utils/constants.js b/server/utils/constants.js index a266424629..644cb39f6b 100644 --- a/server/utils/constants.js +++ b/server/utils/constants.js @@ -601,6 +601,8 @@ const EVENTS = { CHECK_ALERTS: 'weather.check-alerts', ALERT_RAISED: 'weather.alert-raised', ALERT_ENDED: 'weather.alert-ended', + CHECK_TRIGGERS: 'weather.check-triggers', + MATCHED: 'weather.matched', }, EXTERNAL_INTEGRATION: { STATUS_CHANGED: 'external-integration.status-changed', @@ -1504,6 +1506,18 @@ const WEATHER_UNITS = { METRIC: 'metric', }; +// The pivot weather properties a "weather" scene trigger can watch. The +// core always polls the provider in metric units, so the compared values +// are: °C for the temperature, km/h for the wind speed (the pivot carries +// m/s, the trigger converts it like the dashboard widget does), % for the +// humidity and the pivot condition enum for the condition. +const WEATHER_TRIGGER_FIELDS = { + TEMPERATURE: 'temperature', + WIND_SPEED: 'wind_speed', + HUMIDITY: 'humidity', + CONDITION: 'condition', +}; + const DEVICE_FEATURE_UNITS_BY_CATEGORY = { [DEVICE_FEATURE_CATEGORIES.SWITCH]: [ DEVICE_FEATURE_UNITS.AMPERE, @@ -2246,6 +2260,8 @@ module.exports.DASHBOARD_BOX_TYPE_LIST = DASHBOARD_BOX_TYPE_LIST; module.exports.ERROR_MESSAGES = ERROR_MESSAGES; module.exports.WEATHER_UNITS = WEATHER_UNITS; +module.exports.WEATHER_TRIGGER_FIELDS = WEATHER_TRIGGER_FIELDS; +module.exports.WEATHER_TRIGGER_FIELDS_LIST = createList(WEATHER_TRIGGER_FIELDS); module.exports.DEVICE_FEATURE_STATE_AGGREGATE_TYPES = DEVICE_FEATURE_STATE_AGGREGATE_TYPES; module.exports.DEVICE_FEATURE_STATE_AGGREGATE_TYPES_LIST = DEVICE_FEATURE_STATE_AGGREGATE_TYPES_LIST; From ffacb01382a9a5f75a0d375004e8d5b165f7ec57 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 08:14:43 +0000 Subject: [PATCH 2/4] fix(weather): share the provider pulls and compare what the widget shows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on the weather scene trigger: - the alert check and the weather-trigger check no longer pull the same house twice when they overlap (every half hour, and on a freshness nudge): while any check run is in flight, a house is pulled once and the payload is handed to both. The sharing window is exactly the overlap of the runs, so a check never compares against data older than its own poll - the trigger check only polls the houses a `weather.matched` trigger actually watches, instead of every house with coordinates; a house leaving the watched set drops its baseline so it re-baselines when it comes back - the compared numbers are rounded like the dashboard widget displays them: 5.55 m/s reads "20 km/h" on the widget, so it now matches `>= 20` instead of comparing 19.98 - the trigger editor displays and reads the threshold in the unit system of the user (°F, mph) and keeps storing metric, instead of hardcoding °C / km/h next to an imperial dashboard - B.18 points 5 and 7 restate the floors (15 min thresholds / 30 min alerts), the per-check serialization and the shared pulls Autofix-Pass: 1 --- docs/specs/external-integrations.md | 6 +- front/src/config/i18n/de.json | 2 + front/src/config/i18n/en.json | 2 + front/src/config/i18n/fr.json | 2 + .../edit-scene/triggers/WeatherTrigger.jsx | 83 ++++++++++++++++--- ...xternalIntegration.handleWeatherRefresh.js | 5 +- server/lib/scene/scene.triggers.js | 12 ++- server/lib/weather/index.js | 10 +++ server/lib/weather/weather.checkAlerts.js | 10 +-- server/lib/weather/weather.checkTriggers.js | 57 ++++++++----- server/lib/weather/weather.pullForChecks.js | 62 ++++++++++++++ .../triggers/scene.trigger.weather.test.js | 34 ++++++++ .../lib/weather/weather.checkTriggers.test.js | 55 +++++++++++- 13 files changed, 293 insertions(+), 47 deletions(-) create mode 100644 server/lib/weather/weather.pullForChecks.js diff --git a/docs/specs/external-integrations.md b/docs/specs/external-integrations.md index f58de25721..ffff553cb6 100644 --- a/docs/specs/external-integrations.md +++ b/docs/specs/external-integrations.md @@ -397,15 +397,15 @@ The core widget renders the base fields as today and the extensions **when prese **4. The weather-alert scene trigger (core-owned — works with every provider, zero integration contract)**. The field need (the MF pilot again): "run a scene when the vigilance level raises". The trigger is fed by the **core**, not by a push channel: a scheduled job (`check-weather-alerts`, every 30 min) — **gated**: it runs only when at least one active scene carries a weather-alert trigger, so users without such scenes cost their provider zero extra API calls — pulls the weather of every house with coordinates through the normal provider loop, and **diffs the normalized alerts** against the previous poll. Diff identity: the alert `type` when present, the trimmed lowercased `event` text otherwise — one more reason `type` exists. Severity is ranked `minor < moderate < severe < extreme`; a new alert **or a severity increase** fires `weather.alert-raised`, a disappeared alert fires `weather.alert-ended`, a de-escalation that does not clear the alert fires nothing (the alert is still on — re-announcing it adds noise, its end will fire). The first poll after a core start is a **baseline**: no events (a restart during an ongoing storm must not re-fire every scene). Trigger configuration: house, optional phenomenon type filter (`any` by default), minimal severity (`minor` by default). Because the diff runs on **normalized pivot alerts**, the trigger works identically for every provider — OpenWeather One Call alerts included — and the integration has nothing to implement. -**5. The freshness nudge — "trigger, not data" (B.17 doctrine, transposed)**. 30 min is the floor without integration cooperation; a provider that *knows* something changed can do better, but never by pushing data: **`external-integration.weather.refresh`** (integration → core, empty payload, no `message_id`, no ack — fire-and-forget) means only "re-pull me now". The core then runs the exact same gated checks as the scheduled jobs — the alert diff (point 4) and the weather scene trigger (point 7): the data re-enters through the audited pull path (`weather.get` → `normalizeWeather` → diff), the push channel carries **zero state**. Accepted from "weather" integrations only; rate-limited to 1 per minute per integration, silently dropped beyond (fire-and-forget has no error path, and a dropped nudge costs at most the 30-min floor). The checks are **serialized**: a nudge (or the scheduled job) landing while a check is still in flight is dropped — two overlapping runs would diff the same baseline and fire every transition twice. The MF integration polls vigilance upstream, nudges on change: the scene fires seconds later. +**5. The freshness nudge — "trigger, not data" (B.17 doctrine, transposed)**. The scheduled floors are 30 min for the alert diff (point 4) and 15 min for the weather scene trigger (point 7); without integration cooperation, that is as fresh as it gets. A provider that *knows* something changed can do better, but never by pushing data: **`external-integration.weather.refresh`** (integration → core, empty payload, no `message_id`, no ack — fire-and-forget) means only "re-pull me now". The core then runs the exact same gated checks as the scheduled jobs: the data re-enters through the audited pull path (`weather.get` → `normalizeWeather` → diff), the push channel carries **zero state**. Accepted from "weather" integrations only; rate-limited to 1 per minute per integration, silently dropped beyond (fire-and-forget has no error path, and a dropped nudge costs at most the floor of the check it would have relaunched). Each check is **serialized with itself**: a nudge (or the scheduled job) landing while that check is still in flight is dropped — two overlapping runs of the same check would diff the same baseline and fire every transition twice. The two checks, on the other hand, do run **concurrently** — they have different floors and coincide every half hour, and the nudge relaunches both — so they **share their provider pulls**: while any check run is in flight, a house is pulled once and the payload is handed to both (the sharing window is exactly the overlap of the runs, so no check ever compares against data older than its own poll). Two check types therefore cost one `weather.get` per house, not two. The MF integration polls vigilance upstream, nudges on change: the scene fires seconds later. **6. Provider images (vigilance map, rain radar, satellite view)**. The pivot format is deliberately numeric/textual — images do not belong in a weather payload, but weather providers genuinely have them (MF vigilance map, Buienradar/DWD/MeteoSwiss rain radars). The generic channel keeps the trust boundary intact: the payload only ever declares **metadata** — optional `images` (≤ 3 entries): `key` (`^[a-z0-9][a-z0-9-]{0,31}$`) and optional multi-language `label` (values ≤ 50 chars) — and the bytes travel **on demand** over `external-integration.weather.get-image` `{ message_id, key }` → `command-result` `data.image` (raw base64, no data-URI prefix; 15 s ack, the camera exception). The core validates the **decoded bytes** (PNG or JPEG magic numbers only, ≤ 500 KB), caches them 10 min per (integration, key), and serves them to the frontend from its own origin (`GET /api/v1/house/:house_selector/weather/image/:key`, standard user auth) as a data URI — the browser **never loads a third-party URL** (no user-IP leak on every dashboard render, the same doctrine as the catalog cover re-hosting, C.1). The widget gains one "provider images" toggle and renders every declared image with its label; a provider without images loses nothing. The request path is **allowlisted end to end**: the core shape-checks the requested key against the declaration regex before consulting any provider, and the proxy only relays a key **declared in the last normalized payload** of that integration — an undeclared key 404s without a single byte sent to the integration, so an authenticated caller can neither probe the integration with arbitrary keys nor grow the 10-min cache past the declared set (≤ 3 entries per integration, the images cap). Provider images are **instance-global, not per-house**: neither the route (whose house selector only scopes authentication and 404) nor `weather.get-image` carries coordinates — the provider serves the images it knows (a national vigilance map, or the area of the last `weather.get` it answered). If a per-house need materializes, threading the house coordinates into the command is a purely additive extension. **7. The weather scene trigger (core-owned, same doctrine as point 4)**. The field need (community request, topic 10523): "close the blinds when the wind goes above 20 km/h, stop the watering when it rains, alert me when it freezes". Scope note from the request thread: the weather is deliberately **not** exposed as a device whose features would be usable everywhere — it is a **dedicated trigger**. A weather *condition* (evaluated inside a running scene) is a separate request and is not part of this point. -Same shape as the alert trigger, so it works with every provider and costs nothing to integrations: a scheduled job (`check-weather-triggers`, every 15 min — weather thresholds are more time-sensitive than vigilance bulletins) — **gated**: it runs only when at least one active scene carries a `weather.matched` trigger — pulls the weather of every house with coordinates through the normal provider loop (`units: metric`), and emits the trigger check with **both the current and the previous payload**. The first poll after a core start is a **baseline**: no event, for the same reason as point 4. +Same shape as the alert trigger, so it works with every provider and costs nothing to integrations: a scheduled job (`check-weather-triggers`, every 15 min — weather thresholds are more time-sensitive than vigilance bulletins) — **gated twice**: it runs only when at least one active scene carries a `weather.matched` trigger, and it then pulls **only the houses those triggers actually watch** (a trigger matches on its own house, so polling the others would buy nothing and cost a provider call every 15 min) — through the normal provider loop (`units: metric`), and emits the trigger check with **both the current and the previous payload**. When it overlaps the alert check, the pull of a house is shared between the two (point 5). The first poll after a core start is a **baseline**: no event, for the same reason as point 4 — and a house that leaves the watched set (scene deactivated, trigger removed) drops its baseline, so it re-baselines when it comes back instead of comparing against a payload from another day. -Trigger configuration: house, watched pivot property (`temperature`, `wind_speed`, `humidity`, `condition`), and the **shared `operator` / `value` couple of the threshold triggers** (`=`, `!=`, `>`, `>=`, `<`, `<=` — no new operator vocabulary). Compared values: °C for the temperature, % for the humidity, the pivot **condition enum** as a string for the condition, and **km/h** for the wind speed — the pivot carries m/s, the trigger converts it exactly like the dashboard widget, because km/h is the unit users write their rules in. A property the provider does not expose never matches, and a numeric rule whose value is empty or unparseable never matches. +Trigger configuration: house, watched pivot property (`temperature`, `wind_speed`, `humidity`, `condition`), and the **shared `operator` / `value` couple of the threshold triggers** (`=`, `!=`, `>`, `>=`, `<`, `<=` — no new operator vocabulary). Compared values: °C for the temperature, % for the humidity, the pivot **condition enum** as a string for the condition, and **km/h** for the wind speed — the pivot carries m/s, the trigger converts it exactly like the dashboard widget, because km/h is the unit users write their rules in. The numbers are compared **as the widget displays them**, rounded (`Math.round`, the widget's own formatting): a rule is written from what the dashboard shows, so 5.55 m/s — displayed as 20 km/h — matches `>= 20` instead of comparing 19.98 behind the user's back. A property the provider does not expose never matches, and a numeric rule whose value is empty or unparseable never matches. The stored value is always metric; the scene editor **displays and reads it in the unit system of the user** (`temperature_unit_preference`, `distance_unit_preference` — °F and mph shown and converted back on save), so a threshold copied from an imperial dashboard means what it reads. **Edge-triggered, never repeating**: the scene runs when the rule matches on the current payload and did **not** match on the previous one. So "wind > 20 km/h → close the blinds" runs once when the wind picks up, not every 15 minutes for as long as it blows. Because both payloads travel in the event, the matcher is stateless: editing a scene, adding a trigger or changing a threshold resets nothing. diff --git a/front/src/config/i18n/de.json b/front/src/config/i18n/de.json index 5afc183a50..5fcbc5c6fe 100644 --- a/front/src/config/i18n/de.json +++ b/front/src/config/i18n/de.json @@ -3660,7 +3660,9 @@ "operatorLabel": "Operator", "valueLabel": "Wert", "unitTemperature": "°C", + "unitTemperatureImperial": "°F", "unitWindSpeed": "km/h", + "unitWindSpeedImperial": "mph", "unitHumidity": "%" } } diff --git a/front/src/config/i18n/en.json b/front/src/config/i18n/en.json index 3b64c83ac1..8ad45e5e7c 100644 --- a/front/src/config/i18n/en.json +++ b/front/src/config/i18n/en.json @@ -3660,7 +3660,9 @@ "operatorLabel": "Operator", "valueLabel": "Value", "unitTemperature": "°C", + "unitTemperatureImperial": "°F", "unitWindSpeed": "km/h", + "unitWindSpeedImperial": "mph", "unitHumidity": "%" } } diff --git a/front/src/config/i18n/fr.json b/front/src/config/i18n/fr.json index d100cda013..5257c5dd48 100644 --- a/front/src/config/i18n/fr.json +++ b/front/src/config/i18n/fr.json @@ -3660,7 +3660,9 @@ "operatorLabel": "Opérateur", "valueLabel": "Valeur", "unitTemperature": "°C", + "unitTemperatureImperial": "°F", "unitWindSpeed": "km/h", + "unitWindSpeedImperial": "mph", "unitHumidity": "%" } } diff --git a/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx b/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx index 76d0dadb47..7223bf4f36 100644 --- a/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx +++ b/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx @@ -3,7 +3,12 @@ import { connect } from 'unistore/preact'; import { Text } from 'preact-i18n'; import { RequestStatus } from '../../../../utils/consts'; -import { WEATHER_TRIGGER_FIELDS } from '../../../../../../server/utils/constants'; +import { + WEATHER_TRIGGER_FIELDS, + DEVICE_FEATURE_UNITS, + MEASUREMENT_UNITS +} from '../../../../../../server/utils/constants'; +import { celsiusToFahrenheit, fahrenheitToCelsius } from '../../../../../../server/utils/units'; // the generic condition enum of the weather pivot format (B.18). 'night' // and 'unknown' are left out on purpose: they are not weather situations @@ -35,14 +40,59 @@ const OPERATOR_LABELS = { '!=': 'editScene.triggersCard.newState.different' }; -const UNIT_LABELS = { - [WEATHER_TRIGGER_FIELDS.TEMPERATURE]: 'editScene.triggersCard.weather.unitTemperature', - [WEATHER_TRIGGER_FIELDS.WIND_SPEED]: 'editScene.triggersCard.weather.unitWindSpeed', - [WEATHER_TRIGGER_FIELDS.HUMIDITY]: 'editScene.triggersCard.weather.unitHumidity' +// the core polls the provider in metric and compares in °C / km/h / %, so +// a trigger always *stores* a metric value. The editor displays and reads +// it in the unit system of the user (same preferences as the weather +// widget and EditRoomTemperatureBox), otherwise a user on Fahrenheit would +// copy a threshold from their dashboard and save a rule meaning something +// else. +const KM_PER_MILE = 1.60934; + +const isImperialTemperature = user => !!user && user.temperature_unit_preference === DEVICE_FEATURE_UNITS.FAHRENHEIT; +const isImperialWindSpeed = user => !!user && user.distance_unit_preference === MEASUREMENT_UNITS.US; + +const roundValue = value => Math.round(value * 10) / 10; + +const toDisplayValue = (value, field, user) => { + if (typeof value !== 'number') { + return value; + } + if (field === WEATHER_TRIGGER_FIELDS.TEMPERATURE && isImperialTemperature(user)) { + return roundValue(celsiusToFahrenheit(value)); + } + if (field === WEATHER_TRIGGER_FIELDS.WIND_SPEED && isImperialWindSpeed(user)) { + return roundValue(value / KM_PER_MILE); + } + return value; +}; + +const toStoredValue = (value, field, user) => { + if (field === WEATHER_TRIGGER_FIELDS.TEMPERATURE && isImperialTemperature(user)) { + return fahrenheitToCelsius(value); + } + if (field === WEATHER_TRIGGER_FIELDS.WIND_SPEED && isImperialWindSpeed(user)) { + return value * KM_PER_MILE; + } + return value; +}; + +const getUnitLabel = (field, user) => { + if (field === WEATHER_TRIGGER_FIELDS.TEMPERATURE) { + return isImperialTemperature(user) + ? 'editScene.triggersCard.weather.unitTemperatureImperial' + : 'editScene.triggersCard.weather.unitTemperature'; + } + if (field === WEATHER_TRIGGER_FIELDS.WIND_SPEED) { + return isImperialWindSpeed(user) + ? 'editScene.triggersCard.weather.unitWindSpeedImperial' + : 'editScene.triggersCard.weather.unitWindSpeed'; + } + return 'editScene.triggersCard.weather.unitHumidity'; }; // the wind speed rule of the original request, so a freshly added trigger -// is already meaningful +// is already meaningful. Metric, like every stored value: an imperial user +// sees it converted (20 km/h -> 12.4 mph) const DEFAULT_FIELD = WEATHER_TRIGGER_FIELDS.WIND_SPEED; const DEFAULT_OPERATOR = '>'; const DEFAULT_VALUE = 20; @@ -79,7 +129,7 @@ class WeatherTrigger extends Component { this.props.updateTriggerProperty(this.props.index, 'operator', '='); this.props.updateTriggerProperty(this.props.index, 'value', WEATHER_CONDITIONS[0]); } else { - this.setState({ valueInput: String(DEFAULT_VALUE) }); + this.setState({ valueInput: String(toDisplayValue(DEFAULT_VALUE, field, this.props.user)) }); this.props.updateTriggerProperty(this.props.index, 'operator', DEFAULT_OPERATOR); this.props.updateTriggerProperty(this.props.index, 'value', DEFAULT_VALUE); } @@ -97,9 +147,16 @@ class WeatherTrigger extends Component { const raw = e.target.value; this.setState({ valueInput: raw }); const value = parseFloat(raw.replace(',', '.')); + const field = this.props.trigger.weather_field || DEFAULT_FIELD; // an empty or unparseable input leaves no `value` at all on the trigger: - // the scene stays saveable, and a rule without a value never matches - this.props.updateTriggerProperty(this.props.index, 'value', Number.isNaN(value) ? undefined : value); + // the scene stays saveable, and a rule without a value never matches. + // What the user types is in their own unit system, what is stored is + // metric + this.props.updateTriggerProperty( + this.props.index, + 'value', + Number.isNaN(value) ? undefined : toStoredValue(value, field, this.props.user) + ); }; constructor(props) { @@ -108,9 +165,9 @@ class WeatherTrigger extends Component { const isCondition = props.trigger.weather_field === WEATHER_TRIGGER_FIELDS.CONDITION; let valueInput = ''; if (isFreshTrigger) { - valueInput = String(DEFAULT_VALUE); + valueInput = String(toDisplayValue(DEFAULT_VALUE, DEFAULT_FIELD, props.user)); } else if (!isCondition && props.trigger.value !== undefined) { - valueInput = String(props.trigger.value); + valueInput = String(toDisplayValue(props.trigger.value, props.trigger.weather_field, props.user)); } this.state = { houses: [], @@ -128,7 +185,7 @@ class WeatherTrigger extends Component { } } - render({ trigger }, { houses, valueInput }) { + render({ trigger, user }, { houses, valueInput }) { const field = trigger.weather_field || DEFAULT_FIELD; const isCondition = field === WEATHER_TRIGGER_FIELDS.CONDITION; const operators = isCondition ? CONDITION_OPERATORS : NUMERIC_OPERATORS; @@ -198,7 +255,7 @@ class WeatherTrigger extends Component { - + diff --git a/server/lib/external-integration/externalIntegration.handleWeatherRefresh.js b/server/lib/external-integration/externalIntegration.handleWeatherRefresh.js index be9b03fc21..08b1f69c56 100644 --- a/server/lib/external-integration/externalIntegration.handleWeatherRefresh.js +++ b/server/lib/external-integration/externalIntegration.handleWeatherRefresh.js @@ -7,7 +7,10 @@ const { WEATHER_REFRESH_MIN_INTERVAL_MS } = require('./constants'); * (B.18 point 5, "trigger, not data"): re-run the same gated alert check * as the 30-min scheduled poll, and the same gated weather-trigger check * as the 15-min one — the data re-enters through the audited pull path, - * the nudge itself carries nothing. Fire-and-forget: a nudge from a + * the nudge itself carries nothing. Both checks keep their own gate and + * their own in-flight guard, but they run at once here: they share one + * `weather.get` per house (see weather.pullForChecks), so a nudge costs a + * provider a single call per watched house. Fire-and-forget: a nudge from a * non-weather integration or beyond the rate limit is silently dropped * (no error path, the scheduled floors catch up). * @param {object} service - The sending integration service. diff --git a/server/lib/scene/scene.triggers.js b/server/lib/scene/scene.triggers.js index 101ff46ec6..8a4e33c5f2 100644 --- a/server/lib/scene/scene.triggers.js +++ b/server/lib/scene/scene.triggers.js @@ -30,11 +30,17 @@ const matchWeatherAlert = (self, sceneSelector, event, trigger) => // values are °C, % and the pivot condition enum as-is. The pivot wind // speed is in m/s: it is converted to km/h, the unit the dashboard widget // displays and the one users configure their scenes with. +// The numbers are compared **as the widget displays them** (Math.round, +// like WeatherBox): a user writes a rule from what they read on their +// dashboard, so 5.55 m/s — shown as 20 km/h — must match `>= 20` instead +// of silently comparing 19.98. const WEATHER_TRIGGER_VALUE_GETTERS = { - [WEATHER_TRIGGER_FIELDS.TEMPERATURE]: (weather) => weather.temperature, - [WEATHER_TRIGGER_FIELDS.HUMIDITY]: (weather) => weather.humidity, + [WEATHER_TRIGGER_FIELDS.TEMPERATURE]: (weather) => + typeof weather.temperature === 'number' ? Math.round(weather.temperature) : undefined, + [WEATHER_TRIGGER_FIELDS.HUMIDITY]: (weather) => + typeof weather.humidity === 'number' ? Math.round(weather.humidity) : undefined, [WEATHER_TRIGGER_FIELDS.WIND_SPEED]: (weather) => - typeof weather.wind_speed === 'number' ? weather.wind_speed * 3.6 : undefined, + typeof weather.wind_speed === 'number' ? Math.round(weather.wind_speed * 3.6) : undefined, [WEATHER_TRIGGER_FIELDS.CONDITION]: (weather) => weather.weather, }; diff --git a/server/lib/weather/index.js b/server/lib/weather/index.js index 5a434caaa9..7660d7e7fe 100644 --- a/server/lib/weather/index.js +++ b/server/lib/weather/index.js @@ -4,6 +4,7 @@ const { getProviders } = require('./weather.getProviders'); const { command } = require('./weather.command'); const { checkAlerts } = require('./weather.checkAlerts'); const { checkTriggers } = require('./weather.checkTriggers'); +const { beginSharedPulls, endSharedPulls, pullForChecks } = require('./weather.pullForChecks'); const { INTENTS, EVENTS } = require('../../utils/constants'); const { eventFunctionWrapper } = require('../../utils/functionsWrapper'); @@ -24,6 +25,12 @@ const Weather = function Weather(service, event, messageManager, house) { this.houseWeather = new Map(); // in-flight guard of checkTriggers, same reason as checkAlerts this.checkTriggersRunning = false; + // weather pulls shared between the scene checks running right now, so + // the alert check and the weather-trigger check overlapping on the same + // tick (or relaunched together by a freshness nudge) cost one provider + // call per house instead of two; emptied when the last run ends + this.checkPulls = new Map(); + this.sharedPullRuns = 0; this.event.on(INTENTS.WEATHER.GET, this.command.bind(this)); this.event.on(INTENTS.WEATHER.TOMORROW, this.command.bind(this)); this.event.on(INTENTS.WEATHER.AFTER_TOMORROW, this.command.bind(this)); @@ -38,5 +45,8 @@ Weather.prototype.getProviders = getProviders; Weather.prototype.command = command; Weather.prototype.checkAlerts = checkAlerts; Weather.prototype.checkTriggers = checkTriggers; +Weather.prototype.beginSharedPulls = beginSharedPulls; +Weather.prototype.endSharedPulls = endSharedPulls; +Weather.prototype.pullForChecks = pullForChecks; module.exports = Weather; diff --git a/server/lib/weather/weather.checkAlerts.js b/server/lib/weather/weather.checkAlerts.js index d5771c860d..32f8824991 100644 --- a/server/lib/weather/weather.checkAlerts.js +++ b/server/lib/weather/weather.checkAlerts.js @@ -51,12 +51,8 @@ async function runCheck() { await Promise.each(locatedHouses, async (house) => { let weather; try { - weather = await this.get({ - latitude: house.latitude, - longitude: house.longitude, - language: 'en', - units: 'metric', - }); + // shared with the weather-trigger check when both run at once + weather = await this.pullForChecks(house); } catch (e) { // no provider configured or provider down: nothing to diff, the // previous baseline is kept so recovery does not re-fire scenes @@ -117,9 +113,11 @@ async function checkAlerts() { return; } this.checkAlertsRunning = true; + this.beginSharedPulls(); try { await runCheck.call(this); } finally { + this.endSharedPulls(); this.checkAlertsRunning = false; } } diff --git a/server/lib/weather/weather.checkTriggers.js b/server/lib/weather/weather.checkTriggers.js index 3eac491e33..c9a750db52 100644 --- a/server/lib/weather/weather.checkTriggers.js +++ b/server/lib/weather/weather.checkTriggers.js @@ -1,7 +1,7 @@ const Promise = require('bluebird'); const db = require('../../models'); const logger = require('../../utils/logger'); -const { EVENTS, WEATHER_UNITS } = require('../../utils/constants'); +const { EVENTS } = require('../../utils/constants'); /** * @description The actual check, always called under the in-flight guard @@ -18,24 +18,40 @@ async function runCheck() { where: { active: true }, attributes: ['triggers'], }); - // the triggers column is NOT NULL: every scene carries an array - const someSceneListens = activeScenes.some((scene) => - scene.triggers.some((trigger) => trigger.type === EVENTS.WEATHER.MATCHED), + // the triggers column is NOT NULL: every scene carries an array. Only + // the houses actually watched by a trigger are polled: a trigger matches + // on its own house, so polling the others would buy nothing and cost a + // provider call every 15 min + const watchedHouses = new Set(); + activeScenes.forEach((scene) => + scene.triggers.forEach((trigger) => { + if (trigger.type === EVENTS.WEATHER.MATCHED && trigger.house) { + watchedHouses.add(trigger.house); + } + }), ); - if (!someSceneListens) { + if (watchedHouses.size === 0) { + this.houseWeather.clear(); return; } const houses = await this.house.get(); - const locatedHouses = houses.filter((house) => house.latitude !== null && house.longitude !== null); - await Promise.each(locatedHouses, async (house) => { + const housesToCheck = houses.filter( + (house) => house.latitude !== null && house.longitude !== null && watchedHouses.has(house.selector), + ); + // a house leaving the watched set (scene deactivated, trigger removed) + // drops its baseline: coming back, its first poll re-baselines instead + // of comparing against a payload from another day + const checkedSelectors = new Set(housesToCheck.map((house) => house.selector)); + this.houseWeather.forEach((payload, selector) => { + if (!checkedSelectors.has(selector)) { + this.houseWeather.delete(selector); + } + }); + await Promise.each(housesToCheck, async (house) => { let weather; try { - weather = await this.get({ - latitude: house.latitude, - longitude: house.longitude, - language: 'en', - units: WEATHER_UNITS.METRIC, - }); + // shared with the alert check when both run at once + weather = await this.pullForChecks(house); } catch (e) { // no provider configured or provider down: nothing to compare, the // previous payload is kept so recovery does not fire a scene on a @@ -60,12 +76,13 @@ async function runCheck() { } /** - * @description Poll the weather of every located house and feed the - * dedicated weather scene trigger with the current and the previous - * payload, so the scene engine only fires on a transition. Runs every 15 - * minutes (scheduler job check-weather-triggers) and on an integration - * freshness nudge. Gated: no active scene with a weather trigger means - * zero third-party calls. + * @description Poll the weather of the houses watched by a weather scene + * trigger and feed the trigger with the current and the previous payload, + * so the scene engine only fires on a transition. Runs every 15 minutes + * (scheduler job check-weather-triggers) and on an integration freshness + * nudge. Gated: no active scene with a weather trigger means zero + * third-party calls, and a house no trigger watches is never polled. The + * pull is shared with the alert check when both run at once. * @returns {Promise} Resolves when every house has been checked. * @example * await weather.checkTriggers(); @@ -80,9 +97,11 @@ async function checkTriggers() { return; } this.checkTriggersRunning = true; + this.beginSharedPulls(); try { await runCheck.call(this); } finally { + this.endSharedPulls(); this.checkTriggersRunning = false; } } diff --git a/server/lib/weather/weather.pullForChecks.js b/server/lib/weather/weather.pullForChecks.js new file mode 100644 index 0000000000..ad4ded3ce8 --- /dev/null +++ b/server/lib/weather/weather.pullForChecks.js @@ -0,0 +1,62 @@ +const { WEATHER_UNITS } = require('../../utils/constants'); + +/** + * @description Open a shared-pull window: while at least one scene check + * run is in flight, every weather pull of a given house is done once and + * shared. The alert check (every 30 min) and the weather-trigger check + * (every 15 min) land on the same tick every half hour and a freshness + * nudge relaunches both at once: without this, the same house was pulled + * twice from the provider on every overlap. + * @example + * this.beginSharedPulls(); + */ +function beginSharedPulls() { + this.sharedPullRuns += 1; +} + +/** + * @description Close a shared-pull window. The shared payloads are dropped + * as soon as the last run ends: the sharing window is exactly the overlap + * of the runs, so a check never compares against data older than its own + * poll (a freshness nudge landing after the runs pulls for real). + * @example + * this.endSharedPulls(); + */ +function endSharedPulls() { + this.sharedPullRuns = Math.max(0, this.sharedPullRuns - 1); + if (this.sharedPullRuns === 0) { + this.checkPulls.clear(); + } +} + +/** + * @description Pull the weather of a house for the scene checks, through + * the normal provider loop, sharing one call with any other check run + * currently in flight. + * @param {object} house - The house to pull, with its coordinates. + * @returns {Promise} Resolves with the normalized pivot weather. + * @example + * const weather = await this.pullForChecks(house); + */ +function pullForChecks(house) { + const sharedPull = this.checkPulls.get(house.selector); + if (sharedPull !== undefined) { + return sharedPull; + } + // the promise itself is shared, so two runs starting on the same tick + // wait on one provider call instead of racing two + const pull = this.get({ + latitude: house.latitude, + longitude: house.longitude, + language: 'en', + units: WEATHER_UNITS.METRIC, + }); + this.checkPulls.set(house.selector, pull); + return pull; +} + +module.exports = { + beginSharedPulls, + endSharedPulls, + pullForChecks, +}; diff --git a/server/test/lib/scene/triggers/scene.trigger.weather.test.js b/server/test/lib/scene/triggers/scene.trigger.weather.test.js index dee6681622..00702f74a0 100644 --- a/server/test/lib/scene/triggers/scene.trigger.weather.test.js +++ b/server/test/lib/scene/triggers/scene.trigger.weather.test.js @@ -123,6 +123,40 @@ describe('Scene.triggers.weather', () => { expect(matcher(null, 'scene', buildEvent({ temperature: 12 }, { wind_speed: 1 }), trigger)).to.equal(false); }); + it('should compare the numbers as the dashboard widget displays them', () => { + const windTrigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.WIND_SPEED, + operator: '>=', + value: 20, + }; + // 5.55 m/s = 19.98 km/h, displayed as "20 km/h" by the widget: the + // rule the user reads on their dashboard must match + expect(matcher(null, 'scene', buildEvent({ wind_speed: 5.55 }, { wind_speed: 1 }), windTrigger)).to.equal(true); + // 5.41 m/s = 19.47 km/h, displayed as "19 km/h": still below + expect(matcher(null, 'scene', buildEvent({ wind_speed: 5.41 }, { wind_speed: 1 }), windTrigger)).to.equal(false); + + // same display-vs-compare rule for the temperature and the humidity + const frostTrigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.TEMPERATURE, + operator: '<=', + value: 0, + }; + // 0.4 °C is displayed as "0°": the frost alert fires + expect(matcher(null, 'scene', buildEvent({ temperature: 0.4 }, { temperature: 5 }), frostTrigger)).to.equal(true); + expect(matcher(null, 'scene', buildEvent({ temperature: 0.6 }, { temperature: 5 }), frostTrigger)).to.equal(false); + + const humidityTrigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.HUMIDITY, + operator: '>=', + value: 90, + }; + expect(matcher(null, 'scene', buildEvent({ humidity: 89.5 }, { humidity: 40 }), humidityTrigger)).to.equal(true); + expect(matcher(null, 'scene', buildEvent({ humidity: 89.4 }, { humidity: 40 }), humidityTrigger)).to.equal(false); + }); + it('should only fire on the transition, not while the rule stays true', () => { const trigger = { house: 'my-house', diff --git a/server/test/lib/weather/weather.checkTriggers.test.js b/server/test/lib/weather/weather.checkTriggers.test.js index 4342c3a669..f4d4a7ec8a 100644 --- a/server/test/lib/weather/weather.checkTriggers.test.js +++ b/server/test/lib/weather/weather.checkTriggers.test.js @@ -8,6 +8,8 @@ const Weather = require('../../../lib/weather'); const { EVENTS, WEATHER_TRIGGER_FIELDS } = require('../../../utils/constants'); const HOUSE = { selector: 'weather-trigger-house', latitude: 48.85, longitude: 2.35 }; +// located, but no weather trigger watches it: the check must never poll it +const OTHER_HOUSE = { selector: 'other-house', latitude: 45.76, longitude: 4.83 }; const buildWeather = (getResults) => { // getResults: array of payloads (or Error) returned by successive polls @@ -31,7 +33,9 @@ const buildWeather = (getResults) => { }, }; const event = { on: fake.returns(null), emit: fake.returns(null) }; - const house = { get: fake.resolves([HOUSE, { selector: 'no-gps-house', latitude: null, longitude: null }]) }; + const house = { + get: fake.resolves([HOUSE, OTHER_HOUSE, { selector: 'no-gps-house', latitude: null, longitude: null }]), + }; const weather = new Weather(service, event, {}, house); return { weather, event, provider, house }; }; @@ -92,8 +96,55 @@ describe('weather.checkTriggers', () => { weather: second, previous_weather: first, }); - // the house without coordinates is skipped: one call per poll + // the house without coordinates and the house no trigger watches are + // both skipped: one call per poll + expect(provider.weather.get.callCount).to.equal(2); + }); + + it('should only poll the houses watched by a weather trigger', async () => { + const { weather, provider } = buildWeather([{ temperature: 12 }]); + await weather.checkTriggers(); + expect(provider.weather.get.callCount).to.equal(1); + expect(provider.weather.get.firstCall.args[0]).to.include({ + latitude: HOUSE.latitude, + longitude: HOUSE.longitude, + }); + }); + + it('should re-baseline a house that left the watched set', async () => { + const { weather, event, provider } = buildWeather([{ wind_speed: 2 }, { wind_speed: 8 }]); + // baseline + await weather.checkTriggers(); + // the scene is deactivated: nothing is polled and the baseline is dropped + await db.Scene.update({ active: false }, { where: { id: scene.id } }); + await weather.checkTriggers(); + expect(provider.weather.get.callCount).to.equal(1); + // back: the first poll baselines again instead of comparing against a + // payload from another day + await db.Scene.update({ active: true }, { where: { id: scene.id } }); + await weather.checkTriggers(); + expect(triggerCheckCalls(event)).to.have.lengthOf(0); + }); + + it('should share one provider call per house with the alert check', async () => { + // an alert scene polls every located house, the trigger scene only its + // own: running both at once must cost one call per house, not two for + // the house they both need + await db.Scene.create({ + name: 'Weather alert scene test', + icon: 'fe-cloud', + active: true, + triggers: [{ type: EVENTS.WEATHER.ALERT_RAISED, house: HOUSE.selector }], + actions: [[]], + }); + const { weather, provider } = buildWeather([{ temperature: 12, alerts: [] }]); + await Promise.all([weather.checkAlerts(), weather.checkTriggers()]); + // HOUSE (both checks) + OTHER_HOUSE (alert check only) expect(provider.weather.get.callCount).to.equal(2); + const watchedHouseCalls = provider.weather.get + .getCalls() + .filter((callObject) => callObject.args[0].latitude === HOUSE.latitude); + expect(watchedHouseCalls).to.have.lengthOf(1); }); it('should drop a check landing while another one is still in flight', async () => { From 47729f5a720efad3a66c318518682e98022b9340 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 11:03:43 +0000 Subject: [PATCH 3/4] fix(weather): keep the trigger threshold on the compared integer grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on the weather scene trigger: - the editor rounds the value it converts before storing it, and the matcher rounds the threshold it reads: the compared value is already rounded like the dashboard widget displays it, so a raw conversion missed the rule it was copied from — `>= 20 mph` stored as 32.1868 km/h never matched the 32 km/h shown for that wind, and `>= 70 °F` stored as 21.111 °C never matched the 21 °C shown. Rounding both ends also keeps the round-trip stable: 20 mph reads back as 20 mph - matcher tests on those two conversions (`20 * 1.60934`, `fahrenheitToCelsius(70)`), which `32 °F` did not catch since it converts exactly - a two-house test for the baseline drop: unwatching one house leaves the other one compared, and the unwatched house re-baselines when it comes back instead of firing against the payload it left with - B.18 point 7 records that both ends of the conversion are rounded Autofix-Pass: 2 --- docs/specs/external-integrations.md | 2 +- .../edit-scene/triggers/WeatherTrigger.jsx | 22 ++++--- server/lib/scene/scene.triggers.js | 9 ++- .../triggers/scene.trigger.weather.test.js | 43 +++++++++++++ .../lib/weather/weather.checkTriggers.test.js | 61 +++++++++++++++++++ 5 files changed, 125 insertions(+), 12 deletions(-) diff --git a/docs/specs/external-integrations.md b/docs/specs/external-integrations.md index ffff553cb6..fac2f0608c 100644 --- a/docs/specs/external-integrations.md +++ b/docs/specs/external-integrations.md @@ -405,7 +405,7 @@ The core widget renders the base fields as today and the extensions **when prese Same shape as the alert trigger, so it works with every provider and costs nothing to integrations: a scheduled job (`check-weather-triggers`, every 15 min — weather thresholds are more time-sensitive than vigilance bulletins) — **gated twice**: it runs only when at least one active scene carries a `weather.matched` trigger, and it then pulls **only the houses those triggers actually watch** (a trigger matches on its own house, so polling the others would buy nothing and cost a provider call every 15 min) — through the normal provider loop (`units: metric`), and emits the trigger check with **both the current and the previous payload**. When it overlaps the alert check, the pull of a house is shared between the two (point 5). The first poll after a core start is a **baseline**: no event, for the same reason as point 4 — and a house that leaves the watched set (scene deactivated, trigger removed) drops its baseline, so it re-baselines when it comes back instead of comparing against a payload from another day. -Trigger configuration: house, watched pivot property (`temperature`, `wind_speed`, `humidity`, `condition`), and the **shared `operator` / `value` couple of the threshold triggers** (`=`, `!=`, `>`, `>=`, `<`, `<=` — no new operator vocabulary). Compared values: °C for the temperature, % for the humidity, the pivot **condition enum** as a string for the condition, and **km/h** for the wind speed — the pivot carries m/s, the trigger converts it exactly like the dashboard widget, because km/h is the unit users write their rules in. The numbers are compared **as the widget displays them**, rounded (`Math.round`, the widget's own formatting): a rule is written from what the dashboard shows, so 5.55 m/s — displayed as 20 km/h — matches `>= 20` instead of comparing 19.98 behind the user's back. A property the provider does not expose never matches, and a numeric rule whose value is empty or unparseable never matches. The stored value is always metric; the scene editor **displays and reads it in the unit system of the user** (`temperature_unit_preference`, `distance_unit_preference` — °F and mph shown and converted back on save), so a threshold copied from an imperial dashboard means what it reads. +Trigger configuration: house, watched pivot property (`temperature`, `wind_speed`, `humidity`, `condition`), and the **shared `operator` / `value` couple of the threshold triggers** (`=`, `!=`, `>`, `>=`, `<`, `<=` — no new operator vocabulary). Compared values: °C for the temperature, % for the humidity, the pivot **condition enum** as a string for the condition, and **km/h** for the wind speed — the pivot carries m/s, the trigger converts it exactly like the dashboard widget, because km/h is the unit users write their rules in. The numbers are compared **as the widget displays them**, rounded (`Math.round`, the widget's own formatting): a rule is written from what the dashboard shows, so 5.55 m/s — displayed as 20 km/h — matches `>= 20` instead of comparing 19.98 behind the user's back. A property the provider does not expose never matches, and a numeric rule whose value is empty or unparseable never matches. The stored value is always metric; the scene editor **displays and reads it in the unit system of the user** (`temperature_unit_preference`, `distance_unit_preference` — °F and mph shown and converted back on save), so a threshold copied from an imperial dashboard means what it reads. The threshold lives on the **same rounded grid as the compared value**: the editor rounds the conversion it stores and the matcher rounds the value it reads, otherwise `>= 20 mph` — stored as 32.1868 km/h — would never match the 32 km/h the widget displays for that wind. **Edge-triggered, never repeating**: the scene runs when the rule matches on the current payload and did **not** match on the previous one. So "wind > 20 km/h → close the blinds" runs once when the wind picks up, not every 15 minutes for as long as it blows. Because both payloads travel in the event, the matcher is stateless: editing a scene, adding a trigger or changing a threshold resets nothing. diff --git a/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx b/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx index 7223bf4f36..55c88942f0 100644 --- a/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx +++ b/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx @@ -51,29 +51,33 @@ const KM_PER_MILE = 1.60934; const isImperialTemperature = user => !!user && user.temperature_unit_preference === DEVICE_FEATURE_UNITS.FAHRENHEIT; const isImperialWindSpeed = user => !!user && user.distance_unit_preference === MEASUREMENT_UNITS.US; -const roundValue = value => Math.round(value * 10) / 10; - +// the matcher compares the observed value as the dashboard widget displays +// it (Math.round on °C, km/h and %), so a threshold only means what it says +// on that same integer grid: 20 mph stored as the raw 32.1868 km/h would +// never match the 32 km/h the widget shows. Both ends of the conversion are +// therefore rounded, which also keeps the round-trip stable (20 mph -> 32 +// km/h stored -> 20 mph displayed). const toDisplayValue = (value, field, user) => { if (typeof value !== 'number') { return value; } if (field === WEATHER_TRIGGER_FIELDS.TEMPERATURE && isImperialTemperature(user)) { - return roundValue(celsiusToFahrenheit(value)); + return Math.round(celsiusToFahrenheit(value)); } if (field === WEATHER_TRIGGER_FIELDS.WIND_SPEED && isImperialWindSpeed(user)) { - return roundValue(value / KM_PER_MILE); + return Math.round(value / KM_PER_MILE); } - return value; + return Math.round(value); }; const toStoredValue = (value, field, user) => { if (field === WEATHER_TRIGGER_FIELDS.TEMPERATURE && isImperialTemperature(user)) { - return fahrenheitToCelsius(value); + return Math.round(fahrenheitToCelsius(value)); } if (field === WEATHER_TRIGGER_FIELDS.WIND_SPEED && isImperialWindSpeed(user)) { - return value * KM_PER_MILE; + return Math.round(value * KM_PER_MILE); } - return value; + return Math.round(value); }; const getUnitLabel = (field, user) => { @@ -92,7 +96,7 @@ const getUnitLabel = (field, user) => { // the wind speed rule of the original request, so a freshly added trigger // is already meaningful. Metric, like every stored value: an imperial user -// sees it converted (20 km/h -> 12.4 mph) +// sees it converted (20 km/h -> 12 mph) const DEFAULT_FIELD = WEATHER_TRIGGER_FIELDS.WIND_SPEED; const DEFAULT_OPERATOR = '>'; const DEFAULT_VALUE = 20; diff --git a/server/lib/scene/scene.triggers.js b/server/lib/scene/scene.triggers.js index 8a4e33c5f2..f6cf5f84ff 100644 --- a/server/lib/scene/scene.triggers.js +++ b/server/lib/scene/scene.triggers.js @@ -65,9 +65,14 @@ const matchWeather = (self, sceneSelector, event, trigger) => { return false; } // the condition is compared as a string of the pivot enum, everything - // else as a number — a value left empty in the UI never matches + // else as a number — a value left empty in the UI never matches. + // The threshold is put on the same integer grid as the observed value: + // a value stored by an imperial editor is a converted float (20 mph = + // 32.1868 km/h, 70 °F = 21.111 °C), and comparing it against the rounded + // 32 km/h / 21 °C the dashboard displays would miss the rule the user + // copied from their widget. const isCondition = trigger.weather_field === WEATHER_TRIGGER_FIELDS.CONDITION; - const expectedValue = isCondition ? trigger.value : Number(trigger.value); + const expectedValue = isCondition ? trigger.value : Math.round(Number(trigger.value)); if (!isCondition && Number.isNaN(expectedValue)) { return false; } diff --git a/server/test/lib/scene/triggers/scene.trigger.weather.test.js b/server/test/lib/scene/triggers/scene.trigger.weather.test.js index 00702f74a0..2f974d23b9 100644 --- a/server/test/lib/scene/triggers/scene.trigger.weather.test.js +++ b/server/test/lib/scene/triggers/scene.trigger.weather.test.js @@ -8,6 +8,7 @@ const StateManager = require('../../../../lib/state'); const SceneManager = require('../../../../lib/scene'); const { triggersFunc } = require('../../../../lib/scene/scene.triggers'); const { ACTIONS, EVENTS, WEATHER_TRIGGER_FIELDS } = require('../../../../utils/constants'); +const { fahrenheitToCelsius } = require('../../../../utils/units'); const event = new EventEmitter(); @@ -157,6 +158,48 @@ describe('Scene.triggers.weather', () => { expect(matcher(null, 'scene', buildEvent({ humidity: 89.4 }, { humidity: 40 }), humidityTrigger)).to.equal(false); }); + it('should compare a threshold converted from an imperial unit on the same grid', () => { + // what the editor stores for a rule typed in mph / °F: a converted + // float. The observed value is rounded like the widget displays it, so + // the threshold has to be read on that same integer grid — otherwise + // the rule copied from the dashboard silently never fires. + const windTrigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.WIND_SPEED, + operator: '>=', + // "wind >= 20 mph" -> 32.1868 km/h + value: 20 * 1.60934, + }; + // 8.94 m/s is displayed as "20 mph" (32 km/h) by the widget + expect(matcher(null, 'scene', buildEvent({ wind_speed: 8.94 }, { wind_speed: 1 }), windTrigger)).to.equal(true); + // 8.6 m/s = 31 km/h, "19 mph": still below + expect(matcher(null, 'scene', buildEvent({ wind_speed: 8.6 }, { wind_speed: 1 }), windTrigger)).to.equal(false); + + const temperatureTrigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.TEMPERATURE, + operator: '>=', + // "temperature >= 70 °F" -> 21.111… °C + value: fahrenheitToCelsius(70), + }; + // 21.1 °C is displayed as "21°" (70 °F) + expect(matcher(null, 'scene', buildEvent({ temperature: 21.1 }, { temperature: 12 }), temperatureTrigger)).to.equal( + true, + ); + expect(matcher(null, 'scene', buildEvent({ temperature: 20.4 }, { temperature: 12 }), temperatureTrigger)).to.equal( + false, + ); + + // 32 °F converts exactly, so frost keeps working either way + const frostTrigger = { + house: 'my-house', + weather_field: WEATHER_TRIGGER_FIELDS.TEMPERATURE, + operator: '<=', + value: fahrenheitToCelsius(32), + }; + expect(matcher(null, 'scene', buildEvent({ temperature: -2 }, { temperature: 5 }), frostTrigger)).to.equal(true); + }); + it('should only fire on the transition, not while the rule stays true', () => { const trigger = { house: 'my-house', diff --git a/server/test/lib/weather/weather.checkTriggers.test.js b/server/test/lib/weather/weather.checkTriggers.test.js index f4d4a7ec8a..cbd43631c7 100644 --- a/server/test/lib/weather/weather.checkTriggers.test.js +++ b/server/test/lib/weather/weather.checkTriggers.test.js @@ -126,6 +126,67 @@ describe('weather.checkTriggers', () => { expect(triggerCheckCalls(event)).to.have.lengthOf(0); }); + it('should drop only the baseline of the house that left the watched set', async () => { + // two watched houses, one of them stops being watched: its baseline is + // dropped while the other one keeps being compared. Each poll of a + // house returns a payload it has never returned before, so a stale + // baseline would show up as an event. + const otherScene = await db.Scene.create({ + name: 'Weather trigger scene test, other house', + icon: 'fe-cloud', + active: true, + triggers: [ + { + type: EVENTS.WEATHER.MATCHED, + house: OTHER_HOUSE.selector, + weather_field: WEATHER_TRIGGER_FIELDS.WIND_SPEED, + operator: '>', + value: 20, + }, + ], + actions: [[]], + }); + const pollsPerHouse = new Map(); + const provider = { + weather: { + get: fake(async ({ latitude }) => { + const poll = (pollsPerHouse.get(latitude) || 0) + 1; + pollsPerHouse.set(latitude, poll); + return { wind_speed: poll }; + }), + }, + }; + const service = { getService: () => provider, stateManager: { getAllKeys: () => ['ext-fake-weather'] } }; + const event = { on: fake.returns(null), emit: fake.returns(null) }; + const house = { get: fake.resolves([HOUSE, OTHER_HOUSE]) }; + const weather = new Weather(service, event, {}, house); + + // poll 1: both houses baseline + await weather.checkTriggers(); + expect(triggerCheckCalls(event)).to.have.lengthOf(0); + expect(provider.weather.get.callCount).to.equal(2); + + // OTHER_HOUSE is no longer watched: it is not polled and its baseline + // is dropped, while HOUSE keeps comparing against its own + await db.Scene.update({ active: false }, { where: { id: otherScene.id } }); + await weather.checkTriggers(); + expect(provider.weather.get.callCount).to.equal(3); + let calls = triggerCheckCalls(event); + expect(calls).to.have.lengthOf(1); + expect(calls[0].args[1]).to.include({ house: HOUSE.selector }); + expect(calls[0].args[1].previous_weather).to.deep.equal({ wind_speed: 1 }); + + // watched again: its first poll re-baselines instead of firing against + // the payload it left with + await db.Scene.update({ active: true }, { where: { id: otherScene.id } }); + await weather.checkTriggers(); + expect(provider.weather.get.callCount).to.equal(5); + calls = triggerCheckCalls(event); + expect(calls).to.have.lengthOf(2); + expect(calls[1].args[1]).to.include({ house: HOUSE.selector }); + expect(calls.filter((callObject) => callObject.args[1].house === OTHER_HOUSE.selector)).to.have.lengthOf(0); + }); + it('should share one provider call per house with the alert check', async () => { // an alert scene polls every located house, the trigger scene only its // own: running both at once must cost one call per house, not two for From 8d2628b66e0f7c31ecad8fb01f620aaf0b76857e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 13:57:21 +0000 Subject: [PATCH 4/4] fix(weather): store the Fahrenheit threshold unrounded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on the weather scene trigger: - the editor no longer rounds the °F -> °C conversion it stores. The matcher already puts the threshold on the compared integer grid (`Math.round(Number(trigger.value))`), so rounding it a second time at the storage end changed no match — it only moved the number under the user: 71 °F stored as 22 °C reads back as 72 °F on the next open, and 71 of the 161 integer °F values between -40 and 120 drift that way. Local state hid it until the component remounted, so a saved scene silently rewrote its own threshold - the mph -> km/h conversion stays rounded: it round-trips exactly over that range, so it costs no drift and keeps the stored value tidy - B.18 point 7 records that the rounding belongs to the comparison, not to the storage Autofix-Pass: 3 --- docs/specs/external-integrations.md | 2 +- .../scene/edit-scene/triggers/WeatherTrigger.jsx | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/specs/external-integrations.md b/docs/specs/external-integrations.md index fac2f0608c..dbe49f66dd 100644 --- a/docs/specs/external-integrations.md +++ b/docs/specs/external-integrations.md @@ -405,7 +405,7 @@ The core widget renders the base fields as today and the extensions **when prese Same shape as the alert trigger, so it works with every provider and costs nothing to integrations: a scheduled job (`check-weather-triggers`, every 15 min — weather thresholds are more time-sensitive than vigilance bulletins) — **gated twice**: it runs only when at least one active scene carries a `weather.matched` trigger, and it then pulls **only the houses those triggers actually watch** (a trigger matches on its own house, so polling the others would buy nothing and cost a provider call every 15 min) — through the normal provider loop (`units: metric`), and emits the trigger check with **both the current and the previous payload**. When it overlaps the alert check, the pull of a house is shared between the two (point 5). The first poll after a core start is a **baseline**: no event, for the same reason as point 4 — and a house that leaves the watched set (scene deactivated, trigger removed) drops its baseline, so it re-baselines when it comes back instead of comparing against a payload from another day. -Trigger configuration: house, watched pivot property (`temperature`, `wind_speed`, `humidity`, `condition`), and the **shared `operator` / `value` couple of the threshold triggers** (`=`, `!=`, `>`, `>=`, `<`, `<=` — no new operator vocabulary). Compared values: °C for the temperature, % for the humidity, the pivot **condition enum** as a string for the condition, and **km/h** for the wind speed — the pivot carries m/s, the trigger converts it exactly like the dashboard widget, because km/h is the unit users write their rules in. The numbers are compared **as the widget displays them**, rounded (`Math.round`, the widget's own formatting): a rule is written from what the dashboard shows, so 5.55 m/s — displayed as 20 km/h — matches `>= 20` instead of comparing 19.98 behind the user's back. A property the provider does not expose never matches, and a numeric rule whose value is empty or unparseable never matches. The stored value is always metric; the scene editor **displays and reads it in the unit system of the user** (`temperature_unit_preference`, `distance_unit_preference` — °F and mph shown and converted back on save), so a threshold copied from an imperial dashboard means what it reads. The threshold lives on the **same rounded grid as the compared value**: the editor rounds the conversion it stores and the matcher rounds the value it reads, otherwise `>= 20 mph` — stored as 32.1868 km/h — would never match the 32 km/h the widget displays for that wind. +Trigger configuration: house, watched pivot property (`temperature`, `wind_speed`, `humidity`, `condition`), and the **shared `operator` / `value` couple of the threshold triggers** (`=`, `!=`, `>`, `>=`, `<`, `<=` — no new operator vocabulary). Compared values: °C for the temperature, % for the humidity, the pivot **condition enum** as a string for the condition, and **km/h** for the wind speed — the pivot carries m/s, the trigger converts it exactly like the dashboard widget, because km/h is the unit users write their rules in. The numbers are compared **as the widget displays them**, rounded (`Math.round`, the widget's own formatting): a rule is written from what the dashboard shows, so 5.55 m/s — displayed as 20 km/h — matches `>= 20` instead of comparing 19.98 behind the user's back. A property the provider does not expose never matches, and a numeric rule whose value is empty or unparseable never matches. The stored value is always metric; the scene editor **displays and reads it in the unit system of the user** (`temperature_unit_preference`, `distance_unit_preference` — °F and mph shown and converted back on save), so a threshold copied from an imperial dashboard means what it reads. The threshold lives on the **same rounded grid as the compared value**: the **matcher** rounds the threshold it reads as well as the value it observes, otherwise `>= 20 mph` — stored as 32.1868 km/h — would never match the 32 km/h the widget displays for that wind. Rounding therefore belongs to the comparison, not to the storage: the editor stores the °F → °C conversion **unrounded**, so that what the user typed is what they read back (rounding it would turn 71 °F into 22 °C, redisplayed as 72 °F on the next open, without changing a single match). **Edge-triggered, never repeating**: the scene runs when the rule matches on the current payload and did **not** match on the previous one. So "wind > 20 km/h → close the blinds" runs once when the wind picks up, not every 15 minutes for as long as it blows. Because both payloads travel in the event, the matcher is stateless: editing a scene, adding a trigger or changing a threshold resets nothing. diff --git a/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx b/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx index 55c88942f0..ec1cf526d8 100644 --- a/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx +++ b/front/src/routes/scene/edit-scene/triggers/WeatherTrigger.jsx @@ -52,11 +52,15 @@ const isImperialTemperature = user => !!user && user.temperature_unit_preference const isImperialWindSpeed = user => !!user && user.distance_unit_preference === MEASUREMENT_UNITS.US; // the matcher compares the observed value as the dashboard widget displays -// it (Math.round on °C, km/h and %), so a threshold only means what it says -// on that same integer grid: 20 mph stored as the raw 32.1868 km/h would -// never match the 32 km/h the widget shows. Both ends of the conversion are -// therefore rounded, which also keeps the round-trip stable (20 mph -> 32 -// km/h stored -> 20 mph displayed). +// it (Math.round on °C, km/h and %), and it puts the threshold on that same +// integer grid itself (`Math.round(Number(trigger.value))`), so what is +// stored here only has to survive the round-trip. What the user typed is +// therefore what they read back: the displayed value is rounded, the stored +// °F -> °C conversion is not. Rounding it would move the threshold under the +// user (71 °F -> 22 °C -> 72 °F on the next open: 71 of the 161 integer °F +// values between -40 and 120 drift by one degree), without changing a single +// match. The mph -> km/h conversion is rounded because it round-trips exactly +// (no drift over the same kind of range) and keeps the stored value tidy. const toDisplayValue = (value, field, user) => { if (typeof value !== 'number') { return value; @@ -72,7 +76,7 @@ const toDisplayValue = (value, field, user) => { const toStoredValue = (value, field, user) => { if (field === WEATHER_TRIGGER_FIELDS.TEMPERATURE && isImperialTemperature(user)) { - return Math.round(fahrenheitToCelsius(value)); + return fahrenheitToCelsius(value); } if (field === WEATHER_TRIGGER_FIELDS.WIND_SPEED && isImperialWindSpeed(user)) { return Math.round(value * KM_PER_MILE);