Hello Kirk,
> I have decided to take a look into this patch.
Thanks for the feedback.
> I noticed in your refactored code that CSTATE_CHOOSE_SCRIPT may now also
> change to CSTATE_FINISHED when timer is exceeded. (Before, it only
> changes to either CSTATE_START_THROTTLE or CSTATE_START_TX) But the
> change has not been indicated YET in the typedef enum part for
> CSTATE_CHOOSE_SCRIPT.
Indeed, the comment is inaccurate and need updating.
> As for the behavioral change, it is assumed that there will be 1 script
> per transaction run. After code reading, I interpreted the "modified"
> state changes below:
>
> 1. CSTATE_CHOOSE_SCRIPT may now switch CSTATE_FINISHED
> Previously, it may only switch to CSTATE_THROTTLE or CSTATE_START_TX.
Yes, I have added a shortcut there.
> 2. CSTATE_START_THROTTLE (Similar to #1) may now switch to CSTATE_FINISHED.
> Previously, it may only switch to CSTATE_THROTTLE or CSTATE_START_TX.
This was somehow already the case before the patch in some cases, but I
moved the decision after the while throttle so that it catches more cases.
> 3. CSTATE_START_TX: Transactions, once started, cannot be interrupted
> while running, and now proceeds to first command. Interrupt may only be
> allowed either after tx error or when tx run ends.
Yes, I removed all interruptions.
> 4. CSTATE_SKIP_COMMAND
> No particular change in code logic, but clarified in the typedef that this state can move forward several commands
untilit meets next commands or finishes script.
Yes.
> 5. CSTATE_END_TX: either starts over with new script (CSTATE_CHOOSE_SCRIPT) or finishes (CSTATE_FINISHED).
> Previously, it either defaults to CSTATE_START_THROTTLE when choosing a new script, or finishes.
Nope, it was already jumping between CHOOSE & FINISHED, however I
simplified the logic there to avoid doCustom to stay in one client
by always returning.
Attached is a v3, where I have updated inaccurate comments.
--
Fabien.
