replace AsyncMqttClient with espMqttClient

[proddy]

I've been looking into https://github.com/bertmelis/espMqttClient which is new version of asyncMqttClient from one of the maintainers. It looks more lightweight and we can probably not use async and just send out the messages immediately because of the ESP32's dual core. I'm not sure though.

looks nice. Also the current async-mqtt has a queue and processes qos, works also direct sending. The ems-esp queue is only usefull to queue before connected. But the new lib looks good. We should test it.

Originally posted by @MichaelDvP in #605 (comment)

[MichaelDvP]

Seems to work with only liitle renamings/changes:

• rename type AsyncMqttClientto espMqttClient
• rename type AsyncMqttClientDisconnectReason to espMqttClientTypes::DisconnectReason
• remove _mqttClient.setMaxTopicLength (fixed in library)
• the lib supports only single callbacks, so don't register on_connect/on_disconnect in mqtt.cpp, and call them from MqttSettingsService as emsesp::EMSESP::mqtt_.on_connect();,etc. otherwise reconnect does not work.

Codesize with the lib is 15k less, but free heap is 7k lower and maxAlloc is 8k lower. Maybe the memory consumption cn be tuned.

[proddy]

That was quick! The heap memory usage is a shame. Does this happen when the library is instantiated or after it's being used with a few publishes? One thing I saw which is worth checking out is the TLS support, which sits on top of the Wifi/Network layer. Would be good to see if that works for us, also with Ethernet.

[MichaelDvP]

The memory usage is also with mqtt disabled. I've pushed this test to my dev, you can check. It's only the esp build, the standalone needs more work for a dummy lib.

[MichaelDvP]

The memory usage is mainly the extra task with 5k stacksize, there is a option to skip the extra task an handle _mqttClient.loop() in our MqttSettings-loop. Then heap is identical with async-mqtt-client.

[MichaelDvP]

Qos2 is broken, sending a lot of PUBCOMP with increasing message id. Also leaking memory, i think the PUBCOMP are all kept in queue. I've found where the wrong PUBCOMP is generated, but removing this ends up in sending repeated PUBREL with message-id counting up and eating memory. I'll open an issue on the bertmelis repo when i knew a bit more what's wrong.

BTW: There is also mqtt integrated in IDF: https://github.com/espressif/esp-mqtt with tls and mqtt 5 support, but i think it needs a lot af changes.

[proddy]

BTW: There is also mqtt integrated in IDF: https://github.com/espressif/esp-mqtt with tls and mqtt 5 support, but i think it needs a lot af changes.

I've been itching to move to the native IDF libs for both mqtt and the webserver (see me-no-dev/ESPAsyncWebServer#1287)

[proddy]

The memory usage is mainly the extra task with 5k stacksize, there is a option to skip the extra task an handle _mqttClient.loop() in our MqttSettings-loop. Then heap is identical with async-mqtt-client.

bertmelis/espMqttClient#13 (comment)

[MichaelDvP]

I think i've fixed the qos2 bug.
In my dev it's now working with same heap usage as asyncmqtt and ~15k less flash.
I'll test now longer time if there are reconnect issues or something other, but looks good so far.

BTW: i've also added the "Apply changes" button to the manage users page. It's not perfect, not comparing any change, only checking for save/cancel from edit-user dialog.

[proddy]

