changes to the new home thing

[carandraug]

Commit 9c50cb943077eb074862183bfd8573a1f04c4948 adds a new need_homed property to the ABC and its documentation suggests that concrete implementations may or may not add a home method based on that return value. I think if that's the direction we want to go, we might as well have two separate classes of stages. We're indirectly querying the object about whether it has a method at which point we should make it a separate class.

The problem is that enable may trigger a stage move. The need_homed signals whether home needs to be called first. Not documented is what happens if enable is called when homing is required. I'm guessing it should fail which is new backwards incompatible behaviour. But what happens is that need_homed returns False (because that's the default in the base class) signalling the user that it will not move but then enable does make a stage move (because that's the previous behaviour).

However, I think we can do it in another way. The thing we're trying to do is know ahead of time if enable will trigger a stage move. If so, then we can simply have a enable_may_trigger_move. We don't need a separate home method, we can have enable do it just as before. Would that work?

[iandobbie]

I don't think this is the best solution. Even if the a stage needs homing to properly operate you might want to enable the stage without running the home command. If the stage has previously been homed, or if you need to run the stage in a specific way without homing. Automatically homing on enable is fundamentally a bad idea. If you have to restart cockpit but not power cycle the stage you may be able to reconnect with full functionality without rehoming.

[iandobbie]

I also don't see why adding a new device type is a required step.

[carandraug]

Even if the a stage needs homing to properly operate you might want to enable the stage without running the home command.

To do what? The stages that I worked on, the only thing they do on enable is homing.

I also don't see why adding a new device type is a required step.

It's not required but I think at this point we might as well do it. It effectively splits the stages in two types, those who can and those cannot home. Two types == two classes, might as well make it explicit by having a separate class (or Mixin). This is the sort of thing that one can't do on static typing languages.

[carandraug]

Me and @iandobbie met and talked about this in person to clarify somethings. We agreed that none of us wants to have a separate class. We also agreed that we will remove the new home() method and make clear that enable() will only home the stage if that is needed. This mean that if, for example, a frontend crashes and needs to restart and reconnect to the stage, calling enable() a second time will not cause the stage to home. We still need a new method that will tell whether calling enable is likely to make the stage move so that frontends can warn the users at startup.

[carandraug]

This is now done, together with the required adjustments to cockpit. Closing.