wip: Put terminal type details in one place

[silby]

Hi Ben, here's an illustration of what I mean about replacing the if termtype == 'ossi4' etc. with data structures holding the terminal details. Not tested or anything but gets the point across.

[benroy73]

Thanks! I see what you're intending here and generally agree that can be a helpful pattern.

There are some subtle nuances that make this tricky here. For instance the vt220 cancel is just a send not a sendline like the others, and the expected bytes expressions haven't been tested to work in all the possible places that you might think of reusing them if they came from a namedtuple object or separate class.

In this particular code I would like to keep it the way I have it now since I can see exactly what I'm sending and expecting at each place, and by abstracting it I would have to look in multiple places to see what was being done.

As I looked at how each of the 3 places this happens are used, they are very simple and easy to understand. In the case of last one where it verifies the prompt, it could probably even be eliminated with some testing of a way to combine the 2 different prompts in a single regex, but would make the regex harder to understand.

[silby]

Yeah I see your point regarding the minimal surface area here. I won't push for this PR further, but if this driver for the controllers ever grows more features I expect it'll want refactoring.

Simpler changes that would still make the code a bit better in my eyes would be creating an enum of terminal types instead of just comparing to strings, and using if termtype == a: elif termtype==b: instead of relying on on the only-two-terminal-types stuff.

[benroy73]

I don't really like the string comparison either and something like an enum sound good. I'll look at that more.