Skip to content

Commit

Permalink
Merge pull request #624 from BCDA-APS/493-v2-lakeshore-cleanup
Browse files Browse the repository at this point in the history 
Remove auto_heater and tweaks to PVPositionerSoftDone
  • Loading branch information
@gfabbris
gfabbris authored Jan 20, 2022
2 parents a3c3c92 + 67bfb47 commit 7049d0f
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 218 deletions.
251 changes: 36 additions & 215 deletions apstools/devices/lakeshore_controllers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,6 @@
HOUR = 60 * MINUTE


class BooleanSignal(Signal):
"""Signal that forces value to be a boolean."""

def check_value(self, value):
"""
Check if the value is a boolean.
raises ``ValueError``
"""
if not isinstance(value, bool):
raise ValueError("value can only be True or False.")


class LakeShore336_LoopControl(PVPositionerSoftDoneWithStop):
"""
LakeShore 336 temperature controller -- with heater control.
Expand All @@ -54,32 +41,22 @@ class LakeShore336_LoopControl(PVPositionerSoftDoneWithStop):
"""

# position
# FIXME: Resolve this code:
# setpoint is the temperature to be reached
# readback is the current temperature
# XXXXX is the interim setpoint for the PID loop
readback = FormattedComponent(
EpicsSignalRO, "{prefix}IN{loop_number}", auto_monitor=True, kind="hinted"
)
# readback is provided by PVPositionerSoftDoneWithStop

setpoint = FormattedComponent(
EpicsSignalWithRBV,
"{prefix}OUT{loop_number}:SP",
put_complete=True,
kind="normal",
EpicsSignalWithRBV, "{prefix}OUT{loop_number}:SP", put_complete=True
)

# FIXME: refactor (per above)
# Due to ramping the setpoint will change slowly and the readback may catch
# up even if the motion is not done.
target = Component(Signal, value=0, kind="omitted")

# configuration
units = FormattedComponent(
EpicsSignalWithRBV, "{prefix}IN{loop_number}:Units", kind="config"
)

heater = FormattedComponent(
EpicsSignalRO, "{prefix}HTR{loop_number}", auto_monitor=True, kind="normal"
EpicsSignalRO,
"{prefix}HTR{loop_number}",
auto_monitor=True,
kind="normal"
)
heater_range = FormattedComponent(
EpicsSignalWithRBV,
Expand Down Expand Up @@ -119,15 +96,14 @@ class LakeShore336_LoopControl(PVPositionerSoftDoneWithStop):
EpicsSignalWithRBV, "{prefix}OUT{loop_number}:Mode", kind="config"
)

auto_heater = Component(BooleanSignal, value=False, kind="config")

# This must be modified either here, or before using auto_heater.
_auto_ranges = None

def __init__(self, *args, loop_number=None, timeout=10 * HOUR, **kwargs):
self.loop_number = loop_number
super().__init__(
*args, timeout=timeout, tolerance=0.1, target_attr="target", **kwargs
*args,
timeout=timeout,
tolerance=0.1,
readback_pv=f"IN{loop_number}",
**kwargs
)
self._settle_time = 0

Expand All @@ -142,58 +118,9 @@ def settle_time(self, value):
else:
self._settle_time = value

@property
def egu(self):
return self.units.get(as_string=True)

def stop(self, *, success=False):
if success is False:
self.setpoint.put(self._position)
super().stop(success=success)

def pause(self):
self.setpoint.put(self._position)

@auto_heater.sub_value
def _subscribe_auto_heater(self, value=None, **kwargs):
if value:
self.setpointRO.subscribe(self._switch_heater, event_type="value")
else:
self.setpointRO.clear_subs(self._switch_heater)

def _switch_heater(self, value=None, **kwargs):
# TODO: Find a better way to do this, perhaps with an array?
for _heater_range, _temp_range in self._auto_ranges.items():
if _temp_range:
if _temp_range[0] < value <= _temp_range[1]:
self.heater_range.put(_heater_range)

@property
def auto_ranges(self):
return self._auto_ranges

@auto_ranges.setter
def auto_ranges(self, value):
if not isinstance(value, dict):
raise TypeError("auto_ranges must be a dictionary.")

for _heater_range, _temp_range in value.items():
if _heater_range not in self.heater_range.enum_strs:
raise ValueError(
"The input dictionary keys must be one of "
f"these: {self.heater_range.enum_strs}, but "
f"{_heater_range} was entered."
)

if _temp_range is not None and len(_temp_range) != 2:
raise ValueError(
f"The value {_temp_range} is invalid! It "
"must be either None or an iterable with two "
"items."
)

self._auto_ranges = value


class LakeShore336_LoopRO(Device):
"""
Expand All @@ -216,69 +143,6 @@ def __init__(self, *args, loop_number=None, **kwargs):
super().__init__(*args, **kwargs)


class LakeShore336_LoopBase(PVPositionerSoftDoneWithStop):
"""
One control loop of the LakeShore 336 temperature controller.
Each control loop is a separate process controller.
DEFAULTS:
- timeout: 10 hours
- tolerance: 0.1
"""

readback = FormattedComponent(
EpicsSignalRO, "{prefix}IN{loop_number}", auto_monitor=True, kind="hinted"
)
setpoint = FormattedComponent(
EpicsSignalWithRBV,
"{prefix}OUT{loop_number}:SP",
put_complete=True,
kind="normal",
)

def __init__(self, *args, loop_number=None, timeout=10 * HOUR, **kwargs):
self.loop_number = loop_number
super().__init__(*args, timeout=timeout, tolerance=0.1, **kwargs)


class LakeShore336_LoopMore(LakeShore336_LoopBase):
"""
Additional controls for LakeShore 336 loop1 and loop2: heater, pid, & ramp.
Only used on loops 1 & 2.
"""

heater = FormattedComponent(
EpicsSignalRO, "{prefix}HTR{loop_number}", auto_monitor=True, kind="normal"
)
heater_range = FormattedComponent(
EpicsSignalWithRBV,
"{prefix}HTR{loop_number}:Range",
kind="config",
auto_monitor=True,
string=True,
)

pid_P = FormattedComponent(
EpicsSignalWithRBV, "{prefix}P{loop_number}", kind="config"
)
pid_I = FormattedComponent(
EpicsSignalWithRBV, "{prefix}I{loop_number}", kind="config"
)
pid_D = FormattedComponent(
EpicsSignalWithRBV, "{prefix}D{loop_number}", kind="config"
)

ramp_rate = FormattedComponent(
EpicsSignalWithRBV, "{prefix}RampR{loop_number}", kind="config"
)
ramp_on = FormattedComponent(
EpicsSignalWithRBV, "{prefix}OnRamp{loop_number}", kind="config"
)


class LakeShore336Device(Device):
"""
LakeShore 336 temperature controller.
Expand All @@ -289,10 +153,18 @@ class LakeShore336Device(Device):
- loop 4: temperature positioner
"""

loop1 = FormattedComponent(LakeShore336_LoopMore, "{self.prefix}", loop_number=1)
loop2 = FormattedComponent(LakeShore336_LoopMore, "{self.prefix}", loop_number=2)
loop3 = FormattedComponent(LakeShore336_LoopBase, "{self.prefix}", loop_number=3)
loop4 = FormattedComponent(LakeShore336_LoopBase, "{self.prefix}", loop_number=4)
loop1 = FormattedComponent(
LakeShore336_LoopControl, "{self.prefix}", loop_number=1
)
loop2 = FormattedComponent(
LakeShore336_LoopControl, "{self.prefix}", loop_number=2
)
loop3 = FormattedComponent(
LakeShore336_LoopRO, "{self.prefix}", loop_number=3
)
loop4 = FormattedComponent(
LakeShore336_LoopRO, "{self.prefix}", loop_number=4
)

# same names as apstools.synApps._common.EpicsRecordDeviceCommonAll
scanning_rate = Component(EpicsSignal, "read.SCAN", kind="omitted")
Expand All @@ -302,11 +174,11 @@ class LakeShore336Device(Device):
serial = Component(AsynRecord, "serial", kind="omitted")


class LS340_LoopBase(PVPositionerSoftDoneWithStop): # TODO: check, check, check
class LS340_LoopBase(PVPositionerSoftDoneWithStop):
""" Base settings for both sample and control loops. """

# position
readback = Component(Signal, value=0)
# readback is provided by PVPositionerSoftDoneWithStop
# TODO: Polar reports: sometimes the setpoint PV is not updated
setpoint = FormattedComponent(
EpicsSignal,
Expand All @@ -316,10 +188,6 @@ class LS340_LoopBase(PVPositionerSoftDoneWithStop): # TODO: check, check, check
put_complete=True,
)

# This is here only because of ramping, because then setpoint will change
# slowly.
target = Component(Signal, value=0, kind="omitted")

# configuration
units = Component(Signal, value="K", kind="config")

Expand Down Expand Up @@ -355,7 +223,11 @@ class LS340_LoopBase(PVPositionerSoftDoneWithStop): # TODO: check, check, check
def __init__(self, *args, loop_number=None, timeout=10 * HOUR, **kwargs):
self.loop_number = loop_number
super().__init__(
*args, timeout=timeout, tolerance=0.1, target_attr="target", **kwargs
*args,
readback_pv="ignore",
timeout=timeout,
tolerance=0.1,
**kwargs
)
self._settle_time = 0

Expand All @@ -374,11 +246,6 @@ def settle_time(self, value):
def egu(self):
return self.units.get(as_string=True)

def stop(self, *, success=False):
if success is False:
self.setpoint.put(self._position)
super().stop(success=success)

def pause(self):
self.setpoint.put(self._position, wait=True)

Expand Down Expand Up @@ -407,63 +274,17 @@ class LakeShore340Device(Device):

heater = Component(EpicsSignalRO, "Heater")
heater_range = Component(
EpicsSignal, "Rg_rdbk", write_pv="HeatRg", kind="normal", put_complete=True
EpicsSignal,
"Rg_rdbk",
write_pv="HeatRg",
kind="normal",
put_complete=True
)

auto_heater = Component(BooleanSignal, value=False, kind="config")

read_pid = Component(EpicsSignal, "readPID.PROC", kind="omitted")

# same names as apstools.synApps._common.EpicsRecordDeviceCommonAll
scanning_rate = Component(EpicsSignal, "read.SCAN", kind="omitted")
process_record = Component(EpicsSignal, "read.PROC", kind="omitted")

serial = Component(AsynRecord, "serial", kind="omitted")

# This must be modified either here, or before using auto_heater.
_auto_ranges = None

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# TODO: I don't know why this has to be done, otherwise it gets hinted.
self.control.readback.kind = "normal"

@auto_heater.sub_value
def _subscribe_auto_heater(self, value=None, **kwargs):
if value:
self.control.setpoint.subscribe(self._switch_heater, event_type="value")
else:
self.control.setpoint.clear_subs(self._switch_heater)

def _switch_heater(self, value=None, **kwargs):
# TODO: Find a better way to do this, perhaps with an array?
for _heater_range, _temp_range in self._auto_ranges.items():
if _temp_range:
if _temp_range[0] < value <= _temp_range[1]:
self.heater_range.put(_heater_range)

@property
def auto_ranges(self):
return self._auto_ranges

@auto_ranges.setter
def auto_ranges(self, value):
if not isinstance(value, dict):
raise TypeError("auto_ranges must be a dictionary.")

for _heater_range, _temp_range in value.items():
if _heater_range not in self.heater_range.enum_strs:
raise ValueError(
"The input dictionary keys must be one of "
f"these: {self.heater_range.enum_strs}, but "
f"{_heater_range} was entered."
)

if _temp_range is not None and len(_temp_range) != 2:
raise ValueError(
f"The value {_temp_range} is invalid! It "
"must be either None or an iterable with two "
"items."
)

self._auto_ranges = value
2 changes: 1 addition & 1 deletion apstools/devices/positioner_soft_done.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class PVPositionerSoftDone(PVPositioner):
tolerance = Component(Signal, value=-1, kind="config")
report_dmov_changes = Component(Signal, value=False, kind="omitted")

target = Component(Signal, value=None, kind="config")
target = Component(Signal, value="None", kind="config")

@property
def precision(self):
Expand Down
4 changes: 2 additions & 2 deletions apstools/devices/tests/test_positioner_soft_done.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def delayed_stop(pos, delay=1):

def test_same_sp_and_rb():
with pytest.raises(ValueError) as exc:
PVPositionerSoftDone("", name="pos")
PVPositionerSoftDone("", readback_pv="test", setpoint_pv="test", name="pos")
assert str(exc.value).endswith("must have different values")


Expand All @@ -67,7 +67,7 @@ def test_structure(device, has_inposition):
assert pos.setpoint.pvname == "v"
assert pos.done.get() is True
assert pos.done_value is True
assert pos.target.get() is None
assert pos.target.get() == "None"
assert pos.tolerance.get() == -1


Expand Down

0 comments on commit 7049d0f

Please sign in to comment.