Skip to content

Conversation

@embRoobin
Copy link
Contributor

#77631 introduces default non configurable values for zbus heap. This may result in insufficient heap size for application or soc,

This change :

  • introduces the option to configure the heap size, without changing the usability.
  • updates the documentation to refer to automatic heap calculation configuration when using zbus

Recreation of #89012 to avoid CI failures.

Copy link
Contributor

@rodrigopex rodrigopex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw you merged the main into your branch. You need to use rebase only. Please, apply the requested changes and let's see what the CI says.

a pool for the message subscriber for a set of channels;
* :kconfig:option:`CONFIG_ZBUS_MSG_SUBSCRIBER_NET_BUF_STATIC_DATA_SIZE` the biggest message of zbus
channels to be transported into a message buffer;
* :kconfig:option:`CONFIG_HEAP_MEM_POOL_ADD_SIZE_ZBUS` the reserved heap size for message buffer allocation;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not only for message subscribers but also for zbus as a whole.


config HEAP_MEM_POOL_ADD_SIZE_ZBUS
int
int "Zbus message subscriber heap pool size."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heap is not only for message subscribers.

Suggested change
int "Zbus message subscriber heap pool size."
int "Zbus requested heap pool size."

Curiosity only. No other subsystem in Zephyr has a prompt for adding heap size. All of them are defined internally. I see value in it, allowing the user to change that in the application configuration, since the applications will differ.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This made me interested and I found at least CONFIG_HEAP_MEM_POOL_ADD_SIZE_TLS_CRED_SHELL also implementing an configurable heap size 😄

Comment on lines 88 to 89
This configures heap memory pool available for configured net buffers via ZBUS_MSG_SUBSCRIBER_NET_BUF_POOL_SIZE.
Can be set, if more heap is required, or default heap size is too big for the soc.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make that more generic since zbus also uses heap for other means. You can comment on that to state the importance of this configuration.

@embRoobin embRoobin force-pushed the zephyr_zBusHeapConfig branch from 0c9a0cd to 7960d4a Compare May 15, 2025 04:45
@embRoobin
Copy link
Contributor Author

I saw you merged the main into your branch. You need to use rebase only. Please, apply the requested changes and let's see what the CI says.

Should be done!

rodrigopex
rodrigopex previously approved these changes May 18, 2025
@rodrigopex rodrigopex requested review from pdgendt and ubieda May 18, 2025 11:49
pdgendt
pdgendt previously approved these changes May 21, 2025
@pdgendt
Copy link
Contributor

pdgendt commented May 21, 2025

Needs a rebase on main

@pdgendt
Copy link
Contributor

pdgendt commented May 22, 2025

Please squash commits that change the same kconfig text.

embRoobin added 2 commits June 2, 2025 14:21
Zbus currently does only support default heap sizes.
The configuration option allows to decrease rom-footprint for embedded
applications. Adding the option to ZBus improves its flexibility and
while not increasing its complexity.

Add default of 0 value to compensate for previous cases where
HEAP_MEM_POOL_ADD_SIZE_ZBUS was defined as "".

Consistent naming 'Zbus'/'zbus' to 'ZBus'

Signed-off-by: Matthias Plöger <matthias.ploeger@phoenixcontact.com>
Add and exchange global heap configuration CONFIG_HEAP_MEM_POOL_SIZE
against configuration used for auto heap calculation
CONFIG_HEAP_MEM_POOL_ADD_SIZE_ZBUS.
Add CONFIG_HEAP_MEM_POOL_ADD_SIZE_ZBUS as related configuration

Clarify that HEAP is used for zbus in a whole

Consistent naming 'Zbus'/'zbus' to 'ZBus'

Signed-off-by: Matthias Plöger <matthias.ploeger@phoenixcontact.com>
@embRoobin embRoobin force-pushed the zephyr_zBusHeapConfig branch from 04b48f0 to d36a96d Compare June 2, 2025 12:33
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 2, 2025

@embRoobin embRoobin requested review from pdgendt and rodrigopex June 11, 2025 03:37
@embRoobin
Copy link
Contributor Author

@rodrigopex @ubieda
Could you please review the changes?

@danieldegrasse danieldegrasse added this to the v4.3.0 milestone Jul 9, 2025
@aescolar aescolar merged commit b42013a into zephyrproject-rtos:main Jul 19, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants