Skip to content

Make NTP server configurable via Homie settings and fix Rule class linking error#21

Merged
stritti merged 21 commits intomain-v3from
copilot/configurable-ntp-server
Feb 15, 2026
Merged

Make NTP server configurable via Homie settings and fix Rule class linking error#21
stritti merged 21 commits intomain-v3from
copilot/configurable-ntp-server

Conversation

Copy link

Copilot AI commented Jan 27, 2026

Configurable NTP Server Implementation

  • Add HomieSetting for NTP server configuration to PoolController.hpp
  • Update TimeClientHelper to use configurable NTP server instead of hardcoded value
  • Update timeClientSetup() to accept NTP server parameter
  • Initialize NTP client with configured server in PoolController::setupHandler
  • Update README.md to reflect new feature (remove from planned features)
  • Add validator to ensure NTP server is not null or empty
  • Synchronized with latest main-v3 branch (including timezone support PR Add configurable timezone support with DST handling #23)
  • Fix spelling errors: "priniples" → "principles", "mesurement" → "measurement", "intervall" → "interval"
  • Fix duplicate library conflict with lib_ignore
  • Fix Rule class linking error by making virtual methods pure virtual
  • Add virtual destructor to Rule class for safe polymorphic deletion
  • Fix NTPClient initialization to use dynamic allocation

NTP Server Configuration

Replaces hardcoded europe.pool.ntp.org with a configurable Homie setting. Users can now set regional NTP pools (asia, europe, north-america, oceania, south-america) or custom servers via MQTT.

Implementation:

  • Added ntpServerSetting_ to PoolControllerContext with default value "pool.ntp.org"
  • Changed NTPClient from static object to pointer, dynamically allocated in timeClientSetup()
  • Constructor now receives the configured server name, avoiding the need for non-existent setPoolServerName() method
  • Added validator ensuring NTP server is non-null and non-empty
  • Integrated NTP setup in setupHandler() using the configured setting value

Usage: Users configure via Homie MQTT: homie/<device-id>/ntp-server/set → "europe.pool.ntp.org"

Home Assistant Integration: The NTP server setting is automatically available in Home Assistant through the Homie MQTT integration as an ntp-server entity.

The setting is persistent across reboots through Homie's flash storage.

Library Dependency Fixes

ESPAsyncWebServer Configuration: Fixed multiple build issues related to library conflicts:

  • Replaced ESP32-specific libraries with ESP8266-compatible versions
  • Used direct GitHub URL https://github.com/me-no-dev/ESPAsyncWebServer.git for proper library resolution
  • Added lib_ldf_mode = chain+ and lib_ignore = ESP Async WebServer to prevent duplicate library conflicts from transitive dependencies

Rule Class Fixes

Issue: The Rule base class declared virtual methods getMode() and loop() without implementations, causing linker error: undefined reference to '_ZTV4Rule'. Additionally, it was missing a virtual destructor needed for safe polymorphic deletion.

Solution:

  • Made both virtual methods pure virtual (= 0) to properly define Rule as an abstract base class
  • Added virtual destructor (virtual ~Rule() = default) to ensure derived class destructors are called correctly when deleted through base pointer
  • All subclasses (RuleAuto, RuleManu, RuleBoost, RuleTimer) already override these methods, so no other changes were needed

This fixes a pre-existing build issue and potential undefined behavior in the main-v3 base branch.

Original prompt

This section details on the original issue you should resolve

<issue_title>Configurable NTP-Server</issue_title>
<issue_description>Instead of hardcoded ntp-Server it should be configurable.</issue_description>

Comments on the Issue (you are @copilot in this section)

@stritti List of official NTP-Servers:
Area Server
Worldwide pool.ntp.org
Asia asia.pool.ntp.org
Europe europe.pool.ntp.org
North America north-america.pool.ntp.org
Oceania oceania.pool.ntp.org
South America south-america.pool.ntp.org

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 5 commits January 27, 2026 20:38
Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Copilot AI changed the title [WIP] Make NTP server configurable instead of hardcoded Make NTP server configurable via Homie settings Jan 27, 2026
Copilot AI requested a review from stritti January 27, 2026 20:44
stritti

This comment was marked as outdated.

Copy link
Author

Copilot AI commented Jan 27, 2026

Wird die Einstellungen des NTP-Servers auch für reboote Persistent gemacht?

Yes, the NTP server setting is persistent across reboots. HomieSetting automatically stores all configuration values in flash memory. When the device reboots, Homie restores the NTP server setting from flash and timeClientSetup() uses the persisted value in setupHandler().

Copilot AI requested a review from stritti January 27, 2026 20:49
@stritti stritti requested review from Copilot and removed request for stritti January 27, 2026 20:53
Copy link

Copilot AI left a 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 makes the NTP server configurable through Homie settings instead of using a hardcoded value. The change allows users to specify their preferred NTP server (defaulting to pool.ntp.org), improving flexibility for deployments in different geographic regions.

Changes:

  • Added a new HomieSetting for NTP server configuration with validation
  • Updated TimeClientHelper to accept NTP server as a parameter instead of using a hardcoded constant
  • Updated documentation to reflect the new configurable feature

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/main.cpp Added HomieSetting for NTP server with validation and initialization in setupHandler
src/TimeClientHelper.hpp Updated function signature to accept NTP server parameter
src/TimeClientHelper.cpp Removed hardcoded NTP server constant and updated initialization to use configured value
README.md Updated feature list to reflect configurable NTP server

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@stritti stritti changed the base branch from master to main-v3 January 27, 2026 20:57
@stritti stritti marked this pull request as ready for review January 27, 2026 21:05
@stritti stritti requested a review from Copilot January 29, 2026 20:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@stritti stritti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Löse bitte die merge-Konflikte

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Copilot AI requested a review from stritti February 14, 2026 22:50
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@stritti
Copy link
Contributor

stritti commented Feb 15, 2026

… server

Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Copy link
Author

Copilot AI commented Feb 15, 2026

@copilot https://github.com/smart-swimmingpool/pool-controller/actions/runs/22036609871/job/63670485766#step:5:1

Fixed in commit 9615fa1. The NTPClient library v3.1.0 doesn't have a setPoolServerName() method. Changed to dynamically allocate NTPClient in timeClientSetup(), passing the configured server to the constructor. Updated all references to use pointer syntax (timeClient-> instead of timeClient.).

@stritti
Copy link
Contributor

stritti commented Feb 15, 2026

@copilot Da wir ja nun auch Homeassistant supporten. Kann auch über Homeassistant der NTP-Server geändert werden?

Copy link
Author

Copilot AI commented Feb 15, 2026

@copilot Da wir ja nun auch Homeassistant supporten. Kann auch über Homeassistant der NTP-Server geändert werden?

Ja, die NTP-Server-Einstellung ist über Home Assistant änderbar. Alle HomieSetting-Konfigurationen werden automatisch über das Homie MQTT-Protokoll veröffentlicht. In Home Assistant erscheint die Einstellung als ntp-server Entity und kann über die Home Assistant UI oder Automationen geändert werden. Der Pfad ist homie/<device-id>/ntp-server/set – genau wie bei anderen Homie-Settings (z.B. timezone, loop-interval).

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Copilot AI requested a review from stritti February 15, 2026 16:30
@stritti stritti merged commit 49d1791 into main-v3 Feb 15, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Configurable NTP-Server

2 participants