turgu1 / ESP-IDF-InkPlate

A porting effort to the ESP-IDF framework for the e-Radionica InkPlate software.
15 stars 9 forks source link

A fix for a new regression in stable v5.0 #16

Closed tajnymag closed 1 year ago

tajnymag commented 1 year ago

Related issue: https://github.com/espressif/esp-idf/issues/10379

I just changed %d to %ld and the compiler seems to be happy now.

EDIT: I've also changed components version number in idf_component.yml back to 0.9.8 because idf component manager complained about the version not being semantic.

turgu1 commented 1 year ago

Hello tajnymag, It is a bit weird as a change. Normally, %ld, %lu are for 64 bits values (%lld for 128 bits), but the parameters are 32 bits. Have you found the rationale for this change somewhere on the net?

tajnymag commented 1 year ago

Hi, yeah, %ld might not be the completely correct way of fixing this. However, I know what caused the regression. In the new version of Xtensa compiler, int32_t and uint32_t types are no longer defined as int but as long, so they no longer fit in %d.

Official migration doc: https://docs.espressif.com/projects/esp-idf/en/release-v5.0/esp32/migration-guides/release-5.x/gcc.html#int32-t-and-uint32-t-for-xtensa-compiler GitHub issues explanation: https://github.com/espressif/esp-idf/issues/9511#issuecomment-1226160520

turgu1 commented 1 year ago

Ok. I've just read the issue here. Your request would resolve the issue for v5.0 but will cause a bug with older versions.

To get this to work with all versions, there is a need to use "inttypes.h" content and use PRIu32 and PRIi32 in the formatting strings instead of %ld or %ud. This would be a change required to all pieces of code that must go to version 5.0

Do you want to make the change and reconduct a fix? I can do it if you can't.

tajnymag commented 1 year ago

Makes sense. I can do it tomorrow.

turgu1 commented 1 year ago

As an example, the following:

ESP_LOGI(TAG, "STA Event, Base: %08x, Event: %ld.", (unsigned int) event_base, event_id);

would become:

ESP_LOGI(TAG, "STA Event, Base: %08" PRx32 ", Event: %" PRIi32 ".", (unsigned int) event_base, event_id);

Not sure if (unsigned int) would require some change...

turgu1 commented 1 year ago

Another one: lld would be PRIi64 instead