-
Notifications
You must be signed in to change notification settings - Fork 388
Single render, set GMT timezone in runtime, desired use of darkBorder = true. #258
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: dev
Are you sure you want to change the base?
Conversation
fe8ce84 to
6982b07
Compare
6982b07 to
d95f6bb
Compare
Update platformio.ini
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 implements several enhancements to the Watchy firmware (version 1.4.14), focusing on timezone management, display optimization, and new features for the ESP32S3 variant.
- Runtime GMT timezone configuration: Introduces a flexible timezone system with 28 GMT offset options and support for location-based timezones
- Display optimizations: Changes to single-render approach with
darkBorderenabled by default and modified refresh parameters throughout - Moon phase calculation: Adds astronomical moon phase tracking with detailed orbital information
- ESP32S3 USB detection: Implements USB plug detection for charging status display
Reviewed changes
Copilot reviewed 29 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/config.h | Version bump to 1.4.14, added timezone menu item, adjusted menu dimensions |
| src/TimezonesGMT.h | New timezone management system with GMT offsets and location timezone support |
| src/Watchy32KRTC.cpp | Timezone initialization logic using new GMT system |
| src/Watchy.h | Added moon phase support, USB detection, and changed gmtOffset type to long |
| src/Watchy.cpp | Integrated timezone selection in setTime(), moon phase calculations, USB detection for ESP32S3, changed all display.display() calls to use partial refresh (true) |
| src/MoonPhase.h/cpp | New moon phase calculation implementation with astronomical data |
| src/Display.h/cpp | darkBorder default changed to true, added drawDarkBorder() and isDarkBorder() methods |
| examples/WatchFaces/7_SEG/ | Added USB charge icon support for ESP32S3, adjusted battery level thresholds |
| examples/WatchFaces/pipboy/ | New PipBoy-themed watchface example with custom fonts and graphics |
| README.md | Simplified setup instructions to reference documentation website |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case 7: | ||
| showMoonPhase(); |
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.
Missing break statement after case 7. This will cause fall-through to the default case, which could lead to unexpected behavior.
| case 7: | ||
| showMoonPhase(); |
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.
Missing break statement after case 7. This will cause fall-through to the default case, which could lead to unexpected behavior.
| //This line overrides the intended behaviour that I want for the | ||
| //darkBorder variable. I want to set the darkBorder variable to dark | ||
| //and then paint the border always dark, not always putting the opposite | ||
| //colour of the background, like it is done here. | ||
| //darkBorder = dark; |
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 comment describes the intended behavior but then doesn't follow it. The line darkBorder = dark is commented out, but the function is supposed to set darkBorder while drawDarkBorder() should only paint. This creates confusion about the actual design. Consider either updating the comment to match the implementation or creating a clearer separation between setting state and drawing.
| //This line overrides the intended behaviour that I want for the | |
| //darkBorder variable. I want to set the darkBorder variable to dark | |
| //and then paint the border always dark, not always putting the opposite | |
| //colour of the background, like it is done here. | |
| //darkBorder = dark; | |
| // This function only paints the border with the specified color. | |
| // It does not modify the darkBorder state variable. | |
| // To set the darkBorder state and paint, use setDarkBorder(). |
| display.setTextColor(blink ? GxEPD_WHITE : GxEPD_BLACK); | ||
| } | ||
| display.print(2000 + year); | ||
| display.print(1970 + 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 year calculation changed from 2000 + year to 1970 + year. This appears to be a significant change that could affect time display. Verify this is intentional and that year variable now represents years since 1970 (Unix epoch) rather than years since 2000 (Y2K epoch). This could be a breaking change for existing watch face implementations.
Hi,
I explain the changes I have made in the merge to master of my branch.
crossplatformdev#1