[WIP] HW interface switching revisited

[mathias-luedtke]

Based on the discussions in #184 we have prepared a new version of the interface switching.

Interface rationals:

• Conflicts between controllers (and their interfaces) can be checked in checkForConflict
• New member canStart checks if the given controller can be started (including HW interface switches) from the current state. For strict checking the switch fails if one controller cannot be started, otherwise it is filtered out.
• We assume that controllers can be stopped at any time
• After all checks are passed new member doSwitch is called with filtered lists of controllers that start or stop, but not restart
• All controller information needed for switching is contained in ControllerInfo

Alternative implementations:

• canStart was chosen in favour of the discussed canSwitch, because this simplifies the filtering and handling in general
• doSwitch only gets updates, not the the full list, this should simplify the handling of stopped interfaces
• doSwitch runs in the non-RT part, do you see any pros or cons for running it in the real-time part?

Example usage:
@ipa-fxm has prepared a gazebo plugin that uses this interface, you can find it here: https://github.com/ipa-fxm/cob_gazebo_plugins/tree/hwi_switch/cob_gazebo_ros_control (source+documentation)

What do you think?

[adolfo-rt]

I like the signature you propose for doSwitch. It's more complete than my original proposal:

virtual void doSwitch(const std::list<ControllerInfo> &start_list,
                      const std::list<ControllerInfo> &stop_list);

Having knowledge of the controllers to stop (and not only of those to start) is relevant, as one might want to take action on resources that become free. For instance, one might want to switch a joint to an idle, low-power, or safe mode.

• canStart was chosen in favour of the discussed canSwitch, because this simplifies the filtering and handling in general

I'm not so fond of canStart's signature:

virtual bool canStart(const ControllerInfo &info) const;

To me, the purpose of this method (or canSwitch, as it was before) is to verify whether doSwitch can be successfully performed, without actually switching anything. I'm expecting the method parameters to be similar. I see two downsides to the proposed signature:

• It checks controllers individually. Certain inconsistent configurations can only be detected with full knowledge of the end controller configuration. The previously discussed example of a differential transmission illustrates it. Two different controllers might want to use one joint of a differential each, but in different joint modes. Your hardware might not be able to support this.
  • It has less information than doSwitch. In particular, it has no knowledge of the controllers to stop, so it can't really check everything that could be potentially done by doSwitch.

Even if the required logic implied by canSwitch is longer, it provides stronger consistency guarantees. Also, canSwitch should be then made aware of the strictness policy.

virtual bool canSwitch(const std::list<ControllerInfo> &start_list,
                       const std::list<ControllerInfo> &stop_list,
                       int strictness) const;

• doSwitch only gets updates, not the the full list, this should simplify the handling of stopped interfaces

OK, sounds reasonable.

• doSwitch runs in the non-RT part, do you see any pros or cons for running it in the real-time part?

The entire switchControllers method runs in non real-time, so it should be OK.

[mathias-luedtke]

I see your point with inter-controller mode dependencies, but I came to the conclusion that those are rare cases and can be handled in checkForConflict as well.

The main issue here is the best-effort switching.
To really support this inter-controller checking with best-effort, we need a rather complicated interface:

virtual bool canSwitch( const std::list<ControllerInfo> &is_running,
                        const std::list<ControllerInfo> &to_start,
                        const std::list<ControllerInfo> &to_stop,
                        std::list<ControllerInfo> &cannot_start,
                        std::list<ControllerInfo> &cannot_stop,
                        int strictness ) const;

In addition start_request_ and stop_request_ must be updated based on the result, which introduces multiple iterations on all of these lists.
Furthermore the controller (!) switching logic would be partially offloaded to this canSwitch function that is maintained by somebody else.
In particular the RobotHW implementor must pay attention to:

• which controllers are going to be restarted (no switching needed)
• which controllers to omit if multiple options may apply

I prefer the simpler interface, because it is less error-prone.
It is not full-featured, but should be sufficient for most cases.

We have a couple of options now:

1. canStart with best-effort; strict inter-controller checks in checkForConflict
2. canStart with best-effort; strict inter-controller checks in checkForConflict-like member (same interface, other name for semantic reasons)
3. strict canSwitch(start_list, stop_list)
4. full-fledged canSwitch (is_running, to_start, to_stop, cannot_start, cannot_stop, strictness)
5. any more?

If you really want to go for the 4th option, you are more than welcome to provide a working implementation.

I started to write one, but the effort didn't justify the benefit for me.
I guess it would be easier to rewrite the ControllerManager from scratch with proper look-up tables and interfaces..

[fmessmer]

Anything new here?
Which API are we going to use eventually?

[adolfo-rt]

I'll give some feedback before the end of the week.

[adolfo-rt]

Initial disclaimer: I expect to have slow response times until mid-April, but will try to stay in the loop as much as possible. Development efforts from my side will likely have to wait until this date.

By looking at the switchController code again, I realized that checkForConflict does not take the strictness parameter into account, it's actually always strict. If it did, the complications raised by @ipa-mdl in his previous post would also apply to it.

So, we can say from the existing implementation that the strictness parameter only applies to controller names and not resources. If now, for consistency's sake, we apply the same reasoning to the mode switching code, then the problem becomes much more tractable. I'm sorry for the noise introduced by trying to fit in the strictness parameter, I just hadn't made the above simple realization.

If canSwitch is strict, I believe we could then get away with these signatures:

virtual bool canSwitch(const std::list<ControllerInfo> &start_list,
                       const std::list<ControllerInfo> &stop_list) const;

virtual void doSwitch(const std::list<ControllerInfo> &start_list,
                      const std::list<ControllerInfo> &stop_list);

What do you think?.

[mathias-luedtke]

We are fine with strict canSwitch, I will issue a new PR with this interface as soon as I can :)
One last question: Should restarted controllers be filtered out? Otherwise the RobotHW implementation needs to take care.

[adolfo-rt]

One last question: Should restarted controllers be filtered out? Otherwise the RobotHW implementation needs to take care.

Yes, why not. Having it in a single place instead of in every specialized RobotHW will save code. Anyway, if you see some inconvenience along the way you can defer the filtering to RobotHW. Switching a resource to the mode it's currently on should not be hard to handle.

Thanks for offering to prepare the PR. I greatly appreciate the effort you're putting into this.

[fmessmer]

I guess this PR can be closed, too, as #200 is merged!