Refactor to isolate sync algorithms from platform-specific code

[jmtayloruk]

No description provided.

[ChasNelson1990]

In case you're already working on this... I think I've made a couple of comments on the pull request that we should just clarify between ourselves first.

[ChasNelson1990]

I have removed the following comment (optical_gater_server.py):

        # TODO: JT writes: UGH! Who has responsibility for the settings object? What is the distinction between settings and pog_settings?
        #                  Surely settings["frame_buffer_length"] should be in pog_settings?
        #                  Maybe hold off doing anything about this until after the refactors I believe are needed - but we should make sure this is tidied and clarified eventually

As I believe it will be dealt with as part of the refactor

[jmtayloruk]

As discussed, we will do this with a base OpticalGater object, and subclasses that implement this on specific platforms, e.g. RPiOpticalGater, FileOpticalGater, SocketOpticalGater etc.

One slight reservation I do have with that approach is that it does not seem to offer what I would consider a tidy way of substituting different future algorithms as the base class. We can make it work, but in ways that are too clunky for my liking. For example, we could conditionally import a different file implementing the OpticalGater base class. But I'll get over it.

[ChasNelson1990]

(First time using the GitHub Android app)

So, i would expect to control different underlying algorithms with user settings. So, i never implemented it, but always intended to, a adaptive_algorithm = 'cnw' flag could be passed into the existing call to the adaptive updater (and it's been designed to handle that (although it does expect different parameters it can (i think) deal with or without any and has sensible defaults)). Would this not satisfy things?

[jmtayloruk]

Given that we're doing classes (and these classes do very probably have internal state, which might be different for different algorithms), it feels right to me to be subclassing a base class, rather than having a single OpticalGater class which tests a variable to decide which code to execute. For me, the natural way to handle that scenario is with subclasses.

For the LTU implemented as a separate object, that's fine, we can subclass it. But for the OpticalGater, we've already subclassed that for platform-specific options, so there's no nice way to subclass it for specific algorithms in a way that lets us switch on the fly.

We don't need to switch on the fly when running live, so we could make it work to inherit e.g. [OpticalGater -> POGOpticalGater [or CleverNewAlgorithmOpticalGater] -> RPiOpticalGater]. However I'm pretty sure that there would be no tidy and responsible way to test by running [OpticalGater -> POGOpticalGater -> FileOpticalGater] and [OpticalGater -> CleverNewAlgorithmOpticalGater -> FileOpticalGater] alongside each other.

Does that make sense (in terms of what my desire, and my concern, is)? Might be quicker to clear up with a quick in-person discussion. It's possible this is not urgent, and just me looking to the future, but it seemed like it might be worth considering at this point, just in case it causes us to tweak how we structure things.