Lakeshore Model 372: include heater submodules into snapshot, don't snapshot blocking_t parameter, add output parameter to heaters

[ThorvaldLarsen]

• include heater submodules into snapshot
• Also add parameter to get the heater output level from the instrument.
• don't snapshot blocking_t parameter

This has been tested on a Model 372. I have not looked into if similar problems with snapshot of output channels are present in drivers for other models.

Ping @astafan8 as you most recently committed to this driver.

( original description was edited )

[astafan8]

@ThorvaldLarsen is there a particular reason for the heaters to be in a list as opposed to JUST be submodules of the instrument added via add_submodule(...) call?

[ThorvaldLarsen]

Nope, add_submodule would also be fine for me. Ended up making a channellist because I couldnt get the add_submodule thing to work - turned out my issue was that I loaded from a wrong file in my tests rather than the add_submodule XD Just left it as channellists after that.

I have no preference one way or the other.

[codecov]

Codecov Report

Merging #1746 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1746   +/-   ##
=======================================
  Coverage   67.56%   67.56%           
=======================================
  Files         146      146           
  Lines       18347    18347           
=======================================
  Hits        12397    12397           
  Misses       5950     5950

[astafan8]

@ThorvaldLarsen I pushed some edits. Could you perform the test again?

[ThorvaldLarsen]

Yes, will test it again next week.
One thing: The get_parser in set_blocking_t really needs to be removed otherwise the snapshot thing gives warnings (the parameter does not have a get_cmd). I can of course test as is but that was my experience when testing last time for initial version of this PR.

[astafan8]

ok, will remove the get_parser and investigate the thing separately

[ThorvaldLarsen]

@astafan8 I just tested the current version of this driver and all the changes worked:
Tested reading out heater range. setting temperature, and reading snapshot.

Two things to comment on:
First: The heater will return 0.005 even when the heater is off. Seems 0.005 is the lowest value in can return even if it should be zero. Values in normal operation mode works fine. To me it is fine as is but might be good to add a comment about it?

Second: I found a different bug in wait_until_set_point_reached. When the measured temperature is outside the calibrated range the lakeshore will return 0 kelvin as the value. This means abs(t_reading - t_setpoint) / t_reading > tolerance will throw an error due to zero division. There should be a check that t_reading is not equal to zero. Dont know if this should be fixed in a separate PR.

[astafan8]

@ThorvaldLarsen Thanks a lot! Then I merge this PR.

The heater will return 0.005 even when the heater is off.

For now, I'll add this as a note in the description of the parameter in this PR. If needed, we can implement something smarter in the driver.

I found a different bug in wait_until_set_point_reached ...... Dont know if this should be fixed in a separate PR.

Thanks! But it is to be fixed, for sure, in a separate PR :) This PR already contains 3 subtly related changes.