ZMS updates for v1.0.0 #510

MirkoCovizzi · 2025-11-25T18:46:28Z

This PR merges the following changes:

github-actions · 2025-11-25T18:47:43Z

You can find the documentation preview for this PR here.

peknis · 2025-11-26T07:14:02Z

doc/nrf-bm/release_notes/release_notes_changelog.rst

+     * The ``bm_zms_register`` function.
+       The event handler configuration is now done with the struct :c:struct:`bm_zms_fs_config`.
+
+   * Removed the selection of the :kconfig:option:`CONFIG_EXPERIMENTAL` Kconfig option.


Suggested change

* The ``bm_zms_register`` function.

The event handler configuration is now done with the struct :c:struct:`bm_zms_fs_config`.

* Removed the selection of the :kconfig:option:`CONFIG_EXPERIMENTAL` Kconfig option.

* The ``bm_zms_register`` function.

The event handler configuration is now done with the struct :c:struct:`bm_zms_fs_config`.

* The selection of the :kconfig:option:`CONFIG_EXPERIMENTAL` Kconfig option.

samples/subsys/fs/bm_zms/src/main.c

tejlmand · 2025-11-27T08:28:21Z

subsys/storage/bm_storage/Kconfig

+if BM_STORAGE_BACKEND_NATIVE_SIM
+
+config BM_STORAGE_BACKEND_NATIVE_SIM_ASYNC
+	bool "Asynchronous execution of storage operations"


why is there no help text ?

The prompt already gives enough information.

tejlmand · 2025-11-27T08:28:28Z

subsys/storage/bm_storage/Kconfig


+config BM_STORAGE_BACKEND_NATIVE_SIM
+	bool "Bare Metal native simulator storage backend"
+	depends on BOARD_NATIVE_SIM


why is there no help text ?

I added a clarification to this one so that it's even more obvious that it should be used only for integration tests.

* Removes `bm_zms_register` and moves the event handler configuration into `bm_zms_fs_config`. * Renames the `bm_zms_cb_t` type to `bm_zms_evt_handler_t`. Signed-off-by: Mirko Covizzi <[email protected]>

Fixes a bug where `BM_ZMS_EVT_CLEAR` events would not be propagated because the event handler was being nullified before sending the event. Signed-off-by: Mirko Covizzi <[email protected]>

* Updates the sample to wait for event notifications. * Adds a mount and clear at the beginning of the sample to reset the state of the storage partition in case of errors. Signed-off-by: Mirko Covizzi <[email protected]>

Adds native sim backend, to be used for ztests. Signed-off-by: Mirko Covizzi <[email protected]>

* Updates the ztest to include the native_sim board target. * Fixes some configuration bugs in the test suite. * Fixes a bug where `bm_zms_clear` was not waited on until completion. * Refactor to wait for notifications. * Other cleanups. Signed-off-by: Mirko Covizzi <[email protected]>

* Doxygen cleanups. * Adds notes regarding event propagation. Signed-off-by: Mirko Covizzi <[email protected]>

doc/nrf-bm/libraries/bm_zms.rst

* Adds a sections that explains how to clear the storage system. * Updates some other parts of the documentation. Signed-off-by: Mirko Covizzi <[email protected]>

Removes experimental flagging. Signed-off-by: Mirko Covizzi <[email protected]>

* Adds build only step. * Bumps RunsOn runner. Signed-off-by: Mirko Covizzi <[email protected]>

rghaddab · 2025-11-28T10:34:28Z

samples/subsys/fs/bm_zms/src/main.c

