samples: subsys: fs: bm_zms: add wait after clear

[MirkoCovizzi]

The bm_zms_clear function executes write operations under the hood that can be asynchronous.
The condition of termination for the operation is when the file system is set to uninitialized.
Adds a check for this condition before terminating the sample.

[github-actions]

You can find the documentation preview for this PR here.

[b-gent]

Suggested change

	The :c:func:`bm_zms_clear` function must be called to clear the storage.
	Call the :c:func:`bm_zms_clear` function to clear the storage.

[b-gent]

Suggested change

	For asynchronous storage backends, you must wait for the uninitialization to finish before re-initializing the storage system.
	For asynchronous storage backends, you must wait for the uninitialization to finish before reinitializing the storage system.

[eivindj-nordic]

Perhaps state explicitly here that it uinintializes the zms instance.

Suggested change

	Call the :c:func:`bm_zms_clear` function to clear the storage.
	Call the :c:func:`bm_zms_clear` function to clear the storage and uninitialize the ZMS instance.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[lemrey]

Something's wrong here; it cannot be that un-initialization should be checked by the application by polling an internal flag in the instance structure-- this whole library has an asynchronous interface and the uninitialization should be signalled by an event. If un-initialization hasn't completed at the time the event is sent, then it's a bug but clearly we can't solve it like this.

[MirkoCovizzi]

Something's wrong here; it cannot be that un-initialization should be checked by the application by polling an internal flag in the instance structure-- this whole library has an asynchronous interface and the uninitialization should be signalled by an event. If un-initialization hasn't completed at the time the event is sent, then it's a bug but clearly we can't solve it like this.

I agree, that requires a bit of a redesign of the ZMS API and logic. Currently, the subsystem is operating on flags when it comes to specific actions. My contribution is simply addressing the current existing implementation and how it lacks adequate documentation for its behavior.

@rghaddab Any opinions on this topic?

[lemrey]

Can you try to explain exactly what the problem is?

[MirkoCovizzi]

Can you try to explain exactly what the problem is?

Currently the problem is that we are not documenting to the user how to make sure that a clear has completed. I'm currently looking into the event system, and while there is an event for CLEAR, it is never propagated because as I just discovered, we are effectively resetting the handlers to NULL on CLEAR, before sending the event. That, instead, is a bug.

[lemrey]

there is an event for CLEAR, it is never propagated because as I just discovered, we are effectively resetting the handlers to NULL on CLEAR, before sending the event. That, instead, is a bug.

Yes, I fully agree with that. It's a bug, the _CLEAR event should be sent just like the other events. Would it be sufficient to never unset the handler? Once the library is uninitialized, API calls other than _mount() would return an error, so there would not be any other events dispatched to the handler. The only API call allowed, _mount() would overwrite it, so is there any need to set it to NULL?

[MirkoCovizzi]

there is an event for CLEAR, it is never propagated because as I just discovered, we are effectively resetting the handlers to NULL on CLEAR, before sending the event. That, instead, is a bug.

Yes, I fully agree with that. It's a bug, the _CLEAR event should be sent just like the other events. Would it be sufficient to never unset the handler? Once the library is uninitialized, API calls other than _mount() would return an error, so there would not be any other events dispatched to the handler. The only API call allowed, _mount() would overwrite it, so is there any need to set it to NULL?

Yes, that's what I planned to do too. I'm working on a PR that addresses this behavior.