Nesting depth tracking

[gjosipovic]

In the current implementations of the QHsm state machines in both QP/C and QP/C++, there is a MAX_NEST_DEPTH_ limit for the maximum nesting depth of the state machine. This limit is checked at various steps in the init and dispatch methods. However, these checks fail to capture situations where initial or regular transitions lead to nested states deeper than the limit. To address this, the nesting depth should be tracked during entry to and exit from each state.
This pull request provides a proposed solution to resolve this issue.

GJ

[quantum-leaps]

Hi Goran,
You are correct to observe that the MAX_NEST_DEPTH_ constant does not really track the state nesting level. Instead, it simply limits the size of the stack-based path[] array, which "remembers" the entry-path to the states in the entry-segment of the transitions. All the checks present in the existing code aim at ensuring that the indexes into the path[] array stay in the range. Also, the current 100% MC/DC test coverage of the QHsm implementation shows that all these checks are reachable and effective.

I think that the best course of action would be to change the name of the constant to better reflect its purpose (e.g., MAX_PATH_LEN_). I don't really see a need to track and artificially limit the state nesting. Especially, if this comes at a cost of increasing each and every instance of the state machine class (the new m_nestDepth attribute.)

Please let me know what you think.

--MMS

[gjosipovic]

Hi Miro,
Thank you for the explanation. If that was the main purpose, then yes, it does not make sense to add additional members and multiple checks. What confused me the most was the comment “maximum depth of state nesting in a QHsm (including the top level).”

MAX_PATH_LEN_ is a better name, but I think we should also clearly state in the comment that it is the application developer’s responsibility to ensure that the entry path of any transition between any two states is shorter than or equal to this limit.

Best regards,
GJ

[quantum-leaps]

Hi Goran,

MAX_PATH_LEN_ is a better name, but I think we should also clearly state in the comment that it is the application developer’s responsibility to ensure that the entry path of any transition between any two states is shorter than or equal to this limit.

Sure, a better comment is certainly in order. However, the situation is actually more subtle. The path[] array only stores the states (state handlers) that the transition "cuts" in the entry portion of the transition. So, correspondingly, the MAX_PATH_LEN_ constant represents only this limit.

On the other hand, the simplified original statement about "the state nesting depth" is justified insofar that only that many levels have been tested. The QHsm class can probably work for much deeper state nesting, but testing necessarily requires setting some (arbitrary) limit. For safety-critical systems (like in the SafeQP framework), I would rather keep the limit as the required "Condition of Use (COU)", which is needed for safety certification. I hope you understand this functional safety aspect.

--MMS

[gjosipovic]

Ok, I see what you mean. You are basically saying to the application developer that the framework will "support" the application and has been tested up to this level. If you cross it, you might still get a functional application, but the framework cannot guarantee that.
What I (wrongly) understood is that it was the framework's responsibility to stop the application if the threshold is crossed.
I’m not sure how to write a proper and succinct comment to convey this message clearly.

[quantum-leaps]

Your understanding seems correct. I would actually prefer to tell the application developer sooner rather than later (or not at all) when the state nesting level exceeds the tested threshold. But there is only so much I am willing to pay for it in terms of memory and CPU cycles. I hope this makes sense to you.

[gjosipovic]

Yes it makes perfect sense; overhead of this "fuse" would be too much for the added benefit.

[quantum-leaps]

So, to close this PR, I propose the following course of action:

1. change the name of the constant to something along the lines; TRAN_ENTRY_PATH_MAX_LEN_
2. provide a comment explaining the semantics of this constant.
3. modify the QM modeling tool to generate a warning when the state nesting levels exceed the threshold (?)

If these action items are agreeable to you, perhaps you can close the PR.
--MMS

[gjosipovic]

Based on our last messages, I think we should keep "nest depth" in the name. It feels more descriptive and better explains the purpose from the application developer's perspective (i.e., as a guideline for hierarchy design).

I've gone with MAX_TESTED_NEST_DEPTH_ in a follow-up commit, along with an updated comment trying to capture the "tested limit + developer responsibility" nuance we discussed

I realize this might still be a bit confusing in some technical senses, and there's probably no perfect name for this constant.

Regarding your suggestion: yes, the QM modeling tool should definitely issue a warning when state nesting exceeds this threshold—it would catch potential issues at design time without runtime overhead.

What do you think?
Goran