Updates to Pfeiffer agent. #145
Conversation
…re turned off and prints a statement to the logs. Also sets pressure value to zero if the channel was off. Prior to this, it threw an error when querying pressure values when channels are off
There was a problem hiding this comment.
Thanks for the PR Tanay, it'll be great to get that error fixed.
While this currently sets the pressure value for any non-OK state to 0, a valid reading that I expect would solve the block mismatch issue, there are a few questions that I have about the return values from the two commands you're using, and some other minor comments to address.
| if any(chan == 0 for chan in power_states): | ||
| print("Something is off!") |
There was a problem hiding this comment.
It would be more descriptive to state which channels are off here. Also, the way this currently written seems to me to imply that we always want to read out every channel all the time. Is there ever an instance where the readout system doesn't have a fully populated six pressure sensors? (There might always be on the LATR, but other labs which might use this might not have six all the time?)
Also, for the logging, it would be great if you could follow the recommendations about logging that were recently added to the docs.
| if any(state != 0 for state in gauge_states): | ||
| index = np.where(gauge_states != 0) | ||
| for j in range(len(index[0])): | ||
| channel = np.int(index[0][j]) | ||
| pressures[channel] = 0. |
There was a problem hiding this comment.
A few things here:
- What's the expected pressure value when a sensor is off that you're setting to
0.now? - Looking at the manual for comms with the Pfeiffer device, there are more states than just
0for OK, and non-zero for "Sensor switched off". State 5 seems to indicate no gauge and outputs2.0E-2for pressure. Are other states of interest? And do they return values other than2.0E-2? They could provide more information about a sensor that right now would make it appear to just be "off", when it could mean under or over range, error, no gauge, or ID error. - Rather than doing
for j in range(len(index[0])):you should use the enumerate() built-in function. But even then, you don't really need the index here, and could get away with:
for j in index[0]:
pressures[j] = 0.
np.intis deprecated, useint, though I believe thatnp.whereshould give you only integers already, since it's the indices wheregauge_states != 0, so I excluded the casting in the above example.
| @@ -136,6 +159,7 @@ def start_acq(self, session, params=None): | |||
| 'block_name': 'pressures', | |||
| 'data': {} | |||
| } | |||
| self.gauge.channel_power() | |||
There was a problem hiding this comment.
Currently this will generate a "Something is off!" log message at 2.5 Hz if conditions are met. I would suggest perhaps caching channel states in the Pfeiffer object, and logging state changes only when they change, rather than on every iteration of the loop.
There was a problem hiding this comment.
Will the txaio module accomplish this?
There was a problem hiding this comment.
Not by itself, it just replaces the print statement. You'd have to still cache the data and check if the state has changed.
| Returns: | ||
| print statement warning about channels being off | ||
| ''' | ||
| msg = 'SEN\r\n' |
There was a problem hiding this comment.
Question about this command. In the manual it looks like state 0 means "gauge cannot be switched on/off" 1 is "gauge is off" and 2 is "gauge is on". Should we be checking for state 1 for seeing if gauges are off? Or am I misunderstanding the command docs?
It now catches when the monitor channels are turned off and prints a statement to the logs. Also sets pressure value to zero if the channel was off. Prior to this, it threw an error when querying pressure values when channels are off
Description
I added an if statement that first queries the status of the channel. If the status is "on", then the agent goes and queries the pressure reading. If the channel status is off, it returns a pressure value of 0.
Motivation and Context
The agent used to crash when trying to query a pressure gauge channel that was off. This error is detailed in Issue #100
How Has This Been Tested?
I have been testing this on the software development computer at Penn. It has been running on this computer over the weekend, with no crashes, despite turning the pressure gauge channels on and off.
Types of changes
Checklist:
developbranch.