Lib: SMF: Add return code to signal event propagation

See Discussion #83659
for information about the purpose of this change.

Modifies run actions of hierarchical state machines
to return a value indicating if the event was handled
by the run action or should be propagated up to the
parent run action. Flat state machines are not affected,
and their run action returns void.

smf_set_handled() has been removed and replaced by
this return value. smf_set_state() will not propagate
events regardless of the return value as the transition
is considered to have occurred.

Documentation, tests, samples, has been updated.
USB-C and hawkBit use SMF and have been updated to use
the new return codes.

Signed-off-by: Glenn Andrews <[email protected]>

Author: glenn-andrews

doc/releases/migration-guide-4.2.rst

@@ -324,6 +324,17 @@ ZBus
   which was previously allocated through :c:func:`k_malloc` internally. The structure must remain valid
   in memory until :c:func:`zbus_chan_rem_obs` is called.
 
+State Machine Framework
+=======================
+
+* :c:func:`smf_set_handled` has been removed.
+* State run actions now return an :c:enum:`smf_state_result` value instead of void. and the return
+  code determines if the event is propagated to parent run actions or has been handled. A run action
+  that handles the event completely should return :c:enum:`SMF_EVENT_HANDLED`, and run actions that
+  propagate handling to parent states should return :c:enum:`SMF_EVENT_PROPAGATE`.
+* Flat state machines ignore the return value; returning :c:enum:`SMF_EVENT_HANDLED`
+  would be the most technically accurate response.
+
 Modules
 *******
 

doc/services/smf/index.rst

@@ -18,10 +18,11 @@ State Creation
 
 A state is represented by three functions, where one function implements the
 Entry actions, another function implements the Run actions, and the last
