You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
⚠️ This PR was opened by an automated Claude Code run. It needs human review and real-life testing before being merged.
Description
Adds a new scene trigger "Sun position", so a scene can start when the sun reaches a given elevation above the horizon and/or a given compass direction. The use case from the forum: open/close the shutters of a specific facade when the sun actually hits it.
Server
Two new events in server/utils/constants.js:
time.check-sun-position (internal, scheduled every minute in scene.init, like the calendar check),
time.sun-position (the new trigger type).
New server/lib/scene/scene.checkSunPositionTriggers.js:
runs every minute, but returns immediately when no active scene uses the trigger (so house.get() is not called for nothing),
computes the position with the suncalc dependency already used by scene.dailyUpdate.js, for every house with coordinates,
suncalc.getPosition() returns radians with an azimuth measured from South; it is converted to degrees using the usual compass convention (0 = North, 90 = East, 180 = South, 270 = West), which is what the UI documents and what the user enters. This is explained in the JSDoc of convertSunPositionToDegrees,
keeps the last known position per house (sceneManager.sunPositions) and emits trigger.check with the current and previous position. The very first check of a house only stores the position.
New matcher in server/lib/scene/scene.triggers.js: the trigger has an optional condition on the altitude and an optional one on the azimuth (operators >, <, =; = matches within 0.5°, since the position is sampled every minute and never lands on an exact degree). Both conditions are combined with an AND, and the scene is started when the sun enters the configured area — it is not re-executed at every check while the sun stays in it.
Joi validation of the new trigger properties (sun_altitude_operator, sun_altitude in -90..90, sun_azimuth_operator, sun_azimuth in 0..360) in server/models/scene.js.
Front
New SunPositionTrigger.jsx (house selector with the existing "house without coordinates" warning, one operator + degrees input per axis, with help texts explaining both conventions), wired in TriggerCard.jsx and in the trigger type list of ChooseTriggerTypeCard.jsx.
Translations added in en, fr and de.
Notes for the reviewer
The altitude is negative at night: an azimuth-only trigger will also match at night, hence the help text suggesting to combine it with an altitude condition.
The check runs on the same one-minute tick as the calendar check; a scene therefore fires at most one minute after the sun reaches the configured position.
Tests pass: new tests added in server/test/lib/scene/scene.checkSunPositionTriggers.test.js (17 assertions on the conversion, the early return, the first-check behaviour and the emitted payload) and server/test/lib/scene/triggers/scene.trigger.sunPosition.test.js (matching, non-matching house, = tolerance, no re-trigger while the sun stays in the area, missing/invalid values, Joi validation). test/lib/scene/** + test/models/** run green (331 passing). The full server suite was also run: the only failures are pre-existing environment ones (gateway backup/restore needing the sqlite3 binary, docker and network tests), none in scene. npm run coverage and Cypress were not run in this environment.
Linter and prettier pass on both front and server (npm run eslint → 0 errors, prettier --check clean on the touched files, npm run compare-translations clean, npm run build on the front succeeds).
Add a new scene trigger "Sun position" that starts a scene when the sun
reaches a given elevation above the horizon and/or a given compass
direction, so that shutters can be opened/closed when the sun actually
hits a facade.
Server:
- new events time.check-sun-position (internal, scheduled every minute in
scene.init) and time.sun-position (the trigger type)
- scene.checkSunPositionTriggers computes the sun position with suncalc
for every located house, but only when at least one active scene uses
the trigger. suncalc returns radians with an azimuth measured from
South: it is converted to degrees with the usual compass convention
(0 = North, 90 = East, 180 = South, 270 = West)
- the trigger has an optional condition on the altitude and an optional
one on the azimuth (operators >, < and =, "=" matching within 0.5°).
The scene is started when the sun *enters* the configured area, so it
is not re-executed at every check while the sun stays there
- Joi validation of the new trigger properties
Front:
- new SunPositionTrigger component, wired in TriggerCard and in the
trigger type list
- en, fr and de translations
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013yxguVaLdJ8ZKmw5x3HePT
@Pierre-Gilles, you've reached your PR review limit, so we couldn't start this review.
Next review available in:1 minute
You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.
How can I continue?
After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.
To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.
How do review limits work?
CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.
For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.51%. Comparing base (2f7ef52) to head (b93b1fd). ⚠️ Report is 4 commits behind head on master.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Thanks for a clean first cut of this trigger: early-return when no scene listens, enter-area matching (not re-fire while the sun stays in the zone), compass conversion documented, and solid unit tests for the happy path.
This is notrisk:high (additive scene trigger, no migration, no auth/host/gateway). I am adding needs:human-review because it introduces a new scene trigger UX (half-plane vs azimuth window, night-time azimuth, enter-only after restart) and the PR itself asks for a human pass. Requesting @atrovato (author is Pierre-Gilles).
Requested changes
Azimuth is circular, the matcher is not. Conversion can store 360 instead of 0, and = uses linear distance. Due north (including solar midnight) will not match = 0 / = 360 reliably, and > / < mis-detect enter/leave when the sun wraps 359° → 1°. Normalize to [0, 360) and use circular distance for =.
Missing night warning in the UI. The PR text says azimuth-only should be combined with an altitude condition; that help string is not in en/fr/de.
Facade use case vs a single operator. A shutter facade is an azimuth range. With one >/< per axis you cannot express “between 90° and 180°”. Please confirm with a human whether v1 is threshold-crossing (two scenes for open/close) or whether the trigger should be a from/to window.
Non-blocking
MCP sceneSchemas.js does not know time.sun-position (Gladys Plus / AI cannot create it). Weather-alert triggers have the same gap, so not a merge blocker, but it should be added in this PR or a fast follow-up.
triggerEvent.altitude / azimuth are passed to the scene but not registered in the variable picker (setVariablesTrigger), unlike calendar.
Joi allows saving with no house and with both axes on “No condition”; those scenes never fire.
No DEVICE_FEATURE_CATEGORIES / types changes.
I would not merge until (1) and (2) are fixed and a human has signed off on the range vs threshold UX.
…n trigger
Address review feedback on the sun position trigger:
- convertSunPositionToDegrees now normalizes the azimuth into [0, 360[
after rounding: an azimuth just below North used to round to exactly
360 instead of 0
- the "=" operator now uses the circular distance on the azimuth axis,
so "= 0" also matches an azimuth of 359.8°. The altitude keeps a plain
linear distance, and ">" / "<" stay plain comparisons on [0, 360[:
an azimuth area crossing North cannot be described with a single
condition, which is now documented in the code and in the UI help text
- the azimuth help text (en, fr, de) explains that the sun is due North
below the horizon around solar midnight, so an azimuth condition should
be combined with an altitude one to only start the scene during the day
Autofix-Pass: 1
The reason will be displayed to describe this comment to others. Learn more.
Re-review of b93b1fd (follow-up on 30bbaa36). The two code blockers from the last pass are fixed, tests cover the North wrap, and CI is green (including codecov/patch). This is notrisk:high: additive time.sun-position trigger, no migration, no auth/host/gateway, no DEVICE_FEATURE_CATEGORIES / types.
Addressed
Azimuth is normalized to [0, 360) after rounding, so due North is never stored as 360.
= on azimuth uses circular distance (= 0 matches 359.8°).
azimuthHelp (en/fr/de) now warns that azimuth-only also matches at night (combine with altitude, e.g. > 0) and that a single > / < cannot describe a sector that crosses North.
Still needs:human-review — leaving the label and @atrovato (author is Pierre-Gilles / Claude Code). This is a new scene-trigger UX; the forum case is a facade window, and v1 is two threshold scenes (open azimuth > 90, close azimuth > 180) rather than a from/to range. That is a coherent v1 and is documented, but a maintainer should still sign off before merge, plus real-life testing as the PR asks.
Non-blocking residuals
MCP sceneSchemas.js still does not know time.sun-position (same gap as weather-alert triggers). Gladys Plus / AI cannot create this trigger until a follow-up.
triggerEvent.altitude / azimuth are not registered in the scene variable picker (setVariablesTrigger).
Conversion is duplicated with house.getSunState (sun dashboard). That helper can still round to 360; this trigger will not. Worth sharing later so the widget and the trigger always agree at North.
Joi still allows saving with no house and with both axes on “No condition” (those scenes never fire). First check after restart still only stores the position (enter-area by design).
Ready to merge from a code standpoint once a human is happy with the half-plane / two-scene facade UX.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
area:server
Pierre-Gilles
added
the
needs:human-review
Cursor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Implements feature request: https://community.gladysassistant.com/t/ajouter-un-declencheur-de-scenes-selon-l-altitude-et-l-azimut-du-soleil/7881
Description
Adds a new scene trigger "Sun position", so a scene can start when the sun reaches a given elevation above the horizon and/or a given compass direction. The use case from the forum: open/close the shutters of a specific facade when the sun actually hits it.
Server
server/utils/constants.js:time.check-sun-position(internal, scheduled every minute inscene.init, like the calendar check),time.sun-position(the new trigger type).server/lib/scene/scene.checkSunPositionTriggers.js:house.get()is not called for nothing),suncalcdependency already used byscene.dailyUpdate.js, for every house with coordinates,suncalc.getPosition()returns radians with an azimuth measured from South; it is converted to degrees using the usual compass convention (0 = North, 90 = East, 180 = South, 270 = West), which is what the UI documents and what the user enters. This is explained in the JSDoc ofconvertSunPositionToDegrees,sceneManager.sunPositions) and emitstrigger.checkwith the current and previous position. The very first check of a house only stores the position.server/lib/scene/scene.triggers.js: the trigger has an optional condition on the altitude and an optional one on the azimuth (operators>,<,=;=matches within 0.5°, since the position is sampled every minute and never lands on an exact degree). Both conditions are combined with an AND, and the scene is started when the sun enters the configured area — it is not re-executed at every check while the sun stays in it.sun_altitude_operator,sun_altitudein -90..90,sun_azimuth_operator,sun_azimuthin 0..360) inserver/models/scene.js.Front
SunPositionTrigger.jsx(house selector with the existing "house without coordinates" warning, one operator + degrees input per axis, with help texts explaining both conventions), wired inTriggerCard.jsxand in the trigger type list ofChooseTriggerTypeCard.jsx.en,frandde.Notes for the reviewer
Forum
Forum: https://community.gladysassistant.com/t/ajouter-un-declencheur-de-scenes-selon-l-altitude-et-l-azimut-du-soleil/7881
Checklist
server/test/lib/scene/scene.checkSunPositionTriggers.test.js(17 assertions on the conversion, the early return, the first-check behaviour and the emitted payload) andserver/test/lib/scene/triggers/scene.trigger.sunPosition.test.js(matching, non-matching house,=tolerance, no re-trigger while the sun stays in the area, missing/invalid values, Joi validation).test/lib/scene/**+test/models/**run green (331 passing). The full server suite was also run: the only failures are pre-existing environment ones (gateway backup/restore needing thesqlite3binary, docker and network tests), none in scene.npm run coverageand Cypress were not run in this environment.npm run eslint→ 0 errors,prettier --checkclean on the touched files,npm run compare-translationsclean,npm run buildon the front succeeds).Generated by Claude Code