clips-executive: fix action timeout on nowait action

[morxa]

Switch any nowait action that is in the state SENSED-EFFECTS-WAIT to
SENSED-EFFECTS-HOLD.

This fixes a bug introduced by merge 1cf063f which would cause all nowait
actions to block and then eventually timeout.

This restores the behavior introduced with commit f486472.

[timn]

What about the domain-pending-sensed-fact facts? When are they eventually cleaned up in the case they are never observed? Couldn't they keep piling up?

What is a use case for having pending effects if they are ignored?

[morxa]

Previously, we always expected that we would always observe all effects of an action, even if it is a nowait action. With the change in f486472, we can now actually track those effects and react if an expected effect does not occur.

Use case:

1. Prepare a machine, do not wait until it is READY-AT-OUTPUT.
2. Drive to the output side
3. Machine is not yet ready, monitoring needs to decide what to do (e,g., wait or fail). For this, we need to know whether we expect the machine state to change.

I agree that this is not the best way to solve this issue, e.g., we need to clean up the pending-sensed-facts, but it served its purpose.

Anyway, this PR is actually only about changing the condition when to switch a nowait action to SENSED-EFFECTS-HOLD. As it is, the rule may not fire because the previous rule changes the state to SENSED-EFFECTS-WAIT.

[timn]

The use case helps to understand the motivation. However, it casts doubt onto f486472. It seems incomplete and maybe it should be removed and re-added when fully finished.

We need a mechanism to cleanup pending sensed effects in suitable situations, e.g., when the corresponding action no longer exists or is marked FAILED or FINAL. I'm also wondering whether such an action goes through the (mandatory) EXECUTION-SUCCEEDED state. It has to occur in any transition sequence for an action to become FINAL.

To better understand this, a minimal working example that shows the intended rule activation flow and state transitions of the action would be useful. Currently, it looks like a hack that provided some utility in some situation, but may have implications or violations of the intended state graph. Maybe something along the lines of the test scenario to show what should occur to follow the trace through an instance. Let's also consider an on-line conversation about this, could help clarify potential misunderstandings quicker.

[morxa]

So yes, the action always goes through EXECUTION-SUCCEEDED. But yes, let's move the discussion to Slack. I'll ping you next week

[morxa]

However, could we still merge this and then reconsider what the best approach is? Currently, nowait actions do not work at all in master...

[timn]

We could also revert the previous change which would make the problem not exist. With merging it exists but we ignore it. I do see the usefulness of knowing what is missing, but we need some cleanup.

To find something in between, I'd suggest you file a detailed issue explaining the problem. I'll take that into account for the review to approve it. But this way we make sure it's not lost. If we cannot find a solution in a reasonable time, we'll revert this and the previous commits.

Once you are back in Edinburgh please provide some of the info I asked for and we chat a little on how to approach this.

[timn]

Ok for now, follow up in #17.

[morxa]

Thanks for your comments!