-function implements the Exit actions. The prototype for these functions is as
-follows: ``void funct(void *obj)``, where the ``obj`` parameter is a user
-defined structure that has the state machine context, :c:struct:`smf_ctx`, as
-its first member. For example::
+function implements the Exit actions. The prototype for the entry and exit
+functions are as follows: ``void funct(void *obj)``, and the prototype for the
+run action is ``enum smf_state_result funct(void *obj)`` where the ``obj``
+parameter is a user defined structure that has the state machine context,
+:c:struct:`smf_ctx`, as its first member. For example::
 
    struct user_object {
       struct smf_ctx ctx;
@@ -39,6 +40,16 @@ By default, a state can have no ancestor states, resulting in a flat state
 machine. But to enable the creation of a hierarchical state machine, the
 :kconfig:option:`CONFIG_SMF_ANCESTOR_SUPPORT` option must be enabled.
 
+The return value of the run action, :c:enum:`smf_state_result` determines if the
+state machine propagates the event to parent run actions
+(:c:enum:`SMF_EVENT_PROPAGATE`) or if the event was handled by the run action
+(:c:enum:`SMF_EVENT_HANDLED`). Flat state machines do not have parent actions,
+so the return code is ignored; returning :c:enum:`SMF_EVENT_HANDLED` is
+recommended.
+
+Calling :c:func:`smf_set_state` prevents calling parent run
+actions, even if :c:enum:`SMF_EVENT_PROPAGATE` is returned.
+
 By default, the hierarchical state machines do not support initial transitions
 to child states on entering a superstate. To enable them the
 :kconfig:option:`CONFIG_SMF_INITIAL_TRANSITION` option must be enabled.
@@ -111,13 +122,6 @@ To run the state machine, the :c:func:`smf_run_state` function should be
 called in some application dependent way. An application should cease calling
 smf_run_state if it returns a non-zero value.
 
-Preventing Parent Run Actions
-=============================
-
-Calling :c:func:`smf_set_handled` prevents calling the run action of parent
-states. It is not required to call :c:func:`smf_set_handled` if the state
-calls :c:func:`smf_set_state`.
-
 State Machine Termination
 =========================
 
@@ -197,19 +201,21 @@ Code::
 	{
 		/* Do something */
 	}
-	static void s0_run(void *o)
+	static enum smf_state_result s0_run(void *o)
 	{
 		smf_set_state(SMF_CTX(&s_obj), &demo_states[S1]);
+		return SMF_EVENT_HANDLED;
 	}
 	static void s0_exit(void *o)
 	{
 		/* Do something */
 	}
 
 	/* State S1 */
-	static void s1_run(void *o)
+	static enum smf_state_result s1_run(void *o)
 	{
 		smf_set_state(SMF_CTX(&s_obj), &demo_states[S2]);
+		return SMF_EVENT_HANDLED;
 	}
 	static void s1_exit(void *o)
 	{
@@ -221,9 +227,10 @@ Code::
 	{
 		/* Do something */
 	}
-	static void s2_run(void *o)
+	static enum smf_state_result s2_run(void *o)
 	{
 		smf_set_state(SMF_CTX(&s_obj), &demo_states[S0]);
+		return SMF_EVENT_HANDLED;
 	}
 
 	/* Populate state table */
@@ -311,21 +318,24 @@ Code::
 	}
 
 	/* State S0 */
-	static void s0_run(void *o)
+	static enum smf_state_result s0_run(void *o)
 	{
 		smf_set_state(SMF_CTX(&s_obj), &demo_states[S1]);
+		return SMF_EVENT_HANDLED;
 	}
 
 	/* State S1 */
-	static void s1_run(void *o)
+	static enum smf_state_result s1_run(void *o)
 	{
 		smf_set_state(SMF_CTX(&s_obj), &demo_states[S2]);
+		return SMF_EVENT_HANDLED;
 	}
 
 	/* State S2 */
-	static void s2_run(void *o)
+	static enum smf_state_result s2_run(void *o)
 	{
 		smf_set_state(SMF_CTX(&s_obj), &demo_states[S0]);
+		return SMF_EVENT_HANDLED;
 	}
 
 	/* Populate state table */
@@ -369,7 +379,7 @@ When designing hierarchical state machines, the following should be considered:
    state. For example, the s1_exit function is called before the parent_exit
    function is called.
  - The parent_run function only executes if the child_run function does not
-   call either :c:func:`smf_set_state` or :c:func:`smf_set_handled`.
+   call either :c:func:`smf_set_state` or return :c:enum:`SMF_EVENT_HANDLED`.
 
 Event Driven State Machine Example
 **********************************
@@ -439,6 +449,7 @@ Code::
 		if (s->events & EVENT_BTN_PRESS) {
 			smf_set_state(SMF_CTX(&s_obj), &demo_states[S1]);
 		}
+		return SMF_EVENT_HANDLED;
 	}
 
 	/* State S1 */
@@ -455,6 +466,7 @@ Code::
 		if (s->events & EVENT_BTN_PRESS) {
 			smf_set_state(SMF_CTX(&s_obj), &demo_states[S0]);
 		}
+		return SMF_EVENT_HANDLED;
 	}
 
 	/* Populate state table */

include/zephyr/smf.h

@@ -32,6 +32,7 @@
  * @param _parent  State parent object or NULL
  * @param _initial State initial transition object or NULL
  */
+/* clang-format off */
 #define SMF_CREATE_STATE(_entry, _run, _exit, _parent, _initial)           \
 {                                                                          \
 	.entry   = _entry,                                                 \
@@ -40,6 +41,7 @@
 	IF_ENABLED(CONFIG_SMF_ANCESTOR_SUPPORT, (.parent = _parent,))      \
 	IF_ENABLED(CONFIG_SMF_INITIAL_TRANSITION, (.initial = _initial,))  \
 }
+/* clang-format on */
 
 /**
  * @brief Macro to cast user defined object to state machine
@@ -56,23 +58,43 @@ extern "C" {
 #include <zephyr/kernel.h>
 
 /**
- * @brief Function pointer that implements a portion of a state
+ * @brief enum for the return value of a state_execution function
+ */
+enum smf_state_result {
+	SMF_EVENT_HANDLED,
+	SMF_EVENT_PROPAGATE,
+};
+
+/**
+ * @brief Function pointer that implements a entry and exit actions
+ *        of a state
+ *
+ * @param obj pointer user defined object
+ */
+typedef void (*state_method)(void *obj);
+
+/**
+ * @brief Function pointer that implements a the run action of a state
  *
  * @param obj pointer user defined object
+ * @return If the event should be propagated to parent states or not
+ *         (Ignored when CONFIG_SMF_ANCESTOR_SUPPORT not defined)
  */