nice work. Is it using asynctcp or not asynchronous anymore (i.e. calling from the loop?

[MichaelDvP]

It's not async:
Init without own task here:
https://github.com/MichaelDvP/EMS-ESP32/blob/cbc9607b26b434c0c51452efe24aff660a1e142d/lib/framework/MqttSettingsService.cpp#LL37C32-L37C32
and loop here
https://github.com/MichaelDvP/EMS-ESP32/blob/cbc9607b26b434c0c51452efe24aff660a1e142d/lib/framework/MqttSettingsService.cpp#L60

I've added a queue-size output to the espmqtt-lib and will try to only use the queue ot the library.

[MichaelDvP]

I've changed mqtt.cpp to only use the espMqttClient queue, removed the ems-esp queue and the qos resends, this is done by the lib. Sendout is much faster.
Check: MichaelDvP@b28865a

[proddy]

I've changed mqtt.cpp to only use the espMqttClient queue, removed the ems-esp queue and the qos resends, this is done by the lib. Sendout is much faster. Check: MichaelDvP@b28865a

I'll check it out. Looks promising! Should we also look at adding TLS?

[MichaelDvP]

Should we also look at adding TLS?

My normal broker is iobroker.mqtt and can not handle secure connections. I have to setup a new mosquitto to check.
I've tried to change espMqttClient to espMqttClientSecure and start with mqttClient.setInsecure();, but this does not connect to normal broker. Don't know how to configure to have both choices selectable in UI.

[proddy]

Should we also look at adding TLS?

My normal broker is iobroker.mqtt and can not handle secure connections. I have to setup a new mosquitto to check. I've tried to change espMqttClient to espMqttClientSecure and start with mqttClient.setInsecure();, but this does not connect to normal broker. Don't know how to configure to have both choices selectable in UI.

This is something we can try later then. I have mosquitto with TLS so would be easy for me to test. First though is completing the axios->alova port in the web front-end (which will save 20KB in flash)

[proddy]

Hi @MichaelDvP - shall we prepare a merge? Your dev looks and works great and I have my web re-write using Alova instead of Axios. Together these to PRs will save 17K in Flash. Shall I make a new branch called dev2 and merge my changes in first, and then together manually merge yours. We can test it together and when happy merge it back into dev?

[MichaelDvP]

Yes, i'll add the italian translations to my dev2 and make a PR if you have created your dev2.

With it-translation included my dev fits in 2M-flash.

[proddy]

Merged my alova port into dev2

before:

RAM:   [==        ]  15.9% (used 52060 bytes from 327680 bytes)
Flash: [==========]  99.1% (used 2013221 bytes from 2031616 bytes)

after:

RAM:   [==        ]  15.8% (used 51756 bytes from 327680 bytes)
Flash: [==========]  98.9% (used 2009033 bytes from 2031616 bytes)

Only saves 4188 bytes in Flash. Not as impressive as your espMqttClient

[MichaelDvP]

With the extra language we are here:

RAM:   [==        ]  16.3% (used 53300 bytes from 327680 bytes)
Flash: [==========]  99.3% (used 2016821 bytes from 2031616 bytes)

[MichaelDvP]

In the discussion about reconnect i see that tbnobody uses normal/tls connections this way:

if (config.Mqtt_Tls) {
        mqttClient = static_cast<MqttClient*>(new espMqttClientSecure);
    } else {
        mqttClient = static_cast<MqttClient*>(new espMqttClient);
    }

and configs here https://github.com/tbnobody/OpenDTU/blob/9b0d2ff25f48fd39155894351d4069fba5900efa/src/MqttSettings.cpp#L104-L126

I think this is the way to implement usecure/secure clients on demand. Should i make a test?
I actual have not secure mqtt-broker, what option should i use, setCACert, or setPreSharedKey, or another: https://www.emelis.net/espMqttClient/ We have to add the input fields in mqtt-settings.
The secure client need ~15k more flash and will not fit in symmetric 4M.

[proddy]

Yes! Adding TLS has been on our list for a while.

I would create and hardcode the certificate and use mqttClient->setCACert().

And to test I would download a local copy of mosquitto.exe, setup a config file, add the certs, and run it manually from Windows (not as a service). http://www.steves-internet-guide.com/mosquitto-tls/

BTW I was looking at tbnobody's (Thomas Basler) OpenDTU project, interesting that we followed the same path on the front-end and back-end, yet also quite different approaches to solving some problems. Many of the features between our projects are also similar (web log, api, update etc). Maybe some ideas we can exchange later. Anwyay would be good to reach out and connect with this guy.

[MichaelDvP]

I've updated my dev2 with optional Mqtt-TLS.
Haven't setup a mosquitto yet to test,
Automatic build failed due to large size, but S3 builds here and works for unsecure connection.
If you like you can check, certificate could be set in mqtt-settings as string.

[proddy]

nice! I'll check. I think for now we can hardcode a default CA and later, if needed, make it editable.

[MichaelDvP]

There is already an input field for the certificate, leave it blank to disable security. Changing the certificate needs and triggers a reboot. Certificate input is only the certificate, the begin/end lines are added in code.

[proddy]

I've setup a local Mosquitto broker configured for TLS, generated the cerst and keys and tested locally and the MQTT TLS works. But EMS-ESP fails to connect. So I hardcoded the CA into the code to see if that was the problem, but it still fails. So will need to go deeper and debug.

It's not a library issue as the example https://github.com/bertmelis/espMqttClient/blob/main/examples/tls-esp32/tls-esp32.ino works.

[MichaelDvP]

Same here, setup mosquitto on a raspberry 3b with tls, tested positive with mqtt-explorer. No connection with ems-esp. Also with hardcoded certificate (do we need \nin each line?). It's mainly the example from espMqttClient with the static_cast<> declarations, that works fine with no tsl.
I'll try with a espMqttClientSecure only, but need some time.

[proddy]

I added my test to a GH repo here: https://github.com/proddy/mqtt-test

It uses the same logic as you coded, with espMqttClientSecure and espMqttClient using the static casts and it works fine. So the problem is somewhere in the EMS-ESP code. Perhaps the order it's initiated? The hardcoded \n in the CA string is needed.

[MichaelDvP]

Played a bit with your mqtt-test, thank for making this:

• The \n is only needed for header/footer and before footer. \n at the end of each other line is optional.
• I need to add setInsecure() before setCACert, otherwise i can not connect. This skips the rootCA check and it's because most of my small devices have no access to internet. (Just to be prepared for issues, tsl does not make sense if the devices is only in local network.)
• setCACert() have to be the first config command, move to begin() and ems-esp works.
• tsl connection takes a lot of ram, with S3 no problem, but esp32 is maybe critical.

I'll update my dev2.

With S3:
Normal Mqtt:
Secure Mqtt:

[proddy]

ah, good now that it works. I'll test again. I also put the setCACert() in the begin() (before the connect callback) but it didn't work either - I guess the trick was the missing setInsecure().

A few other ideas

• make the TLS CA input box on multi-line by adding multiline to the TextField
• make a new Get endpoint which when called returns config settings from EMS-ESP. We can use this to bring back the chip type (S3, C3...) and flash size (4, 16 MB) and then use this to show/hide/disable settings, like the TLS option.

[MichaelDvP]

Sorry, i have to do some more tests, not working correctly.

[MichaelDvP]

With your mqtt-test i always get with hardcoded certificate

Connecting to MQTT...
[ 43120][E][ssl_client.cpp:37] _handle_error(): [start_ssl_client():273]: (-9984) X509 - Certificate verification failed, e.g. CRL, CA or signature check failed
[ 43123][E][WiFiClientSecure.cpp:144] connect(): start_ssl_client: -9984
Disconnected from MQTT: 7.

when removing the \n from header or footer it gives

Connecting to MQTT...
[ 24996][E][ssl_client.cpp:37] _handle_error(): [start_ssl_client():187]: (-10112) X509 - Format not recognized as 
DER or PEM
[ 24996][E][WiFiClientSecure.cpp:144] connect(): start_ssl_client: -10112
Disconnected from MQTT: 7.

so format is right, but verification failed. Seems my mosquitto installation is wrong. I thought it's working because mqtt-explorer connects with tsl and this certificate set, but if i change the certificate in mqtt-explorer it also connects. (insecure).

[proddy]

Can I help with testing?

For my mosquitto setup I used the scripts at http://www.steves-internet-guide.com/download/ssl-certificate-shell-scripts-linux/ and did

• only set the Common Name to the IP address of where the mosquitto broker is installed (192.168.1.105 in my case), all other settings I left blank
• use the same Pass Phrase on all steps
• ignored any extra challenge passwords when asked
• when creating the server.crt I didn't use an external file, but this command openssl x509 -req -in server.csr -CA ca.crt -CAkey ca.key -CAcreateserial -out server.crt -days 720 -copy_extensions copyall

and my mosquitto.conf is just

allow_anonymous true
listener 8883
cafile C:\Users\paul\OneDrive\Desktop\mosquitto\certs\ca.crt
keyfile C:\Users\paul\OneDrive\Desktop\mosquitto\certs\server.key
certfile C:\Users\paul\OneDrive\Desktop\mosquitto\certs\server.crt

And to test I used, on Windows:

C:\Program Files\mosquitto\mosquitto_pub.exe" -h 192.168.1.105 -t foo/bar --cafile certs/ca.crt -m "test message" -p 8883

[MichaelDvP]

I've made a mistake generating the certificates, now with FQDN 192.168.0.25 from the broker it works.
I'm now testing ems-esp on S3 only with options: tsl cert input insecure, hardcoded or full certificate. The cert input don't like CRLF, better generate a single line or make sure each line ends only in \n. For web i use rootCA?: string and data.rootCA !== undefined to show the input.

[MichaelDvP]

Ok, updated my dev2. TLS is only for esp32-S3, the esp32 version compiles to fit 2M partition. Certificate is striped down to pure certificate after input, just copy&paste the cert to the input field, header/footer/cr/lf/blanks are removed.
if you enter insecure to certificate input, it connects insecure.
Works now for me. Please check.

[proddy]

works fine, both with a CA and using "insecure", I'll leave it on for a few hours and monitor. Great work!

What I did find is

• leaving TLS blank but connecting to TLS server will give a wrong version error on the broker. Maybe always default to 'insecure' when it's left blank, and show it as blank in the web.
• changing the TLS in the Web does a restart, but it's not needed so removed restartNeeded in MqttSettings::update()?

I'll work on #1218 so then the TLS web input can be disabled if the firmware is not compiled for an S3.

[MichaelDvP]

Leaving blank connects to a standard broker with emsMqttClient, unsecure connects to a tsl-broker skiping the certificate. We need both options. Maybe we can check the port and use unsecure for 88xx and normal client for 18xx.
Restart is needed to use espMqttClient or espMqttClientSecure in begin(). We can change and reinitialize, but now the mqtt.cpp gets the pointer on start, this also needs update. I don't want to change to much until i get a stable connection.

[MichaelDvP]

Changed to no restart needed. Also check for port and set cert to unsecure if port>8800 and empty cert.

[proddy]

great. I've done #1218 so you can add a disabled flag to the MQTTSettings page and check for features.platform. It's in my latest PR on dev2 so probably merge first

[MichaelDvP]

Thanks, but i think the feature.platform ist not needed for the mqttSettings. Check only in .ccp file is only one single file to change if we add to other platforms and leaving rootCA undefined works.

[proddy]

if you build on platformio 5.3.0 for a 4MB ESP32, it'll show the TLS box in the Web, when the option is technically unavailable?

[MichaelDvP]

Actually the tsl-box is only shown on ESP32-S3 platform.
The rootCA string is only send on this platform and the web do not show the input fileld if rooCA is undefined.
For other chips lke a ESP32 with PSRAM we can later enable in MqttSettingsService.cpp and it will show in he web.
I tested on my MH-ET (ESP32, asymmetic partitions, platformio 6.0) and on BBQKees S3 prototype.

[proddy]

ah I see. nice!

[proddy]

happy for you to merge your dev2 into the ems-esp repo when you're ready

[proddy]

closing! A fabulous addition. Thanks again Michael for picking this one up.

[proddy]

@MichaelDvP I had to make some very minor changes to espMqttClient to allow it to compile on Linux and OSX. They don't impact the ESP platform. I'll mention this to Bert or do a minor PR on his repo later. Changes are all marked as "// modified by proddy for EMS-ESP compiling standalone" and in
MqttClient.cpp, MqttClient.h and ClientPosix.cpp

[MichaelDvP]

If you do the PR, there are also some small changes from me in the Mqtt lib.

• added getQueue() to check the size of the queue
• changed two lines in qos2 handling to fix an issue with retries if a message get lost.