Fix cases where the FTDI driver crashes test scripts preventing the power supply from being shut down

[Riley-King]

If the FTDI driver raises an exception, it raises a BaseException which is a root exception that catch Exception as e WILL NOT catch. This will kill every script if this occurs and the shutdown functions will not be called which has resulted in power supplies being left powered. This does not necessarily fix the power supply being left on but it does give the scripts a fighting chance.

The second commit 948f4d5 takes this a step further and removes ALL uses of BaseException as it offers no advantage over Exception unless we are trying to kill the script.

See Why is it recommended to derive from Exception instead of BaseException class in Python?.

[clint-lawrence]

This is a good move in general and it is probably O.K. to just makes this change as is. However the exception handling in the sequencer is also a mess, and catches BaseException in a few places. Which is probably worse than raising some base exceptions :(

So I wonder if we should also review the sequencer and check all the except statements also?

[clint-lawrence]

Since these are all getting touched, It's probably worth making a top level FixateError and deriving all the more specific errors from that.

[clint-lawrence]

Perhaps it grows the scope of this PR too much, but generally python exceptions will be suffixed with Error. We could rename them, keeping an alias for compatibility (but probably not worth the churn)

[Riley-King]

@clint-lawrence I don't think that the sequencer should have the except BaseException changed to use Exception because if the program were to be killed in some way (e.g; Ctrl+C, sys.exit, etc), the application would immediately crash and not run the teardown functions for the test, which could leave a power supply energized. I do agree that catching all exceptions is generally bad practice but I believe it to be justified in this case.

Ideally, Fixate would have a resource manager that uses the __enter__ and __exit__ metafunctions - this would mean that even if an exception is not caught, the cleanup function would still run. The hardest part of this would be updating existing scripts to use it, though this could be deferred to an as-needed basis provided that the underlying drivers are still exposed. It would save some of the boilerplate cleanup code used in scripts and make opening and closing resources more consistent.

[clint-lawrence]

I do agree that catching all exceptions is generally bad practice but I believe it to be justified in this case.

Perhaps, but the sequencer is still a mess. And I'm fairly sure it catches BaseException more than it should. However, I'm working of fuzzy memory, rather than rigorous analysis :)

There is one spot that might be worth checking. In this block, the sequencer catches BaseException and re-raises AbortException. I'm not sure how that then gets handled further up the call stack. It's possible changing the base class of AbortException will change how that works.

Fixate/src/fixate/sequencer.py

Line 370 in 9f42646

except BaseException as e:

[Riley-King]

BaseException being a catch-all means that changing fixate errors to Exception should not be an issue here, however control flow that depends on catching all non-fixate errors could break because they would now be catching exceptions that aren't meant for them. Given the convoluted control flow in sequencer.py, it is pretty hard to pin down any side effects of this change. The easiest fix would be to shrink the except scope to just that of non-fixate exceptions within the except block (e.g; IOError, RuntimeError, etc).