-typedef void (*state_execution)(void *obj);
+typedef enum smf_state_result (*state_execution)(void *obj);
 
 /** General state that can be used in multiple state machines. */
 struct smf_state {
 	/** Optional method that will be run when this state is entered */
-	const state_execution entry;
+	const state_method entry;
+
 	/**
 	 * Optional method that will be run repeatedly during state machine
 	 * loop.
 	 */
 	const state_execution run;
+
 	/** Optional method that will be run when this state exists */
-	const state_execution exit;
+	const state_method exit;
 #ifdef CONFIG_SMF_ANCESTOR_SUPPORT
 	/**
 	 * Optional parent state that contains common entry/run/exit
@@ -147,15 +169,6 @@ void smf_set_state(struct smf_ctx *ctx, const struct smf_state *new_state);
  */
 void smf_set_terminate(struct smf_ctx *ctx, int32_t val);
 
-/**
- * @brief Tell the SMF to stop propagating the event to ancestors. This allows
- *        HSMs to implement 'programming by difference' where substates can
- *        handle events on their own or propagate up to a common handler.
- *
- * @param ctx  State machine context
- */
-void smf_set_handled(struct smf_ctx *ctx);
-
 /**
  * @brief Runs one iteration of a state machine (including any parent states)
  *

lib/smf/smf.c

@@ -158,7 +158,11 @@ static bool smf_execute_ancestor_run_actions(struct smf_ctx *ctx)
 		ctx->executing = tmp_state;
 		/* Execute parent run action */
 		if (tmp_state->run) {
-			tmp_state->run(ctx);
+			enum smf_state_result rc = tmp_state->run(ctx);
+
+			if (rc == SMF_EVENT_HANDLED) {
+				internal->handled = true;
+			}
 			/* No need to continue if terminate was set */
 			if (internal->terminate) {
 				return true;
@@ -188,8 +192,7 @@ static bool smf_execute_all_exit_actions(struct smf_ctx *const ctx, const struct
 	struct internal_ctx *const internal = (void *)&ctx->internal;
 
 	for (const struct smf_state *to_execute = ctx->current;
-	     to_execute != NULL && to_execute != topmost;
-	     to_execute = to_execute->parent) {
+	     to_execute != NULL && to_execute != topmost; to_execute = to_execute->parent) {
 		if (to_execute->exit) {
 			to_execute->exit(ctx);
 
@@ -381,13 +384,6 @@ void smf_set_terminate(struct smf_ctx *ctx, int32_t val)
 	ctx->terminate_val = val;
 }
 
-void smf_set_handled(struct smf_ctx *ctx)
-{
-	struct internal_ctx *const internal = (void *)&ctx->internal;
-
-	internal->handled = true;
-}
-
 int32_t smf_run_state(struct smf_ctx *const ctx)
 {
 	struct internal_ctx *const internal = (void *)&ctx->internal;
@@ -406,11 +402,20 @@ int32_t smf_run_state(struct smf_ctx *const ctx)
 	ctx->executing = ctx->current;
 #endif
 
+#ifndef CONFIG_SMF_ANCESTOR_SUPPORT
 	if (ctx->current->run) {
 		ctx->current->run(ctx);
 	}
+#else
+	ctx->executing = ctx->current;
+	if (ctx->current->run) {
+		enum smf_state_result rc = ctx->current->run(ctx);
+
+		if (rc == SMF_EVENT_HANDLED) {
+			internal->handled = true;
+		}
+	}
 
-#ifdef CONFIG_SMF_ANCESTOR_SUPPORT
 	if (smf_execute_ancestor_run_actions(ctx)) {
 		return ctx->terminate_val;
 	}