Add configurable timezone support with DST handling#23
Conversation
Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
…sistence notes Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds configurable timezone support with automatic DST handling to replace the hardcoded Central European Time configuration. The implementation allows both persistent configuration via Homie settings and runtime changes via MQTT.
Changes:
- Extended timezone definitions from 2 to 10 major timezones with proper DST rules
- Added timezone selection API with validation and persistent storage
- Integrated MQTT properties for runtime timezone control and information display
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/TimeClientHelper.cpp | Extended timezone definitions to 10 zones and implemented timezone selection API |
| src/TimeClientHelper.hpp | Added function declarations for timezone getter/setter |
| src/Timer.cpp | Changed from hardcoded timezone index to dynamic timezone retrieval |
| src/main.cpp | Added HomieSetting for timezone with validation and applied configuration in setup |
| src/OperationModeNode.hpp | Added property constants for timezone and timezone-info |
| src/OperationModeNode.cpp | Implemented MQTT properties for runtime timezone control |
| platformio.ini | Updated library dependencies and platform version |
| docs/users-guide.md | Documented timezone configuration options and usage |
| README.md | Added timezone feature to feature list |
| _codeql_detected_source_root | CodeQL configuration file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/TimeClientHelper.cpp
Outdated
| TimeChangeRule CST_CN = {"CST", First, Sun, Mar, 0, 8 * 60}; // UTC + 8 hours | ||
| Timezone China(CST_CN, CST_CN); |
There was a problem hiding this comment.
The variable name CST_CN conflicts with the US Central Standard Time abbreviation 'CST' defined on line 35. Consider renaming to 'CST_CHINA' or 'CN_CST' to avoid confusion.
| TimeChangeRule CST_CN = {"CST", First, Sun, Mar, 0, 8 * 60}; // UTC + 8 hours | |
| Timezone China(CST_CN, CST_CN); | |
| TimeChangeRule CST_CHINA = {"CST", First, Sun, Mar, 0, 8 * 60}; // UTC + 8 hours | |
| Timezone China(CST_CHINA, CST_CHINA); |
There was a problem hiding this comment.
Fixed in commit 57bbf48. Renamed to CST_CHINA to avoid confusion with US Central Standard Time.
platformio.ini
Outdated
| ArduinoJson @ 6.18.0 | ||
| me-no-dev/ESP Async WebServer | ||
| ESP32Async/ESPAsyncWebServer @ ^3.6.0 ; Asynchronous HTTP/WebSocket Server [oai_citation_attribution:1‡PlatformIO Community](https://community.platformio.org/t/how-come-lib-deps-esp-async-webserver-works/24853?utm_source=chatgpt.com) | ||
| ESP32Async/ESPAsyncTCP @ ^2.0.0 ; TCP-Layer für ESP826 |
There was a problem hiding this comment.
Corrected 'ESP826' to 'ESP8266' in the comment.
| ESP32Async/ESPAsyncTCP @ ^2.0.0 ; TCP-Layer für ESP826 | |
| ESP32Async/ESPAsyncTCP @ ^2.0.0 ; TCP-Layer für ESP8266 |
There was a problem hiding this comment.
Fixed in commit 57bbf48. Corrected ESP826 to ESP8266.
src/OperationModeNode.cpp
Outdated
| // Note: This is a runtime change. To persist across reboots, update the | ||
| // 'timezone' HomieSetting in the configuration file. | ||
| retval = true; | ||
| } else { | ||
| Homie.getLogger() << cIndent << F("✖ Invalid timezone index: ") << tzIndex << endl; | ||
| retval = false; | ||
| } | ||
|
|
There was a problem hiding this comment.
This comment explains a user-facing behavior but appears in code that won't be visible to end users. Consider moving this information exclusively to user documentation (where it already exists in users-guide.md) to avoid maintenance duplication.
| // Note: This is a runtime change. To persist across reboots, update the | |
| // 'timezone' HomieSetting in the configuration file. | |
| retval = true; | |
| } else { | |
| Homie.getLogger() << cIndent << F("✖ Invalid timezone index: ") << tzIndex << endl; | |
| retval = false; | |
| } | |
| // Note: This only updates the timezone at runtime; persistence is handled via configuration. | |
| retval = true; | |
| } else { | |
| Homie.getLogger() << cIndent << F("✖ Invalid timezone index: ") << tzIndex << endl; | |
| retval = false; | |
| } |
There was a problem hiding this comment.
Fixed in commit 57bbf48. Simplified the comment while keeping brief documentation reference.
stritti
left a comment
There was a problem hiding this comment.
Kannst du die merge-Konflikte bitte lösen.
…typo, simplify comment Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Merge-Konflikte gelöst in commit 47bb7f0. Die Timezone-Unterstützung wurde erfolgreich in die neue PoolController-Struktur integriert. |
Timezone was hardcoded to Central European Time (index 0). This adds configurable timezone selection with 10 major timezones supporting automatic DST transitions.
Changes
Timezone Definitions (
TimeClientHelper.cpp)CST_CHINAinstead ofCST_CNto avoid confusion with US Central Standard TimeConfiguration API (
TimeClientHelper.cpp/hpp)setTimezoneIndex(int)/getTimezoneIndex()with bounds validation_selectedTimezoneIndexstores active selection (default: 0)Runtime Integration (
Timer.cpp)Persistent Configuration (
PoolController.cpp/hpp)HomieSetting<long> timezoneSetting_to PoolControllerContext class with validator[0, getTzCount())setupHandler()viasetTimezoneIndex(this->timezoneSetting_.get())MQTT Runtime Control (
OperationModeNode.cpp/hpp)timezone(integer 0-9, settable) - runtime overridetimezone-info(string, read-only) - displays current timezone nameUsage
Persistent (survives reboot):
{ "timezone": 3 } // US EasternRuntime via MQTT:
Backward Compatibility
Defaults to index 0 (Central European Time) - existing deployments unchanged.
Integration with main-v3
This PR is based on the main-v3 branch which includes a major refactoring of the codebase. The timezone configuration has been successfully integrated with the new PoolController structure:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.