make state transitions event-driven

[jack-oquin]

The current transitions.TRANS_TABLE is a frozenset of (old_status, new_status) pairs.

That does not seem adequate. We need a table that takes into account what event is happening. A better representation is: (prev_status, event) -> new_status.

For example, a cancel() event should have no effect when the status is already RELEASED. Otherwise it sets status to RELEASING.

[jack-oquin]

The _reconcile() methods are not single events. They merge the results of several different events from the other side of the protocol.

How should that be handled?

[jack-oquin]

The previous commit introduces a new _EventTransitions data structure with a dictionary containing {old_status: new_status} for each of the existing five event types: cancel(), free(), grant(), preempt(), and wait(). That allows more flexible state transitions. For example, preempt() is now permitted, though it has no effect, in several additional states, including NEW, RESERVED and WAITING.

The _reconcile() methods still use _validate(), which still depends on the TRANS_TABLE frozenset implementation. That is undesirable because it repeats certain event transitions, making things harder to understand and maintain.

[jack-oquin]

See also: robotics-in-concert/rocon_msgs#60.

[jack-oquin]

New transition graph documentation:

• http://farnsworth.csres.utexas.edu/docs/rocon_scheduler_requests/html/transitions.html

[jack-oquin]

Commit ee22cb0444367a4e27cfac7db0a8881d2aa88016 fixed a bug in the _reconcile() logic for #19.

There is a timing problem with the merge() logic that causes the scheduler and requester to get out of sync. They keep sending each other messages alternately with and without a previously-canceled request. Those messages bounce back an forth so quickly the two sides never have a chance to catch up with each other. See #20 for details.

[jack-oquin]

When scheduler timeouts are implemented (#7), we will need additional transitions to the terminal state without notifying the lost requester.

I dislike the event method name free(), which takes a CANCELING request to the CANCELED state. I think close() is a better name for that event.

So, we should also consider using CLOSED instead of CANCELED for the terminal state label.

I closed the pull request for robotics-in-concert/rocon_msgs#60 until we resolve these questions.

@stonier: any comments or suggestions?

[jack-oquin]

The previous commit depends on update robotics-in-concert/rocon_msgs@af874de86d80933fa935e7e3d757181f50adccc4 for robotics-in-concert/rocon_msgs#60.

[jack-oquin]

So far, I am reasonably happy with the way this turned out.

There are probably still some murky corner cases in the merge() logic. I hope that more and better test cases will uncover them.

[jack-oquin]

One more thing is missing: when a disconnected requester reconnects, the merge() of its initial message should be handled as a special-case.

• NEW and RESERVED requests should be accepted as usual.
• Requests in any other state would normally be ignored, but for a newly-connected requester they should be preempted with reason=TIMEOUT.