Azure / azure-iot-middleware-freertos

Azure IoT Middleware for FreeRTOS
https://azure.github.io/azure-iot-middleware-freertos/
MIT License
79 stars 25 forks source link

Request for compatibility with ESP-IDF v5.1 #301

Closed wreyford closed 1 year ago

wreyford commented 1 year ago

Our project uses ESP-IDF 5.1 Would like to be able to just clone the repository, but has required hundreds of hacks to the code.

Of note: #include "freertos/FreeRTOS.h" #include "freertos/task.h"

instead of: #include "FreeRTOS.h" #include "task.h"

Hundreds of format errors with logging eg.: AZLogInfo( ( "%.*s", az_span_size( xMessage ), az_span_ptr( xMessage ) ) ); reports: Description Resource Path Location Type field precision specifier '.*' expects argument of type 'int', but argument 6 has type 'int32_t' {aka 'long int'} [-Werror=format=] azure_iot.c /LoadAssist_IoT/libs/azure-iot-middleware-freertos/source line 39 C/C++ Problem casting to an expected (int)az_span_size solves that.

I've been slowly but surely fixing all these errors, and hopefully will eventually be able to compile and use it. The only problem is, I will have to repeatedly do this every time you update your repository, so a s to keep up to date with your latest innovations.

This is just a simple request, but also required to keep at pace with Espressif's latest ESP-IDF innovations.

ewertons commented 1 year ago

Hi @wreyford , We have a PR in progress to make this library compatible with ESP-IDF v5.0 as well. Hopefully it will suffice to make it compatible with ESP-IDF v5.1 as well. We will not support past ESP-IDF v5.0 yet because that is the latest stable version published by ESPRESSIF.

ewertons commented 1 year ago

Hi @wreyford , this PR (#302) has been merged. If you could share your experience with us, if it addresses your compilation issues, we would appreciate it.

wreyford commented 1 year ago

Many hundreds of changes required. I started by moving trimming the not required functionality of all the other STM etc support, resulting in many cmake changes to support new ../../../ depth.

Besides that, here are the major impactors...I hope I remember them all.

Some of the changes I was required to make to get it to compile with v5.1: eg. in sample_azure_iot_pnp.c: #include "FreeRTOS.h" to #include "freertos/FreeRTOS.h" #include "freertos/task.h" This was required in all the classes, also in the libs. I could not figure out a way to not cause this breaking change using cmake, but sure you will get it right, so we don't have to break the code for the other supported architectures.

LogInfo required major changes for v5.1 %d no longer will compile with uint32_t, eg: LogInfo( ( "Successfully sent command response %d", ulResponseStatus ) ); need to be changed to LogInfo( ( "Successfully sent command response %lu", ulResponseStatus ) );

another: LogDebug( ( "Property document payload : %.*s \r\n", pxMessage->ulPayloadLength, ( const char * ) pxMessage->pvMessagePayload ) ); I couldn't figure a fix, so I just cast it: LogDebug( ( "Property document payload : %.*s \r\n", (int)pxMessage->ulPayloadLength, ( const char * ) pxMessage->pvMessagePayload ) );

All semaphore types need to be renamed: eg. in demos\projects\ESPRESSIF\esp32\main\azure_iot_freertos_esp32_main.c static xSemaphoreHandle s_semph_get_ip_addrs; to static SemaphoreHandle_t s_semph_get_ip_addrs;

Even better yet, could you please support this code base of yours as a component, that can be pulled in from the components registry: https://components.espressif.com/ of espressif.

Thx a lot for all your efforts.

wreyford commented 1 year ago

I see you changed azure_iot.c as follows for AZLogError (150) AZLogError( ( "az_base64_decode failed: core error=0x%08x", ( unsigned int ) xCoreResult ) ); and I changed as follows: AZLogError( ( "az_base64_decode failed: core error=0x%08lx", xCoreResult ) );

I suppose it doesn't matter.

ewertons commented 1 year ago

Yes, all the logging has been addressed as you pointed above.

For now we do not have updates in regards to the relative paths in the freertos includes. We will discuss the possibility of making this library an ESP-IDF component. We will update this again in a few days.

Thank you for your feedback, @wreyford !

ewertons commented 1 year ago

Hi @wreyford , we have decided to keep the design of the freertos include paths as it is today, without the "freertos/" relative path. In your CMakeLists.txt you can adjust to include the relative include paths, as we have done here.

ewertons commented 1 year ago

We will close this issue as resolved, but please feel free to reopen it or open a new one if you would like any follow up. Thanks, Azure IoT SDK Team