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

0.5.74 getNTPTime Änderung 'retries' funktioniert nicht wie sie soll #609

Closed
beegee3 opened this issue Jan 20, 2023 · 7 comments
Closed
Assignees
Labels
bug Something isn't working fixed dev fixed

Comments

@beegee3
Copy link
Contributor

beegee3 commented Jan 20, 2023

Wenn nach den 5 getNtpTime Versuchen bei vorhandenem mUtcTimerstamp alle Ticker "zwangsaktiviert" werden, setzt app::tickNtpUpdate den NTP Update Trigger auf 12 h. Da aber keine Rückmeldung vom NTP Server kam, sollte der Trigger bei 5 sec bleiben, d.h. die Zeile nxtTrig = 43200; muss gelöscht werden. Es ist auch übersichtlicher, wenn die Triggerzeiten nur noch in ahoywifi::getNtpTime gesetzt werden. Die Anweisungen dort passen, lesen sich aber merkwürdig:

  • *nxtTrig = 43200; braucht man ja nur bei erfolgreichem Update, kann also dort stehen.
  • mRetries wird nur bei 0 != *mUtcTimestamp zurückgesetzt, sonst läuft das auf 0 und bleibt dort (was natürlich auch unproblematisch ist).
  • die Debug Meldung "try to getNtpTime" sollte nur kommen, wenn es auch wirklich versucht wird.

Alles in allem folgender Vorschlag:
in app::tickNtpUpdate(void) die Zeile nxtTrig = 43200; löschen und die erste Zeile ändern auf
uint32_t nxtTrig; // getNtpTime sets the value
und

bool ahoywifi::getNtpTime(uint32_t *nxtTrig) {
    *nxtTrig = 5;  // default: check again in 5 sec
    if(0 != mRetries)
        mRetries--;
    else {
        mRetries = NTP_RETRIES;
        if(0 != *mUtcTimestamp) // time is availabe, but NTP not
            return true;        // true is necessary to enable all timers even if NTP was not reachable
    }

    if(GOT_IP != mStaConn)
        return false;

    DPRINTLN(DBG_INFO, "try to getNtpTime");

    ...
                *mUtcTimestamp = secsSince1900 - 2208988800UL; // UTC time
                *nxtTrig = 43200; // NTP was successful, check again in 12h
                DPRINTLN(DBG_INFO, "[NTP]: " + ah::getDateTimeStr(*mUtcTimestamp) + " UTC");
    ...
}
@beegee3
Copy link
Contributor Author

beegee3 commented Jan 20, 2023

... und nochmals der Hiweis:
Auch wenn die Internet NTP Server fast immer erreichbar sind, ist es vorteilhaft einen lokalen NTP Server (wie die Fritz-Box; Einstellung: s. Hilfe dort) zu nutzen. Erstens bekommt man immer sofort Rückmeldung und zweitens haben die meistens die Möglichkeit, mehrere Internet NTP Server abzufragen.

@stefan123t stefan123t added the bug Something isn't working label Jan 20, 2023
@lumapu
Copy link
Owner

lumapu commented Jan 20, 2023

@beegee3 bitte auch nochmal die neusete Version checken, ich war selbst nicht zufrieden und sehe erst jetzt deinen Issue!

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 21, 2023

@lumapu die neue Version 0.5.75 bringt's nicht: wenn bei vorhandener Zeit das erste getNtpTime nicht funktioniert, wird dort mLastNtpFailed = false gesetzt. Nach 5 sec wird getNtpTime erneut aufgerufen, liefert aber sofort true zurück und versucht gar nicht, den NTP Server abzufragen.
Dann doch lieber getNtpTime der ursprünglichen Version 0.5.73 und die Regelung in app::tickNtpUpdate, hier beispielhaft mit einem Neuversuch nach 60 Sekunden, falls bei vorhandener Zeit das NTP Update erfolglos ist (nach belieben anpassen).

void app::tickNtpUpdate(void) {
    uint32_t nxtTrig = 5;  // default: check again in 5 sec (if mTimestamp = 0)
    bool isOK = mWifi.getNtpTime();
    if (isOK || mTimestamp != 0) {
        if (mMqttReconnect && mMqttEnabled) {
            ...
        }

        nxtTrig = isOK ? 43200 : 60; // depending on NTP update success check again in 12 h or in 1 min

        if((mSunrise == 0) && (mConfig->sun.lat) && (mConfig->sun.lon)) {
            ...
        }
    }
    once(std::bind(&app::tickNtpUpdate, this), nxtTrig, "ntp");
}

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 21, 2023

@lumapu ... und wenn du dabei bist, könntest du vielleicht auch das AP Modus Problem korrigieren (s. 1. Bemerkung in #611)

@lumapu
Copy link
Owner

lumapu commented Jan 21, 2023

@beegee3 hab's gesehen, danke fürs vorbereiten / entwickeln.
Bin noch nicht dazu gekommen sein Rezept zu kochen 😊

@lumapu
Copy link
Owner

lumapu commented Jan 21, 2023

warum habe ich nur so kompliziert gedacht, wenn es so einfach geht =)

@lumapu lumapu added the fixed dev fixed label Jan 21, 2023
lumapu added a commit that referenced this issue Jan 22, 2023
@beegee3
Copy link
Contributor Author

beegee3 commented Jan 22, 2023

👍 passt!

@beegee3 beegee3 closed this as completed Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed dev fixed
Projects
None yet
Development

No branches or pull requests

3 participants