-
Notifications
You must be signed in to change notification settings - Fork 386
Moonphase #264
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?
Moonphase #264
Conversation
crossplatformdev
commented
Jul 21, 2024


|
Added moon phase app. |
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 PR adds moon phase calculation functionality to the Watchy smartwatch library along with several UI improvements and bug fixes. The changes include timezone management enhancements, menu system updates, and a new "PipBoy" watchface example inspired by Fallout.
Key Changes
- Added moon phase calculation with astronomical data (phase, age, distance, latitude, longitude, zodiac)
- Implemented timezone management system with GMT offset support and POSIX timezone strings
- Expanded menu system from 7 to 8 items to include "Moon Phase" display
- Changed display refresh behavior from
false(full refresh) totrue(partial refresh) throughout the codebase - Added USB detection support for ESP32-S3 devices
- Included new "PipBoy" themed watchface example with custom fonts and graphics
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, menu configuration updates (8 items, adjusted height), added timezone setting index |
| src/Watchy32KRTC.cpp/h | Timezone initialization using POSIX TZ strings, removed unused microseconds variable |
| src/Watchy.h/cpp | Moon phase integration, menu expansion, USB detection for ESP32-S3, display refresh parameter changes |
| src/TimezonesGMT.h | New timezone management system with GMT offsets and POSIX timezone strings |
| src/MoonPhase.h/cpp | New moon phase calculation implementation using astronomical algorithms |
| src/Display.h/cpp | Dark border default changed to true, separated draw and set functions for border control |
| platformio.ini | New build configuration for multiple Watchy hardware versions |
| library.properties/json | Version updates, WiFiManager dependency changed to master branch |
| examples/WatchFaces/pipboy/* | Complete new watchface example with custom fonts and graphics |
| .gitignore | Added PlatformIO and VSCode directories |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case 7: | ||
| showMoonPhase(); |
Copilot
AI
Nov 16, 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. The same issue exists in another switch statement in this file (lines 176-177).
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.
@copilot open a new pull request to apply changes based on this feedback
| display.setTextColor(blink ? GxEPD_WHITE : GxEPD_BLACK); | ||
| } | ||
| display.print(2000 + year); | ||
| display.print(1970 + year); |
Copilot
AI
Nov 16, 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 display calculation has changed from 2000 + year to 1970 + year. This appears to be incorrect as the tmYearToY2k function returns years since 2000 (Y2K), not since 1970 (Unix epoch). This will display the wrong year (e.g., 2024 would be shown as 1994).
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.
@copilot open a new pull request to apply changes based on this feedback
| // WiFi and remember to turn it back off | ||
| WiFiUDP ntpUDP; | ||
| NTPClient timeClient(ntpUDP, ntpServer.c_str(), gmt); | ||
| timeClient.setTimeOffset(gmt); |
Copilot
AI
Nov 16, 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] The setTimeOffset() call appears redundant since the offset is already passed to the NTPClient constructor. The constructor's gmt parameter sets the time offset, making this additional call unnecessary.
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.
@copilot open a new pull request to apply changes based on this feedback
| case 7: | ||
| showMoonPhase(); |
Copilot
AI
Nov 16, 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 when selecting the Moon Phase menu item.
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.
@copilot open a new pull request to apply changes based on this feedback