Added ability to change task notification index for streambuffers

glemco commented 8 months ago

Description

This PR adds the modification described in https://forums.freertos.org/t/multiple-message-buffers-per-task-interferences/17663:

The code change necessary to enable your scenario should be relatively simple though - adding a member to the stream buffer structure that holds the task notification index to use with that stream buffer, and a utility API that sets that index

In short, all calls to xTaskNotify* APIs are now using the *Indexed version and a new field was added to StreamBuffer_t to store the notification value used for that buffer. Now, this value can be get and set using new functions (uxStreamBufferGetStreamBufferNotificationIndex and vStreamBufferSetStreamBufferNotificationIndex) to avoid changing the overall API.

Test Steps

Create any stream buffer, by default this will use 0 (tskDEFAULT_INDEX_TO_NOTIFY) as index.
Change the index using vStreamBufferSetStreamBufferNotificationIndex
- Of course the configuration should allow this with a proper value of configTASK_NOTIFICATION_ARRAY_ENTRIES
- The value must be changed before any task waits for the buffer, otherwise we may never wake them up
Use the buffer as normal, it should block and resume as usual
Sending a notification to the task to a different index should not wake up the task.

Checklist:

[x] I have tested my changes. No regression in existing tests.
[x] I have modified and/or added unit-tests to cover the code changes in this Pull Request.
- https://github.com/FreeRTOS/FreeRTOS/pull/1150

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov[bot] commented 8 months ago

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (4568507) 93.78% compared to head (42e5cea) 93.43%.

Files	Patch %	Lines
stream_buffer.c	36.84%	12 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #939 +/- ## ========================================== - Coverage 93.78% 93.43% -0.35% ========================================== Files 6 6 Lines 3186 3199 +13 Branches 885 890 +5 ========================================== + Hits 2988 2989 +1 - Misses 91 103 +12 Partials 107 107 ``` | [Flag](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/939/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | `93.43% <36.84%> (-0.35%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

glemco commented 8 months ago

/bot run formatting

glemco commented 8 months ago

Thanks @AniruddhaKanhere for the approval, I'm trying to figure out how to fix the failed steps, besides formatting which is trivial, I'm not quite sure why is the link verifier complaining and how to convince the coverage report that the functions are checked (or at least they should be in https://github.com/FreeRTOS/FreeRTOS/pull/1150, which obviously cannot be merged before this change in the kernel)

AniruddhaKanhere commented 7 months ago

Hey @glemco,

Apologies for the late response.

The link check issue seems to have been a transient failure. We are aware of this circular dependency between tests and the PR checks. We can ignore the failing unit tests for now.

Spell check is failing though. To fix that, you need to add the list of outlier spellings in this file.

glemco commented 7 months ago

Thanks @AniruddhaKanhere for the explanation! It seems after your merge the spellcheck is not failing any more (probably another transient failure? It didn't seem related to the code I changed) Does it mean this PR is now ready or is there anything else I could take care of?

sonarcloud[bot] commented 7 months ago

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

AniruddhaKanhere commented 7 months ago

Thank you for this PR @glemco and I apologize for the confusion with the checks. We are looking into what caused those transient failures. Maybe it was just that this branch was out of sync with the main repository.

Thank you @aggarg for the approval!

I shall be merging this PR now.

FreeRTOS / FreeRTOS-Kernel