Skip to content

Conversation

mellodev
Copy link
Contributor

@mellodev mellodev commented Apr 22, 2025

Changes Proposed

Various Fixes:

  • removed data-appversion attribute in index.html
  • added elvis operator to checkAnalogSensorAvail to prevent crash at app launch
  • renamed programview.js to program-view.js and updated imports
  • renamed CURRENT_FW related constants to MIN_REQ_FW for improved readibility
  • refactored notification channel ids to Constants.CHANNELS; refactored all uses of channel ids; refactored if statements to switch
  • removed testing comment
  • language._ "OpenSprinkler Sensor Export on"
  • "Adjustment-Name" to "Adjustment Name"
  • name: "Adjustment" to name: OSApp.Language._("Adjustment")
  • "close" to "Close" x3
  • "inverse" to "Inverse" x5
  • "Sensor-Nr" to "Sensor Nr"
  • refactor UNITS to constants; refactor options list; refactor switch statement
  • "download log" to "Download Log"
  • "delete log" to "Delete Log"
  • window.alert() to OSApp.Errors.showError() x2
  • refactor backup types to constants (sensor, adjustments, monitor, all); update getExportMethodSensors() and getImportMethodSensors(); update callers of those functions
  • updated text to "Please update to firmware version x.x.x or later"
  • no boards detected/unknown adapter options using Language translations
  • use OSApp.Analog.Constants.SENSOR_GROUP_MIN rather than hardcoded integer 1000
  • "Refresh Sensordata" to "Refresh Sensor data"
  • combineWithSep() refactored to OSApp.Utils.combineWithSep() and caller updated
  • "SET SN 1/2" to "Set SN 1/2"
  • "TIME" to "time"
  • "REMOTE" to "Remote"
  • "last 48h" to "Last 48hrs"
  • "last weeks" to "Last Week"
  • "last months" to "Last Month"
  • "degree celius temperature" to "Degree Celcius Temperature"
  • "degree farenheit temperature" to "Degree Fahrenheit Temperature"
  • refactored isNumber(), formatVal() and formatValUnit() to OSApp.Utils namespace; updated callers
  • refactored getUnit() to use Constants; refactored buildGraph() to better use constants
  • "liter/min" using Language translations
  • added a 30s default timeout to sendToOS()
  • added OSApp.Utils.buildOptionsForObj() to automate building tags from constants
  • refactored Options.showOptions to use new DASHBOARD_MODE constant
  • "Monitoring-warnings level low" to "Monitoring warning level: Low"
  • "Monitoring-warnings level medium" to "Monitoring warning level: Medium"
  • "Monitoring-warnings level high" to "Monitoring warning level: High"
  • SystemDiagnostics.showDiagnostics() refactored to conditionally call /du endpoint for OS v 233; passed duStatus section strings through Language translations
  • Updated apexcharts.min.js from 3.35.1 to latest 3.54.1

Demo Video or Screenshots

Please include a short video or screenshot(s) to assist with pr review

@mellodev
Copy link
Contributor Author

mellodev commented Apr 22, 2025

Edit - disregard. This seems to be a dev env issue related to one of my Chrome extensions. It does not repro in Firefox or in a Chrome incognito tab.

hey @opensprinklershop, no matter which version of apexcharts I try, I get a console error when visiting the "Edit Programs" page. I have tried v3.35.0, v3.50.0 and v3.54.1 but they all fail in similar manner. I'm not sure if I've broken something with my branch, can you check if you see this on your end?

image

@salbahra salbahra requested a review from Copilot May 16, 2025 01:26
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR continues the analog branch cleanup efforts by refining naming conventions, refactoring integration code, and updating user‐facing text and error handling. Key changes include removal of unused markup attributes and testing comments, renaming files and constants for improved readability, and refactoring functions (e.g. error handling in sendToOS) to add a default timeout and support new dashboard and flow alert features.

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
www/sw.js Added program-view.js to the service worker cache list.
www/locale/messages_en.po Added/updated msgid entries for new texts and corrections.
www/locale/de.js Updated German translations to reflect new strings.
www/js/modules/utils.js Introduced escapeJSON2 and new utility functions; minor refactor.
www/js/modules/ui-dom.js Initialized analog module via asb_init.
www/js/modules/system-diagnostics.js Refactored diagnostic integration to load du status for fw 233+.
www/js/modules/supported.js Added support check for Flow Alert.
www/js/modules/stations.js Updated stopAllStations to accept an optional callback.
www/js/modules/sites.js Added a call to update monitors after analog sensor update.
www/js/modules/programs.js Added a new pid case; fixed error message case sensitivity.
www/js/modules/program-view.js Introduced a new module for program visualization using ApexCharts.
www/js/modules/options.js Extended options display with new dashboard mode and influxdb config.
www/js/modules/import-export.js Updated JSON escaping for configuration import/export.
www/js/modules/firmware.js Added a default timeout parameter in sendToOS.
www/js/modules/dashboard.js Updated dashboards to support flow alert setpoints and new views.
www/index.html Included program-view.js in main index.
www/css/analog.css Updated media queries and button styles for responsive design.

@salbahra
Copy link
Member

salbahra commented Sep 2, 2025

@mellodev Is this PR ready for merge? @rayshobby do we want to merge this?

@mellodev
Copy link
Contributor Author

mellodev commented Sep 2, 2025

Hi Samer/Ray! This branch looked good to me when sent, however we were waiting on someone to get actual sensors to do some testing and verification before merging. I'm not sure if any QA work like that has been completed? Additionally I see that we have a couple merge conflicts that need resolving now. We should make sure that program-view.js change to use = rather than == is implemented, that was a great catch by the copilot!

@salbahra
Copy link
Member

salbahra commented Sep 2, 2025

Got it, thank you Joel! Ray also let me know he is still pending testing the sensors. So we can hold off on the conflict fixes if we want until that is verified.

Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants