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: Wifi Handling Code Vorschlag #571

Closed
beegee3 opened this issue Jan 9, 2023 · 10 comments
Closed

Feature Request: Wifi Handling Code Vorschlag #571

beegee3 opened this issue Jan 9, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request fixed dev fixed

Comments

@beegee3
Copy link
Contributor

beegee3 commented Jan 9, 2023

Wie in #548 angekündigt, hier ein Vorschlag zum Wifi Handling. Hatte die Idee dort grob skizziert, hier nun der passende Code.
Solange eine Wifi Verbindung aufgebaut wird, soll nichts anderes laufen. Dafür braucht man eine Schnittstelle von ahoywifi zu app und muss z.B. zum reconnect alle laufenden Ticker abschalten, d.h.

scheduler.h, der Ticker reset aus setup ist nun in einer eigenen Funktion (für Aufruf aus app):

            void setup() {
                mUptime     = 0;
                mTimestamp  = 0;
                mMax        = 0;
                mPrevMillis = millis();
                resetTicker();
            }

            inline void resetTicker(void) {
                for (uint8_t i = 0; i < MAX_NUM_TICKER; i++)
                    mTickerInUse[i] = false;
            }

ahoywifi.h, Callback zu app

class ahoywifi {
    public:
        typedef std::function<void(bool)> appWifiCb;

        ahoywifi();

        void setup(settings_t *config, uint32_t *utcTimestamp, appWifiCb cb);

    ...

        appWifiCb mAppWifiCb;
};

ahoywifi.cpp, Callback Nutzung:

void ahoywifi::setup(settings_t *config, uint32_t *utcTimestamp, appWifiCb cb) {
    mConfig = config;
    mUtcTimestamp = utcTimestamp;
    mAppWifiCb = cb;

    ...
}

void ahoywifi::connectionEvent(WiFiStatus_t status) {
    ...
        case GOT_IP:
            ...
            mAppWifiCb(true);
            break;

        case DISCONNECTED:
                ...
                mAppWifiCb(false);
                DPRINTLN(DBG_INFO, "[WiFi] Connection Lost");
            ...
    ...
}

die app::loop nutzt nun einen Callback für die eigentlichen Aufgaben. Die ursprüngliche loop ist in loopStandard umbenannt.
So kann man leicht zwischen Wifi Verbindung aufbauen (loopWifi) und Standardaufgaben (loopStandard) switchen.
(Könnte man auch mit einer if Anweisung machen, ist dann aber eine Überprüfung, die zusätzlich permanent den Prozessor beschäftigt; finde es so eleganter.)
Außerdem erfordert die Verlegung vom MQTT Server connect nach tickNtpUpdate, dass die Informationen zu Sun und IVCommunication mit Hilfe von Tickern übertragen werden, da beim Aufruf nicht sicher ist, dass die MQTT Verbindung steht.

app.h

        ...
        void loop(void);
        void loopWifi(void);
        void loopStandard(void);
        void onWifi(bool gotIp);
        ...
    private:
        typedef std::function<void()> innerLoopCb;
        ...
        void tickSun(void);
        void tickComm(void);
        ...
        innerLoopCb mInnerLoopCb;
        bool mMqttReconnect;
        ...

app.cpp, die bisher ungenutzte Variable mMqttActive wird auf (mConfig->mqtt.broker[0] > 0) gesetzt. Jeder weitere mMqtt Bezug ist von mMqttActive abhängig (würde sie in mMqttEnabled umbenennen; sie sollte wohl mal einen anderen Zweck erfüllen?!).

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

    ah::Scheduler::setup();

    resetSystem();

    mSettings.setup();
    mSettings.getPtr(mConfig);
    DPRINTLN(DBG_INFO, F("Settings valid: ") + String((mSettings.getValid()) ? F("true") : F("false")));

    mSys = new HmSystemType();
    mSys->enableDebug();
    mSys->setup(mConfig->nrf.amplifierPower, mConfig->nrf.pinIrq, mConfig->nrf.pinCe, mConfig->nrf.pinCs);
    mPayload.addListener(std::bind(&app::payloadEventListener, this, std::placeholders::_1));

    #if defined(AP_ONLY)
    mInnerLoopCb = std::bind(&app::loopStandard, this);
    #else
    mInnerLoopCb = std::bind(&app::loopWifi, this);
    #endif

    mWifi.setup(mConfig, &mTimestamp, std::bind(&app::onWifi, this, std::placeholders::_1));
    #if !defined(AP_ONLY)
    everySec(std::bind(&ahoywifi::tickWifiLoop, &mWifi));
    #endif

    mSys->addInverters(&mConfig->inst);
    mPayload.setup(this, mSys, &mStat, mConfig->nrf.maxRetransPerPyld, &mTimestamp);
    mPayload.enableSerialDebug(mConfig->serial.debug);

    if(!mSys->Radio.isChipConnected())
        DPRINTLN(DBG_WARN, F("WARNING! your NRF24 module can't be reached, check the wiring"));

    // when WiFi is in client mode, then enable mqtt broker
    #if !defined(AP_ONLY)
    mMqttActive = (mConfig->mqtt.broker[0] > 0);
    if (mMqttActive) {
        mMqtt.setup(&mConfig->mqtt, mConfig->sys.deviceName, mVersion, mSys, &mTimestamp);
        mMqtt.setSubscriptionCb(std::bind(&app::mqttSubRxCb, this, std::placeholders::_1));
    }
    #endif
    setupLed();

    mWeb.setup(this, mSys, mConfig);
    mWeb.setProtection(strlen(mConfig->sys.adminPwd) != 0);
    everySec(std::bind(&WebType::tickSecond, &mWeb));

    mApi.setup(this, mSys, mWeb.getWebSrvPtr(), mConfig);

    // Plugins
    #if defined(ENA_NOKIA) || defined(ENA_SSD1306) || defined(ENA_SH1106)
    mMonoDisplay.setup(mSys, &mTimestamp);
    everySec(std::bind(&MonoDisplayType::tickerSecond, &mMonoDisplay));
    #endif

    mPubSerial.setup(mConfig, mSys, &mTimestamp);
    every(std::bind(&PubSerialType::tick, &mPubSerial), mConfig->serial.interval);
    //everySec(std::bind(&app::tickSerial, this));
}


//-----------------------------------------------------------------------------
void app::loop(void) {
    mInnerLoopCb();
}

//-----------------------------------------------------------------------------
void app::loopWifi(void) {
    DPRINTLN(DBG_VERBOSE, F("app::loop Wifi"));

    ah::Scheduler::loop();
    yield();
}

...

//-----------------------------------------------------------------------------
void app::onWifi(bool gotIp) {
    ah::Scheduler::resetTicker();
    if (gotIp) {
        mInnerLoopCb = std::bind(&app::loopStandard, this);
        mSendTickerId = every(std::bind(&app::tickSend, this), mConfig->nrf.sendInterval);
        mMqttReconnect = true;
        once(std::bind(&app::tickNtpUpdate, this), 2);
    }
    else {
        mInnerLoopCb = std::bind(&app::loopWifi, this);
        everySec(std::bind(&ahoywifi::tickWifiLoop, &mWifi));
    }
}

//-----------------------------------------------------------------------------
void app::tickNtpUpdate(void) {
    uint32_t nxtTrig = 5;  // default: check again in 5 sec
    if (mWifi.getNtpTime()) {
        if (mMqttReconnect && mMqttActive) {
            mMqtt.connect();
            everySec(std::bind(&PubMqttType::tickerSecond, &mMqtt));
            everyMin(std::bind(&PubMqttType::tickerMinute, &mMqtt));
            uint32_t nxtTrig = mTimestamp - ((mTimestamp - 1) % 86400) + 86400; // next midnight
            if(mConfig->mqtt.rstYieldMidNight)
                onceAt(std::bind(&app::tickMidnight, this), nxtTrig);
            mMqttReconnect = false;
        }

        nxtTrig = 43200;    // check again in 12 h
    ...
}

//-----------------------------------------------------------------------------
void app::tickCalcSunrise(void) {
    ...
    if (mMqttActive)
        tickSun();
}

//-----------------------------------------------------------------------------
void app::tickIVCommunication(void) {
    ...
    if (mMqttActive)
        tickComm();
}

...

//-----------------------------------------------------------------------------
void app::tickSun(void) {
    // only used and enabled by MQTT (see setup())
    if (!mMqtt.tickerSun(mSunrise, mSunset, mConfig->sun.offsetSec, mConfig->sun.disNightCom))
        once(std::bind(&app::tickSun, this), 1);    // MQTT not connected, retry
}

//-----------------------------------------------------------------------------
void app::tickComm(void) {
    // only used and enabled by MQTT (see setup())
    if (!mMqtt.tickerComm(!mIVCommunicationOn))
        once(std::bind(&app::tickComm, this), 1);    // MQTT not connected, retry
}

pubMqtt.h, baut selbst keine Verbindung mehr auf, d.h. mEnReconnect und onWifi... komplett entfernen (NICHT aber onConnect und onDisconnect), dafür

        void connect() {
            if (!mClient.connected())
                mClient.connect();
        }

einfügen (wird in app::tickNtpUpdate aufgerufen).

Einzig bei MQTT_SERVER_UNAVAILABLE bin ich mir nicht sicher, ob da nicht ein mClient.connect(); für den reconnect eingefügt werden muss, da ja kein Wifi disconnect stattfand, sondern evtl. 'nur' die MQTT Server Verbindung abgebrochen ist.

tickerSun und tickerComm geben nur Erfolgsmeldungen zurück:

        bool tickerSun(uint32_t sunrise, uint32_t sunset, uint32_t offs, bool disNightCom) {
            if (!mClient.connected())
                return false;
            ...
            return true;
        }

        bool tickerComm(bool disabled) {
            if (!mClient.connected())
                return false;
            ...
            return true;
        }

Hoffentlich habe ich nichts übersehen, konnte erst nach Sunset testen, i.W. also Wifi connect/disconnect/reconnect.
Daher ist es vielleicht besser, das bei Sonne nochmal zu testen, bevor evtl. etwas davon in die dev.03 übernommen wird.

@knickohr
Copy link

knickohr commented Jan 9, 2023

Zum Thema WiFi hätte ich auch noch einen Vorschlag.

Ahoy kann ja schon im Setting den stärksten AP anzeigen wenn man mehrere APs mit der gleichen SSID hat. Wäre es nicht sinnvoll wenn dann auch immer mit dem stärksten AP (gleiche SSID) im Netz verbunden wird ?

Bei mir kommt es öfters vor, keine Ahnung nach welchen Kriterien, das sich die DTU NICHT mit dem stärksten verbindet. Als Beispiel, bei den Shellies gibt es eine Option wo man einstellen kann, ab wieviel dBm sich der Shelly versucht mit einem stärkeren zu verbinden.

4B01DDD5-BF65-489B-9461-7BBA285AA05D

Was auch wunderbar funktioniert 😉

@lumapu
Copy link
Owner

lumapu commented Jan 9, 2023

wow, das ist schon ein gewisser Aufwand das umzusetzen und zu verstehen. Ich hoffe ich komme die nächsten Tage dazu.

Evtl. kannst du nach dem Test es auch als pull request einreichen, dann habe ich es leichter 😉

Bisher waren deine Anpassungen immer vollständig und sehr verständlich, daher kann man mit deinem 'Rezept' die Änderungen bestimmt leicht einbauen.

@knickohr
Copy link

knickohr commented Jan 9, 2023

Mir schnattern echt die Ohren was @beegee3 da so raus läßt. Da bleibt kein Stein, ähhh Bit mehr auf dem anderen 😅

weiter so 👍 Das wird immer besser 😎

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 10, 2023

@lumapu ja, pull request wäre hier vernünftig. Programmiere in einer abgeschotteten virtuellen Umgebung, ohne direkten Github Zugang. Das muss ich erst anpassen, sorry.
Mir ist aufgefallen, dass nach einem Switch ein paar Ticker nicht wieder installiert werden (für web, display und serial).
Daher sind diese Ticker jetzt in einer eigenen Funktion, die von setup und onWifi aufgerufen wird.
(setup und onWifi zur besseren Übersicht nochmal komplett)

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

    ah::Scheduler::setup();

    resetSystem();

    mSettings.setup();
    mSettings.getPtr(mConfig);
    DPRINTLN(DBG_INFO, F("Settings valid: ") + String((mSettings.getValid()) ? F("true") : F("false")));

    mSys = new HmSystemType();
    mSys->enableDebug();
    mSys->setup(mConfig->nrf.amplifierPower, mConfig->nrf.pinIrq, mConfig->nrf.pinCe, mConfig->nrf.pinCs);
    mPayload.addListener(std::bind(&app::payloadEventListener, this, std::placeholders::_1));

    #if defined(AP_ONLY)
    mInnerLoopCb = std::bind(&app::loopStandard, this);
    #else
    mInnerLoopCb = std::bind(&app::loopWifi, this);
    #endif

    mWifi.setup(mConfig, &mTimestamp, std::bind(&app::onWifi, this, std::placeholders::_1));
    #if !defined(AP_ONLY)
    everySec(std::bind(&ahoywifi::tickWifiLoop, &mWifi));
    #endif

    mSys->addInverters(&mConfig->inst);
    mPayload.setup(this, mSys, &mStat, mConfig->nrf.maxRetransPerPyld, &mTimestamp);
    mPayload.enableSerialDebug(mConfig->serial.debug);

    if(!mSys->Radio.isChipConnected())
        DPRINTLN(DBG_WARN, F("WARNING! your NRF24 module can't be reached, check the wiring"));

    // when WiFi is in client mode, then enable mqtt broker
    #if !defined(AP_ONLY)
    mMqttActive = (mConfig->mqtt.broker[0] > 0);
    if (mMqttActive) {
        mMqtt.setup(&mConfig->mqtt, mConfig->sys.deviceName, mVersion, mSys, &mTimestamp);
        mMqtt.setSubscriptionCb(std::bind(&app::mqttSubRxCb, this, std::placeholders::_1));
    }
    #endif
    setupLed();

    mWeb.setup(this, mSys, mConfig);
    mWeb.setProtection(strlen(mConfig->sys.adminPwd) != 0);

    mApi.setup(this, mSys, mWeb.getWebSrvPtr(), mConfig);

    // Plugins
    #if defined(ENA_NOKIA) || defined(ENA_SSD1306) || defined(ENA_SH1106)
    mMonoDisplay.setup(mSys, &mTimestamp);
    #endif

    mPubSerial.setup(mConfig, mSys, &mTimestamp);

    regularTickers();
}

//-----------------------------------------------------------------------------
void app::regularTickers(void) {
    everySec(std::bind(&WebType::tickSecond, &mWeb));
    // Plugins
    #if defined(ENA_NOKIA) || defined(ENA_SSD1306) || defined(ENA_SH1106)
    everySec(std::bind(&MonoDisplayType::tickerSecond, &mMonoDisplay));
    #endif
    every(std::bind(&PubSerialType::tick, &mPubSerial), mConfig->serial.interval);
}

//-----------------------------------------------------------------------------
void app::onWifi(bool gotIp) {
    ah::Scheduler::resetTicker();
    regularTickers();   // reinstall regular tickers
    if (gotIp) {
        mInnerLoopCb = std::bind(&app::loopStandard, this);
        mSendTickerId = every(std::bind(&app::tickSend, this), mConfig->nrf.sendInterval);
        mMqttReconnect = true;
        once(std::bind(&app::tickNtpUpdate, this), 2);
    }
    else {
        mInnerLoopCb = std::bind(&app::loopWifi, this);
        everySec(std::bind(&ahoywifi::tickWifiLoop, &mWifi));
    }
}

@knickohr

Zum Thema WiFi hätte ich auch noch einen Vorschlag.

Ist mir auch aufgefallen, bei mir drängelt sich immer der schwächste AP vor 😞
Hab' aber auf die Schnelle aber keine Programmierlösung gefunden. (ESP ist jetzt bei diesem AP auf der blacklist 😄)

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 10, 2023

noch ein Zusatz:
dachte ja, wenn nur die MQTT Server Verbindung abbricht, nicht aber das Wifi, bekommt man ein MQTT_SERVER_UNAVAILABLE im pubMqtt onDisconnect. Das ist wohl nicht so, zumindest bei mir führt das zu TCP_DISCONNECTED. Daher muss dort ein connect(); für den reconnect Versuch stehen. Dann läuft das pingpong-artig solange, bis die Verbindung wieder hergestellt ist. Es dauert ca. 3s vom reconnect Versuch bis zum nächsten TCP_DISCONNECTED, was den Prozessor also nicht sehr belastet.
Mit dieser Anmerkung und der Korrektur funktioniert alles so, wie es soll - Test abgeschlossen 😁.

@stefan123t
Copy link
Collaborator

@beegee3 in app:setup und app:regularTickers stehen ja jetzt die plugin Ticker:
Also das Web Interface (UI), die drei Displays und die Serial Console PubSerialType.
Wo kommt denn das MQTT, der Prometheus und ggf. andere "Output" Plugins hin ?

@stefan123t stefan123t changed the title Wifi Handling Code Vorschlag Feature Request: Wifi Handling Code Vorschlag Jan 12, 2023
@stefan123t stefan123t added the enhancement New feature or request label Jan 12, 2023
@beegee3
Copy link
Contributor Author

beegee3 commented Jan 12, 2023

@stefan123t die MQTT Ticker werden in tickNtpUpdate bei erfolgreichem(!) NTP Update gesetzt. Das verhindert unsinnige 'timestamps' beim Publishing. (Gut, dass du nachgefragt hast, mir ist aufgefallen, dass dort noch die Zeile mMqttReconnect = false; fehlte. Ohne diese würden die Ticker zu oft gesetzt. Hab's im Code oben korrigiert.)
Zu Prometheus und JSON gibt es keine Ticker. Als reine 'Request' Abfragen liefern sie bei Bedarf die aktuellen Werte.
Andere (zukünftige) "Output" Plugin Ticker sollten entweder in regularTickers, oder (falls 'timespamps' benutzt werden) in tickNtpUpdate eingetragen werden.

@lumapu lumapu added the fixed dev fixed label Jan 16, 2023
@lumapu lumapu self-assigned this Jan 16, 2023
lumapu added a commit that referenced this issue Jan 16, 2023
fixed YieldTotal correction calculation #589
fixed serial output of power limit acknowledge #569
@lumapu
Copy link
Owner

lumapu commented Jan 16, 2023

@beegee3 heute habe ich dein Rezept in Ahoy reingebacken. Es fühlt sich sehr gut an.
Kannst du bei Gelegenheit nochmal kurz drüber schauen, ob ich gewissenhaft gearbeitet habe?
Vielen Dank an dieser Stelle nochmal 😊

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 16, 2023

@lumapu 👍 sieht sehr gut aus, Perfekte Umsetzung 😄.

Ein Ansatz zu @knickohr

Zum Thema WiFi hätte ich auch noch einen Vorschlag :

bei WiFi.begin(...) kann man auch die BSSID eines einzelnen AP angeben. Die erhält man bei einem WiFi.scanNetworks.
Zusammen mit WiFi.RSSI lässt sich so zum 'stärksten' AP verbinden.
Werde da mal ein bisschen probieren und ggfs. einen neuen 'Feature Request' aufmachen.

Dieser 'Feature Request' kann m.E. geschlossen werden.

@lumapu
Copy link
Owner

lumapu commented Jan 17, 2023

super, danke! 😎
bin gespannt.

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