Pull request for Alicat Python Library

[marinapalese]

Several functions were added to the flowmeter and flowcontroller classes. The functions added include most serial override commands, and well as a way to create and write gas mixes to the Alicat (introduced in R22 5v firmware and onward).

• Override Commands Added: Volumetric & pressure tare, reset totalizer, valve hold and cancel valve hold, lock/unlock display.
• set_gas() was edited to accommodate numerical and string inputs. This will give access the full gas list, and is the best method to set a gas mix that has been created. I did not make any efforts add additional gases to self.gases, since there's at least 60 that would need to be added and they are not often used.
• I also changed the method of checking if the gas was set correctly by directly reading register 46. This register contains the gas in bits 1-256 (Air is zero), and zero dead band in bits 512-32768. By subtracting out the enabled high bits the gas can be read accurately. The current method has the possibility of incorrectly raising errors if there is any status code on the device. Directly checking register 46 also works well with both a numerical and string gas input.
• In _set_setpoint in the FlowController class, current was change from float(line.split()[-2]) to float(line.split()[5]). Checking from the back, again, has the possibility of incorrectly calling an error if there is a status code on the device. The setpoint will always be index 5 from the front on a controller, so there is no issue. This method was not employed in set_gas() because if the device has a totalizer, the totalized flow will appear before the gas.

[patrickfuller]

Thanks for the PR! Reviewing will be a fun Friday afternoon distraction so I'll get you something back later today.

Regarding your comments on registers, I've always felt that this driver in general would be better if we moved away from using the space-separated API and relied more heavily on registers. That is, I like the path you're going down there and would be very open to more of it.

Thanks again!

[marinapalese]

This is really cool! I have a lot of little comments that I hope you find educational, and I'm happy to discuss more if needed.

Hope this helps!

Thank you for the feedback! I will work on making the changes as time allows. Thanks again!

[marinapalese]

Hi Patrick! I'm sorry this took so long.... some things happened when COVID started really affecting us and I wasn't allowed to work on this for a long time. I've dusted this off and made all the changes you requested now that I've finally been able to work and this and got all my other projects out of the way.

If you wouldn't mind taking another look, I'd really appreciate it. Happy new year!

[patrickfuller]

@marinapalese Happy new year and no worries! It's been a wild couple of years for all of us.

I'll review ASAP. Also - any chance you're able to test this driver on an array of MFCs? (We could probably string something together at our company but I figured you might have access to a nicer test setup!)

[marinapalese]

@marinapalese Happy new year and no worries! It's been a wild couple of years for all of us.

I'll review ASAP. Also - any chance you're able to test this driver on an array of MFCs? (We could probably string something together at our company but I figured you might have access to a nicer test setup!)

I have a box of controllers with all our main firmware except 4v - can't remember when that was "borrowed". Initially tested all the code for each of the firmware except that one, and then I've been testing the refactored version with our new 9v firmware.

[marinapalese]

@marinapalese hope this helps! I'm being extra nitpicky in the hope of showing off the "pythonic" way of doing things. None of my comments really matter if we can test and show this works on a few live devices, but I'm still hoping you find it helpful to read through!

Thank you so much! I can't express enough how much I appreciate you taking the time to show me the pythonic way. I did go through and make all the changes you recommended, as well as test them on some of my devices.

[patrickfuller]

I made a few tweaks and uploaded to PyPI.

@marinapalese - can you confirm it works by updating your driver (pip install --upgrade alicat) and testing on your MFCs?

@alexrudd2 - I deleted the TCP driver and added a note that we can re-add it in if needed. If we end up needing it, we'll want to follow #16 to separate out the transport (serial, TCP, etc) and protocol (the strings the Alicat expects) so we're not repeating ourselves.

Thanks again!

[alexrudd2]

@alexrudd2 - I deleted the TCP driver and added a note that we can re-add it in if needed. If we end up needing it, we'll want to follow #16 to separate out the transport (serial, TCP, etc) and protocol (the strings the Alicat expects) so we're not repeating ourselves.

Thanks again!

?!? The TCP driver was the only one I ever used! It was powering the pilot-lab. "Fortunately" we've now killed that controller and replaced it with kilo-lab which uses a P2000 to talk to the Alicats. So now we don't actually use this driver at all in production.

[patrickfuller]

Great! I left a comment in telling people to let us know if they're using the TCP driver. It's a couple hours of refactor if we want to add it back in right. Not a big deal but not a priority if it's going unused.

[marinapalese]

I made a few tweaks and uploaded to PyPI.

@marinapalese - can you confirm it works by updating your driver (pip install --upgrade alicat) and testing on your MFCs?

@alexrudd2 - I deleted the TCP driver and added a note that we can re-add it in if needed. If we end up needing it, we'll want to follow #16 to separate out the transport (serial, TCP, etc) and protocol (the strings the Alicat expects) so we're not repeating ourselves.

Thanks again!

@patrickfuller I checked it out and everything works with some small things.

• README.md lists the function get_PID() when it is actually get_pid(), causing it not to work. It works fine once I call the actual function.
• This was probably there before, but README.md also says "Tare volumetric hold." instead of "Tare volumetric flow." while describing other features.
• All versions of the firmware work great, except GP. I have mixed feelings about this since it's 10 years old now and these devices aren't circulating much. The loop type can't be changed (and register 85 doesn't exist yet) so that messes get_pid(), though set_pid() can be used if the loop isn't specified. The totalizer command is missing the dollar signs ('{addr}T\r' instead of '{addr}$$T\r' so it doesn't like this. Only GP actually uses the dollar signs, though everything else is backwards compatible. And lastly, the {addr}VE command also wasn't around yet, so GP isn't recognized to stop it from continue to create_mix() function. {addr}??m8 would work, but only for GP which is annoying.

[patrickfuller]

@marinapalese thanks for reviewing! I updated the README to handle the first two points.

Your third point is related to #14 and could be worth thinking through further. I'd recommend moving the conversation to that thread but here's a brief overview:

Ideally, FlowController would ask for the connected device's version (and maybe other properties like max flow) on initialization. The class would then select from a series of subclasses (ie. FlowControllerGP) that implement all the features available for that specific device version. This is more complicated but provides back-compatibility and modern device features without compromising on either. There are many patterns (mixins w/ protocol vs. transport subclasses, yaml config file containing protocols, etc) that could be tried out based on how unique the handling needs to be.

Also ideally, we'd read/write the registers directly for all functions. Not only does this open up more features (as you've already shown in this PR!) but it allows greater accuracy for getting/setting flow rates on more modern MFCs.

This is a full library rewrite... but this is a small library so it's not out of the question. What would be needed is 1. a detailed list of all Alicat MFC versions and their communication protocols and 2. a good hardware test system with all supported MFCs that we could build a real test suite against.

If it's something you're interested in leading or helping out with, let me know and I'd be happy to find a way to support.