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

Adding complete Espressif IDF compatibility #148

Merged
merged 87 commits into from
Oct 10, 2023

Conversation

MathewHDYT
Copy link
Contributor

@MathewHDYT MathewHDYT commented Sep 13, 2023

I removed the hard coded dependency to all Arduino libraries, especially the PubSubClient.

It is now possible to pass the IMQTT_Client as a interface implementation instead and that implementation will be used to send the data instead of directly using the PubSubClient.

This modularity with the previously created IUpdater interface, allows to use even Espressif components to initalize and use the library.

It has been tested with the ESP32 and compiles with both PlatformIO and the ESP-IDF extension, for the latter CmakeLists.txt files have been added that configure which files to compile and which not to compile.

Additionally even some of the Configurations are settable via the ESP-IDF menuconfig.

Furthermore both sending and receiving data works flawlessly, even fully getting an OTA udpate over MQTT worked when using Espressif IDF.

For sending telemetry data and processing OTA examples have been added to the library, for the other features it is simply possible to copy the needed code section from the Arduino examples.

Fixes #146, fixes #149, fixes #153


Additionally as long as the libraries exist, even Arduino framworks users profit from the change, because some internal components can now directly use Espressif instead of having to rely on Arduino middleware which simply wraps the calls.

Thanks to that it was possible to improve for example the Callback_Watchdog for ESP32 users, where the memory allocations and de-allocations have been drastically reduced.

Furthermore it is also possible to use the Espressif_Updater implementation, instead of Arduino_ESP32_Updater implementation, this has been adjusted in the examples and is highly recommended, because the underlying Arduino library used by Arduino_ESP32_Updater has some fatal memory leaks and undefined behaviour.

Meaning those issues have now been resolved as well and will make the library even more stable and efficient, when using OTA.

The documentation has also been updated to reflect the possible usage with Espressif IDF. The same has been done for the internal documentation additionally making the description of some types more detailed or adding documentation where there was previously none.

I also changed the way the vectors are initialised when using the callback helper classes, especially the shared attribute update and update request callback wrappers. Those now simply forward multiple passed arguments to the std::vector constructor or assign method instead. Which allows to create the vector in more ways then previously were only the copying from the given start to the given end iterator was possible.


@imbeacon Would be nice if this could be merged as v0.12.0 and if possible it would be nice if you could release this library as a component on the ESP IDF component registry as thingsboard/ThingsBoard.

This would allow users to very easily install the library when using Espressif IDF and make ThingsBoard easily accessible to an even wider userbase.

Additionally it would be nice if you could change the description of the GitHub repository to Library to easily connect and communicate with the ThingsBoard IoT Platform, to remove the reference to Arduino because that is not required anymore.

@MathewHDYT
Copy link
Contributor Author

I additionally added more GitHub Actions that will compile the created examples on esp-idf to ensure any Pull Request or Commit does not break the internal examples folder.

@imbeacon Any progress on the merge, because you can still compile for Arduino. Therefore I would prefer simply calling the library thingsboard-sdk.

@imbeacon
Copy link
Member

Hi @MathewHDYT,

Yes, we will update the name of the repository to thingsboard-cpp-sdk, because we already have some sdk repositories and it won't be clear for users what kind of sdk. Also, now I'm updating documentations with new examples and check links to the repository. Also, after renaming I will update the Arduino libraries repo.

@ashvayka
Copy link
Member

Hi @MathewHDYT, great contribution! We have renamed the project and briefly changed the description. @imbeacon will review and merge your PR. We will also update the necessary repositories like Arduino Libraries, Platformio, etc.

@MathewHDYT
Copy link
Contributor Author

Hi @MathewHDYT, great contribution! We have renamed the project and briefly changed the description. @imbeacon will review and merge your PR. We will also update the necessary repositories like Arduino Libraries, Platformio, etc.

Thanks a lot the project is in a pretty good state right now feature wise and there currently is no library that allows to easily connect and send data to ThingsBoard when using espressif. As far as I know most people simply use the esp_mqtt client directly and hardcode the calls to ThingsBoard by hand.

So making this library solve both will definitely help a lot of people.

