From a5612f11610e4b34c9c90272c14583ca801fc882 Mon Sep 17 00:00:00 2001 From: Dhanya Thattil Date: Mon, 23 Mar 2026 12:09:09 +0100 Subject: [PATCH] fixes for python api --- python/slsdet/ctb.py | 4 +- python/slsdet/detector.py | 6 +- python/slsdet/powers.py | 202 +++++------------- python/tests/test_det_api.py | 4 +- slsDetectorSoftware/src/DetectorImpl.h | 4 +- .../Caller/test-Caller-chiptestboard.cpp | 12 +- 6 files changed, 68 insertions(+), 164 deletions(-) diff --git a/python/slsdet/ctb.py b/python/slsdet/ctb.py index 1295af9cb..f8893eb95 100644 --- a/python/slsdet/ctb.py +++ b/python/slsdet/ctb.py @@ -3,7 +3,7 @@ from .detector import Detector, freeze from .utils import element_if_equal from .dacs import DetectorDacs, NamedDacs -from .powers import DetectorPowers, NamedPowers +from .powers import DetectorPowers from . import _slsdet dacIndex = _slsdet.slsDetectorDefs.dacIndex from .detector_property import DetectorProperty @@ -16,7 +16,7 @@ class Ctb(Detector): super().__init__(id) self._frozen = False self._dacs = NamedDacs(self) - self._powers = NamedPowers(self) + self._powers = DetectorPowers(self) @property def dacs(self): diff --git a/python/slsdet/detector.py b/python/slsdet/detector.py index 328f4bc5f..50d7cacea 100755 --- a/python/slsdet/detector.py +++ b/python/slsdet/detector.py @@ -4173,13 +4173,11 @@ class Detector(CppDetectorApi): @element def v_limit(self): """[Ctb][Xilinx Ctb] Soft limit for power supplies (ctb only) and DACS in mV.""" - return self.getDAC(dacIndex.V_LIMIT, True) + return self.getVoltageLimit() @v_limit.setter def v_limit(self, value): - value = ut.merge_args(dacIndex.V_LIMIT, value, True) - ut.set_using_dict(self.setDAC, *value) - + ut.set_using_dict(self.setVoltageLimit, value) @property @element diff --git a/python/slsdet/powers.py b/python/slsdet/powers.py index 990f1e0bb..bdeacda53 100755 --- a/python/slsdet/powers.py +++ b/python/slsdet/powers.py @@ -1,63 +1,74 @@ # SPDX-License-Identifier: LGPL-3.0-or-other # Copyright (C) 2021 Contributors to the SLS Detector Package -from .detector_property import DetectorProperty from functools import partial import numpy as np from . import _slsdet from .detector import freeze -dacIndex = _slsdet.slsDetectorDefs.dacIndex -class Power(DetectorProperty): +powerIndex = _slsdet.slsDetectorDefs.powerIndex +class Power: """ - This class represents a power on the Chip Test Board. One instance handles all - powers with the same name for a multi detector instance. (TODO: Not needed for CTB) + This class represents a power supply on the Chip Test Board. .. note :: - This class is used to build up DetectorPowers and is in general + This class is used to build up NamedPowers and is in general not directly accessible to the user. """ + _direct_access = ['_detector'] + def __init__(self, name, enum, default, detector): - - def get_power(modules=[]): - enabled = detector.isPowerEnabled(enum, modules) - values = detector.getDAC(enum, True, modules) - - # replace disabled powers with 0 - return [v if e else 0 for v, e in zip(values, enabled)] - - - def set_power(value, modules=[]): - - # always disable first - detector.setPowerEnabled([enum], False, modules) - - if value > 0: - # set the value - detector.setDAC(enum, value, True, modules) - detector.setPowerEnabled([enum], True, modules) - - super().__init__(get_power, - set_power, - detector.size, - name) - + self._frozen = False + self.__name__ = name + self.enum = enum self.default = default + self.detector = detector + self._frozen = True + @property + def dac(self): + " Returns the dac value for this power supply in mV." + return self.detector.getPowerDAC(self.enum) + + @dac.setter + def dac(self, value): + " Set the dac value for this power supply in mV." + self.detector.setPowerDAC(self.enum, value) + @property + def enable(self): + " Returns whether this power supply is enabled." + return self.detector.isPowerEnabled(self.enum) + + @enable.setter + def enable(self, value): + " Set whether this power supply is enabled." + self.detector.setPowerEnabled([self.enum], value) + + # prevent unknown attributes + def __setattr__(self, name, value): + if not getattr(self, "_frozen", False): + super().__setattr__(name, value) + elif name in ("dac", "enable", "_frozen", "enum", "default", "detector", "__name__"): + super().__setattr__(name, value) + else: + raise AttributeError(f"Cannot set attribute '{name}' on Power. Allowed attributes are 'dac' and 'enable'.") + def __repr__(self): - """String representation for a single power in all modules""" - powerstr = ''.join([f'{item:5d} mV' for item in self.get()]) - return f'{self.__name__:15s}:{powerstr}' + "String representation for a single power supply" + return f'{self.__name__:15s}: {str(self.enable):5s}, {self.dac:5d} mV' -class NamedPowers: + +class DetectorPowers: """ - New implementation of the detector powers. + List implementation of the all the power supplies with its names. """ - _frozen = False _direct_access = ['_detector', '_current', '_powernames'] + def __init__(self, detector): + self._frozen = False + self._detector = detector self._current = 0 @@ -70,24 +81,23 @@ class NamedPowers: # Populate the powers for i,name in enumerate(self._powernames): #name, enum, low, high, default, detector - k = dacIndex(i + int(dacIndex.V_POWER_A)) - setattr(self, name, Power(name, k, 0, detector)) + setattr(self, name, Power(name, powerIndex(i), 0, detector)) self._frozen = True - # def __getattr__(self, name): - # return self.__getattribute__('_' + name) - def __setattr__(self, name, value): - if not self._frozen: + if not getattr(self, "_frozen", False): #durning init we need to be able to set up the class super().__setattr__(name, value) else: #Later we restrict us to manipulate powers and a few fields - if name in self._direct_access: + if name in self._direct_access or name == '_frozen': super().__setattr__(name, value) elif name in self._powernames: - return self.__getattribute__(name).__setitem__(slice(None, None), value) + raise AttributeError( + f"Cannot assign directly to power '{name}'. " + f"Use '{name}.dac = value' or '{name}.enable = value'" + ) else: raise AttributeError(f'Power not found: {name}') @@ -107,107 +117,3 @@ class NamedPowers: r_str = ['========== POWERS ========='] r_str += [repr(power) for power in self] return '\n'.join(r_str) - def get_asarray(self): - """ - Read the powers into a numpy array with dimensions [npowers, nmodules] - """ - power_array = np.zeros((len(self._powernames), len(self._detector))) - for i, _d in enumerate(self): - power_array[i,:] = _d[:] - return power_array - - def to_array(self): - return self.get_asarray() - - def set_from_array(self, power_array): - """ - Set the power from an numpy array with power values. [npowers, nmodules] - """ - power_array = power_array.astype(np.int) - for i, _d in enumerate(self): - _d[:] = power_array[i] - - def from_array(self, power_array): - self.set_from_array(power_array) - -class DetectorPowers: - _powers = [] - _powernames = [_d[0] for _d in _powers] - _allowed_attr = ['_detector', '_current'] - _frozen = False - - def __init__(self, detector): - # We need to at least initially know which detector we are connected to - self._detector = detector - - # Index to support iteration - self._current = 0 - - # Name the attributes? - for _d in self._powers: - setattr(self, '_'+_d[0], Power(*_d, detector)) - - self._frozen = True - - def __getattr__(self, name): - return self.__getattribute__('_' + name) - - @property - def powernames(self): - return [_d[0] for _d in _powers] - - def __setattr__(self, name, value): - if name in self._powernames: - return self.__getattribute__('_' + name).__setitem__(slice(None, None), value) - else: - if self._frozen == True and name not in self._allowed_attr: - raise AttributeError(f'Power not found: {name}') - super().__setattr__(name, value) - - - def __next__(self): - if self._current >= len(self._powers): - self._current = 0 - raise StopIteration - else: - self._current += 1 - return self.__getattr__(self._powernames[self._current-1]) - - def __iter__(self): - return self - - def __repr__(self): - r_str = ['========== POWERS ========='] - r_str += [repr(power) for power in self] - return '\n'.join(r_str) - - def get_asarray(self): - """ - Read the powers into a numpy array with dimensions [npowers, nmodules] - """ - power_array = np.zeros((len(self._powers), len(self._detector))) - for i, _d in enumerate(self): - power_array[i,:] = _d[:] - return power_array - - def to_array(self): - return self.get_asarray() - - def set_from_array(self, power_array): - """ - Set the powers from an numpy array with power values. [npowers, nmodules] - """ - power_array = power_array.astype(np.int) - for i, _d in enumerate(self): - _d[:] = power_array[i] - - def from_array(self, power_array): - self.set_from_array(power_array) - - def set_default(self): - """ - Set all powers to their default values - """ - for _d in self: - _d[:] = _d.default - diff --git a/python/tests/test_det_api.py b/python/tests/test_det_api.py index 79d012e20..9b12c2c77 100644 --- a/python/tests/test_det_api.py +++ b/python/tests/test_det_api.py @@ -405,8 +405,8 @@ def test_v_limit(session_simulator, request): if det_type in ['ctb', 'xilinx_ctb']: # save previous value + prev_val = d.getVoltageLimit() from slsdet import dacIndex - prev_val = d.getDAC(dacIndex.V_LIMIT, True) prev_dac_val = d.getDAC(dacIndex.DAC_0, False) with pytest.raises(Exception): @@ -426,8 +426,8 @@ def test_v_limit(session_simulator, request): d.setDAC(dacIndex.DAC_0, 1501, True, [0]) # restore previous value + d.setVoltageLimit(prev_val) for i in range(len(d)): - d.setDAC(dacIndex.V_LIMIT, prev_val[i], True, [i]) d.setDAC(dacIndex.DAC_0, prev_dac_val[i], False, [i]) else: diff --git a/slsDetectorSoftware/src/DetectorImpl.h b/slsDetectorSoftware/src/DetectorImpl.h index 894865b4c..d7fcc655d 100644 --- a/slsDetectorSoftware/src/DetectorImpl.h +++ b/slsDetectorSoftware/src/DetectorImpl.h @@ -190,7 +190,9 @@ class DetectorImpl : public virtual slsDetectorDefs { inline void verifyChipTestBoard(const std::string &funcName) const { if (!isChipTestBoard()) - throw RuntimeError(funcName + " is only valid for chip test board"); + throw RuntimeError(funcName + + " is not implemented for this detector. It is " + "only valid for chip test board"); if (size() != 1) throw RuntimeError( funcName + diff --git a/slsDetectorSoftware/tests/Caller/test-Caller-chiptestboard.cpp b/slsDetectorSoftware/tests/Caller/test-Caller-chiptestboard.cpp index 5ac1e8e49..4889ed1b3 100644 --- a/slsDetectorSoftware/tests/Caller/test-Caller-chiptestboard.cpp +++ b/slsDetectorSoftware/tests/Caller/test-Caller-chiptestboard.cpp @@ -916,14 +916,12 @@ TEST_CASE("v_limit", "[.detectorintegration]") { REQUIRE_THROWS(caller.call("v_limit", {"1200", "mV"}, -1, PUT)); REQUIRE_THROWS(caller.call("v_limit", {"-100"}, -1, PUT)); { - std::ostringstream oss; + std::ostringstream oss, oss2; caller.call("v_limit", {"0"}, -1, PUT, oss); REQUIRE(oss.str() == "v_limit 0\n"); - } - { - std::ostringstream oss; - caller.call("v_limit", {}, -1, GET, oss); - REQUIRE(oss.str() == "v_limit 0\n"); + caller.call("v_limit", {}, -1, GET, oss2); + REQUIRE(oss2.str() == "v_limit 0\n"); + REQUIRE_NOTHROW(caller.call("dac", {"0", "1200", "mV"}, -1, PUT)); } { std::ostringstream oss; @@ -931,8 +929,8 @@ TEST_CASE("v_limit", "[.detectorintegration]") { REQUIRE(oss.str() == "v_limit 1500\n"); REQUIRE_THROWS(caller.call("dac", {"0", "1501", "mV"}, -1, PUT)); } + det.setVoltageLimit(prev_val); for (int i = 0; i != det.size(); ++i) { - det.setVoltageLimit(prev_val); det.setDAC(defs::DAC_0, prev_dac_val[i], false, {i}); } } else {