Add clock app #61
Add clock app #61
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new clock application to the badge that displays time synchronized via NTP, with automatic timezone detection based on IP geolocation. The app connects to WiFi, syncs time from NTP servers, detects the user's timezone, and displays both local and UTC time with automatic hourly re-sync functionality.
Key Changes
- New clock app with NTP time synchronization and RTC integration
- Automatic timezone detection using IP geolocation API (ipapi.co)
- Auto-sync functionality that connects to WiFi on startup and hourly intervals
- Manual sync controls and battery-saving WiFi disconnection after sync
Reviewed Changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| badge/apps/clock/icon.png | 24x24 PNG icon for the clock app to appear in the menu launcher |
| badge/apps/clock/init.py | Complete clock app implementation with NTP sync, timezone detection, and time display functionality |
| sys.path.insert(0, "/system/apps/clock") | ||
| os.chdir("/system/apps/clock") | ||
|
|
||
| from badgeware import io, brushes, shapes, screen, PixelFont, run, Image |
There was a problem hiding this comment.
The Image import is unused in this code. According to the coding guidelines, imports should only be included if they are used. Remove Image from the import statement.
| from badgeware import io, brushes, shapes, screen, PixelFont, run, Image | |
| from badgeware import io, brushes, shapes, screen, PixelFont, run |
| from badgeware import io, brushes, shapes, screen, PixelFont, run, Image | ||
| import network | ||
| import ntptime | ||
| import time |
There was a problem hiding this comment.
The time module is imported but never used in the code. Remove this unused import.
| import time |
|
|
||
| # Load fonts | ||
| small_font = PixelFont.load("/system/assets/fonts/ark.ppf") | ||
| large_font = PixelFont.load("/system/assets/fonts/absolute.ppf") |
There was a problem hiding this comment.
The large_font variable is loaded but never used in the code. Remove this unused font loading to save memory.
| large_font = PixelFont.load("/system/assets/fonts/absolute.ppf") |
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| run(update) |
There was a problem hiding this comment.
The init() function is defined but not passed to run(). According to the coding guidelines and the pattern shown in other apps (e.g., monapet), the init function should be passed as a parameter: run(update, init=init). Without this, WiFi credentials won't be loaded and RTC won't be initialized on app startup.
| run(update) | |
| run(update, init=init) |
| if __name__ == "__main__": | ||
| run(update) |
There was a problem hiding this comment.
Missing on_exit() function to properly clean up WiFi when exiting the app. According to best practices in the codebase and the coding guidelines, apps that connect to WiFi should implement an on_exit() function to call disconnect_wifi() and ensure the WiFi radio is turned off to save battery. Add an on_exit() function and pass it to run() as run(update, init=init, on_exit=on_exit).
| if __name__ == "__main__": | |
| run(update) | |
| def disconnect_wifi(): | |
| """Disconnect from WiFi and turn off the radio to save battery.""" | |
| try: | |
| wlan = network.WLAN(network.STA_IF) | |
| if wlan.active(): | |
| wlan.disconnect() | |
| wlan.active(False) | |
| except Exception: | |
| pass # Ignore errors during cleanup | |
| def on_exit(): | |
| """Cleanup WiFi on app exit.""" | |
| disconnect_wifi() | |
| if __name__ == "__main__": | |
| run(update, init=init, on_exit=on_exit) |
|
|
||
| # Clear screen | ||
| screen.brush = background | ||
| screen.draw(shapes.rectangle(0, 0, 160, 120)) |
There was a problem hiding this comment.
Screen clearing should use screen.clear() instead of drawing a full-screen rectangle. According to the coding guidelines best practice #7, use screen.clear() with brush color for screen clearing as it's the fastest method. Change to: screen.brush = background followed by screen.clear().
| screen.draw(shapes.rectangle(0, 0, 160, 120)) | |
| screen.clear() |
| days_offset = 0 | ||
| if total_seconds < 0: | ||
| days_offset = -1 | ||
| total_seconds += 86400 | ||
| elif total_seconds >= 86400: | ||
| days_offset = 1 |
There was a problem hiding this comment.
Variable days_offset is not used.
| days_offset = 0 | |
| if total_seconds < 0: | |
| days_offset = -1 | |
| total_seconds += 86400 | |
| elif total_seconds >= 86400: | |
| days_offset = 1 | |
| if total_seconds < 0: | |
| total_seconds += 86400 | |
| elif total_seconds >= 86400: |
| days_offset = 0 | ||
| if total_seconds < 0: | ||
| days_offset = -1 | ||
| total_seconds += 86400 | ||
| elif total_seconds >= 86400: | ||
| days_offset = 1 |
There was a problem hiding this comment.
Variable days_offset is not used.
| days_offset = 0 | |
| if total_seconds < 0: | |
| days_offset = -1 | |
| total_seconds += 86400 | |
| elif total_seconds >= 86400: | |
| days_offset = 1 | |
| if total_seconds < 0: | |
| total_seconds += 86400 | |
| elif total_seconds >= 86400: |
Currently very basic UX, but appreciate someone not in UTC timezone checking if this works for them.
This should set the RTC clock on board the device allowing for accurate time, but also syncs once per hour switching wifi on to sync and then off again when done.
Adjusts local time based on the detected location by IP address and looking up the UTC offset.