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

Feature Request: Vorschläge Code Anpassungen Ticker #515

Closed
beegee3 opened this issue Dec 20, 2022 · 28 comments
Closed

Feature Request: Vorschläge Code Anpassungen Ticker #515

beegee3 opened this issue Dec 20, 2022 · 28 comments
Assignees
Labels
enhancement New feature or request fixed dev fixed

Comments

@beegee3
Copy link
Contributor

beegee3 commented Dec 20, 2022

Der Ticker tickCalcSunrise ist in app::setup() unnötig. Die Sonnenzeiten können sowieso erst berechnet werden, wenn tickNtpUpdate erfolgreich war. Daher kann man das erste tickCalcSunrise dahin verlegen. Und die Prüfung
if ((mTimestamp > 0) && (!mConfig->sun.disNightCom || (mTimestamp >= (mSunrise - mConfig->sun.offsetSec) && mTimestamp <= (mSunset + mConfig->sun.offsetSec)))) in app:tickSend kann man durch eine Variable bool mIVCommunicationOn; ersetzen, die als Default auf true gesetzt ist und ggfs. nach tickCalcSunrise angepasst wird.
Insgesamt sähe das so aus:

void app::resetSystem(void) {
...
mIVCommunicationOn = true;
....
}

void app::setup() {
...
    #if !defined(AP_ONLY)
        once(std::bind(&app::tickNtpUpdate, this), 2);
    #endif
...
}

void app::tickNtpUpdate(void) {
    uint32_t nxtTrig = 5;  // default: check again in 5 sec
    if (mWifi.getNtpTime()) {
        nxtTrig = 43200;    // check again in 12 h
        if((mSunrise == 0) && (mConfig->sun.lat) && (mConfig->sun.lon)) {
            mCalculatedTimezoneOffset = (int8_t)((mConfig->sun.lon >= 0 ? mConfig->sun.lon + 7.5 : mConfig->sun.lon - 7.5) / 15) * 3600;
            tickCalcSunrise();
        }
    }
    once(std::bind(&app::tickNtpUpdate, this), nxtTrig);
}

void app::tickCalcSunrise(void) {
    ah::calculateSunriseSunset(mTimestamp, mCalculatedTimezoneOffset, mConfig->sun.lat, mConfig->sun.lon, &mSunrise, &mSunset);
    tickIVCommunication();
    uint32_t nxtTrig = mTimestamp - ((mTimestamp - 1) % 86400) + 86400; // next midnight, -1 for safety that it is certain next day
    ...
}

void app::tickIVCommunication(void) {
    mIVCommunicationOn = !mConfig->sun.disNightCom; // if sun.disNightCom is false, communication is always on
    if (!mIVCommunicationOn) {  // inverter communication only during the day
        uint32_t nxtTrig;
        if (mTimestamp < mSunrise - mConfig->sun.offsetSec) { // current time is before communication start, set next trigger to communication start
            nxtTrig = mSunrise - mConfig->sun.offsetSec;
        } else {
            if (mTimestamp > mSunset + mConfig->sun.offsetSec) { // current time is past communication stop, nothing to do. Next update will be done at midnight by tickCalcSunrise
                return;
            } else { // current time lies within communication start/stop time, set next trigger to communication stop
                mIVCommunicationOn = true;
                nxtTrig = mSunset + mConfig->sun.offsetSec;
            }
        }
        onceAt(std::bind(&app::tickIVCommunication, this), nxtTrig);
    }
}

