Skip to content

stream_buffer: require batching buffers to exceed the trigger level#1396

Merged
archigup merged 1 commit into
FreeRTOS:mainfrom
officialasishkumar:fix/stream-batching-trigger-threshold
Jun 12, 2026
Merged

stream_buffer: require batching buffers to exceed the trigger level#1396
archigup merged 1 commit into
FreeRTOS:mainfrom
officialasishkumar:fix/stream-batching-trigger-threshold

Conversation

@officialasishkumar

Copy link
Copy Markdown
Contributor

Description

Batching stream buffers are documented to unblock receivers only after the buffered byte count exceeds the trigger level. Both xStreamBufferSend() and xStreamBufferSendFromISR() currently notify as soon as the count reaches that level, which wakes a blocked receiver too early and can make xStreamBufferReceive() return 0 bytes while the buffer still holds exactly trigger-level data.

This change routes both send paths through a shared helper so batching buffers require a strict greater-than check, while stream and message buffers keep their existing trigger semantics.

Test Steps

  1. Built the existing FreeRTOS/Test/CMock/stream_buffer/api harness against the updated FreeRTOS-Kernel working tree.
  2. Ran focused local batching-buffer regression cases against that harness for both the task send path and the ISR send path.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

#1375

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

@officialasishkumar officialasishkumar marked this pull request as ready for review April 4, 2026 11:39
@kstribrnAmzn

Copy link
Copy Markdown
Member

Changes look good though a rebase is needed. Please update your PR to contain the latest commits from the main branch.

I did leave a conceptual comment on #1375. We can greatly simplify code changes by having the batching buffer trigger when data equals the threshold rather than exceeding the threshold. This is probably a breaking change though so your fix here is best.

@kstribrnAmzn

kstribrnAmzn commented Apr 22, 2026

Copy link
Copy Markdown
Member

I've raised #1406 which is the counterpart to your PR. Let me know your thoughts. I'm in favor of simplify the number of conditionals required by better aligning existing stream buffer behavior - though your change is the correct one if we want to keep the existing threshold behavior.

@officialasishkumar officialasishkumar force-pushed the fix/stream-batching-trigger-threshold branch from a651081 to 4df89dc Compare April 23, 2026 20:55
@officialasishkumar

Copy link
Copy Markdown
Contributor Author

Rebased onto current main in 4df89dc.

@sonarqubecloud

Copy link
Copy Markdown

@kstribrnAmzn kstribrnAmzn force-pushed the fix/stream-batching-trigger-threshold branch from 4df89dc to d0a3bc6 Compare June 12, 2026 18:58
@kstribrnAmzn

Copy link
Copy Markdown
Member

Accepting your PR since this preserves the behavior that the user would expect per documentation. In the future, as in when the next major Kernel release is done, I'll back out this change and replace it with #1406 to reduce code.

Batching stream buffers are documented to unblock receivers only after the buffered byte count exceeds the trigger level, but both xStreamBufferSend() paths currently notify as soon as the count reaches it.

That equality case wakes the blocked receiver too early, causing xStreamBufferReceive() to return 0 bytes while the buffer still holds exactly trigger-level data. Route both task and ISR send paths through a shared trigger helper so batching buffers require a strict greater-than check while existing stream and message buffer semantics remain unchanged.

Fixes FreeRTOS#1375

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
@kstribrnAmzn kstribrnAmzn force-pushed the fix/stream-batching-trigger-threshold branch from d0a3bc6 to 3030587 Compare June 12, 2026 20:16
@sonarqubecloud

Copy link
Copy Markdown

@kstribrnAmzn

Copy link
Copy Markdown
Member

The only failing check (WIN/MSVC) was failing due to an ARM gitlab repo. This PR does not impact this. Approving.

@archigup archigup enabled auto-merge (rebase) June 12, 2026 20:25
@archigup archigup merged commit 543558b into FreeRTOS:main Jun 12, 2026
17 checks passed
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

Successfully merging this pull request may close these issues.

3 participants