Skip to content
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

Resync NTP regurlarly #134

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Resync NTP regurlarly #134

wants to merge 1 commit into from

Conversation

CurlyMoo
Copy link

@CurlyMoo CurlyMoo commented May 18, 2024

When the year is 1970 resync every 5 minutes. If the date is properly set, resync every day.

@IgorYbema
Copy link
Owner

Seems ok

@CurlyMoo
Copy link
Author

Let's wait at least for the results of the tests of @McMagellan

@RichieB2B
Copy link

RichieB2B commented May 18, 2024

The rescheduling is done twice: both in timer_cb() every minute and in ntpReload() itself every 5 minutes. So you'll get 2 timerqueue_inserts(), then 4, then 8 etc. Doubling with every call to timer_cb() for the -5 case.

@CurlyMoo
Copy link
Author

@RichieB2B Two things. The difference in time was indeed wrong. However, the double call to timerqueue_insert wasn't necessarily because it would just overwrite the running timer. Despite that, i changed it according to your feedback.

@RichieB2B
Copy link

@CurlyMoo So an extra call to timerqueue_inserts(x, y, -5) would just reset the previous timer. That's much less of an issue than I thought but still good to avoid it.

@CurlyMoo
Copy link
Author

@RichieB2B Exactly.

@McMagellan
Copy link

Feedback: Testing time synchronization. Alpha-467a907, #527
The test failed.

Method:

  1. Disconnect Heishamon and Fritzbox from the power supply.
  2. Turn the power back on.
  3. After about 6 minutes, contact with the Heishamon console could be established again from the browser.
    --> Timestamp 1/1/1970, 1:06, Uptime 6 minutes.
  4. Today morning see Screenshot:
    -> Timestamp 1/1/1970, 9:37, Uptime 8:h 37 min.

Screenshot 2024-05-19 at 07-40-51 Heisha monitor

@geduxas
Copy link

geduxas commented May 19, 2024

@CurlyMoo why not use some basic logic, just 2 variables, one boolen for time sync'ed flag, and one last time synced (timestamp after timesync) and check by that.. if it's never synced (try every 5min) and if synced try after last sync_time timeout.

@CurlyMoo
Copy link
Author

CurlyMoo commented May 19, 2024

@CurlyMoo why not use some basic logic, just 2 variables, one boolen for time sync'ed flag, and one last time synced (timestamp after timesync) and check by that.. if it's never synced (try every 5min) and if synced try after last sync_time timeout.

That's what we're basicly doing here, but centralized through the timepool.

@geduxas
Copy link

geduxas commented May 19, 2024

Oh, sorry, i saw somewhere you mentioned exact dates.. so thought it tied to dates :) didn't check source.

@CurlyMoo
Copy link
Author

CurlyMoo commented May 19, 2024

Oh, sorry, i saw somewhere you mentioned exact dates.. so thought it tied to dates :) didn't check source.

It does, because by definition unix time starts on the 1st of januari 1970. So, checking for a year other than 1970 is similar to that sync flag but with more accuracy of success.

https://en.wikipedia.org/wiki/Unix_time

@CurlyMoo
Copy link
Author

@McMagellan Can you let it run again for a while and check for the Syncing with ntp servers loglines?

@McMagellan
Copy link

Feedback: Testing time synchronization. Alpha baddb94, #531
The test failed again.

How I do it:

  1. Close the console window in the PC browser.
  2. Disconnect the main Fritzbox1 from power
  3. (in the house basement) switch off the power to the Fritzbox2 repeater.
  4. Switch off Heishamon, wait 1 minute.
  5. Switch on Heishamon, switch on Fritzbox2, switch on Fritzbox1.

LED WLAN and DSL on Fritzbox1 flash for about 5 - 6 minutes until the PC browser is connected to the WLAN again.
During this time the Heishamon Setup AP is active and I connected to the console via it to test it.
Message every 15 seconds: Reconnecting to WIFI failed. Waiting a few seconds before trying again.

Screenshot 2024-05-19 at 11-24-56 Heisha monitor