+static void wait_for_write(void)
 {
-	while (fs.ongoing_writes) {
+	while (!write_notif) {


why did you change this to rely on event handler instead of the fs structure ?
fs.ongoing_write is still a reliable variable to verify if a write is still ongoing.

#490 (review)

The user should rely on events, rather than polling some internal flag.

The sample is here to show the user all the different features of bm_zms.
The event handler is there in case the user wants to rely on it and defines a global variable to signal the write events. But that's up to the user to choose how is he going to use it.
However the internal flag is there to signal the user that the write has finished if for example the user doesn't want to implement an event_handler but wants only to make sure that all writes finished.

I think in the end it is about what we want to showcase as the preferred way to the users. For other BM related libraries and BLE services, we are using a event based approach. Thus I think it makes sense to have it event based here as well, although polling is also an option that is available. Thoughts @lemrey?

Asynchronous operation completion should be signalled using events, and polling, when supported, should be exposed via the API, for example with a _is_busy() call. Instance structures are typically opaque, and the user should not care about or rely on specific flags in internal structures to interact with a library. Providing a dedicated API for polling is better because it maintains the instance structure opaque, and abstracts the meaning "being busy" so that the user doesn't need to know which flags (or which combination of flags) represents a certain state (busy).

rghaddab · 2025-11-28T10:37:34Z

tests/subsys/fs/bm_zms/src/main.c

 	zassert_true(len == sizeof(wr_buf), "bm_zms_write failed");

-	wait_for_ongoing_writes(fs);
+	wait_for_write();


Is there a reason why you changed this behavior, relying on the event handlers instead of the fs->ongoing_writes variable ?

MirkoCovizzi self-assigned this Nov 25, 2025

github-actions bot added the doc-required PR must not be merged without tech writer approval. label Nov 25, 2025

MirkoCovizzi force-pushed the zms-release branch 3 times, most recently from 78aaa37 to f26ed91 Compare November 25, 2025 21:03

MirkoCovizzi added this to the v1.0.0 milestone Nov 25, 2025

MirkoCovizzi force-pushed the zms-release branch 8 times, most recently from b26f996 to 5391f43 Compare November 25, 2025 23:09

MirkoCovizzi marked this pull request as ready for review November 25, 2025 23:09

MirkoCovizzi requested review from a team and rghaddab as code owners November 25, 2025 23:09

peknis requested changes Nov 26, 2025

View reviewed changes

MirkoCovizzi force-pushed the zms-release branch 5 times, most recently from 5081e9b to 921a13f Compare November 26, 2025 11:23

eivindj-nordic reviewed Nov 26, 2025

View reviewed changes

samples/subsys/fs/bm_zms/src/main.c Outdated Show resolved Hide resolved

samples/subsys/fs/bm_zms/src/main.c Outdated Show resolved Hide resolved

MirkoCovizzi force-pushed the zms-release branch 3 times, most recently from ec9400e to 9f66652 Compare November 26, 2025 15:02

MirkoCovizzi force-pushed the zms-release branch from 6762082 to af73b85 Compare November 26, 2025 15:15

anhmolt approved these changes Nov 26, 2025

View reviewed changes

MirkoCovizzi force-pushed the zms-release branch from af73b85 to 7f65146 Compare November 26, 2025 16:29

peknis approved these changes Nov 27, 2025

View reviewed changes

tejlmand reviewed Nov 27, 2025

View reviewed changes

MirkoCovizzi force-pushed the zms-release branch 5 times, most recently from 674738a to 2534e48 Compare November 27, 2025 09:40

MirkoCovizzi requested a review from a team as a code owner November 27, 2025 09:55

MirkoCovizzi force-pushed the zms-release branch 2 times, most recently from a9889f6 to f0dd258 Compare November 27, 2025 12:19

MirkoCovizzi added 6 commits November 27, 2025 13:19

subsys: fs: bm_zms: remove bm_zms_register

523b3eb

* Removes `bm_zms_register` and moves the event handler configuration into `bm_zms_fs_config`. * Renames the `bm_zms_cb_t` type to `bm_zms_evt_handler_t`. Signed-off-by: Mirko Covizzi <[email protected]>

subsys: fs: bm_zms: fix clear event not propagated

f410cf8

Fixes a bug where `BM_ZMS_EVT_CLEAR` events would not be propagated because the event handler was being nullified before sending the event. Signed-off-by: Mirko Covizzi <[email protected]>

samples: subsys: fs: bm_zms: use event notifs

bc10f51

* Updates the sample to wait for event notifications. * Adds a mount and clear at the beginning of the sample to reset the state of the storage partition in case of errors. Signed-off-by: Mirko Covizzi <[email protected]>

subsys: storage: bm_storage: add native_sim backend

d91726c

Adds native sim backend, to be used for ztests. Signed-off-by: Mirko Covizzi <[email protected]>

include: bm: fs: bm_zms: cleanup and update

e2902e1

* Doxygen cleanups. * Adds notes regarding event propagation. Signed-off-by: Mirko Covizzi <[email protected]>

MirkoCovizzi force-pushed the zms-release branch from f0dd258 to 9067575 Compare November 27, 2025 12:21

MirkoCovizzi requested a review from tejlmand November 27, 2025 14:37

eivindj-nordic approved these changes Nov 27, 2025

View reviewed changes

doc/nrf-bm/libraries/bm_zms.rst Outdated Show resolved Hide resolved

MirkoCovizzi added 3 commits November 27, 2025 16:39

doc: nrf-bm: libraries: bm_zms: add bm_zms_clear section

9b536b5

* Adds a sections that explains how to clear the storage system. * Updates some other parts of the documentation. Signed-off-by: Mirko Covizzi <[email protected]>

subsys: fs: bm_zms: remove experimental flagging

818af00

Removes experimental flagging. Signed-off-by: Mirko Covizzi <[email protected]>

workflows: twister: add build only step

7f3767c

* Adds build only step. * Bumps RunsOn runner. Signed-off-by: Mirko Covizzi <[email protected]>

MirkoCovizzi force-pushed the zms-release branch from 9067575 to 7f3767c Compare November 27, 2025 15:39

jangalda-nsc approved these changes Nov 28, 2025

View reviewed changes

lemrey approved these changes Nov 28, 2025

View reviewed changes

rghaddab reviewed Nov 28, 2025

View reviewed changes

MirkoCovizzi requested a review from rghaddab November 28, 2025 12:04

ZMS updates for v1.0.0 #510

Are you sure you want to change the base?

ZMS updates for v1.0.0 #510

Uh oh!

Conversation

MirkoCovizzi commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lemrey Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

MirkoCovizzi commented Nov 25, 2025 •

edited

Loading

lemrey Nov 28, 2025 •

edited

Loading