naming of settings in concrete classes

[carandraug]

Creating this issue from an email:

On 06 June 2019 10:00, Mick Phillips (@mickp) wrote:

Currently, the base Camera class exposes an ROI as a setting called 'roi'; atmcd exposes it as 'Roi'. I guess we need either:

• a naming convention for settings;
• a rule not to re-define settings in derived classes that have already been defined on their base class - instead, common settings like this could use private methods on the derived class as setters and getters.

[carandraug]

My view on Settings is that they give a more direct interface to the device. They enable configuring stuff that is unique to a device and so can't easily be included in the base class. This is the place where concrete classes have complete freedom.

Things like ROI and exposure time for cameras are not that. Indeed we already have abstract methods for them. Similarly, we don't have laser power in the settings of lasers and they work fine. As soon as we start including Settings on the base class, we are defining an interface at which point we might as well do it with instance methods.

Instead, I think base class should not create any Setting, the concrete classes should be free to do whatever makes the most sense to them. If that's repeating what they already have on instance methods, fair enough.

Also, coming up with a naming convention defeats the purpose of giving absolute freedom to the concrete classes on settings.

[mickp]

That makes sense. The Settings system is useful for things that have restricted values such as, on some cameras, binning (some will allow non-square binning, others won't) and ROI (some allow free selection of ROI, others are pinned to a corner or edge). Currently, Settings can expose these as an Enum where values are restricted. If we support these through methods instead, we need to:

• either allow the user to query allowable values;
• or return the resulting value or exception when an attempt is made to set an illegal value.

(Cockpit will also need UI adding to MicroscopeCamera to enable manipulation of ROI and Binning via methods.)

[carandraug]

We can also fallback to software instead. For example, if a camera does not support ROI, we can still do the cropping ourselves on CameraDevice._process_data.

On the topic of cockpit, I remember Ian saying something about preventing non-square binning in cockpit at all, even if microscope makes it possible.

[mickp]

Cockpit now handles non-square images correctly: as long as the camera reports the image size after the binning operation, it'll be handled and displayed correctly. I think Ian's point was that we shouldn't go out of our way to support non-square binning, not that we should prohibit it.

[carandraug]

Cockpit handles non-square images only for display. I think the pixel sizes of the data saved are wrong. I'm pretty sure his point was that non square pixels are mad and we should prevent users from ending there. But that was before you fixed the display of non-square pixels so maybe that changed now.