dingo35
commented
1 year ago @arpiecodes First of all, love your coding style, looks like a really clean job!
I understand your struggle with the libraries, been there, done that. I understand you didn't want to include the library, but you had no choice. What I did not like is that you had to add set_credentials, so now we have an unmaintained, modified library. Still, I have no problem to do that as long as it is our last resort.
I did not understand the ethernet/wifi client remark (since we use that wifi.h library), so I fiddled around a bit with the MQTTPubSubClient library; in the zip file you find a .diff that shows how I think the MQTTPubSubClient lib could be used; the code compiles but is otherwise UNTESTED since I currently have no MQTT server running; it will not work since the callback code has to be moved and I didn't want to make the diff unreadable.
Please let me know what you think, would moving towards that lib be feasible? Pro's, cons?
arpiecodes
This pull is my second attempt to add MQTT support (see https://github.com/serkri/SmartEVSE-3/pull/69 for first attempt). It was kept to the minimum to enable MQTT to function properly this time, and only contains minor refactors where it made sense to do so.
The pull request seems to contain a lot of added code, but this is due to the addition of a lightweight MQTT client library (I simply included the whole Git repo contents as it was not possible to directly use it through pio dependencies). The standard Arduino MQTTPubSubClient library would require a Ethernet/WiFiClient, and it seems we are not using that in this project with mixed Arduino/ESP IDF framework code. If someone figures out a better way to implement it properly using standard libraries, contributions to the code are very welcome. I also edited the library to support username/password authentication.
Code has been tested on my SmartEVSE for some time already albeit in another form. "It works on my machine" applies, YMMV, some testing would probably be wise.
The MQTT support can easily be disabled again by defining
MQTT_DISABLED.PS: Swagger will need an update after this is merged to document all the new MQTT setting params. I am willing to take a stab at that when the code is accepted and merged.