FlowMeter is_connected method in serial.py

[Brad-Houska]

Hi, I'm new to GitHub (I'm an intern). I'm not sure if this is an issue, but I will report it in case:

In your _isconnected method, you assert that '_flowsetpoint' is in 'device.keys' to check if connected:

                if cls.__name__ == 'FlowMeter':
                    assert c and 'flow_setpoint' not in device.keys
                elif cls.__name__ == 'FlowController':
                    assert c and 'flow_setpoint' in device.keys

However, in your FlowMeter constructor, you initialize 'setpoint' instead of '_flowsetpoint':

self.keys = ['pressure', 'temperature', 'volumetric_flow', 'mass_flow',
                     'setpoint', 'gas']

I do not see any line in your code where you could have added '_flowsetpoint' to' device.keys'. This makes me think that _isconnected will always return False for FlowController objects.

[patrickfuller]

You're right. I added in the ability for Alicats to control on either pressure or flow in 71f0fb8b1369766016efc863270721eb12a4f711, and renamed flow_setpoint to setpoint. I must have missed updating the is_connected method.

Do you want to try issuing a pull request to fix it?

[Brad-Houska]

One more thing: if you initialize a device as a FlowMeter object that has setpoint in device.keys, then _isconnected would return false.

if cls.__name__ == 'FlowMeter':
                    assert c and 'flow_setpoint' not in device.keys

Do you think the statement is necessary? Because FlowMeter can potentially contain setpoint in device.keys, if a flow controller is initialized as a FlowMeter object.

[Brad-Houska]

Yes I'll do the pull request

[patrickfuller]

I agree with the idea, but not the fix. I use is_connected to determine what type of device is connected to a specific port, and I'd like to know if it's a meter or controller.

However, the error handling could be improved. I could see more verbose error types, such as "nothing's connected" (IOError?), "something's connected but it's not Alicat" (IO/Type/custom?), and "Alicat's connected but it's not a flow controller" (custom).

Let me know if you want to tackle that one, but I think the priority in the current PR is simply switching 'flow_setpoint' to 'setpoint'.

[Brad-Houska]

Ok. For now I'll just replace 'flow_setpoint' with 'setpoint'