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

esp-idf port in release 7.0.1? #22

Closed
gh4emb opened this issue Aug 1, 2022 · 9 comments
Closed

esp-idf port in release 7.0.1? #22

gh4emb opened this issue Aug 1, 2022 · 9 comments

Comments

@gh4emb
Copy link

gh4emb commented Aug 1, 2022

Hello Miro,

Thank you for your substantial contributions to improving embedded software development.

Did release 7.0.1 intentionally remove the espressif IDF port at port/esp-idf? I do not see any mention in the 7.0.1 release notes at https://www.state-machine.com/qpc/history.html#qpc_7_0_1, and I do not see the port/esp-idf folder in the 7.0.1 source code.

Kind regards,
Hal

@quantum-leaps
Copy link
Contributor

quantum-leaps commented Aug 1, 2022

Hi Hal,
Yes, the experimental QP/C port to ESP-IDF is missing from the latest QP/C 7.0.1 release (thanks a lot for noticing it right away).

The decision to remove that experimental port was based on the recent updates to the official QP/C port to FreeRTOS. As described in the QP/C revision history, the official FreeRTOS port now uses the FreeRTOS message queue (StaticQueue_t), instead of the QP event queue (QEQueue) combined with the FreeRTOS task notifications used previously. It turns out that the previous design had a small window for potential race conditions (due to the interplay between FreeRTOS critical section and task notifications).

Anyway, with this finding, the experimental ESP-IDF was deemed inadequate and should be "ported" similar to the new FreeRTOS port. However, the "FreeRTOS" inside ESP-IDF is significantly different from the official FreeRTOS, so the porting (of the port) is tricky and we didn't feel comfortable releasing something untested. I hope you can understand.

On a side note, the FreeRTOS kernel is by far the most difficult RTOS to work with that we have ever encountered. The current QP/C (and QP/C++) ports to FreeRTOS are an order of magnitude bigger and more complex than ports to any other RTOSes (currently embOS, ThreadX, uC/OS-2, and Zephyr). This is mostly due to the bizarre duplication of the FreeRTOS APIs (the "FromISR" stuff). This and other design aspects of FreeRTOS make it quite tedious to work with.

--MMS

@gh4emb
Copy link
Author

gh4emb commented Aug 2, 2022

Thank you for explaining. FreeRTOS certainly contains several...interesting design choices.

Would you expect the same small window for potential race conditions to exist in the ESP-IDF port in release 7.0.0?

Do you have any plans to upgrade the experimental ESP-IDF port to use the FreeRTOS message queue (StaticQueue_t)?

@quantum-leaps
Copy link
Contributor

Would you expect the same small window for potential race conditions to exist in the ESP-IDF port in release 7.0.0?

Yes, most likely. The time window is within the macro QACTIVE_EQUEUE_WAIT_() in the previous ESP-IDF port, which determines how the native QP queue should block and wait for an event. The problem is that after determining that the queue is becoming empty, the critical section must be exited to call ulTaskNotifyTake(). If in the meantime an event is posted to the queue, the task will still block (indefinitely) while an event will be pending...

Do you have any plans to upgrade the experimental ESP-IDF port to use the FreeRTOS message queue (StaticQueue_t)?

Yes, upgrading would be the best solution. However, we currently don't have a way to test the port (or even build it), and we didn't want to hold the whole release hostage to that one experimental port. It's also frustrating to maintain multiple ports of "FreeRTOS". As I described in the previous post, this takes an absolutely disproportionate amount of time and effort compared to any other RTOS kernel.

@quantum-leaps
Copy link
Contributor

quantum-leaps commented Aug 23, 2022

An experimental esp-idf port has been added to QP/C 7.1.0-beta (directory qpc/ports/esp-idf). The port has NOT been tested and it would be nice if someone interested in this port could give it a try.

@gh4emb
Copy link
Author

gh4emb commented Aug 23, 2022

Thank you! I am quite interested to give it a try. Using the experimental esp-idf port in QP v7.0.0, I have run (what I believe to be successfully) DPP in QP/C on an ESP32 development board (ESP32 DevKitC). I would use that same development board to test the QP/C 7.1.0-beta. Is there a set of tests using QSPY and QUTest that I could run to validate the port? (my apologies if such tests are documented and I have missed them. I do see qp/qpc/examples/qutest/self_test)

@quantum-leaps
Copy link
Contributor

Please note that the provided esp-idf port has been designed for QP/C 7.1.0 and will not compile against older QP. (It might not even compile as is, because this was not tried).
Regarding the examples, including QSPY, the tests are not the right place to start. This is because tests run against the "stubbed-QP", and not the "real thing".
A better way to test QSPY would be to adapt the existing DPP application to run on your ESP32 board and produce trace. You might want to look at some existing examples for FreeRTOS at qpc/examples/freertos.

@gh4emb
Copy link
Author

gh4emb commented Aug 29, 2022

Hello Miro, I (what I believe to be) successfully tested the esp-idf port in v7.1.0-beta3 on an ESP32 DevKitC development board, as an example in examples/esp-idf: gh4emb@362c8e8

Added README in this commit: gh4emb@0941027

I had added QSPY via ESP32 UART0 in an earlier project, and I could add it to the branch linked above if value-added.

@quantum-leaps
Copy link
Contributor

Thanks a lot, gh4emb! The updated qpc port for esp-idf as well as the contributed example for esp-idf have been merged into the just released QP/C 7.1.0 and pushed to github. Please check:

Please let me know if you see any problems.
--MMS

@quantum-leaps
Copy link
Contributor

Issue closed as fixed in QP/C 7.1.0.

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

No branches or pull requests

2 participants