Better defaults and better error reporting for incorrect use of state machine

[philipstarkey]

Original report (archived issue) by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

---

Generally it would be good to review the state machine API and consider ways of setting default arguments for the case where they're not relevant, for example:

• create_worker could set primary_worker automatically if primary_worker is not None. That way if there is a single worker, it would already be primary, and if there are multiple workers, the first one created will be primary unless the user specifies otherwise.

• queue_work could default to using the primary worker if none is specified (this would require changing it to be a keyword argument, but still accepting two positional arguments for backward compat).

• Some error checking in tab_base_classes.mainloop() regarding workers, so that you get more informative errors than the following (this I suspect is the result of not setting self.primary_worker, meaning it is None):

The mainloop should be read with a defensive eye to think what could go wrong and how we might provide good defaults and better error messages to minimise mistakes and confusion.

This would make writing new device classes a smoother experience.

[philipstarkey]

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).

---

We should decide whether the concept of "primary" and "secondary" workers should migrate to Tab (from their existing home in DeviceTab). I'm personally leaning towards keeping it in DeviceTab, in which case a solution would be to reimplement create_worker in DeviceTab with the behaviour you describe (rather than relocating primary_worker to Tab). We could also ensure workers get added to secondary_workers if they are not the primary_worker, however I think that there may be instances where you might want to create a worker and not have it managed by DeviceTab methods so force adding workers to secondary_workers is probably a bad idea.

I think if we automatically set primary_worker then there probably isn't a need to mangle queue_work to change it to accept the worker name as a keyword argument. Of course, up-to-date documentation would go a long way to dealing with these kinds of problems!

More error checking sounds good. I think if we type check worker_process, worker_function, worker_args, and worker_kwargs inside the generator loop (here) then that pretty much covers the common things that will go wrong (anything else going wrong would be the result of pretty severe hacking on the part of the user I think).