void app::tickSend(void) {
    if(!mSys->Radio.isChipConnected()) {
        DPRINTLN(DBG_WARN, "NRF24 not connected!");
        return;
    }
    if (mIVCommunicationOn) {
        ...

P.S.: es reicht, in tickCalcSunrise den Trigger auf 1 Sekunde nach Mitternacht einzustellen, da er ja im Scheduler durch
(p->d.timestamp) <= mTimestamp ausgelöst wird.

@lumapu
Copy link
Owner

lumapu commented Dec 20, 2022

danke für die Vorschläge, habe sie nahezu 1:1 übernommen in 0.5.58

lumapu added a commit that referenced this issue Dec 20, 2022
improved wifi initial connection - especially if station wifi is not available #509
removed new operators from web.h (reduce dynamic allocation)
improved sun calculation #515, #505
fixed wifi auto reconnect #509
added disable night communication flag to MQTT #505
changed MQTT publish of `available` and `available_text` to sunset #468
@beegee3
Copy link
Contributor Author

beegee3 commented Dec 21, 2022

😃 👍 In app::resetSystem muss noch mIVCommunicationOn = true; stehen, sonst gibt es im AP_ONLY Modus bzw. ohne erfolgreiches tickNtpUpdate keine Kommunikation.

@lumapu
Copy link
Owner

lumapu commented Dec 21, 2022

ja das hab ich auch schon festgestellt, kommt mit der nächsten Version

lumapu added a commit that referenced this issue Dec 23, 2022
beautified serial.html
added ticker for wifi loop #515
reverted sunrise / sunset ticker to most recent version
@beegee3
Copy link
Contributor Author

beegee3 commented Dec 24, 2022

nehme hier mal das Todesschleife Problem beim Scheduler auf (gehört ja eigentlich nicht in #509)
Idee auf die Schnelle: in checkEvery, Schleife while(NULL != p) die p->d.c nicht sofort ausführen, sondern zwischenspeichern und erst nach Ende der Schleife ausführen. Dann sollten durch p->d.c neu erzeugte Ticker doch immer erst beim nächsten Check berücksichtigt werden, oder?

@beegee3
Copy link
Contributor Author

beegee3 commented Dec 24, 2022

... die resetEveryById der neuen Version 0.5.60 [6bb8a4e] hilft da auch nicht weiter. Das wäre doch nur, einen every Ticker VOR auslösen zu resetten, d.h nichts dafür, wenn man schon drin ist. Meiner Meinung nach kann die Funktion auch wieder gelöscht werden.

@lumapu
Copy link
Owner

lumapu commented Dec 24, 2022

man könnte sich vor ausführen der Liste auch das Ende merken und nur bis dahin ausführen, alles was dazu gekommen ist wird dann nicht ausgeführt

@lumapu
Copy link
Owner

lumapu commented Dec 24, 2022

... die resetEveryById der neuen Version 0.5.60 [6bb8a4e] hilft da auch nicht weiter. Das wäre doch nur, einen every Ticker VOR auslösen zu resetten, d.h nichts dafür, wenn man schon drin ist. Meiner Meinung nach kann die Funktion auch wieder gelöscht werden.

die Idee ist für den sendTicker vorgesehen. Wenn man zB ein Powerlimit setzt muss man in schlechtesten Fall ein ganzes Intervall (30s) warten. Ich will einen Callback oä einführen, mit dem das Limit sofort gesendet wird, indem die sendTicker Routine sofort aufgerufen wird. Damit es zu keinen Komplikationen kommt soll gleichzeitig auch das timeout zurückgesetzt werden.

@fubu2k
Copy link

fubu2k commented Dec 25, 2022

für Code Reviews eignet sich ab und an auch die OpenaI Ki unter chat.openai.com - dort den Code einfügen und eine Frage wie "can you improve the code" stellen. Es werden hier und da echt gute Empfehlungen ausgespuckt :)

@beegee3
Copy link
Contributor Author

beegee3 commented Dec 27, 2022

@lumapu könnte man da nicht eine ganz einfache pseudomäßige Event Queue verwenden? D.h. im Scheduler:

std::list<scdCb> mEvents;

void addEvent(scdCb c)  { mEvents.push_back(c); }

inline void checkEvents(void) {
    int8_t n = mEvents.size();
    while (n > 0) {
       (mEvents.front())();
       mEvents.pop_front();
       n--;
    }
}

checkEvents natürlich am Ende der Scheduler loop aufrufen, dann wird das im Sekundentakt abgearbeitet. Ist im Prinzip nicht wirklich anders als ein once Aufruf, hat aber nicht das oben genannte Schleifenproblem, da mit std::list und nicht mit llist.
(Gibt es eigentlich einen Grund, dass du von der std::list weg zur llist bist?)

Entsprechend zur Verfügung gestellt, könnte z.B. auch die ahoywifi loop komplett darüber gesteuert werden (ohne einen Ticker)
Man braucht nur 3 Aufrufe:
-am Ende des setup, um die loop einmal aufzurufen
-am Ende der loop, solange kein GotIP erhalten wurde
-am Ende von connectionEvent, wenn ein disconnect gemeldet wird
Damit verhält sich die ahoywifi loop bis zum Erhalt einer STA IP wie ein everySec Ticker,
hört dann aber auf und wird erst bei disconnected für das reconnect wieder gestartet.

@lumapu
Copy link
Owner

lumapu commented Dec 27, 2022

Ich habe llist eingeführt, da ich keine dynamischen Datentypen wollte. Mein Gefühl war, das es mit der std::list instabiler läuft, habe dafür aber keine Beweise.
llist reserviert für die gesamte Laufzeit eine fixe Speichergröße

Ich bin hier offen für eine Diskussion und lerne gerne dazu, gerne können wir auch zu std::list migrieren

Das hier finde ich interessant und spiegelt meine Befürchtungen wider:

https://stackoverflow.com/questions/9612588/stl-within-embedded-system-with-very-limited-memory

@beegee3
Copy link
Contributor Author

beegee3 commented Dec 27, 2022

OK, ich glaube zwar nicht, dass es bei den wenigen Tickern zu Problemen mit dynamischem Speicher kommt, aber glauben heißt nicht wissen! Andererseits weiß man bei fixem Speicher im Vorfeld, ob es geht. Letztlich hat llist ja auch nur das Schleifenproblem und ist komplizierter, als man es hier braucht. Wie wäre es daher denn mit Arrays:
ein Array, in dem die Ticker Funktionen hinterlegt werden, ein zweites mit Flags, ob der jeweilige Eintrag im ersten Array aktiv ist, also z.B. sP mEveryArr[20]; und bool mEveryInUseArr[20];. Dazu eine add Funktion, die nach !mEveryInUseArr[i] sucht und dann mEveryArr[i] belegt. In checkEvery macht man eine Kopie von mEveryInUseArr, die man anschließend in einer for Scheife abarbeitet (und so das Schleifenproblem vermeidet). Ein once Ticker wird einfach mit mEveryInUseArr[i] = false; abgeschaltet. Passiert das VOR dem Aufruf der Ticker Funktion, so braucht man auch keinen zusätzlichen Eintrag, wenn die Ticker Funktion ein neues once erzeugt. (Auch wenn das nicht wirklich relevant ist.) Analog für onceAt.

@beegee3
Copy link
Contributor Author

beegee3 commented Dec 27, 2022

Vorschlag für eine Array Version mit leicht angepasster sP Struktur, was Every und At zusammenfasst.
Nur die Anpassungen sind aufgeführt, unveränderter Code durch ... angedeutet. llist wird nicht mehr benötigt.
Die Funktion getStat erfüllt keinen Zweck mehr, wird aber m.E. auch nicht wirklich gebraucht.

namespace ah {
    typedef std::function<void()> scdCb;

    enum {SCD_SEC = 1, SCD_MIN = 60, SCD_HOUR = 3600, SCD_12H = 43200, SCD_DAY = 86400};

    struct sP {
        scdCb c;
        uint32_t timeout;
        uint32_t reload;
        bool isTimestamp;
        sP() : c(NULL), timeout(0), reload(0), isTimestamp(false) {}
        sP(scdCb a, uint32_t tmt, uint32_t rl, bool its) : c(a), timeout(tmt), reload(rl), isTimestamp(its) {}
    };


    class Scheduler {
        public:
            Scheduler() {}

            static const uint8_t MAXTICKER = 30;

            void setup() {
                ...
                for (uint8_t i = 0; i < MAXTICKER; i++)
                    mTickerInUse[i] = false;
            }

            void loop(void) {
                ...
                checkTicker();
            }

            void once(scdCb c, uint32_t timeout)     { addTicker(c, timeout, 0, false); }
            void onceAt(scdCb c, uint32_t timestamp) { addTicker(c, timestamp, 0, true); }
            uint8_t every(scdCb c, uint32_t interval){ return addTicker(c, interval, interval, false); }

            void everySec(scdCb c)  { every(c, SCD_SEC);  }
            void everyMin(scdCb c)  { every(c, SCD_MIN);  }
            void everyHour(scdCb c) { every(c, SCD_HOUR); }
            void every12h(scdCb c)  { every(c, SCD_12H);  }
            void everyDay(scdCb c)  { every(c, SCD_DAY);  }

            ...
            bool resetEveryById(uint8_t id) {
                if (mTickerInUse[id] == false)
                    return false;
                mTicker[id].timeout = mTicker[id].reload;
                return true;
            }

            ...
            void getStat(uint16_t *everyMax, uint16_t *atMax) {
                *everyMax = 0;
                *atMax    = 0;
            }

        ...
        private:
            inline uint8_t addTicker(scdCb c, uint32_t timeout, uint32_t reload, bool isTimestamp) {
                 for (uint8_t i = 0; i < MAXTICKER; i++) {
                    if (!mTickerInUse[i]) {
                        mTickerInUse[i] = true;
                        mTicker[i].c = c;
                        mTicker[i].timeout = timeout;
                        mTicker[i].reload = reload;
                        mTicker[i].isTimestamp = isTimestamp;
                        return i;
                    }
                 }
                 return 0xff;
            }
            inline void checkTicker(void) {
                bool inUse[MAXTICKER];
                for (uint8_t i = 0; i < MAXTICKER; i++)
                    inUse[i] = mTickerInUse[i];
                for (uint8_t i = 0; i < MAXTICKER; i++) {
                    if (inUse[i]) {
                        if (mTicker[i].timeout <= ((mTicker[i].isTimestamp) ? mTimestamp : mDiffSeconds)) { // expired
                            if(0 == mTicker[i].reload)
                                mTickerInUse[i] = false;
                            else
                                mTicker[i].timeout = mTicker[i].reload;
                            (mTicker[i].c)();
                            yield();
                        }
                        else // not expired
                            if (!mTicker[i].isTimestamp)
                                mTicker[i].timeout -= mDiffSeconds;
                    }
                }
            }

            sP mTicker[MAXTICKER];
            bool mTickerInUse[MAXTICKER];
            ...
    };
}

@lumapu
Copy link
Owner

lumapu commented Dec 27, 2022

sehr gute Idee, schaue ich mir noch genauer an. Einfacher ist immer besser

@lumapu
Copy link
Owner

lumapu commented Dec 27, 2022

ist es schneller / resourcenschonender, wenn man das

for (uint8_t i = 0; i < MAXTICKER; i++)
    inUse[i] = mTickerInUse[i];

durch ein

memcpy(inUse, mTickerInUse, MAXTICKER);

ersetzt?

Die getStat Funktion soll dazu dienen, um abzuschätzen ob uns eine fixe Arraygröße vollläuft. Ich habe die Info deshalb auch in /api/system hinzugefügt, wird aber aktuell nicht auf der Website dargestellt.

Ich veröffentliche später noch eine 0.5.63 mit dem optimierten Scheduler - ich teste gerade 😊

lumapu added a commit that referenced this issue Dec 27, 2022
@lumapu lumapu added the fixed dev fixed label Dec 27, 2022
@beegee3
Copy link
Contributor Author

beegee3 commented Dec 27, 2022

memcpy ist immer gut. Ob es hier wirkliche Vorteile bringt, hängt sich davon ab, wie der Compiler die for Schleife verarbeitet.
Der Vorschlag ist ja zunächst auf 'einfach' getrimmt, nicht auf 'optimal'. Eine erste Optimierung wäre, nicht eine 'in use' Liste zuführen und immer alle MAXTICKER Zustände abzufragen, sondern einen Counter für die Anzahl der aktuell genutzten Ticker mit einer Liste der genutzten mTicker Indizes. Dazu folgende Anpassungen:

            sP mTicker[MAXTICKER];
            uint8_t mTickerId[MAXTICKER];
            uint8_t mTickerCnt;

            void setup() {
                ...
                mTickerCnt = 0;
            }

            bool resetEveryById(uint8_t id) {
                for (uint8_t i = 0; i < mTickerCnt; i++)
                    if (mTickerId[i] == id) {
                        mTicker[id].timeout = mTicker[id].reload;
                        return true;
                    }
                return false;
            }

            inline uint8_t addTicker(scdCb c, uint32_t timeout, uint32_t reload, bool isTimestamp) {
                if (mTickerCnt < MAXTICKER) {
                    for (uint8_t id = 0; id < MAXTICKER; id++) {
                        if (NULL == mTicker[id].c) {
                            mTicker[id].c = c;
                            mTicker[id].timeout = timeout;
                            mTicker[id].reload = reload;
                            mTicker[id].isTimestamp = isTimestamp;
                            mTickerId[mTickerCnt] = id;
                            mTickerCnt++;
                            return id;
                        }
                    }
                }
                return 0xff;
            }
            inline void checkTicker(void) {
                uint8_t n = mTickerCnt;
                for (uint8_t i = 0; i < n; i++) {
                    uint8_t id = mTickerId[i];
                    if (mTicker[id].timeout <= ((mTicker[id].isTimestamp) ? mTimestamp : mDiffSeconds)) { // expired
                        scdCb c = mTicker[id].c;
                        if(0 == mTicker[id].reload) {
                            mTicker[id].c = NULL;   // mark mTicker[id] as unused
                            mTickerCnt--;
                            mTickerId[i] = mTickerId[mTickerCnt];
                        }
                        else
                            mTicker[id].timeout = mTicker[id].reload;
                        (c)();
                        yield();
                    }
                    else // not expired
                        if (!mTicker[id].isTimestamp)
                            mTicker[id].timeout -= mDiffSeconds;
                }
            }

@beegee3
Copy link
Contributor Author

beegee3 commented Dec 28, 2022

zwei kleine "bugs":

  • void getSchedulerInfo(...) in app.h: bitte bei der Zeile return getStat(max); das return entfernen.
  • in tickCalcSunrise wird bei nxtTrig der Zeitzonen- Offset nicht berücksichtigt.
    nxtTrig wird als 'next midnight + 10s' UTC berechnet. Z.B. die Zeitzone UTC+12 hat da schon Mittag (der Trigger kommt viel zu spät), die Zeitzone UTC-6 noch 18 Uhr abends (der Trigger ist viel zu früh). D.h. wie bei calculateSunriseSunset muss bei nxtTrig anstelle von mTimestamp mit timezoneTime = mTimestamp + mCalculatedTimezoneOffset gerechnet werden. Das gibt natürlich wieder eine Zeitzonen-Zeit, man braucht aber als Trigger UTC. Also mCalculatedTimezoneOffset wieder abziehen:
    nxtTrig = timezoneTime - ((timezoneTime - 10) % 86400) + 86400 - mCalculatedTimezoneOffset;
    zusammengefasst:
    nxtTrig = mTimestamp - ((mTimestamp + mCalculatedTimezoneOffset - 10) % 86400) + 86400;
    nxtTrig löst so immer 10s nach Mitternacht der Zeitzonen-Zeit aus. In UTC bzw. Lokalzeit sieht das natürlich i.d.R. anders aus. Für die 3 Zeitzonen in Deutschland sind das in Lokalzeit (MEZ):
    UTC+2 (der östlichste Zipfel; Längengrad > 15°): 10s nach 23.00 Uhr
    UTC+1 (mittlerer Bereich; 7.5° < Längengrad < 15°): 10s nach 00.00 Uhr
    UTC+0 (der westliche Bereich; Längengrad < 7.5°): 10s nach 01.00 Uhr

@lumapu
Copy link
Owner

lumapu commented Dec 28, 2022

Ich finde die erste Version des vereinfachten Scheduler übersichtlicher, als die zweite Version, bei der 'umsortiert' wird.

Der Fehler mit der Zeitzone ist mir beim Debuggen auch aufgefallen, er hat immer um 01:00:10 ausgelöst - also eine Stunde zu spät. Ich habe deine Änderung eingebaut und hoffe jetzt stimmt es überall auf der Welt.

lumapu added a commit that referenced this issue Dec 28, 2022
added `/` to MQTT topic and Inverter name
trigger for `calcSunrise` is now using local time #515
fix reconnect timeout for WiFi #509
start AP only after boot, not on WiFi connection loss
improved /system `free_heap` value (measured before JSON-tree is built)
@beegee3
Copy link
Contributor Author

beegee3 commented Dec 29, 2022

erste Version des vereinfachten Scheduler ...

gib dir völlig Recht. Der inUse Teil in checkTicker kostet so gut wie keine Zeit. Und der Rest ist bei beiden Vorschlägen (fast) gleich.

Zeitzone ...

Australien wird es dir danken 😄
Das Problem, nicht unbedingt um Mitternacht (Lokalzeit) auszulösen, lässt sich mit mCalculatedTimezoneOffset allein ja auch nicht lösen (s.o. 3 Zeitzonen in Deutschland ...). Mit Beginn der Sommerzeit ist alles 1 Stunde verschoben. Hier wären "echte" Zeitzonenangaben notwendig (s. z.B. https://werner.rothschopf.net/201802_arduino_esp8266_ntp.htm). Aber braucht man das?
Man will doch nur bei 'Night Communication disabled' den Zeitraum für die Inverter Kommunikation festzulegen. Eigentlich könnte man das auch direkt (z.B. 1s, 10s oder 1min) nach 'Communication stop' für den nächsten Tag aufbereiten. Ist das für dich eine Alternative? Die notwendigen Anpassungen in tickCalcSunrise sind wahrscheinlich gering.

@beegee3
Copy link
Contributor Author

beegee3 commented Dec 29, 2022

hab's erfolgreich mit 1min ausprobiert, man sieht schön, wie sich in der Web Anzeige die Info 'stopped polling at' nach 1min zu 'will start polling at' mit den passenden Zeitangaben ändert:

void app::tickCalcSunrise(void) {
    if (mSunrise == 0)                                          // on boot/reboot calc sun values for current time
        ah::calculateSunriseSunset(mTimestamp, mCalculatedTimezoneOffset, mConfig->sun.lat, mConfig->sun.lon, &mSunrise, &mSunset);

    if (mTimestamp > (mSunset + mConfig->sun.offsetSec))        // current time is past communication stop, calc sun values for next day
        ah::calculateSunriseSunset(mTimestamp + 86400, mCalculatedTimezoneOffset, mConfig->sun.lat, mConfig->sun.lon, &mSunrise, &mSunset);

    tickIVCommunication();

    uint32_t nxtTrig = mSunset + mConfig->sun.offsetSec + 60;    // set next trigger to communication stop, +60 for safety that it is certain past communication stop
    onceAt(std::bind(&app::tickCalcSunrise, this), nxtTrig);
    if (mConfig->mqtt.broker[0] > 0) {
        mMqtt.tickerSun(mSunrise, mSunset, mConfig->sun.offsetSec, mConfig->sun.disNightCom);
    }
}

Dazu noch das at midnight im tickIVCommunication Kommentar // ... past communication stop ... löschen. 🤔 😄
Vielleicht auch die 'Sun' und 'Communication' Texte dynamisch gestalten, je nachdem, ob sich die Zeitangaben auf den aktuellen, oder den nächsten Tag beziehen.

@beegee3
Copy link
Contributor Author

beegee3 commented Dec 30, 2022

@lumapu komme nochmal auf deine resetEveryById Idee ist für den sendTicker zurück und habe mir tickSend angesehen. Ist ein hartes Stück Brot! Es ist doch richtig, dass pro Aufruf nur ein Inverter abgearbeitet wird?! Ist das Absicht, oder soll die
do { ... } while ((NULL == iv) && ((maxLoop--) > 0)); eigentlich auch das nachfolgende if (NULL != iv) { ... } umfassen?
Irgendwie ist das komisch, mPayload.process in der if bearbeitet ja alle Inverter (nicht nur die mit iv->config->enabled).
Soll mPayload.process vielleicht mit *iv als Parameter nur diesen verarbeiten? (In app::loop kann es ja in einer Schleife über die Inverter laufen).
Macht es Sinn, den Inhalt von tickSend in eine eigene Funktion ivSend(Inverter<> *iv) zu transferieren? tickSend ruft sie dann für jeden Inverter auf. Dann könnte z.B. für 'Powerlimit setzen' ivSend direkt gestartet werden (keine Wartezeit).

@lumapu
Copy link
Owner

lumapu commented Dec 30, 2022

Macht es Sinn, den Inhalt von tickSend in eine eigene Funktion ivSend(Inverter<> *iv) zu transferieren? tickSend ruft sie dann für jeden Inverter auf. Dann könnte z.B. für 'Powerlimit setzen' ivSend direkt gestartet werden (keine Wartezeit).

Das klingt sehr gut. Ja die Senderoutine kommt noch aus der 'Steinzeit' (April 2022). Hier muss noch einiges geschraubt werden, deine Ideen lesen sich sehr gut!

Es ist doch richtig, dass pro Aufruf nur ein Inverter abgearbeitet wird?! Ist das Absicht, oder soll die

Ja es soll nur ein Inverter pro Schleifendurchgang gepollt werden. Dadurch versuchen wir Kollisionen durch gleichzeitige Abfrage zu verhindern und hatten bisher sehr gute Ergebnisse. Macht natürlich Sinn bei der Abfrage eines einzelnen Inverters in Processpayload nicht alle abzuklappern.

Einen Guten Rutsch!

@lumapu lumapu closed this as completed Dec 30, 2022
@beegee3
Copy link
Contributor Author

beegee3 commented Dec 31, 2022

@lumapu 🎊 🚀 release 0.5.66 yes you did it!!! Herzlichen Glückwunsch! Die Zeit war reif dafür, so viele Verbesserungen!

Nur eine kurze Frage (will dafür dieses issue nicht neu aufmachen, nur kurzes ja/nein als Antwort):
ist der Vorschlag von oben, tickCalcSunrise 1min nach 'Communication stop' zu starten, eine Option für dich?
Nachdem ich das ausprobiert habe, gefällt es mir persönlich besser als die Mitternachtssache, wo die Auslösung je nach Sommer-/Winterzeit um 1 Stunde verschoben erscheint (Lokalzeit).
In der Web System Anzeige könnte in der Sun Headline ja entsprechend 'Sun (currently)' bzw. 'Sun (next)' stehen.

Lehn dich zurück, du hast es dir verdient,
einen Guten Rutsch!

@lumapu
Copy link
Owner

lumapu commented Dec 31, 2022

danke, ja ich denke es war mehr als notwendig und schafft bei mir auch innere Ruhe.

Die Idee ist sicherlich gut, wir müssen nur überlegen, ob das auch bei einem reboot funktioniert, der zu einer beliebigen Zeit kommen kann.

@beegee3
Copy link
Contributor Author

beegee3 commented Dec 31, 2022

tut es, hab's ausprobiert. Nach einem reboot wird sowieso immer mit mSunrise=0 gestartet.

@lumapu lumapu reopened this Dec 31, 2022
lumapu added a commit that referenced this issue Jan 1, 2023
… comm. stop #515

moved payload send to `payload.h`, function `ivSend` #515
@lumapu
Copy link
Owner

lumapu commented Jan 1, 2023

Macht es Sinn, den Inhalt von tickSend in eine eigene Funktion ivSend(Inverter<> *iv) zu transferieren? tickSend ruft sie dann für jeden Inverter auf. Dann könnte z.B. für 'Powerlimit setzen' ivSend direkt gestartet werden (keine Wartezeit).

Ich habe jetzt angefangen, die Funktion in payload.h auszulagern. Wenn wir die Funktion direkt aufrufen wollen, müssen wir zuerst die schon 'halb' empfangene Payload zurücksetzen und das Kommando sofort schicken. Ich schaue mir das morgen nochmal genauer an.
Payload.process hat bisher weiterhin eine Schleife über alle Inverter, hier müssen wir wie du oben geschrieben hast auch nochmal drüber schauen.

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 2, 2023

@lumapu hatte irgendwo im Zusammenhang mit 'Nachtabschaltung' etwas von deepSleep erwähnt;
bringe es jetzt hier unter (oder wäre ein neues issue besser?)
Habe mal einen Testcode gestrickt, bei dem nach dem ersten Erhalt der NTP Zeit der ESP für 3 x 10s in den Tiefschlaf geht.
Leider kann ich z.Zt. nur ESP32 testen, mangels ESP8266. Würdest du, wenn's zeitlich passt, den Code checken und austesten?

app::app() : ah::Scheduler() {}

// ESP deep sleep vars
#if defined(ESP8266)
struct {
    uint16_t sleepCntDwn;
    uint32_t sleepMs;                           // for testing purposes only
    bool sleepTimerDisabled;                    // for testing purposes only
} rtcData;
#else
RTC_DATA_ATTR struct {
    uint16_t sleepCntDwn;
    uint32_t sleepMs;                           // for testing purposes only
    bool sleepTimerDisabled;                    // for testing purposes only
} rtcData;
#endif

//-----------------------------------------------------------------------------
void app::setup() {
    Serial.begin(115200);
    while (!Serial)
        yield();

    #if defined(ESP8266)
    if(!ESP.rtcUserMemoryRead(0, (uint32_t *)&rtcData, sizeof(rtcData))) {
        rtcData.sleepCntDwn = 0;
        rtcData.sleepMs = 0;
        rtcData.sleepTimerDisabled = false;
    }
    #endif
    if (rtcData.sleepCntDwn > 0) {
        Serial.println("start " + String("sleep another ") + String(10*rtcData.sleepCntDwn) + " s");
        Serial.flush();
        --rtcData.sleepCntDwn;
        rtcData.sleepMs += 10000-millis();
        #if defined(ESP8266)
        ESP.rtcUserMemoryWrite(0, (uint32_t *)&rtcData, sizeof(rtcData));
        #endif
        ESP.deepSleep(10e6-1000*millis()); // sleep 10s minus current start time delay
    }
    if (rtcData.sleepMs > 0)
        Serial.println("slept " + String(rtcData.sleepMs) + " ms");

    ah::Scheduler::setup();
    ...
}

und in tickNtpUpdate(void) nach tickCalcSunrise();

    ...
            if (!rtcData.sleepTimerDisabled) {
                rtcData.sleepTimerDisabled = true;
                rtcData.sleepCntDwn = 2;            // (2+1)*10s = 30s all in all
                rtcData.sleepMs = 10000;
                #if defined(ESP8266)
                ESP.rtcUserMemoryWrite(0, (uint32_t *)&rtcData, sizeof(rtcData));
                #endif
                Serial.println("init deepSleep for 10s");
                Serial.flush();
                ESP.deepSleep(10e6);        // sleep 10s
            }
            else {
                rtcData.sleepTimerDisabled = false;
                rtcData.sleepCntDwn = 0;
                rtcData.sleepMs = 0;
                #if defined(ESP8266)
                ESP.rtcUserMemoryWrite(0, (uint32_t *)&rtcData, sizeof(rtcData));
                #endif
            }
    ...

sleepTimerDisabled wird hier nur gebraucht, damit nach dem deepSleep und erneutem NTP Update keine Endlosschleife entsteht, sleepMs ist nur, um zu sehen wie exakt die 30s eingehalten werden. Beides wird nur für den Test benötigt.

Wenn es dir gefällt, könnte im Setup 'ESP sleep begin' und 'end' als HH:MM Eingabe stehen. Bei gleicher Zeit gibt's kein sleep,
ansonsten kann man (analog zu tickIVCommunication) die Tageszeiten für die beiden Werte bestimmen, einen Ticker für den Start setzen und darin die Dauer in Minuten ausrechnen, als Countdown Wert im RTC hinterlegen und 1min deepSleep starten.

@lumapu
Copy link
Owner

lumapu commented Jan 3, 2023

kann ich gerne ausprobieren.
Soll der ESP immer für 30s schlafen, dann kurz kommunizieren? Oder soll er nur über Nacht schlafen? Habe nur deinen Text gelesen und noch nicht in den Code geschaut

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 3, 2023

die 30s sind nur zur Anschauung und für's testen. Am besten dabei seriell mitschneiden, da sieht man gut die 3 x 10s ESP Pause und dass danach das ganze wie ein reboot neu startet. Daraus kann man eine 'Nachtabschaltung' machen, die im Prinzip so funktioniert als hätte man den ESP eine vorgegebene Zeit vom Strom abgeklemmt.

@stefan123t stefan123t changed the title Vorschläge Code Anpassungen Feature Request: Vorschläge Code Anpassungen Jan 12, 2023
@stefan123t stefan123t added the enhancement New feature or request label Jan 12, 2023
@stefan123t stefan123t changed the title Feature Request: Vorschläge Code Anpassungen Feature Request: Vorschläge Code Anpassungen Ticker Jan 12, 2023
@lumapu lumapu closed this as completed Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed dev fixed
Projects
None yet
Development

No branches or pull requests

4 participants