smf: support initial transition to nested states

[bdunlay]

Is your enhancement proposal related to a problem? Please describe.
SMF's hierarchical state machine doesn't support nested initial state transitions. This is a useful feature to have so that child states can remain abstracted from higher level states in the state machine.

Today, the only way to transition to a child state is to update the state machine to the sub-state directly. And this is problematic because setting the state always calls the exit action, even when an exit might not be occurring. For example, transitioning to a child state from a parent state is not an exit from the parent state.

Describe the solution you'd like
I would like to be able to define initial transitions so that I can set the current state to a parent state, which will then immediately transition to the designated initial child state, all without calling the exit action on the parent state.

Describe alternatives you've considered
I considered putting a smf_set_state call in the entry action of the parent state, but this appears to call the current state's exit function before transitioning to the child state.

Additional context
Here's a visual of what I want to happen. BUTTON_PRESS from OFF state should transition to ON state, and ON state should transition immediately to START_MODIAL_INTERACTION state.

[X64X2]

Noted

[bdunlay]

Tagging maintainer @sambhurst and collaborator @keith-zephyr.

[keith-zephyr]

I think the hierarchical model can support this today. In your case a button press in the OFF state should call smf_set_state(START_MODIAL_INTERACTION). This call does the following:

• Call the the OFF exit routine
• Call the ON entry routine
• Call the START_MODIAL_INTERACTION entry

Note that if START_MODIAL_INTERACTION calls smf_set_state() for either SUPPLY_INVERSE_REACTIVE_CURRENT or SYNCHRONIZE_CARDINAL_GRAMMETER, the parent state ON is not exited. Only transitions to states that are not children of ON will trigger the exit from ON

[bdunlay]

I agree that transitions to child states from parent siblings are supported today as you describe. But the current implementation leaks the implementation details of the parent state that should could be abstracted away by the parent state. Why must OFF have to know how ON is implemented?

I can definitely make this work, but initial transitions would be nice.

[bdunlay]

I also agree with your example about transitioning between sibling states in ON. My example was about transitioning from a parent to a child. What is the expected behavior when transitioning from ON to START_MODIAL_INTERACTION?

[sambhurst]

@bdunlay This does seem like a useful feature. Will you be submitting an initial PR? I spoke to @keith-zephyr offline and he said we could add something like the following to smf_set_state()

    /* Exit current state, only if not transitioning to a child state of the current */
    if ((target->parent != ctx->current) && ctx->current-exit)
        ctx->current->exit();

You might have an issue with the child entry being called twice, once from smf_set_state() and again because it's a child state.

[bdunlay]

I'd need to spend some time digging in to understand how it can be changed to avoid improper entry/exit calls. But in the meantime, would you be open to changing the signature of the SMF_CREATE_STATE macro for HSMs to support initial child states? smf_set_state() could call itself with initial as the target whenever a state's initial member is set.

#define SMF_CREATE_STATE(_entry, _run, _exit, _parent, _initial) \
{ \
	.entry  = _entry, \
	.run    = _run,   \
	.exit   = _exit,  \
	.parent = _parent \
        .initial = _initial \ # <--- Adding an `initial` member to `struct smf_state`
}

[keith-zephyr]

I'd need to spend some time digging in to understand how it can be changed to avoid improper entry/exit calls. But in the meantime, would you be open to changing the signature of the SMF_CREATE_STATE macro for HSMs to support initial child states? smf_set_state() could call itself with initial as the target whenever a state's initial member is set.

#define SMF_CREATE_STATE(_entry, _run, _exit, _parent, _initial) \
{ \
	.entry  = _entry, \
	.run    = _run,   \
	.exit   = _exit,  \
	.parent = _parent \
        .initial = _initial \ # <--- Adding an `initial` member to `struct smf_state`
}

I'd be reluctant to add another member to struct smf_state as this will impact flash space. The USB-C subsystem requires lots of states (approximately 32 now, and more will be added).

[bdunlay]

4 bytes per state for a pointer to support initial states on a 32 bit machine, yes. I gather that your flash requirements must be tight since you're probably using this in very constrained microcontrollers for managing usb-c devices. Would consuming another 132 or say 200 bytes severely impact those systems?

I'm not sure how else initial states could be reasonably supported in first-class manner. I suppose one approach could be an optional feature, but seems a bit brutalist to have to turn that on in addition to the ancestor feature. Do you have any suggestions?

And just to clarify, maybe I didn't understand @sambhurst when he commented that "this might be a useful feature." Was that in reference to first-class support for nested initial states? Or just fixing the issue that transitioning from parent to child (or any descendant) causes exit and reentry of that same parent? Obviously the latter will have to come in either way.

[YaronShragai]

Is a potential workaround here, that in the parent state's entry function, you issue smf_set_initial() rather than smf_set_state() to get into the parent state's initial sub-state? It looks like the only difference between smf_set_initial() and smf_set_state(), effectively, is that ..._initial() does not execute the exit state and ..._state() does.

The trick is that ..._initial() would still re-execute the parent's entry state.

[YaronShragai]

I spoke to @keith-zephyr offline and he said we could add something like the following to smf_set_state()

    /* Exit current state, only if not transitioning to a child state of the current */
    if ((target->parent != ctx->current) && ctx->current-exit)
        ctx->current->exit();

A problem with this is that it only catches the issue if the target state is a direct, 1st-level child of the current state. If it's more than 1 level down, it'll still execute the exit function. There probably needs to be some for loop involving get_child_of().

[YaronShragai]

I would argue that this is not just an "enhancement", this is a bug fix. This is in the way of being able to implement HSMs that follow the generally accepted behavior of HSMs (e.g., the behavior expounded in Miro Samek's book).

[bdunlay]

@YaronShragai That was exactly my motivation behind this issue, Miro Samek's model, after having worked with his QP frameworks in the past.

As an side note, unfortunately I am no longer employed by the company I was working for when I wrote this issue, so it's very unlikely I'll be able to commit any time to this going forward. I appreciate the renewed interested however, as I believe this would really be a useful change.

[glenn-andrews]

I'd be reluctant to add another member to struct smf_state as this will impact flash space. The USB-C subsystem requires lots of states (approximately 32 now, and more will be added).

Could we do it as a config option: CONFIG_SMF_SUPPORT_INITIAL_TRANSITION or something?

[glenn-andrews]

#66753 should solve this issue.

[glenn-andrews]

#71252 fixes all issues with initial transitions. This can be closed.