@imbeacon imbeacon merged commit d6d1722 into thingsboard:master Oct 10, 2023
5 of 8 checks passed
@MathewHDYT MathewHDYT deleted the feat_mqtt_interface branch October 10, 2023 09:29
@MathewHDYT
Copy link
Contributor Author

MathewHDYT commented Oct 10, 2023

@imbeacon Thanks a lot for the merge would be nice if you could update the PlattformIO and Arduino registry if possible as well.

Additionally it would be highly appreciated if you could add the project as thingsboard/thingsboard, so the same name as in PlattformIO and Arduino registry to the EspressifIDF registry. This would make using the project with Espressif IDF build tools much easier.

@MathewHDYT
Copy link
Contributor Author

@imbeacon The project has not been yet registered on the EspressifIDF registry as thingsboard/thingsboard. Would be nice if that could be done as well so that the library can be included in projects using the Espressif IDF toolchain more easily.

@imbeacon
Copy link
Member

Hi @MathewHDYT ,

I have written to Espressif and wait for response to create a ThingsBoard namespace. Then I will be able to push the component to this namespace.

@spirosbond
Copy link
Contributor

spirosbond commented Nov 7, 2023

Hey guys, jumping to v0.12.0 I have a question about using my SimCom7000 with ESP32-S2 to be used as a client for Thingsboard. Here is the code I had working with v0.11.1:

#define TINY_GSM_MODEM_SIM7000
TinyGsm        modem(SerialAT)
TinyGsmClient client(modem);
ThingsBoard tb(client);

With v.0.12, I saw the example Espressif_MQTT_Client.cpp and tried that but still doesn't compile:

// Initalize the Mqtt client instance
Espressif_MQTT_Client mqttClient;
// Initialize ThingsBoard instance with the maximum needed buffer size
ThingsBoard tb(mqttClient, MAX_MESSAGE_SIZE);

Error i get: no known conversion for argument 1 from 'TinyGsmClient' {aka 'TinyGsmSim7000::GsmClientSim7000'} to 'const Espressif_MQTT_Client&'

However, if I use the Arduino_MQTT_Client it seems to work ok:

Arduino_MQTT_Client mqttClient(client);
ThingsBoard tb(mqttClient);

Can you suggest what is the best to do here? Is this how it is supposed to be? I feel like v0.11.1 was cleaner.
Thanks for clarifying

@MathewHDYT
Copy link
Contributor Author

MathewHDYT commented Nov 7, 2023

@spirosbond Sorry for the confusion, the examples in the library all contain a README.md file, which explains which boards and Frameworks they support. In your case the second code option is most likely what you need to use, because the Espressif_MQTT_Client is only meant for users which use the Espressif Toolchain and the Espressif base and are not using Arduino.


Below you can find a more detailed description of why we changed the internals of the library:

v0.11.1 was easier to use because we always expected a Client interface to be passed to the ThingsBoard and then passed that Client to the PubSubClient. Which was then used to send and receive MQTT data.

This makes using the library with anything besides platforms and boards that support the PubSubClient impossible tough.

Therefore this hard dependency has been removed, instead the ThingsBoard construct expects a IMQTT_Client implementation, which simply has to follow the interface but allows using basically anything as the intermediate connection class.

This of course slightly increased the complexity on the user side, but in my opinion it is 100% worth it. Because currently there is no ThingsBoard library that acts as an intermediate when using the Espressif IDF Toolchain and the support for other MQTT clients like the base client from Arduino can now be easily integrated by an end-user as well which is a nice bonus.

Because of that increased flexibility I wrote some default implementation to make the library still easy to use, that's the reason why there is a Arduino_MQTT_Client, which is as the name implies meant for Arduino boards and then the Espressif_MQTT_Client, which is as the name again implies meant for the Espressif IDF toolchain.


Error i get: no known conversion for argument 1 from 'TinyGsmClient' {aka 'TinyGsmSim7000::GsmClientSim7000'} to 'const Espressif_MQTT_Client&'

The specific error you got is simply, because the Espressif_MQTT_Client does not expect any constructor arguments, because it does not use the Arduino Client interface, which you attempted to pass with the TinyGsmClient. Instead it uses the esp-mqtt client in the background which utilises a completely different network stack from Arduino.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESP32-OTA thingsboard mqtt OTA fails esp idf
4 participants