For the next attempt, I activated "Debug log to MQTT topic from start" to have the messages recorded in the influxdb immediately after reconnection.
In the log file (see logfile) the first 4 lines are from the previous correct connection. The first data set is recorded after 3 minutes 11 seconds after power returns (start uptime new).
panasonic heat pump_log.csv

The timestamp at the beginning of the line comes from the Raspberry.
The Heishamon time is then at 1/1/1970 and 1:03:11 and runs synchronously with the uptime.
There was no further synchronization.

@McMagellan
Copy link

@CurlyMoo: How can i do this?
@McMagellan Can you let it run again for a while and check for the Syncing with ntp servers loglines?

@McMagellan
Copy link

I did another test on synchronizing the date and time.
After about 30 minutes, dated January 1st, 1970, I changed and saved the Heishamon settings in a second console window.
The time was then correctly synchronized and the uptime and rules continued.

This process was recorded in the first console window. You can see that the time has been updated.

Console log update Settings

Security notice:
While the settings is being updated, all parameters of the profile are displayed in the log file. This also includes the password for the currently connected WiFi network. Marked in blue here in the picture.
FYI.

@stumbaumr
Copy link

stumbaumr commented Jun 7, 2024

Somehow yesterday afternoon the HeishaMon rebooted, left rules active, but did not get the proper time. Only found out this morning when looking at the graphs and seeing a DHW production attempt at 4:55. Looks like I need to help to get this PR into main...
grafik

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 7, 2024

Looks like I need to help to get this PR into main...

Please do!

@stumbaumr
Copy link

stumbaumr commented Jun 7, 2024

Why do you need two timers? The code from timer -6 could look like this:

      case -6: {
          time_t now = time(NULL);
          struct tm *tm_struct = localtime(&now);
          if(tm_struct->tm_year == 1970) {
            logprintln_P(F("I am still in the year 1970, syncing with ntp servers, check again in one minute"));
            timerqueue_insert(60, 0, -6);
          } else {
            logprintln_P(F("Time probably correct, syncing with ntp servers, check again in a day"));
            timerqueue_insert(86400, 0, -6);
          }
          ntpReload(&heishamonSettings);
        } break;

and then in systemboot just call it after a minute.

      logprintln_P(F("Syncing with ntp servers, check again in one minute"));
      ntpReload(&heishamonSettings);
      timerqueue_insert(60, 0, -6);

And then delete timer -5...
...and rename -6 to -5...

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 7, 2024

Why do you need two timers?

Because in your case you're always doing a ntpReload even if it's not necessary.

@stumbaumr
Copy link

Why do you need two timers?

Because in your case you're always doing a ntpReload even if it's not necessary.

Only once, right after the systemboot.
But you could also remove that, doing things like this

      case -6: {
          ntpReload(&heishamonSettings);
          time_t now = time(NULL);
          struct tm *tm_struct = localtime(&now);
          if(tm_struct->tm_year == 1970) {
            logprintln_P(F("I am still in the year 1970, check again in one minute"));
            timerqueue_insert(60, 0, -6);
          } else {
            logprintln_P(F("Time probably correct, check again in a day"));
            timerqueue_insert(86400, 0, -6);
          }
        } break;

and

      logprintln_P(F("Syncing with ntp servers after boot"));
      timerqueue_insert(1, 0, -6);

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 7, 2024

Same thing. You are already doing a ntpReload while you haven't checked if the previous ntpReload succeeded.

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 7, 2024

And i don't see why using two timers is an issue? They're not active at the same time so aren't a performance issue at all.

@stumbaumr
Copy link

Same thing. You are already doing a ntpReload while you haven't checked if the previous ntpReload succeeded.

It is just doing it the other way around:

  • Do the ntpReload, if it failed, schedule another one and try then again.
  • If successful, schedule it for the next day anyway.

And i don't see why using two timers is an issue? They're not active at the same time so aren't a performance issue at all.

It is less code and easier to read. Less code is easier to maintain.

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 7, 2024 via email

@stumbaumr
Copy link

Oh, ok... sorry. My spaghetti-code-brain has problems processing things like that and I didn't know it is async...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants