Multiday Watering Levels, Weather Restriction, Current Fault Detect#272
Conversation
…App into multidayETo
…App into multidayETo
…App into multidayETo
…App into multidayETo
…App into multidayETo
…er to preserve station run order in the annotation
|
Visit the preview URL for this PR (updated for commit b1f06e4): https://opensprinkler-devui--pr272-multidayeto-saxr97a9.web.app (expires Fri, 05 Sep 2025 18:10:40 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 9b7a984849dffe48c01bc0242f5c0f93b14ed1df |
|
For now, I can only help with PRs you've created. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for multiday watering levels using historical weather data, weather restrictions, and current fault detection, while also updating the timeline visualization library and fixing time display issues.
- Adds multiday watering level calculations using historical weather data for programs with regular intervals
- Implements weather restriction options (rain amount/days, temperature thresholds, California restriction)
- Adds overcurrent and undercurrent fault detection with UI notifications
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| www/js/modules/weather.js | Updates weather delay adjustment options and forecast display with precipitation data |
| www/js/modules/utils.js | Fixes JSON escaping method to properly handle braces |
| www/js/modules/system-diagnostics.js | Adds multiday levels and weather restriction status to diagnostics display |
| www/js/modules/supported.js | Adds version check for weather restrictions feature |
| www/js/modules/status.js | Changes status colors and adds overcurrent fault detection |
| www/js/modules/stations.js | Updates run-once submission to support annotations and improved dialog handling |
| www/js/modules/programs.js | Migrates from links-timeline to vis-timeline library and implements multiday watering level logic |
| www/js/modules/options.js | Adds weather restriction configuration UI and updates current threshold options |
| www/js/modules/logs.js | Updates timeline visualization to use vis-timeline library with proper time formatting |
| www/js/modules/language.js | Adds call to update weather restriction notice |
| www/js/modules/firmware.js | Improves firmware update detection with minor version handling |
| www/js/modules/dates.js | Fixes 12/24-hour time format display throughout the application |
| www/js/modules/dashboard.js | Adds weather restriction notice to dashboard and fixes remote station hex encoding |
| www/js/main.js | Updates option constants for current limits |
| www/index.html | Adds vis-timeline library dependencies |
| www/css/vis-timeline-graph2d.min.css | New CSS file for vis-timeline library |
| www/css/main.css | Updates timeline styling for new vis-timeline library |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if ( typeof OSApp.currentSession.weather.forecast[ i ].precip !== "undefined") { | ||
| list += "<span>" + OSApp.Language._( "Forecasted Rain" ) + "</span><span>: " + OSApp.Weather.formatPrecip( OSApp.currentSession.weather.forecast[ i ].precip ) + "</span>" |
There was a problem hiding this comment.
Missing semicolon after the string concatenation on line 843. This could cause JavaScript parsing issues.
| if ( typeof OSApp.currentSession.weather.forecast[ i ].precip !== "undefined") { | |
| list += "<span>" + OSApp.Language._( "Forecasted Rain" ) + "</span><span>: " + OSApp.Weather.formatPrecip( OSApp.currentSession.weather.forecast[ i ].precip ) + "</span>" | |
| list += "<span>" + OSApp.Language._( "Forecasted Rain" ) + "</span><span>: " + OSApp.Weather.formatPrecip( OSApp.currentSession.weather.forecast[ i ].precip ) + "</span>"; |
| return wto && OSApp.Firmware.checkOSVersion( 2213 ); | ||
| } |
There was a problem hiding this comment.
Missing semicolon after the function declaration on line 100.
| return wto && OSApp.Firmware.checkOSVersion( 2213 ); | |
| } | |
| }; |
|
|
||
| // Show overcurrent fault status | ||
| if ( OSApp.currentSession.controller.settings.ocs ) { | ||
| OSApp.Status.changeStatus( 0, "red", "<p class='running-text center'>" + OSApp.Language._( "Overcurrent Fault detected" ) + ( OSApp.currentSession.controller.settings.ocs === 255 ? "" : ( " when opening Station " + OSApp.currentSession.controller.settings.ocs ) ) + "!!</p>" ); |
There was a problem hiding this comment.
[nitpick] The double exclamation marks ('!!') in the error message are unprofessional. Consider using a single exclamation mark or removing them entirely for a more professional error message.
| OSApp.Status.changeStatus( 0, "red", "<p class='running-text center'>" + OSApp.Language._( "Overcurrent Fault detected" ) + ( OSApp.currentSession.controller.settings.ocs === 255 ? "" : ( " when opening Station " + OSApp.currentSession.controller.settings.ocs ) ) + "!!</p>" ); | |
| OSApp.Status.changeStatus( 0, "red", "<p class='running-text center'>" + OSApp.Language._( "Overcurrent Fault detected" ) + ( OSApp.currentSession.controller.settings.ocs === 255 ? "" : ( " when opening Station " + OSApp.currentSession.controller.settings.ocs ) ) + "!</p>" ); |
| } else { | ||
| $( "#restr-active" ).addClass("hidden"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing semicolon after the function declaration.
| } | |
| }; |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| // Show overcurrent fault status | ||
| if ( OSApp.currentSession.controller.settings.ocs ) { | ||
| OSApp.Status.changeStatus( 0, "red", "<p class='running-text center pointer'>" + OSApp.Language._( "Overcurrent Fault detected" ) + | ||
| ( OSApp.currentSession.controller.settings.ocs === 255 ? "" : ( OSApp.Language._( " when opening Station " ) + OSApp.currentSession.controller.settings.ocs ) ) + "!!</p>", |
There was a problem hiding this comment.
The double exclamation mark is not needed here. Rachio notifies me all the time over and under current and it isn't as alarming.
Summary