rogerdahl / m5stickc-idf

ESP-IDF component to work with M5StickC without Arduino
MIT License
12 stars 5 forks source link

esp_event_loop_handle_t #1

Closed teuteuguy closed 4 years ago

teuteuguy commented 4 years ago

Simply trying to run this, i get errors that esp_event_loop_handle_t is not declared ...

teuteuguy commented 4 years ago

Googling around, it seems that the esp_event_loop_handle_t is in the 4.1 idf.

Any chance code could be versioned to support 3.1 ?

pablobacho commented 4 years ago

Hi @teuteuguy,

It uses the current stable version of ESP-IDF, which is v3.3. Unless there is a specific reason not to do so, I would recommend using that ESP-IDF version, it is a stable version and it should be backwards compatible with previous v3.x releases.

However, I'll take a look at it to see what I can do to make it work with v3.1. The event library is used to detect special button actions such as "click" and "hold", and that functionality will be lost without it. But you will still be able to read whether a button is pressed or not, and everything else should work as expected.

I'll make the changes asap and will update the issue.

teuteuguy commented 4 years ago

Hi @pablobacho,

Reason for change of ESP-IDF is it seems the packaged idf version within amazon-freertos is a v3.1xxx.

I've taken a look at the code and made the following crude changes you can see on the fork i made to your repo: https://github.com/teuteuguy/m5stickc-idf/commit/4087a6ce9710cda3d43cafe22a60a508a6ca896d

Keen to know what you think about it.

pablobacho commented 4 years ago

It is a good reason indeed...

Would you be interested in merging your contributions? I will take a closer look at them and get back to you.

I greatly appreciate your input and help.

teuteuguy commented 4 years ago

We should push for M5 to officially support such a project themselves. Am trying to get them to do that.

pablobacho commented 4 years ago

I checked out your changes. The use of a queue allows you to detect clicks and holds, and I think it is a good solution. However, they break the display timeout feature.

The upside of the event library is that it allows buttons generate events that can be consumed by multiple parts of the code, including other parts of the component and the user program at the same time. This is not true for queues, at least not without arbitration (which is what the event library does).

I am hesitant to commit to support pre-v3.3 if it means losing functionality or complicating the code-base too much. On the other hand, supporting Amazon FreeRTOS is important.

I backported the event library and packed it into the component. I branched out master into feature-backport-events. Please check it out and let me know if it works on AmazonFreeRTOS. I have been able to compile and run the example program using ESP-IDF v3.1.5 with success. There is an option in menuconfig (Components > M5Stick-C) for activating the backported libraries. The backported version is v3.3.

Would you give it a try and let me know if it works on AmazonFreeRTOS?

On a different note, if you make a PR with your changes for the LED it could be merged right away.

teuteuguy commented 4 years ago

Hi Pablo, thanks for looking into this. I'll test it ASAP. I do have some concerns about how everything is tightly interlocked. Meaning that for example button presses and display backlight in my mind are user-based business logic, and user should be free to disconnect those capabilities. Wondering if theres a way of simplifying the code to the bare minimum functionalities ? Will update as soon as I've tested this backport capability.

teuteuguy commented 4 years ago

Hi Pablo, I've tried your feature backpord on an amazon-freertos project. Works. I think it's worth merging into the master project.

teuteuguy commented 4 years ago

Any updates on being able to accept the pull requests ?

pablobacho commented 4 years ago

Great!

Opening issue about everything being tightly interlocked at #7.

Let's discuss PR #6 on its comments page.

Closing this issue. Thanks for all your contributions @teuteuguy!