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

Add support for FreeRTOS task notification arrays (IDFGH-7819) #9349

Closed
andrewteta opened this issue Jul 13, 2022 · 6 comments
Closed

Add support for FreeRTOS task notification arrays (IDFGH-7819) #9349

andrewteta opened this issue Jul 13, 2022 · 6 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@andrewteta
Copy link

The ESP-IDF FreeRTOS configuration does not support direct to task notifications at notification index > 0. This seems to only be a limitation of the FreeRTOSConfig.h file, which sets #define configTASK_NOTIFICATION_ARRAY_ENTRIES 1.

A clear and concise description of what you want to happen.
I think the cleanest solution would be to add a menuconfig option for FREERTOS_TASK_NOTIFICATION_ARRAY_ENTRIES to configure this value at compile time.

I changed the value in ~/esp/esp-idf/components/freertos/include/esp_additions/freertos/FreeRTOSConfig.h from 1 to 32 with no apparent issues.

@andrewteta andrewteta added the Type: Feature Request Feature request for IDF label Jul 13, 2022
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 13, 2022
@github-actions github-actions bot changed the title Add support for FreeRTOS task notification arrays Add support for FreeRTOS task notification arrays (IDFGH-7819) Jul 13, 2022
@0xjakob
Copy link
Collaborator

0xjakob commented Jul 18, 2022

@andrewteta I agree, this could be made configurable.

Please keep in mind, though, that according to the documentation in task.h, each notification value is 32 bits long. This means that in your case, you'll add 31 * 4 bytes * number of tasks in your system. You will fill up 1KB of memory with less than 10 FreeRTOS tasks, including the system tasks (esp_timer, esp_event, etc.).

@andrewteta
Copy link
Author

Thanks for reminding me of that. I think depending on the application, though the low overhead of direct-to-task notifications could outweigh some of the other IPC mechanisms (queue, for example). With memory consumption in mind, for example, I could get away with only 3 or 4 notifications, so it should be configurable to any value (1-32). I would like to add the feature myself, but I don't really know where to start with the menuconfig build system. I've been digging into the workings of esp-idf for a long time now, though so with some guidance I could probably make a pull request. Thanks for getting back to me!

@0xjakob
Copy link
Collaborator

0xjakob commented Jul 28, 2022

@andrewteta I think just making the option configurable should work like this: Instead of defining it to 1, you just set it to a Kconfig option, similar to the tick hook configuration in the same file you mentioned above:

#define configUSE_TICK_HOOK                             CONFIG_FREERTOS_USE_TICK_HOOK

Then you add the Kconfig option to the FreeRTOS Kconfig file, but without the CONFIG_ prefix.

The probably more difficult part is adding a unit test. If we support this option, we want to have a test that checks if increasing the size of the task notification array actually works at runtime. There might be some edge cases, too.

Anyways, a Pull Request will always be welcome. Once you submit it, just be prepared that we add additional unit tests if you haven't provided them yourself. This might take a while since it'll be holiday season soon, so fewer colleagues will be available to review and process PRs.

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new Resolution: NA Issue resolution is unavailable labels Nov 16, 2022
@TobiasUhmannVoss
Copy link

Has this been implemented? In my app I am using ESP-IDF 5.0.3 but I cannot see such an option in the config and I get a failed assertion assert failed: ulTaskGenericNotifyTake tasks.c:5533 (uxIndexToWait < 1) when trying to notify at index 1.

@0xjakob
Copy link
Collaborator

0xjakob commented Sep 11, 2023

@TobiasUhmannVoss This feature is only available from IDF version v5.1. New features are usually not backported to older releases (see our Support Periods documentation for more information). Is it possible for you to update to that version?

@TobiasUhmannVoss
Copy link

@0xjakob Thanks for the info! Until recently, I couldn't use IDF 5.1 due to some compilation problem with the new gcc and my TensorFlow Lite component but I solved my problem using an event group instead of task notifications already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

4 participants