-
Notifications
You must be signed in to change notification settings - Fork 388
Some Custom Improvements. #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 pull request introduces comprehensive timezone support with GMT-based timezone selection, refactors display refresh logic for consistency, and improves display border handling. The changes enable users to select from 28 GMT-based timezones or configure location-based timezones with daylight saving time support.
Key changes:
- Added new timezone infrastructure with GMT offset arrays and selection UI, allowing users to cycle through timezones during time setup
- Standardized all menu and UI operations to use full display refresh (
display.display(true)) instead of partial refreshes - Refactored display border logic with new getter/setter methods and changed the default border to dark mode
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| src/TimezonesGMT.h | New file defining 28 GMT-based timezones with offsets and configuration for location-based timezone override |
| src/config.h | Added SET_TZ constant and renumbered time-setting indices to accommodate timezone selection |
| src/Watchy32KRTC.cpp | Updated RTC initialization to apply timezone settings from TimezonesGMT.h configuration |
| src/Watchy.h | Changed gmtOffset type from int to long to support larger offset values and included TimezonesGMT.h |
| src/Watchy.cpp | Added timezone selection UI in setTime(), standardized display refresh calls to full refresh, added forced weather update on WiFi setup, and updated NTP sync to use gmtOffset |
| src/Display.h | Added drawDarkBorder() and isDarkBorder() methods, changed darkBorder default to true |
| src/Display.cpp | Separated border drawing from border setting logic with new drawDarkBorder() method and clearer documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // This file is used to set up GMT based timezones. | ||
|
|
||
| // You don't need to change anything here to be able to set up GMT based time. | ||
| // If you set TIMEZONES_NON_GMT_OVERRIDE to 1 (as for get summer time and leaps), |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: "get summer time" should be "to get summer time". The comment should read "If you set TIMEZONES_NON_GMT_OVERRIDE to 1 (as to get summer time and leaps),".
| // If you set TIMEZONES_NON_GMT_OVERRIDE to 1 (as for get summer time and leaps), | |
| // If you set TIMEZONES_NON_GMT_OVERRIDE to 1 (as to get summer time and leaps), |
| if(TIMEZONES_NON_GMT_OVERRIDE == 0){ | ||
| setenv("TZ", timeZones[tzIndex].timezone, 1); | ||
| } else{ | ||
| #if TIMEZONES_NON_GMT_OVERRIDE == 1 | ||
| setenv("TZ", tz_override.timezone, 1); | ||
| #else | ||
| setenv("TZ", timeZones[TIMEZONES_SELECTED].timezone, 1); | ||
| #endif | ||
| } |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for applying timezone settings is inconsistent. When TIMEZONES_NON_GMT_OVERRIDE is 1, the code uses tz_override, but when it's any other non-zero value (line 11-12), the code sets "GMT0". However, in the else branch here (line 621-627), when TIMEZONES_NON_GMT_OVERRIDE is not 0, it conditionally checks if it equals 1, otherwise it uses TIMEZONES_SELECTED. This creates inconsistent behavior between initialization and setTime. The init() function should match the logic here for consistency.
| #endif | ||
| gmtOffset = settings.gmtOffset; | ||
| #endif | ||
| gmtOffset = settings.gmtOffset; |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Trailing whitespace on this line. Remove the spaces after gmtOffset = settings.gmtOffset;.
| gmtOffset = settings.gmtOffset; | |
| gmtOffset = settings.gmtOffset; |
| RTC.set(tm); | ||
|
|
||
| showMenu(menuIndex, false); | ||
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Trailing whitespace on line 632. Remove the spaces after the semicolon.
| ; |
| }; | ||
|
|
||
| static long OFFSETS_SEC[] = { | ||
| 0, 3600, 7200, 10800, 14400, 18000, 21600, 25200, 28800, 32400, 36000, 39600, 43200, 0, -3600, -7200, -10800, -14400, -18000, -21600, -25200, -28800, -32400, -36000, -39600, -43200, -46800, -50400 |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OFFSETS_SEC array has incorrect signs for all timezone offsets. In POSIX notation, "Etc/GMT+N" represents a timezone N hours behind UTC (i.e., UTC-N), so it needs a negative offset. Conversely, "Etc/GMT-N" is UTC+N and needs a positive offset. Currently, all the signs are inverted. For example, index 1 is "Etc/GMT+1" (UTC-1) but has offset 3600 when it should be -3600. Index 14 is "Etc/GMT-1" (UTC+1) but has offset -3600 when it should be 3600. All non-zero offset values need their signs flipped.
| 0, 3600, 7200, 10800, 14400, 18000, 21600, 25200, 28800, 32400, 36000, 39600, 43200, 0, -3600, -7200, -10800, -14400, -18000, -21600, -25200, -28800, -32400, -36000, -39600, -43200, -46800, -50400 | |
| 0, -3600, -7200, -10800, -14400, -18000, -21600, -25200, -28800, -32400, -36000, -39600, -43200, 0, 3600, 7200, 10800, 14400, 18000, 21600, 25200, 28800, 32400, 36000, 39600, 43200, 46800, 50400 |
| display.setTextColor(blink ? GxEPD_WHITE : GxEPD_BLACK); | ||
| } | ||
| display.print(2000 + year); | ||
| display.print(year); |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commented-out year conversion functions (tmYearToY2k and y2kYearToTm) suggest uncertainty about the correct year format. The change from 2000 + year (line 580 vs old line) to just year means the display now shows raw year values (0-99) instead of full years (2000-2099). This is likely incorrect and confusing to users. Either uncomment the conversion functions or update the display logic to show 2000 + year for a proper 4-digit year display.
| display.print(year); | |
| display.print(2000 + year); |
| USB_PLUGGED_IN = (digitalRead(USB_DET_PIN) == 1); | ||
| #endif | ||
| gmtOffset = settings.gmtOffset; | ||
| #endif |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Trailing whitespace on this line. Remove the spaces after #endif.
| #endif | |
| #endif |
| } | ||
|
|
||
|
|
||
| gmt = OFFSETS_SEC[tzIndex] / 3600; |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Trailing whitespace on line 534. Remove the spaces after the semicolon.
| gmt = OFFSETS_SEC[tzIndex] / 3600; | |
| gmt = OFFSETS_SEC[tzIndex] / 3600; |
| } | ||
|
|
||
| tzset(); | ||
|
|
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Trailing whitespace on line 630. Remove the spaces after tzset();.
| } else if (TIMEZONES_NON_GMT_OVERRIDE == 1) { | ||
| setenv("TZ", tz_override.timezone, 1); | ||
| } else { | ||
| setenv("TZ", "GMT0", 1); |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Trailing whitespace on line 12. Remove the spaces after setenv("TZ", "GMT0", 1);.
| setenv("TZ", "GMT0", 1); | |
| setenv("TZ", "GMT0", 1); |
This pull request introduces significant improvements to timezone handling and display refresh logic, as well as some refactoring for the display border logic. The main changes include adding support for selecting GMT-based timezones, updating the UI and logic to allow users to set and display the timezone, and ensuring consistent use of full display refreshes. Additionally, the handling of the display border has been clarified and improved with new getter/setter methods.
Timezone support and selection:
TimezonesGMT.hfile, which defines a list of GMT-based timezones, their offsets, and related configuration logic. This enables users to select from a wide range of timezones or specify a location-based override.setTime) to allow users to cycle through and select timezones, display the current GMT offset, and update the environment timezone accordingly. The timezone is now stored and used throughout the system, including for NTP synchronization. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Display refresh consistency:
display.display(true)) instead of a partial or conditional refresh. This ensures consistent and correct updates to the e-paper display for all major UI actions, such as menus, about screens, firmware updates, and WiFi configuration. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]Display border logic improvements:
drawDarkBorder,setDarkBorder, andisDarkBordermethods to clarify intent and improve maintainability. The default value fordarkBorderwas also changed totrue. [1] [2] [3]Other notable changes:
These changes together make the system more robust, user-friendly, and maintainable, especially regarding timezone management and display behavior.