Deal with prior preemption and runaway child states.

[hawesie]

This PR adds two functionalities to Concurrence to deal with error cases during execution:

1. Check for preemption before executing child states. This is necessary because a prior state may have failed to honour a preempt and thus it could be passed to us.

2. Addition of timeout on termination of child states. This is necessary if a child state refuses to preempt or terminate. The use of a timeout prevents the whole state machine freezing at that point.

[hawesie]

Hey, it would be great to get some input on this.

[jbohren]

I'm working on some of the smach backlog, but my current priority is finishing an rqt-smach gui.

[hawesie]

Would be possible to add to the development team a bit? We have a few relative smach experts in our project that might be able to verify this.

[hawesie]

Hey @jbohren any chance of adding some devs to smach so we can get PRs looked at? I could find some volunteers from my team if necessary.

[130s]

@hawesie Could you rebase to the latest indigo-devel so that we can test this PR on CI?

[hawesie]

Will do.

[130s]

Thanks for rebasing. Now CI passed, which indicates this PR might have not broken the package.
I know it's been awhile, but since you're adding new functionalities, would you mind adding testcases? Otherwise it'd be difficult in the future to avoid regression in advance. Also with lack of reviewers I'm inclined to count on the test result as a rationale for merging (I'm mainly helping release maintenance only).

[hawesie]

@130s I understand. I'll add some tests, but it might take a week or two.

[StephanHasler]

Preempting a concurrence before executing child states might help to realize a specific application behavior, but it should be the users choice if he likes to have this and not mandatory.

I think we should have a general discussion about a good preemption strategy. To me it seems that this pull request (and also the implementation of all ROS states) assumes, that the best strategy is to just leave everything as early as possible. For me the best strategy would be, to leave with as much control by the user as possible.

So the concurrence now will just leave before executing the child sates. But maybe the child states
should perform some mandatory processing for the state machine, which in no case should be skipped (e.g. cleaning up something). So it should be up to the user to decide, if this is a good point to service the preempt. The same applies for the ServiceState and the SimpleActionState, they can service the preempt and return 'preempted' before executing the service or action. But why should this always be desired by the user?
Of cource, for the 'clean up' use-case, I could also transition from the 'preempted' to one/some additional states that do the clean up. But this has several disadvantages. First, it makes the state machine more complex. Second, having general clean up nodes requires to keep track of what should be cleaned. Third, the clean up state does not get aware that he was called because of a preempt, because the calling state serviced the preempt, which stops the propagation of the preempt_requested signal. But maybe the clean up state's behavior should depend on this, e.g. return either 'preempted' or 'succeeded' in the end.