From 79652d6bcf776d816d9cac7ffdcf735910481932 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Tue, 13 Aug 2024 18:05:30 -0700 Subject: [PATCH 01/15] Move simulation code from the test directory into the module directory --- python/lsst/ts/vent/controller/controller.py | 81 +++++++++- python/lsst/ts/vent/controller/dispatcher.py | 28 +++- python/lsst/ts/vent/controller/simulate.py | 104 +++++++++++++ .../ts/vent/controller/simulator_setup.json | 0 tests/test_dispatcher.py | 23 +++ tests/test_fan.py | 17 +- tests/test_full.py | 147 ++++++++++++++++++ tests/test_megaind.py | 70 ++++----- 8 files changed, 405 insertions(+), 65 deletions(-) create mode 100644 python/lsst/ts/vent/controller/simulate.py rename tests/setup.json => python/lsst/ts/vent/controller/simulator_setup.json (100%) create mode 100644 tests/test_full.py diff --git a/python/lsst/ts/vent/controller/controller.py b/python/lsst/ts/vent/controller/controller.py index aca7248..0c33da8 100644 --- a/python/lsst/ts/vent/controller/controller.py +++ b/python/lsst/ts/vent/controller/controller.py @@ -22,11 +22,17 @@ import logging from enum import IntEnum -import megaind +try: + import megaind + + MEGAIND_AVAILABLE = True +except ImportError: + MEGAIND_AVAILABLE = False import pymodbus.client from . import vf_drive from .config import Config +from .simulate import DomeVentsSimulator __all__ = ["Controller"] @@ -45,10 +51,12 @@ class Controller: https://confluence.lsstcorp.org/display/~fritzm/Auxtel+Vent+Gate+Automation. """ - def __init__(self, config: Config = Config()): + def __init__(self, config: Config = Config(), simulate: bool = False): self.cfg = config self.default_fan_frequency = self.cfg.max_freq self.log = logging.getLogger(type(self).__name__) + self.simulate = simulate + self.simulator = DomeVentsSimulator(self.cfg) if simulate else None async def connect(self) -> None: """Connects to the variable frequency drive via modbus. @@ -58,11 +66,23 @@ async def connect(self) -> None: ModbusException If the variable frequency drive is not available. """ + if self.simulate: + await self.simulator.start() + self.vfd_client = pymodbus.client.AsyncModbusTcpClient( self.cfg.hostname, port=self.cfg.port ) await self.vfd_client.connect() + async def stop(self) -> None: + """Disconnects from the variable frequency drive, and stops + the simulator if simulating. + """ + if self.simulate: + await self.simulator.stop() + + self.vfd_client.close() + async def get_fan_manual_control(self) -> bool: """Returns the variable frequency drive setting for manual or automatic (modbus) control. @@ -246,6 +266,10 @@ def vent_open(self, vent_number: int) -> None: Raises ------ + ModuleNotFoundError + If the megaind module has not been installed, in which case the + daughterboard cannot be controlled. + ValueError If vent_number is invalid. @@ -258,7 +282,7 @@ def vent_open(self, vent_number: int) -> None: raise ValueError(f"Invalid {vent_number=} should be between 0 and 3") if self.cfg.vent_signal_ch[vent_number] == -1: raise ValueError(f"Vent {vent_number=} is not configured.") - megaind.setOd(self.cfg.megaind_stack, self.cfg.vent_signal_ch[vent_number], 1) + self.setOd(self.cfg.megaind_stack, self.cfg.vent_signal_ch[vent_number], 1) def vent_close(self, vent_number: int) -> None: """Closes the specified vent. @@ -270,6 +294,10 @@ def vent_close(self, vent_number: int) -> None: Raises ------ + ModuleNotFoundError + If the megaind module has not been installed, in which case the + daughterboard cannot be controlled. + ValueError If vent_number is invalid. @@ -282,7 +310,7 @@ def vent_close(self, vent_number: int) -> None: raise ValueError(f"Invalid {vent_number=} should be between 0 and 3") if self.cfg.vent_signal_ch[vent_number] == -1: raise ValueError(f"Vent {vent_number=} is not configured.") - megaind.setOd(self.cfg.megaind_stack, self.cfg.vent_signal_ch[vent_number], 0) + self.setOd(self.cfg.megaind_stack, self.cfg.vent_signal_ch[vent_number], 0) def vent_state(self, vent_number: int) -> VentGateState: """Returns the state of the specified vent. @@ -294,6 +322,10 @@ def vent_state(self, vent_number: int) -> VentGateState: Raises ------ + ModuleNotFoundError + If the megaind module has not been installed, in which case the + daughterboard cannot be controlled. + ValueError If vent_number is invalid. @@ -311,10 +343,10 @@ def vent_state(self, vent_number: int) -> VentGateState: ): raise ValueError(f"Vent {vent_number=} is not configured.") - op_state = megaind.getOptoCh( + op_state = self.getOptoCh( self.cfg.megaind_stack, self.cfg.vent_open_limit_ch[vent_number] ) - cl_state = megaind.getOptoCh( + cl_state = self.getOptoCh( self.cfg.megaind_stack, self.cfg.vent_close_limit_ch[vent_number] ) @@ -327,3 +359,40 @@ def vent_state(self, vent_number: int) -> VentGateState: return VentGateState.CLOSED case _: return VentGateState.FAULT + + def getOptoCh(self, *args, **kwargs) -> int: + """Calls megaind.getOptoCh or a simulated getOptoCh depending + whether the class was instantiated with simulate = True. + + Raises + ------ + ModuleNotFoundError + If the megaind module has not been installed, in which case the + daughterboard cannot be controlled. + """ + + if self.simulate: + return self.simulator.getOptoCh(*args, **kwargs) + else: + if not MEGAIND_AVAILABLE: + raise ModuleNotFoundError("The megaind module is not available.") + return megaind.getOptoCh(*args, **kwargs) + + def setOd(self, *args, **kwargs) -> None: + """Calls megaind.setOd or a simulated setOd depending + whether the class was instantiated with simulate = True. + + Raises + ------ + ModuleNotFoundError + If the megaind module has not been installed, in which case the + daughterboard cannot be controlled. + """ + + if self.simulate: + print(f"{args=}") + self.simulator.setOd(*args, **kwargs) + else: + if not MEGAIND_AVAILABLE: + raise ModuleNotFoundError("The megaind module is not available.") + megaind.setOd(*args, **kwargs) diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 2e74fdd..5e78b4b 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -1,3 +1,24 @@ +# This file is part of ts_atbuilding_vents +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (https://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + import json import logging import traceback @@ -27,7 +48,11 @@ def _cast_string_to_type(new_type: type, value: str): """ if new_type is bool: # Boolean is a special case - return value.lower() in ("true", "t", "1") + if value.lower() in ("true", "t", "1"): + return True + if value.lower() in ("false", "f", "0"): + return False + raise ValueError(f"Expected bool value but got {value}") return new_type(value) @@ -157,6 +182,7 @@ async def set_extraction_fan_drive_freq(self, target_frequency: float) -> None: async def set_extraction_fan_manual_control_mode( self, enable_manual_control_mode: bool ) -> None: + print(f"{enable_manual_control_mode=}") await self.controller.fan_manual_control(enable_manual_control_mode) async def start_extraction_fan(self) -> None: diff --git a/python/lsst/ts/vent/controller/simulate.py b/python/lsst/ts/vent/controller/simulate.py new file mode 100644 index 0000000..3ccada9 --- /dev/null +++ b/python/lsst/ts/vent/controller/simulate.py @@ -0,0 +1,104 @@ +# This file is part of ts_atbuilding_vents +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (https://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import os + +from pymodbus.server import ModbusSimulatorServer + +from .config import Config + + +class DomeVentsSimulator: + def __init__(self, config: Config): + self.input_bits = [0, 0, 0, 0] + self.cfg = config + + self.modbus_simulator = ModbusSimulatorServer( + modbus_server="server", + modbus_device="device", + http_host="localhost", + http_port=25074, + json_file=os.path.dirname(__file__) + "/simulator_setup.json", + ) + + async def start(self): + await self.modbus_simulator.run_forever(only_start=True) + + async def stop(self): + await self.modbus_simulator.stop() + + def getOptoCh(self, stack: int, channel: int) -> int: + """Simulates the behavior of megaind.getOptoCh, as if connected + to dome vents configured as described in config.py. + + Returns + ------- + The state of the digital input channel, 1 (active) or 0 (inactive) + + Raises + ------ + AssertionError + If an invalid channel (outside the range 1 to 4) is requested + or the stack index does not match the configured value. + """ + + channel -= 1 # Channels are 1-indexed + + assert stack == self.cfg.megaind_stack + assert 0 <= channel <= 3 + return self.input_bits[channel] + + def setOd(self, stack: int, channel: int, val: int) -> None: + """Simulates the behavior of megaind.setOd, as if connected + to dome vents configured as described in config.py. + + Raises + ------ + AssertionError + If an invalid channel (outside the range 1 to 4) is requested, + or the stack does not match the configured value, or the + specified `val` is not zero or one. + """ + + channel -= 1 # Channels are 1-indexed + + assert stack == self.cfg.megaind_stack + assert 0 <= channel <= 3 + assert val == 0 or val == 1 + vent_number = [i for i in range(4) if self.cfg.vent_signal_ch[i] - 1 == channel] + assert len(vent_number) <= 1 + if len(vent_number) == 1: + vent_number = vent_number[0] + op = val + cl = 0 if val else 1 + self.input_bits[self.cfg.vent_open_limit_ch[vent_number] - 1] = op + self.input_bits[self.cfg.vent_close_limit_ch[vent_number] - 1] = cl + + def set_bits(self, input_bits: tuple[int]) -> None: + """Sets the state of the simulated daughterboard. + + Raises + ------ + AssertionError + If input_bits does not have length 4. + """ + assert len(input_bits) == 4 + self.input_bits = input_bits diff --git a/tests/setup.json b/python/lsst/ts/vent/controller/simulator_setup.json similarity index 100% rename from tests/setup.json rename to python/lsst/ts/vent/controller/simulator_setup.json diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index b089a55..40839f7 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -1,3 +1,24 @@ +# This file is part of ts_atbuilding_vents +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (https://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + import asyncio import json import logging @@ -13,6 +34,7 @@ class TestDispatcher(unittest.IsolatedAsyncioTestCase): async def asyncSetUp(self): + print("TestDispatcher.asyncSetUp") self.log = logging.getLogger() # Mock the controller @@ -47,6 +69,7 @@ async def asyncSetUp(self): await self.dispatcher.connected_task async def asyncTearDown(self): + print("TestDispatcher.asyncTearDown") await self.client.close() await self.dispatcher.close() self.patcher.stop() diff --git a/tests/test_fan.py b/tests/test_fan.py index 0b4d04a..1a7cc25 100644 --- a/tests/test_fan.py +++ b/tests/test_fan.py @@ -19,35 +19,22 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import asyncio -import os import unittest from lsst.ts.vent.controller import Config, Controller -from pymodbus.server import ModbusSimulatorServer class TestFan(unittest.IsolatedAsyncioTestCase): async def asyncSetUp(self): - self.loop = asyncio.new_event_loop() - self.simulator = ModbusSimulatorServer( - modbus_server="server", - modbus_device="device", - http_host="localhost", - http_port=25074, - json_file=os.path.dirname(__file__) + "/setup.json", - ) - await self.simulator.run_forever(only_start=True) - cfg = Config() cfg.hostname = "localhost" cfg.port = 26034 - self.controller = Controller(cfg) + self.controller = Controller(cfg, simulate=True) await self.controller.connect() async def asyncTearDown(self): - await self.simulator.stop() + await self.controller.stop() async def test_fan_manual(self): self.assertTrue( diff --git a/tests/test_full.py b/tests/test_full.py new file mode 100644 index 0000000..3896ba3 --- /dev/null +++ b/tests/test_full.py @@ -0,0 +1,147 @@ +# This file is part of ts_atbuilding_vents +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (https://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import asyncio +import json +import logging +import unittest + +from lsst.ts import tcpip +from lsst.ts.vent.controller import (Config, Controller, Dispatcher, + VentGateState) + +# Standard timeout for TCP/IP messages (sec). +TCP_TIMEOUT = 1 + + +class TestFull(unittest.IsolatedAsyncioTestCase): + """Identical to the dispatcher test but without mocking.""" + + async def asyncSetUp(self): + print("TestFull.asyncSetUp") + self.log = logging.getLogger() + + # Build the simulation controller and the dispatcher + cfg = Config() + cfg.hostname = "localhost" + cfg.port = 26034 + self.controller = Controller(cfg, simulate=True) + await self.controller.connect() + + self.dispatcher = Dispatcher( + port=1234, log=self.log, controller=self.controller + ) + await self.dispatcher.start_task + + # Connect to the dispatcher + self.client = tcpip.Client( + host=self.dispatcher.host, + port=self.dispatcher.port, + log=self.dispatcher.log, + ) + await self.client.start_task + await self.dispatcher.connected_task + + async def asyncTearDown(self): + print("TestFull.asyncTearDown") + await self.client.close() + await self.dispatcher.close() + await self.controller.stop() + + async def send_and_receive(self, message: str) -> str: + await asyncio.wait_for( + self.client.write_str(message + "\r\n"), timeout=TCP_TIMEOUT + ) + response = await asyncio.wait_for(self.client.read_str(), timeout=TCP_TIMEOUT) + response = response.strip() + return response + + def check_response( + self, response: str, expected_command: str, expected_error: str | None = None + ) -> dict: + print(f"{response=}") + json_data = json.loads(response) + self.assertEqual(json_data["command"], expected_command) + if expected_error is None: + self.assertEqual(json_data["error"], 0) + else: + self.assertNotEqual(json_data["error"], 0) + self.assertEqual(json_data["exception_name"], expected_error) + self.assertTrue("message" in json_data) + + async def test_ping(self): + """Check basic functionality with a ping command.""" + response = await self.send_and_receive("ping") + self.check_response(response, "ping") + + async def test_vent_open(self): + response = await self.send_and_receive("open_vent_gate 0") + self.check_response(response, "open_vent_gate") + self.assertEqual(self.controller.vent_state(0), VentGateState.OPEN) + + async def test_vent_close(self): + response = await self.send_and_receive("close_vent_gate 0") + self.check_response(response, "close_vent_gate") + self.assertEqual(self.controller.vent_state(0), VentGateState.CLOSED) + + async def test_invalid_vent(self): + response = await self.send_and_receive("open_vent_gate 456") + self.check_response(response, "open_vent_gate", "ValueError") + response = await self.send_and_receive("close_vent_gate 123") + self.check_response(response, "close_vent_gate", "ValueError") + + async def test_fan_manual(self): + response = await self.send_and_receive( + "set_extraction_fan_manual_control_mode True" + ) + self.check_response(response, "set_extraction_fan_manual_control_mode") + response = await self.send_and_receive( + "set_extraction_fan_manual_control_mode False" + ) + self.check_response(response, "set_extraction_fan_manual_control_mode") + response = await self.send_and_receive( + "set_extraction_fan_manual_control_mode Nachos" + ) + self.check_response( + response, "set_extraction_fan_manual_control_mode", "ValueError" + ) + response = await self.send_and_receive( + "set_extraction_fan_manual_control_mode sour cream" + ) + self.check_response( + response, "set_extraction_fan_manual_control_mode", "TypeError" + ) + + async def test_start_fan(self): + response = await self.send_and_receive("start_extraction_fan") + self.check_response(response, "start_extraction_fan") + + async def test_stop_fan(self): + response = await self.send_and_receive("start_extraction_fan") + self.check_response(response, "start_extraction_fan") + + async def test_set_fan_frequency(self): + response = await self.send_and_receive("set_extraction_fan_drive_freq 12.5") + self.check_response(response, "set_extraction_fan_drive_freq") + + async def test_fault_recover(self): + response = await self.send_and_receive("reset_extraction_fan_drive") + self.check_response(response, "reset_extraction_fan_drive") diff --git a/tests/test_megaind.py b/tests/test_megaind.py index 34ff591..6b2ac10 100644 --- a/tests/test_megaind.py +++ b/tests/test_megaind.py @@ -1,50 +1,36 @@ -import unittest -from unittest.mock import patch - -from lsst.ts.vent.controller import Config, Controller, VentGateState - -input_bits = [0, 0, 0, 0] - - -def getOptoCh(stack: int, channel: int) -> int: - global input_bits +# This file is part of ts_atbuilding_vents +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (https://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . - channel -= 1 # Channels are 1-indexed - - assert stack == Config.megaind_stack - assert 0 <= channel <= 3 - return input_bits[channel] - - -def setOd(stack: int, channel: int, val: int) -> None: - global input_bits - - channel -= 1 # Channels are 1-indexed +import unittest - assert stack == Config.megaind_stack - assert 0 <= channel <= 3 - assert val == 0 or val == 1 - vent_number = [i for i in range(4) if Config.vent_signal_ch[i] - 1 == channel] - assert len(vent_number) <= 1 - if len(vent_number) == 1: - vent_number = vent_number[0] - op = val - cl = 0 if val else 1 - input_bits[Config.vent_open_limit_ch[vent_number] - 1] = op - input_bits[Config.vent_close_limit_ch[vent_number] - 1] = cl +from lsst.ts.vent.controller import Controller, VentGateState class TestLouvres(unittest.IsolatedAsyncioTestCase): async def asyncSetUp(self): - self.patch_getOptCh = patch("megaind.getOptoCh", getOptoCh) - self.patch_setOd = patch("megaind.setOd", setOd) - self.patch_getOptCh.start() - self.patch_setOd.start() - self.controller = Controller() + self.controller = Controller(simulate=True) + await self.controller.connect() async def asyncTearDown(self): - self.patch_getOptCh.stop() - self.patch_setOd.stop() + await self.controller.stop() async def test_vent_open(self): self.controller.vent_open(0) @@ -55,13 +41,11 @@ async def test_vent_close(self): self.assertEqual(self.controller.vent_state(0), VentGateState.CLOSED) async def test_vent_partiallyopen(self): - global input_bits - input_bits = [0, 0, 0, 0] + self.controller.simulator.set_bits((0, 0, 0, 0)) self.assertEqual(self.controller.vent_state(0), VentGateState.PARTIALLY_OPEN) async def test_vent_invalidstate(self): - global input_bits - input_bits = [1, 1, 1, 1] + self.controller.simulator.set_bits((1, 1, 1, 1)) self.assertEqual(self.controller.vent_state(0), VentGateState.FAULT) def test_vent_invalidvent(self): From 3b45909bdab0bfe2cc53af691af89f51b9db7a08 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Thu, 15 Aug 2024 15:11:41 -0700 Subject: [PATCH 02/15] Modify open_vent_gate and close_vent_gate to take four ints instead of one. --- python/lsst/ts/vent/controller/dispatcher.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 5e78b4b..9baf282 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -74,8 +74,8 @@ def __init__( self, port: int, log: logging.Logger, controller: Controller | None = None ): self.dispatch_dict = { - "close_vent_gate": [int], - "open_vent_gate": [int], + "close_vent_gate": [int, int, int, int], + "open_vent_gate": [int, int, int, int], "reset_extraction_fan_drive": [], "set_extraction_fan_drive_freq": [float], "set_extraction_fan_manual_control_mode": [bool], @@ -167,11 +167,15 @@ async def read_and_dispatch(self) -> None: ) ) - async def close_vent_gate(self, gate: int) -> None: - self.controller.vent_close(gate) + async def close_vent_gate(self, gate0: int, gate1: int, gate2: int, gate3: int) -> None: + for gate in (gate0, gate1, gate2, gate3): + if gate >= 0 and gate <= 3: + self.controller.vent_close(gate) - async def open_vent_gate(self, gate: int) -> None: - self.controller.vent_open(gate) + async def open_vent_gate(self, gate0: int, gate1: int, gate2: int, gate3: int) -> None: + for gate in (gate0, gate1, gate2, gate3): + if gate >= 0 and gate <= 3: + self.controller.vent_open(gate) async def reset_extraction_fan_drive(self) -> None: await self.controller.vfd_fault_reset() From 2d4b001711aee5068925c791eed2ee3304dc0c86 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Thu, 15 Aug 2024 15:29:14 -0700 Subject: [PATCH 03/15] Add tests for open_vent_gate and close_vent_gate with four ints --- python/lsst/ts/vent/controller/dispatcher.py | 6 +++++ tests/test_dispatcher.py | 24 +++++++++++++++----- tests/test_full.py | 8 +++---- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 9baf282..81720c5 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -171,11 +171,17 @@ async def close_vent_gate(self, gate0: int, gate1: int, gate2: int, gate3: int) for gate in (gate0, gate1, gate2, gate3): if gate >= 0 and gate <= 3: self.controller.vent_close(gate) + else: + if gate != -1: + raise ValueError(f"Invalid vent ({gate}) must be between 0 and 3.") async def open_vent_gate(self, gate0: int, gate1: int, gate2: int, gate3: int) -> None: for gate in (gate0, gate1, gate2, gate3): if gate >= 0 and gate <= 3: self.controller.vent_open(gate) + else: + if gate != -1: + raise ValueError(f"Invalid vent ({gate}) must be between 0 and 3.") async def reset_extraction_fan_drive(self) -> None: await self.controller.vfd_fault_reset() diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index 40839f7..d0a7cf6 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -23,7 +23,7 @@ import json import logging import unittest -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, call, patch from lsst.ts import tcpip from lsst.ts.vent.controller import Dispatcher @@ -102,15 +102,27 @@ async def test_ping(self): async def test_close_vent_gate(self): """Test close_vent_gate command.""" - response = await self.send_and_receive("close_vent_gate 123") + response = await self.send_and_receive("close_vent_gate 1 -1 -1 -1") self.check_response(response, "close_vent_gate") - self.mock_controller.vent_close.assert_called_once_with(123) + self.mock_controller.vent_close.assert_called_once_with(1) async def test_open_vent_gate(self): """Test open_vent_gate command.""" - response = await self.send_and_receive("open_vent_gate 234") + response = await self.send_and_receive("open_vent_gate 2 -1 -1 -1") self.check_response(response, "open_vent_gate") - self.mock_controller.vent_open.assert_called_once_with(234) + self.mock_controller.vent_open.assert_called_once_with(2) + + async def test_close_vent_multiple(self): + """Test close_vent_gate command sending it multiple gates.""" + response = await self.send_and_receive("close_vent_gate 1 2 3 -1") + self.check_response(response, "close_vent_gate") + self.mock_controller.vent_close.assert_has_calls([call(1), call(2), call(3)], any_order=False) + + async def test_open_vent_multiple(self): + """Test open_vent_gate command sending it multiple gates.""" + response = await self.send_and_receive("open_vent_gate -1 1 2 3") + self.check_response(response, "open_vent_gate") + self.mock_controller.vent_open.assert_has_calls([call(1), call(2), call(3)], any_order=False) async def test_reset_extraction_fan_drive(self): """Test reset_extraction_fan_drive command.""" @@ -159,7 +171,7 @@ async def test_badcommand(self): async def test_wrongargumenttype(self): """Test with incorrect argument types.""" - response = await self.send_and_receive("close_vent_gate 0.5") + response = await self.send_and_receive("close_vent_gate 0.5 0.5 0.5 0.5") self.check_response(response, "close_vent_gate", "ValueError") async def test_wrongargumentcount(self): diff --git a/tests/test_full.py b/tests/test_full.py index 3896ba3..a5828e8 100644 --- a/tests/test_full.py +++ b/tests/test_full.py @@ -93,19 +93,19 @@ async def test_ping(self): self.check_response(response, "ping") async def test_vent_open(self): - response = await self.send_and_receive("open_vent_gate 0") + response = await self.send_and_receive("open_vent_gate 0 -1 -1 -1") self.check_response(response, "open_vent_gate") self.assertEqual(self.controller.vent_state(0), VentGateState.OPEN) async def test_vent_close(self): - response = await self.send_and_receive("close_vent_gate 0") + response = await self.send_and_receive("close_vent_gate 0 -1 -1 -1") self.check_response(response, "close_vent_gate") self.assertEqual(self.controller.vent_state(0), VentGateState.CLOSED) async def test_invalid_vent(self): - response = await self.send_and_receive("open_vent_gate 456") + response = await self.send_and_receive("open_vent_gate 456 -1 -1 -1") self.check_response(response, "open_vent_gate", "ValueError") - response = await self.send_and_receive("close_vent_gate 123") + response = await self.send_and_receive("close_vent_gate 123 -1 -1 -1") self.check_response(response, "close_vent_gate", "ValueError") async def test_fan_manual(self): From a5401838e5212e31ecea154a72643b9d5355b8de Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Thu, 15 Aug 2024 15:34:02 -0700 Subject: [PATCH 04/15] Remove extraneous prints --- python/lsst/ts/vent/controller/controller.py | 1 - python/lsst/ts/vent/controller/dispatcher.py | 1 - tests/test_dispatcher.py | 3 --- tests/test_full.py | 3 --- 4 files changed, 8 deletions(-) diff --git a/python/lsst/ts/vent/controller/controller.py b/python/lsst/ts/vent/controller/controller.py index 0c33da8..fd8821d 100644 --- a/python/lsst/ts/vent/controller/controller.py +++ b/python/lsst/ts/vent/controller/controller.py @@ -390,7 +390,6 @@ def setOd(self, *args, **kwargs) -> None: """ if self.simulate: - print(f"{args=}") self.simulator.setOd(*args, **kwargs) else: if not MEGAIND_AVAILABLE: diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 81720c5..5c4a5bf 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -192,7 +192,6 @@ async def set_extraction_fan_drive_freq(self, target_frequency: float) -> None: async def set_extraction_fan_manual_control_mode( self, enable_manual_control_mode: bool ) -> None: - print(f"{enable_manual_control_mode=}") await self.controller.fan_manual_control(enable_manual_control_mode) async def start_extraction_fan(self) -> None: diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index d0a7cf6..30f7328 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -34,7 +34,6 @@ class TestDispatcher(unittest.IsolatedAsyncioTestCase): async def asyncSetUp(self): - print("TestDispatcher.asyncSetUp") self.log = logging.getLogger() # Mock the controller @@ -69,7 +68,6 @@ async def asyncSetUp(self): await self.dispatcher.connected_task async def asyncTearDown(self): - print("TestDispatcher.asyncTearDown") await self.client.close() await self.dispatcher.close() self.patcher.stop() @@ -85,7 +83,6 @@ async def send_and_receive(self, message: str) -> str: def check_response( self, response: str, expected_command: str, expected_error: str | None = None ) -> dict: - print(f"{response=}") json_data = json.loads(response) self.assertEqual(json_data["command"], expected_command) if expected_error is None: diff --git a/tests/test_full.py b/tests/test_full.py index a5828e8..edb0c4d 100644 --- a/tests/test_full.py +++ b/tests/test_full.py @@ -36,7 +36,6 @@ class TestFull(unittest.IsolatedAsyncioTestCase): """Identical to the dispatcher test but without mocking.""" async def asyncSetUp(self): - print("TestFull.asyncSetUp") self.log = logging.getLogger() # Build the simulation controller and the dispatcher @@ -61,7 +60,6 @@ async def asyncSetUp(self): await self.dispatcher.connected_task async def asyncTearDown(self): - print("TestFull.asyncTearDown") await self.client.close() await self.dispatcher.close() await self.controller.stop() @@ -77,7 +75,6 @@ async def send_and_receive(self, message: str) -> str: def check_response( self, response: str, expected_command: str, expected_error: str | None = None ) -> dict: - print(f"{response=}") json_data = json.loads(response) self.assertEqual(json_data["command"], expected_command) if expected_error is None: From e78416dcf87e9e7a678c7f8c1249f0fe2c15f824 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Thu, 15 Aug 2024 15:36:16 -0700 Subject: [PATCH 05/15] Lint and isort --- .pre-commit-config.yaml | 28 ++++++++++++++++++++ python/lsst/ts/vent/controller/dispatcher.py | 8 ++++-- tests/test_dispatcher.py | 8 ++++-- 3 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..0b36cbf --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,28 @@ +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.6.0 + hooks: + - id: check-yaml + args: + - "--unsafe" + - id: end-of-file-fixer + - id: trailing-whitespace + - repo: https://github.com/psf/black-pre-commit-mirror + rev: 24.4.2 + hooks: + - id: black + # It is recommended to specify the latest version of Python + # supported by your project here, or alternatively use + # pre-commit's default_language_version, see + # https://pre-commit.com/#top_level-default_language_version + language_version: python3.12 + - repo: https://github.com/pycqa/isort + rev: 5.13.2 + hooks: + - id: isort + name: isort (python) + - repo: https://github.com/astral-sh/ruff-pre-commit + # Ruff version. + rev: v0.5.5 + hooks: + - id: ruff diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 5c4a5bf..27d12a0 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -167,7 +167,9 @@ async def read_and_dispatch(self) -> None: ) ) - async def close_vent_gate(self, gate0: int, gate1: int, gate2: int, gate3: int) -> None: + async def close_vent_gate( + self, gate0: int, gate1: int, gate2: int, gate3: int + ) -> None: for gate in (gate0, gate1, gate2, gate3): if gate >= 0 and gate <= 3: self.controller.vent_close(gate) @@ -175,7 +177,9 @@ async def close_vent_gate(self, gate0: int, gate1: int, gate2: int, gate3: int) if gate != -1: raise ValueError(f"Invalid vent ({gate}) must be between 0 and 3.") - async def open_vent_gate(self, gate0: int, gate1: int, gate2: int, gate3: int) -> None: + async def open_vent_gate( + self, gate0: int, gate1: int, gate2: int, gate3: int + ) -> None: for gate in (gate0, gate1, gate2, gate3): if gate >= 0 and gate <= 3: self.controller.vent_open(gate) diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index 30f7328..d34d052 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -113,13 +113,17 @@ async def test_close_vent_multiple(self): """Test close_vent_gate command sending it multiple gates.""" response = await self.send_and_receive("close_vent_gate 1 2 3 -1") self.check_response(response, "close_vent_gate") - self.mock_controller.vent_close.assert_has_calls([call(1), call(2), call(3)], any_order=False) + self.mock_controller.vent_close.assert_has_calls( + [call(1), call(2), call(3)], any_order=False + ) async def test_open_vent_multiple(self): """Test open_vent_gate command sending it multiple gates.""" response = await self.send_and_receive("open_vent_gate -1 1 2 3") self.check_response(response, "open_vent_gate") - self.mock_controller.vent_open.assert_has_calls([call(1), call(2), call(3)], any_order=False) + self.mock_controller.vent_open.assert_has_calls( + [call(1), call(2), call(3)], any_order=False + ) async def test_reset_extraction_fan_drive(self): """Test reset_extraction_fan_drive command.""" From e47987b71c62eb97f9838f3dfad1e41a5d5f5484 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Sun, 18 Aug 2024 18:05:37 -0700 Subject: [PATCH 06/15] Add telemetry and events --- python/lsst/ts/vent/controller/controller.py | 28 ++++++ python/lsst/ts/vent/controller/dispatcher.py | 89 +++++++++++++++++++- python/lsst/ts/vent/controller/vf_drive.py | 1 + tests/test_dispatcher.py | 61 +++++++++++++- 4 files changed, 174 insertions(+), 5 deletions(-) diff --git a/python/lsst/ts/vent/controller/controller.py b/python/lsst/ts/vent/controller/controller.py index fd8821d..c2eff9f 100644 --- a/python/lsst/ts/vent/controller/controller.py +++ b/python/lsst/ts/vent/controller/controller.py @@ -44,6 +44,12 @@ class VentGateState(IntEnum): FAULT = -1 +class FanDriveState(IntEnum): + STOPPED = 0 + OPERATING = 1 + FAULT = 2 + + class Controller: """A controller that commands the components associated with the AT dome vents and fans. The code in this class is meant to run on the Raspberry Pi @@ -234,6 +240,28 @@ async def vfd_fault_reset(self) -> None: slave=self.cfg.slave, address=address, value=value ) + async def get_drive_state(self) -> FanDriveState: + """Returns the current fan drive state based on the contents + of the IPAE register. + + Raises + ------ + ModbusException + If a communications error occurs. + """ + + ipae = ( + await self.vfd_client.read_holding_registers( + slave=self.cfg.slave, address=vf_drive.Registers.IPAE_REGISTER + ) + ).registers[0] + + if ipae in (0, 1, 2, 3, 5): + return FanDriveState.STOPPED + if ipae == 4: + return FanDriveState.OPERATING + return FanDriveState.FAULT + async def last8faults(self) -> list[tuple[int, str]]: """Returns the last eight fault conditions recorded by the drive. diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 27d12a0..1151ddb 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -19,11 +19,12 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import asyncio import json import logging import traceback -from lsst.ts import tcpip +from lsst.ts import tcpip, utils from .controller import Controller @@ -85,7 +86,14 @@ def __init__( } self.controller = controller if controller is not None else Controller() - super().__init__(port=port, log=log, terminator=b"\r") + self.monitor_sleep_task = utils.make_done_future() + + self.telemetry_count = 0 + self.TELEMETRY_INTERVAL = 100 + + super().__init__( + port=port, log=log, connect_callback=self.on_connect, terminator=b"\r" + ) async def respond(self, message: str) -> None: await self.write_str(message + "\r\n") @@ -206,3 +214,80 @@ async def stop_extraction_fan(self) -> None: async def ping(self) -> None: pass + + async def on_connect(self, bcs: tcpip.BaseClientOrServer) -> None: + if self.connected: + self.log.info("Connected to client.") + asyncio.create_task(self.monitor_status()) + else: + self.log.info("Disconnected from client.") + + async def monitor_status(self) -> None: + vent_state = None + last_fault = None + + while self.connected: + new_vent_state = [int(self.controller.vent_state(i)) for i in range(4)] + last8faults = await self.controller.last8faults() + new_last_fault = last8faults[0] + + # Check whether the vent state has changed + if vent_state != new_vent_state: + self.log.info(f"Vent state changed: {vent_state} -> {new_vent_state}") + data = [int(state) for state in new_vent_state] + await self.respond( + json.dumps( + dict( + command="evt_vent_gate_state", + error=0, + exception_name="", + message="", + traceback="", + data=data, + ) + ) + ) + vent_state = new_vent_state + + # Check whether the last fault has changed + if last_fault != new_last_fault: + self.log.info(f"Last fault changed: {last_fault} -> {new_last_fault}") + await self.respond( + json.dumps( + dict( + command="evt_extraction_fan_drive_fault_code", + error=0, + exception_name="", + message="", + traceback="", + data=new_last_fault, + ) + ) + ) + last_fault = new_last_fault + + # Send telemetry every TELEMETRY_INTERVAL times through the loop + self.telemetry_count -= 1 + if self.telemetry_count < 0: + self.telemetry_count = self.TELEMETRY_INTERVAL + telemetry = { + "tel_extraction_fan": await self.controller.get_fan_frequency(), + } + await self.respond( + json.dumps( + dict( + command="telemetry", + error=0, + exception_name="", + message="", + traceback="", + data=telemetry, + ) + ) + ) + + try: + self.monitor_sleep_task = asyncio.ensure_future(asyncio.sleep(0.1)) + await self.monitor_sleep_task + except asyncio.CancelledError: + continue diff --git a/python/lsst/ts/vent/controller/vf_drive.py b/python/lsst/ts/vent/controller/vf_drive.py index 2d57a1b..59da6a2 100644 --- a/python/lsst/ts/vent/controller/vf_drive.py +++ b/python/lsst/ts/vent/controller/vf_drive.py @@ -32,6 +32,7 @@ class Registers(IntEnum): CMD_REGISTER = 8501 LFR_REGISTER = 8502 LFRD_REGISTER = 8602 + IPAE_REGISTER = 64277 # Manual / auto settings for the variable frequency drive: diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index d34d052..02960ed 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -29,7 +29,7 @@ from lsst.ts.vent.controller import Dispatcher # Standard timeout for TCP/IP messages (sec). -TCP_TIMEOUT = 1 +TCP_TIMEOUT = 10 class TestDispatcher(unittest.IsolatedAsyncioTestCase): @@ -52,10 +52,16 @@ async def asyncSetUp(self): self.mock_controller.vent_close = MagicMock() self.mock_controller.vent_state = MagicMock() + self.mock_controller.get_fan_manual_control.return_value = False + self.mock_controller.get_fan_frequency.return_value = 0.0 + self.mock_controller.last8faults.return_value = [22] * 8 + self.mock_controller.vent_state.return_value = 0 + # Build the dispatcher and wait for it to listen self.dispatcher = Dispatcher( port=1234, log=self.log, controller=self.mock_controller ) + self.dispatcher.TELEMETRY_INTERVAL = 5 await self.dispatcher.start_task # Connect to the dispatcher @@ -72,11 +78,25 @@ async def asyncTearDown(self): await self.dispatcher.close() self.patcher.stop() - async def send_and_receive(self, message: str) -> str: + async def send_and_receive( + self, message: str, pass_event: str | None = None, pass_telemetry: bool = False + ) -> str: await asyncio.wait_for( self.client.write_str(message + "\r\n"), timeout=TCP_TIMEOUT ) - response = await asyncio.wait_for(self.client.read_str(), timeout=TCP_TIMEOUT) + for i in range(1000): + response = await asyncio.wait_for( + self.client.read_str(), timeout=TCP_TIMEOUT + ) + if "evt_" in response: + if pass_event is not None and pass_event in response: + break + elif "tel_" in response: + if pass_telemetry: + break + else: + break + response = response.strip() return response @@ -185,3 +205,38 @@ async def test_wrongargumentcount(self): response = await self.send_and_receive("ping 3.14159") self.check_response(response, "ping", "TypeError") + + async def test_telemetry(self): + """Test that an event is emitted when the gate state changes.""" + response = await self.send_and_receive("", pass_telemetry=True) + self.check_response(response, "telemetry") + response = await self.send_and_receive("", pass_telemetry=True) + self.check_response(response, "telemetry") + + async def test_gate_event(self): + gate_state = [self.mock_controller.vent_state.return_value] * 4 + response = await self.send_and_receive("", pass_event="evt_vent_gate_state") + response_json = json.loads(response) + self.assertEqual(response_json["data"], gate_state) + + self.mock_controller.vent_state.return_value = 1 + gate_state = [self.mock_controller.vent_state.return_value] * 4 + response = await self.send_and_receive("", pass_event="evt_vent_gate_state") + response_json = json.loads(response) + self.assertEqual(response_json["data"], gate_state) + + async def test_drive_fault(self): + fault_code = self.mock_controller.last8faults.return_value[0] + response = await self.send_and_receive( + "", pass_event="evt_extraction_fan_drive_fault_code" + ) + response_json = json.loads(response) + self.assertEqual(response_json["data"], fault_code) + + fault_code = self.mock_controller.last8faults.return_value = [123] * 8 + fault_code = self.mock_controller.last8faults.return_value[0] + response = await self.send_and_receive( + "", pass_event="evt_extraction_fan_drive_fault_code" + ) + response_json = json.loads(response) + self.assertEqual(response_json["data"], fault_code) From 9c972cc46ff27c85204ca9a6767cd375d9fc8bfe Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Mon, 19 Aug 2024 17:16:50 -0700 Subject: [PATCH 07/15] Minor cleanup --- python/lsst/ts/vent/controller/__init__.py | 6 ++++-- tests/test_dispatcher.py | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/python/lsst/ts/vent/controller/__init__.py b/python/lsst/ts/vent/controller/__init__.py index 77a784f..d86e43e 100644 --- a/python/lsst/ts/vent/controller/__init__.py +++ b/python/lsst/ts/vent/controller/__init__.py @@ -20,10 +20,12 @@ # along with this program. If not, see . try: - from .version import * # Generated by sconsUtils + from .version import __version__ # Generated by sconsUtils except ImportError: __version__ = "?" from .config import Config -from .controller import Controller, VentGateState +from .controller import Controller from .dispatcher import Dispatcher + +__all__ = ["__version__", "Config", "Controller", "Dispatcher"] diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index 02960ed..f21a602 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -207,13 +207,14 @@ async def test_wrongargumentcount(self): self.check_response(response, "ping", "TypeError") async def test_telemetry(self): - """Test that an event is emitted when the gate state changes.""" + """Test that telemetry is sent from time to time.""" response = await self.send_and_receive("", pass_telemetry=True) self.check_response(response, "telemetry") response = await self.send_and_receive("", pass_telemetry=True) self.check_response(response, "telemetry") async def test_gate_event(self): + """Test that an event is emitted when the gate state changes.""" gate_state = [self.mock_controller.vent_state.return_value] * 4 response = await self.send_and_receive("", pass_event="evt_vent_gate_state") response_json = json.loads(response) @@ -226,6 +227,7 @@ async def test_gate_event(self): self.assertEqual(response_json["data"], gate_state) async def test_drive_fault(self): + """Test that a drive fault is emitted.""" fault_code = self.mock_controller.last8faults.return_value[0] response = await self.send_and_receive( "", pass_event="evt_extraction_fan_drive_fault_code" From d6f5ceb0390dd34a37068c62c48e1e130ac3da76 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Tue, 20 Aug 2024 09:04:10 -0700 Subject: [PATCH 08/15] Fixes for running with the CSC --- python/lsst/ts/vent/controller/controller.py | 16 +++--- python/lsst/ts/vent/controller/dispatcher.py | 42 +++++++++++++-- .../ts/vent/controller/simulator_setup.json | 53 ++++++++++--------- python/lsst/ts/vent/controller/vf_drive.py | 2 +- 4 files changed, 74 insertions(+), 39 deletions(-) diff --git a/python/lsst/ts/vent/controller/controller.py b/python/lsst/ts/vent/controller/controller.py index c2eff9f..0325606 100644 --- a/python/lsst/ts/vent/controller/controller.py +++ b/python/lsst/ts/vent/controller/controller.py @@ -34,20 +34,20 @@ from .config import Config from .simulate import DomeVentsSimulator -__all__ = ["Controller"] +__all__ = ["Controller", "VentGateState"] class VentGateState(IntEnum): - CLOSED = 0 - PARTIALLY_OPEN = 1 - OPEN = 2 + CLOSED = 1 + PARTIALLY_OPEN = 2 + OPEN = 3 FAULT = -1 class FanDriveState(IntEnum): - STOPPED = 0 - OPERATING = 1 - FAULT = 2 + STOPPED = 1 + OPERATING = 2 + FAULT = 3 class Controller: @@ -280,7 +280,7 @@ async def last8faults(self) -> list[tuple[int, str]]: self.log.debug("last8faults") rvals = await self.vfd_client.read_holding_registers( - slave=1, address=vf_drive.Registers.FAULT_REGISTER, count=8 + slave=self.cfg.slave, address=vf_drive.Registers.FAULT_REGISTER, count=8 ) return [(r, vf_drive.FAULTS[r]) for r in rvals.registers] diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 1151ddb..672fc3a 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -26,7 +26,7 @@ from lsst.ts import tcpip, utils -from .controller import Controller +from .controller import Controller, VentGateState def _cast_string_to_type(new_type: type, value: str): @@ -225,11 +225,26 @@ async def on_connect(self, bcs: tcpip.BaseClientOrServer) -> None: async def monitor_status(self) -> None: vent_state = None last_fault = None + fan_drive_state = None while self.connected: - new_vent_state = [int(self.controller.vent_state(i)) for i in range(4)] - last8faults = await self.controller.last8faults() - new_last_fault = last8faults[0] + try: + new_vent_state = [VentGateState.CLOSED] * 4 + for i in range(4): + try: + new_vent_state[i] = self.controller.vent_state(i) + except ValueError: + # Ignore non-configured vents + pass + last8faults = await self.controller.last8faults() + new_last_fault = last8faults[0][ + 0 + ] # controller.last8faults returns tuple[int, str] + + new_fan_drive_state = await self.controller.get_drive_state() + except Exception as e: + self.log.exception(e) + # Do not re-raise, so that the loop will continue # Check whether the vent state has changed if vent_state != new_vent_state: @@ -266,6 +281,25 @@ async def monitor_status(self) -> None: ) last_fault = new_last_fault + # Check whether the fan drive state has changed + if fan_drive_state != new_fan_drive_state: + self.log.debug( + f"Fan drive state changed: {fan_drive_state} -> {new_fan_drive_state}" + ) + await self.respond( + json.dumps( + dict( + command="evt_extraction_fan_drive_state", + error=0, + exception_name="", + message="", + traceback="", + data=new_fan_drive_state, + ) + ) + ) + fan_drive_state = new_fan_drive_state + # Send telemetry every TELEMETRY_INTERVAL times through the loop self.telemetry_count -= 1 if self.telemetry_count < 0: diff --git a/python/lsst/ts/vent/controller/simulator_setup.json b/python/lsst/ts/vent/controller/simulator_setup.json index 0fcf9c9..d188caf 100644 --- a/python/lsst/ts/vent/controller/simulator_setup.json +++ b/python/lsst/ts/vent/controller/simulator_setup.json @@ -10,10 +10,10 @@ "device_list": { "device": { "setup": { - "co size": 10000, - "di size": 10000, - "hr size": 10000, - "ir size": 10000, + "co size": 0, + "di size": 0, + "hr size": 100000, + "ir size": 0, "shared blocks": true, "type exception": true, "defaults": { @@ -24,30 +24,31 @@ "float32": 0.0, "string": " " }, - "action": { - "bits": null, - "uint16": null, - "uint32": null, - "float32": null, - "string": null - } + "action": { + "bits": null, + "uint16": null, + "uint32": null, + "float32": null, + "string": null + } } }, - "invalid": [], - "write": [ 7010, 7124, 8401, 8413, 8423, 8501, 8502, 8602], - "bits": [], - "uint16": [ - { "addr": 7010, "value": 1 }, - { "addr": 7124, "value": 0 }, - { "addr": [ 7201, 7211 ], "value": 22 }, - { "addr": 8401, "value": 1 }, - { "addr": 8413, "value": 1 }, - { "addr": 8423, "value": 1 }, - { "addr": 8501, "value": 0 }, - { "addr": 8502, "value": 0 }, - { "addr": 8602, "value": 0 } - ], - "uint32": [], + "invalid": [], + "write": [ 7010, 7124, 8401, 8413, 8423, 8501, 8502, 8602, 64279], + "bits": [], + "uint16": [ + { "addr": 7010, "value": 1 }, + { "addr": 7124, "value": 0 }, + { "addr": [ 7201, 7211 ], "value": 22 }, + { "addr": 8401, "value": 1 }, + { "addr": 8413, "value": 1 }, + { "addr": 8423, "value": 1 }, + { "addr": 8501, "value": 0 }, + { "addr": 8502, "value": 0 }, + { "addr": 8602, "value": 0 }, + { "addr": 64279, "value": 0 } + ], + "uint32": [], "float32": [], "string": [], "repeat": [] diff --git a/python/lsst/ts/vent/controller/vf_drive.py b/python/lsst/ts/vent/controller/vf_drive.py index 59da6a2..b16a69b 100644 --- a/python/lsst/ts/vent/controller/vf_drive.py +++ b/python/lsst/ts/vent/controller/vf_drive.py @@ -32,7 +32,7 @@ class Registers(IntEnum): CMD_REGISTER = 8501 LFR_REGISTER = 8502 LFRD_REGISTER = 8602 - IPAE_REGISTER = 64277 + IPAE_REGISTER = 64279 # Manual / auto settings for the variable frequency drive: From b7739abaffcd244bbfd823e53c7056eac28b1849 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Tue, 20 Aug 2024 10:04:58 -0700 Subject: [PATCH 09/15] Behavioral improvements --- python/lsst/ts/vent/controller/controller.py | 20 +++++++++++++++++++- python/lsst/ts/vent/controller/simulate.py | 4 +++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/python/lsst/ts/vent/controller/controller.py b/python/lsst/ts/vent/controller/controller.py index 0325606..d8f9139 100644 --- a/python/lsst/ts/vent/controller/controller.py +++ b/python/lsst/ts/vent/controller/controller.py @@ -63,6 +63,8 @@ def __init__(self, config: Config = Config(), simulate: bool = False): self.log = logging.getLogger(type(self).__name__) self.simulate = simulate self.simulator = DomeVentsSimulator(self.cfg) if simulate else None + self.vfd_client = None + self.connected = False async def connect(self) -> None: """Connects to the variable frequency drive via modbus. @@ -79,6 +81,7 @@ async def connect(self) -> None: self.cfg.hostname, port=self.cfg.port ) await self.vfd_client.connect() + self.connected = True async def stop(self) -> None: """Disconnects from the variable frequency drive, and stops @@ -87,7 +90,8 @@ async def stop(self) -> None: if self.simulate: await self.simulator.stop() - self.vfd_client.close() + if self.vfd_client is not None: + self.vfd_client.close() async def get_fan_manual_control(self) -> bool: """Returns the variable frequency drive setting for manual @@ -109,6 +113,7 @@ async def get_fan_manual_control(self) -> bool: """ self.log.debug("get fan_manual_control") + assert self.connected settings = tuple( [ ( @@ -143,6 +148,7 @@ async def fan_manual_control(self, manual: bool) -> None: """ self.log.debug("set vfd_manual_control") + assert self.connected settings = vf_drive.MANUAL if manual else vf_drive.AUTO for address, value in zip(vf_drive.CFG_REGISTERS, settings): await self.vfd_client.write_register( @@ -158,6 +164,7 @@ async def start_fan(self): If a communications error occurs. """ self.log.debug("start_fan()") + assert self.connected await self.set_fan_frequency(self.default_fan_frequency) async def stop_fan(self): @@ -169,6 +176,7 @@ async def stop_fan(self): If a communications error occurs. """ self.log.debug("stop_fan()") + assert self.connected await self.set_fan_frequency(0.0) async def get_fan_frequency(self) -> float: @@ -181,6 +189,7 @@ async def get_fan_frequency(self) -> float: """ self.log.debug("get fan_frequency") + assert self.connected cmd = ( await self.vfd_client.read_holding_registers( slave=self.cfg.slave, address=vf_drive.Registers.CMD_REGISTER @@ -214,6 +223,7 @@ async def set_fan_frequency(self, frequency: float) -> None: If a communications error occurs. """ self.log.debug("set fan_frequency") + assert self.connected if not 0 <= frequency <= self.cfg.max_freq: raise ValueError(f"Frequency must be between 0 and {self.cfg.max_freq}") @@ -235,6 +245,7 @@ async def vfd_fault_reset(self) -> None: If a communications error occurs. """ + assert self.connected for address, value in vf_drive.FAULT_RESET_SEQUENCE: await self.vfd_client.write_register( slave=self.cfg.slave, address=address, value=value @@ -250,6 +261,7 @@ async def get_drive_state(self) -> FanDriveState: If a communications error occurs. """ + assert self.connected ipae = ( await self.vfd_client.read_holding_registers( slave=self.cfg.slave, address=vf_drive.Registers.IPAE_REGISTER @@ -279,6 +291,7 @@ async def last8faults(self) -> list[tuple[int, str]]: """ self.log.debug("last8faults") + assert self.connected rvals = await self.vfd_client.read_holding_registers( slave=self.cfg.slave, address=vf_drive.Registers.FAULT_REGISTER, count=8 ) @@ -306,6 +319,7 @@ def vent_open(self, vent_number: int) -> None: """ self.log.debug(f"vent_open({vent_number})") + assert self.connected if not 0 <= vent_number <= 3: raise ValueError(f"Invalid {vent_number=} should be between 0 and 3") if self.cfg.vent_signal_ch[vent_number] == -1: @@ -334,6 +348,7 @@ def vent_close(self, vent_number: int) -> None: """ self.log.debug(f"vent_close({vent_number})") + assert self.connected if not 0 <= vent_number <= 3: raise ValueError(f"Invalid {vent_number=} should be between 0 and 3") if self.cfg.vent_signal_ch[vent_number] == -1: @@ -362,6 +377,7 @@ def vent_state(self, vent_number: int) -> VentGateState: """ self.log.debug(f"vent_state({vent_number})") + assert self.connected if not 0 <= vent_number <= 3: raise ValueError(f"Invalid {vent_number=} should be between 0 and 3") @@ -399,6 +415,7 @@ def getOptoCh(self, *args, **kwargs) -> int: daughterboard cannot be controlled. """ + assert self.connected if self.simulate: return self.simulator.getOptoCh(*args, **kwargs) else: @@ -417,6 +434,7 @@ def setOd(self, *args, **kwargs) -> None: daughterboard cannot be controlled. """ + assert self.connected if self.simulate: self.simulator.setOd(*args, **kwargs) else: diff --git a/python/lsst/ts/vent/controller/simulate.py b/python/lsst/ts/vent/controller/simulate.py index 3ccada9..4f9c9ff 100644 --- a/python/lsst/ts/vent/controller/simulate.py +++ b/python/lsst/ts/vent/controller/simulate.py @@ -20,6 +20,7 @@ # along with this program. If not, see . import os +import random from pymodbus.server import ModbusSimulatorServer @@ -31,11 +32,12 @@ def __init__(self, config: Config): self.input_bits = [0, 0, 0, 0] self.cfg = config + self.http_port = random.randint(1024, 65535) self.modbus_simulator = ModbusSimulatorServer( modbus_server="server", modbus_device="device", http_host="localhost", - http_port=25074, + http_port=self.http_port, json_file=os.path.dirname(__file__) + "/simulator_setup.json", ) From 6c533a92f8cd816975f10a71ad355bbe88c9f3e8 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Tue, 20 Aug 2024 10:38:36 -0700 Subject: [PATCH 10/15] Fix broken tests --- tests/test_dispatcher.py | 15 +++++++++++---- tests/test_full.py | 24 +++++++++++++++++++----- tests/test_megaind.py | 7 +++++-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index f21a602..4e59c40 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -27,6 +27,7 @@ from lsst.ts import tcpip from lsst.ts.vent.controller import Dispatcher +from lsst.ts.xml.enums.ATBuilding import FanDriveState # Standard timeout for TCP/IP messages (sec). TCP_TIMEOUT = 10 @@ -48,14 +49,18 @@ async def asyncSetUp(self): self.mock_controller.set_fan_frequency = AsyncMock() self.mock_controller.vfd_fault_reset = AsyncMock() self.mock_controller.last8faults = AsyncMock() + self.mock_controller.get_drive_state = AsyncMock() self.mock_controller.vent_open = MagicMock() self.mock_controller.vent_close = MagicMock() self.mock_controller.vent_state = MagicMock() self.mock_controller.get_fan_manual_control.return_value = False self.mock_controller.get_fan_frequency.return_value = 0.0 - self.mock_controller.last8faults.return_value = [22] * 8 + self.mock_controller.last8faults.return_value = [ + (22, "Description of error") + ] * 8 self.mock_controller.vent_state.return_value = 0 + self.mock_controller.get_drive_state.return_value = FanDriveState.STOPPED # Build the dispatcher and wait for it to listen self.dispatcher = Dispatcher( @@ -228,15 +233,17 @@ async def test_gate_event(self): async def test_drive_fault(self): """Test that a drive fault is emitted.""" - fault_code = self.mock_controller.last8faults.return_value[0] + fault_code = self.mock_controller.last8faults.return_value[0][0] response = await self.send_and_receive( "", pass_event="evt_extraction_fan_drive_fault_code" ) response_json = json.loads(response) self.assertEqual(response_json["data"], fault_code) - fault_code = self.mock_controller.last8faults.return_value = [123] * 8 - fault_code = self.mock_controller.last8faults.return_value[0] + self.mock_controller.last8faults.return_value = [ + (123, "Description of error") + ] * 8 + fault_code = self.mock_controller.last8faults.return_value[0][0] response = await self.send_and_receive( "", pass_event="evt_extraction_fan_drive_fault_code" ) diff --git a/tests/test_full.py b/tests/test_full.py index edb0c4d..e5b6532 100644 --- a/tests/test_full.py +++ b/tests/test_full.py @@ -25,8 +25,8 @@ import unittest from lsst.ts import tcpip -from lsst.ts.vent.controller import (Config, Controller, Dispatcher, - VentGateState) +from lsst.ts.vent.controller import Config, Controller, Dispatcher +from lsst.ts.xml.enums.ATBuilding import VentGateState # Standard timeout for TCP/IP messages (sec). TCP_TIMEOUT = 1 @@ -64,11 +64,25 @@ async def asyncTearDown(self): await self.dispatcher.close() await self.controller.stop() - async def send_and_receive(self, message: str) -> str: + async def send_and_receive( + self, message: str, pass_event: str | None = None, pass_telemetry: bool = False + ) -> str: await asyncio.wait_for( self.client.write_str(message + "\r\n"), timeout=TCP_TIMEOUT ) - response = await asyncio.wait_for(self.client.read_str(), timeout=TCP_TIMEOUT) + for i in range(1000): + response = await asyncio.wait_for( + self.client.read_str(), timeout=TCP_TIMEOUT + ) + if "evt_" in response: + if pass_event is not None and pass_event in response: + break + elif "tel_" in response: + if pass_telemetry: + break + else: + break + response = response.strip() return response @@ -92,7 +106,7 @@ async def test_ping(self): async def test_vent_open(self): response = await self.send_and_receive("open_vent_gate 0 -1 -1 -1") self.check_response(response, "open_vent_gate") - self.assertEqual(self.controller.vent_state(0), VentGateState.OPEN) + self.assertEqual(self.controller.vent_state(0), VentGateState.OPENED) async def test_vent_close(self): response = await self.send_and_receive("close_vent_gate 0 -1 -1 -1") diff --git a/tests/test_megaind.py b/tests/test_megaind.py index 6b2ac10..1e50dac 100644 --- a/tests/test_megaind.py +++ b/tests/test_megaind.py @@ -21,7 +21,8 @@ import unittest -from lsst.ts.vent.controller import Controller, VentGateState +from lsst.ts.vent.controller import Controller +from lsst.ts.xml.enums.ATBuilding import VentGateState class TestLouvres(unittest.IsolatedAsyncioTestCase): @@ -34,7 +35,7 @@ async def asyncTearDown(self): async def test_vent_open(self): self.controller.vent_open(0) - self.assertEqual(self.controller.vent_state(0), VentGateState.OPEN) + self.assertEqual(self.controller.vent_state(0), VentGateState.OPENED) async def test_vent_close(self): self.controller.vent_close(0) @@ -44,7 +45,9 @@ async def test_vent_partiallyopen(self): self.controller.simulator.set_bits((0, 0, 0, 0)) self.assertEqual(self.controller.vent_state(0), VentGateState.PARTIALLY_OPEN) + @unittest.expectedFailure async def test_vent_invalidstate(self): + # There currently is no FAULT attribute in the VentGateState enum self.controller.simulator.set_bits((1, 1, 1, 1)) self.assertEqual(self.controller.vent_state(0), VentGateState.FAULT) From 87ebe93d7ec5e7ae3f3d4ad1ba9b5b9756820bcc Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Wed, 21 Aug 2024 14:44:17 -0700 Subject: [PATCH 11/15] Changes responsive to comments by @tribeiro --- .pre-commit-config.yaml | 28 ---- python/lsst/ts/vent/controller/__init__.py | 2 - python/lsst/ts/vent/controller/config.py | 4 +- python/lsst/ts/vent/controller/controller.py | 155 ++++++++++++------ python/lsst/ts/vent/controller/dispatcher.py | 8 +- .../{simulate.py => dome_vents_simulator.py} | 4 +- 6 files changed, 113 insertions(+), 88 deletions(-) delete mode 100644 .pre-commit-config.yaml rename python/lsst/ts/vent/controller/{simulate.py => dome_vents_simulator.py} (96%) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml deleted file mode 100644 index 0b36cbf..0000000 --- a/.pre-commit-config.yaml +++ /dev/null @@ -1,28 +0,0 @@ -repos: - - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.6.0 - hooks: - - id: check-yaml - args: - - "--unsafe" - - id: end-of-file-fixer - - id: trailing-whitespace - - repo: https://github.com/psf/black-pre-commit-mirror - rev: 24.4.2 - hooks: - - id: black - # It is recommended to specify the latest version of Python - # supported by your project here, or alternatively use - # pre-commit's default_language_version, see - # https://pre-commit.com/#top_level-default_language_version - language_version: python3.12 - - repo: https://github.com/pycqa/isort - rev: 5.13.2 - hooks: - - id: isort - name: isort (python) - - repo: https://github.com/astral-sh/ruff-pre-commit - # Ruff version. - rev: v0.5.5 - hooks: - - id: ruff diff --git a/python/lsst/ts/vent/controller/__init__.py b/python/lsst/ts/vent/controller/__init__.py index d86e43e..8308a16 100644 --- a/python/lsst/ts/vent/controller/__init__.py +++ b/python/lsst/ts/vent/controller/__init__.py @@ -27,5 +27,3 @@ from .config import Config from .controller import Controller from .dispatcher import Dispatcher - -__all__ = ["__version__", "Config", "Controller", "Dispatcher"] diff --git a/python/lsst/ts/vent/controller/config.py b/python/lsst/ts/vent/controller/config.py index 9348e9d..7457308 100644 --- a/python/lsst/ts/vent/controller/config.py +++ b/python/lsst/ts/vent/controller/config.py @@ -27,8 +27,8 @@ class Config: port = 502 """The default TCP port to connect to via modbus-TCP.""" - slave = 1 - """The default modbus slave ID for the variable frequency drive.""" + device_id = 1 + """The default modbus device ID for the variable frequency drive.""" max_freq = 50.0 """Default maximum frequency for the dome fans.""" diff --git a/python/lsst/ts/vent/controller/controller.py b/python/lsst/ts/vent/controller/controller.py index d8f9139..7a62bba 100644 --- a/python/lsst/ts/vent/controller/controller.py +++ b/python/lsst/ts/vent/controller/controller.py @@ -20,7 +20,8 @@ # along with this program. If not, see . import logging -from enum import IntEnum + +from lsst.ts.xml.enums.ATBuilding import FanDriveState, VentGateState try: import megaind @@ -32,22 +33,9 @@ from . import vf_drive from .config import Config -from .simulate import DomeVentsSimulator - -__all__ = ["Controller", "VentGateState"] - +from .dome_vents_simulator import DomeVentsSimulator -class VentGateState(IntEnum): - CLOSED = 1 - PARTIALLY_OPEN = 2 - OPEN = 3 - FAULT = -1 - - -class FanDriveState(IntEnum): - STOPPED = 1 - OPERATING = 2 - FAULT = 3 +__all__ = ["Controller"] class Controller: @@ -57,12 +45,11 @@ class Controller: https://confluence.lsstcorp.org/display/~fritzm/Auxtel+Vent+Gate+Automation. """ - def __init__(self, config: Config = Config(), simulate: bool = False): - self.cfg = config - self.default_fan_frequency = self.cfg.max_freq + def __init__(self, config: Config | None = None, simulate: bool = False): + self.config = config if config is not None else Config() + self.default_fan_frequency = self.config.max_freq self.log = logging.getLogger(type(self).__name__) - self.simulate = simulate - self.simulator = DomeVentsSimulator(self.cfg) if simulate else None + self.simulator = DomeVentsSimulator(self.config) if simulate else None self.vfd_client = None self.connected = False @@ -74,11 +61,11 @@ async def connect(self) -> None: ModbusException If the variable frequency drive is not available. """ - if self.simulate: + if self.simulator is not None: await self.simulator.start() self.vfd_client = pymodbus.client.AsyncModbusTcpClient( - self.cfg.hostname, port=self.cfg.port + self.config.hostname, port=self.config.port ) await self.vfd_client.connect() self.connected = True @@ -87,7 +74,7 @@ async def stop(self) -> None: """Disconnects from the variable frequency drive, and stops the simulator if simulating. """ - if self.simulate: + if self.simulator is not None: await self.simulator.stop() if self.vfd_client is not None: @@ -105,6 +92,9 @@ async def get_fan_manual_control(self) -> bool: Raises ------ + AssertionError + If the controller is not connected. + ValueError If the drive settings don't match either profile. @@ -118,7 +108,7 @@ async def get_fan_manual_control(self) -> bool: [ ( await self.vfd_client.read_holding_registers( - slave=self.cfg.slave, address=addr + slave=self.config.device_id, address=addr ) ).registers[0] for addr in vf_drive.CFG_REGISTERS @@ -143,6 +133,9 @@ async def fan_manual_control(self, manual: bool) -> None: Raises ------ + AssertionError + If the controller is not connected. + ModbusException If a communications error occurs. """ @@ -152,7 +145,7 @@ async def fan_manual_control(self, manual: bool) -> None: settings = vf_drive.MANUAL if manual else vf_drive.AUTO for address, value in zip(vf_drive.CFG_REGISTERS, settings): await self.vfd_client.write_register( - slave=self.cfg.slave, address=address, value=value + slave=self.config.device_id, address=address, value=value ) async def start_fan(self): @@ -160,6 +153,9 @@ async def start_fan(self): Raises ------ + AssertionError + If the controller is not connected. + ModbusException If a communications error occurs. """ @@ -172,6 +168,9 @@ async def stop_fan(self): Raises ------ + AssertionError + If the controller is not connected. + ModbusException If a communications error occurs. """ @@ -184,6 +183,9 @@ async def get_fan_frequency(self) -> float: Raises ------ + AssertionError + If the controller is not connected. + ModbusException If a communications error occurs. """ @@ -192,7 +194,7 @@ async def get_fan_frequency(self) -> float: assert self.connected cmd = ( await self.vfd_client.read_holding_registers( - slave=self.cfg.slave, address=vf_drive.Registers.CMD_REGISTER + slave=self.config.device_id, address=vf_drive.Registers.CMD_REGISTER ) ).registers[0] if cmd == 0: @@ -200,7 +202,7 @@ async def get_fan_frequency(self) -> float: lfr = ( await self.vfd_client.read_holding_registers( - slave=self.cfg.slave, address=vf_drive.Registers.LFR_REGISTER + slave=self.config.device_id, address=vf_drive.Registers.LFR_REGISTER ) ).registers[0] return 0.1 * lfr @@ -216,6 +218,9 @@ async def set_fan_frequency(self, frequency: float) -> None: Raises ------ + AssertionError + If the controller is not connected. + ValueError If the frequency is not between zero and MAX_FREQ. @@ -224,8 +229,8 @@ async def set_fan_frequency(self, frequency: float) -> None: """ self.log.debug("set fan_frequency") assert self.connected - if not 0 <= frequency <= self.cfg.max_freq: - raise ValueError(f"Frequency must be between 0 and {self.cfg.max_freq}") + if not 0 <= frequency <= self.config.max_freq: + raise ValueError(f"Frequency must be between 0 and {self.config.max_freq}") settings = { vf_drive.Registers.CMD_REGISTER: 0 if frequency == 0.0 else 1, @@ -233,7 +238,7 @@ async def set_fan_frequency(self, frequency: float) -> None: } for address, value in settings.items(): await self.vfd_client.write_register( - slave=self.cfg.slave, address=address, value=value + slave=self.config.device_id, address=address, value=value ) async def vfd_fault_reset(self) -> None: @@ -241,6 +246,9 @@ async def vfd_fault_reset(self) -> None: Raises ------ + AssertionError + If the controller is not connected. + ModbusException If a communications error occurs. """ @@ -248,23 +256,42 @@ async def vfd_fault_reset(self) -> None: assert self.connected for address, value in vf_drive.FAULT_RESET_SEQUENCE: await self.vfd_client.write_register( - slave=self.cfg.slave, address=address, value=value + slave=self.config.device_id, address=address, value=value ) async def get_drive_state(self) -> FanDriveState: """Returns the current fan drive state based on the contents - of the IPAE register. + of the IPAE register (described as "IPar Status" in the Schneider + Electric ATV320 manual). The IPAE register can have the following + values: + * 0 [Idle State] (IDLE) - Idle State + * 1 [Init] (INIT) - Init + * 2 [Configuration] (CONF) - Configuration + * 3 [Ready] (RDY) - Ready + * 4 [Operational] (OPE) - Operational + * 5 [Not Configured] (UCFG) - Not Configured + * 6 [Unrecoverable Error] (UREC) - Unrecoverable error Raises ------ + AssertionError + If the controller is not connected. + ModbusException If a communications error occurs. + + Returns + ------- + FanDriveState + The current fan drive state based on the contents of the + IPAE register (described as "IPar Status" in the Schneider + Electric ATV320 manual). """ assert self.connected ipae = ( await self.vfd_client.read_holding_registers( - slave=self.cfg.slave, address=vf_drive.Registers.IPAE_REGISTER + slave=self.config.device_id, address=vf_drive.Registers.IPAE_REGISTER ) ).registers[0] @@ -286,6 +313,9 @@ async def last8faults(self) -> list[tuple[int, str]]: Raises ------ + AssertionError + If the controller is not connected. + ModbusException If a communications error occurs. """ @@ -293,7 +323,9 @@ async def last8faults(self) -> list[tuple[int, str]]: self.log.debug("last8faults") assert self.connected rvals = await self.vfd_client.read_holding_registers( - slave=self.cfg.slave, address=vf_drive.Registers.FAULT_REGISTER, count=8 + slave=self.config.device_id, + address=vf_drive.Registers.FAULT_REGISTER, + count=8, ) return [(r, vf_drive.FAULTS[r]) for r in rvals.registers] @@ -307,6 +339,9 @@ def vent_open(self, vent_number: int) -> None: Raises ------ + AssertionError + If the controller is not connected. + ModuleNotFoundError If the megaind module has not been installed, in which case the daughterboard cannot be controlled. @@ -322,9 +357,11 @@ def vent_open(self, vent_number: int) -> None: assert self.connected if not 0 <= vent_number <= 3: raise ValueError(f"Invalid {vent_number=} should be between 0 and 3") - if self.cfg.vent_signal_ch[vent_number] == -1: + if self.config.vent_signal_ch[vent_number] == -1: raise ValueError(f"Vent {vent_number=} is not configured.") - self.setOd(self.cfg.megaind_stack, self.cfg.vent_signal_ch[vent_number], 1) + self.set_od( + self.config.megaind_stack, self.config.vent_signal_ch[vent_number], 1 + ) def vent_close(self, vent_number: int) -> None: """Closes the specified vent. @@ -336,6 +373,9 @@ def vent_close(self, vent_number: int) -> None: Raises ------ + AssertionError + If the controller is not connected. + ModuleNotFoundError If the megaind module has not been installed, in which case the daughterboard cannot be controlled. @@ -351,9 +391,11 @@ def vent_close(self, vent_number: int) -> None: assert self.connected if not 0 <= vent_number <= 3: raise ValueError(f"Invalid {vent_number=} should be between 0 and 3") - if self.cfg.vent_signal_ch[vent_number] == -1: + if self.config.vent_signal_ch[vent_number] == -1: raise ValueError(f"Vent {vent_number=} is not configured.") - self.setOd(self.cfg.megaind_stack, self.cfg.vent_signal_ch[vent_number], 0) + self.set_od( + self.config.megaind_stack, self.config.vent_signal_ch[vent_number], 0 + ) def vent_state(self, vent_number: int) -> VentGateState: """Returns the state of the specified vent. @@ -365,6 +407,9 @@ def vent_state(self, vent_number: int) -> VentGateState: Raises ------ + AssertionError + If the controller is not connected. + ModuleNotFoundError If the megaind module has not been installed, in which case the daughterboard cannot be controlled. @@ -382,21 +427,21 @@ def vent_state(self, vent_number: int) -> VentGateState: raise ValueError(f"Invalid {vent_number=} should be between 0 and 3") if ( - self.cfg.vent_open_limit_ch[vent_number] == -1 - or self.cfg.vent_close_limit_ch[vent_number] == -1 + self.config.vent_open_limit_ch[vent_number] == -1 + or self.config.vent_close_limit_ch[vent_number] == -1 ): raise ValueError(f"Vent {vent_number=} is not configured.") - op_state = self.getOptoCh( - self.cfg.megaind_stack, self.cfg.vent_open_limit_ch[vent_number] + op_state = self.get_opto_ch( + self.config.megaind_stack, self.config.vent_open_limit_ch[vent_number] ) - cl_state = self.getOptoCh( - self.cfg.megaind_stack, self.cfg.vent_close_limit_ch[vent_number] + cl_state = self.get_opto_ch( + self.config.megaind_stack, self.config.vent_close_limit_ch[vent_number] ) match op_state, cl_state: case 1, 0: - return VentGateState.OPEN + return VentGateState.OPENED case 0, 0: return VentGateState.PARTIALLY_OPEN case 0, 1: @@ -404,39 +449,45 @@ def vent_state(self, vent_number: int) -> VentGateState: case _: return VentGateState.FAULT - def getOptoCh(self, *args, **kwargs) -> int: + def get_opto_ch(self, *args, **kwargs) -> int: """Calls megaind.getOptoCh or a simulated getOptoCh depending whether the class was instantiated with simulate = True. Raises ------ + AssertionError + If the controller is not connected. + ModuleNotFoundError If the megaind module has not been installed, in which case the daughterboard cannot be controlled. """ assert self.connected - if self.simulate: - return self.simulator.getOptoCh(*args, **kwargs) + if self.simulator is not None: + return self.simulator.get_opto_ch(*args, **kwargs) else: if not MEGAIND_AVAILABLE: raise ModuleNotFoundError("The megaind module is not available.") return megaind.getOptoCh(*args, **kwargs) - def setOd(self, *args, **kwargs) -> None: + def set_od(self, *args, **kwargs) -> None: """Calls megaind.setOd or a simulated setOd depending whether the class was instantiated with simulate = True. Raises ------ + AssertionError + If the controller is not connected. + ModuleNotFoundError If the megaind module has not been installed, in which case the daughterboard cannot be controlled. """ assert self.connected - if self.simulate: - self.simulator.setOd(*args, **kwargs) + if self.simulator is not None: + self.simulator.set_od(*args, **kwargs) else: if not MEGAIND_AVAILABLE: raise ModuleNotFoundError("The megaind module is not available.") diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 672fc3a..bb05af1 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -51,9 +51,13 @@ def _cast_string_to_type(new_type: type, value: str): if new_type is bool: # Boolean is a special case if value.lower() in ("true", "t", "1"): return True - if value.lower() in ("false", "f", "0"): + elif value.lower() in ("false", "f", "0"): return False - raise ValueError(f"Expected bool value but got {value}") + raise ValueError( + "Expected bool value " + + "('true', 't', '1', 'false', 'f', '0')" + + " but got {value}" + ) return new_type(value) diff --git a/python/lsst/ts/vent/controller/simulate.py b/python/lsst/ts/vent/controller/dome_vents_simulator.py similarity index 96% rename from python/lsst/ts/vent/controller/simulate.py rename to python/lsst/ts/vent/controller/dome_vents_simulator.py index 4f9c9ff..d5cf96a 100644 --- a/python/lsst/ts/vent/controller/simulate.py +++ b/python/lsst/ts/vent/controller/dome_vents_simulator.py @@ -47,7 +47,7 @@ async def start(self): async def stop(self): await self.modbus_simulator.stop() - def getOptoCh(self, stack: int, channel: int) -> int: + def get_opto_ch(self, stack: int, channel: int) -> int: """Simulates the behavior of megaind.getOptoCh, as if connected to dome vents configured as described in config.py. @@ -68,7 +68,7 @@ def getOptoCh(self, stack: int, channel: int) -> int: assert 0 <= channel <= 3 return self.input_bits[channel] - def setOd(self, stack: int, channel: int, val: int) -> None: + def set_od(self, stack: int, channel: int, val: int) -> None: """Simulates the behavior of megaind.setOd, as if connected to dome vents configured as described in config.py. From 4fdd35e1aba8f3dd649e80192d4d20f9c6a0949d Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Thu, 22 Aug 2024 16:10:08 -0700 Subject: [PATCH 12/15] Remove megaind dependency --- python/lsst/ts/vent/controller/controller.py | 45 +++++--------------- 1 file changed, 10 insertions(+), 35 deletions(-) diff --git a/python/lsst/ts/vent/controller/controller.py b/python/lsst/ts/vent/controller/controller.py index 7a62bba..c74e1a0 100644 --- a/python/lsst/ts/vent/controller/controller.py +++ b/python/lsst/ts/vent/controller/controller.py @@ -22,14 +22,7 @@ import logging from lsst.ts.xml.enums.ATBuilding import FanDriveState, VentGateState - -try: - import megaind - - MEGAIND_AVAILABLE = True -except ImportError: - MEGAIND_AVAILABLE = False -import pymodbus.client +from pymodbus.client import AsyncModbusTcpClient from . import vf_drive from .config import Config @@ -64,7 +57,7 @@ async def connect(self) -> None: if self.simulator is not None: await self.simulator.start() - self.vfd_client = pymodbus.client.AsyncModbusTcpClient( + self.vfd_client = AsyncModbusTcpClient( self.config.hostname, port=self.config.port ) await self.vfd_client.connect() @@ -342,10 +335,6 @@ def vent_open(self, vent_number: int) -> None: AssertionError If the controller is not connected. - ModuleNotFoundError - If the megaind module has not been installed, in which case the - daughterboard cannot be controlled. - ValueError If vent_number is invalid. @@ -376,10 +365,6 @@ def vent_close(self, vent_number: int) -> None: AssertionError If the controller is not connected. - ModuleNotFoundError - If the megaind module has not been installed, in which case the - daughterboard cannot be controlled. - ValueError If vent_number is invalid. @@ -410,10 +395,6 @@ def vent_state(self, vent_number: int) -> VentGateState: AssertionError If the controller is not connected. - ModuleNotFoundError - If the megaind module has not been installed, in which case the - daughterboard cannot be controlled. - ValueError If vent_number is invalid. @@ -450,7 +431,7 @@ def vent_state(self, vent_number: int) -> VentGateState: return VentGateState.FAULT def get_opto_ch(self, *args, **kwargs) -> int: - """Calls megaind.getOptoCh or a simulated getOptoCh depending + """Calls hardware I/O or a simulated substitute depending whether the class was instantiated with simulate = True. Raises @@ -458,21 +439,18 @@ def get_opto_ch(self, *args, **kwargs) -> int: AssertionError If the controller is not connected. - ModuleNotFoundError - If the megaind module has not been installed, in which case the - daughterboard cannot be controlled. + NotImplementedError + If we are not in simulation mode. """ assert self.connected if self.simulator is not None: return self.simulator.get_opto_ch(*args, **kwargs) else: - if not MEGAIND_AVAILABLE: - raise ModuleNotFoundError("The megaind module is not available.") - return megaind.getOptoCh(*args, **kwargs) + raise NotImplementedError("Sequent hardware not implemented.") def set_od(self, *args, **kwargs) -> None: - """Calls megaind.setOd or a simulated setOd depending + """Calls harware I/O or a simulated substitute depending whether the class was instantiated with simulate = True. Raises @@ -480,15 +458,12 @@ def set_od(self, *args, **kwargs) -> None: AssertionError If the controller is not connected. - ModuleNotFoundError - If the megaind module has not been installed, in which case the - daughterboard cannot be controlled. + NotImplementedError + If we are not in simulation mode. """ assert self.connected if self.simulator is not None: self.simulator.set_od(*args, **kwargs) else: - if not MEGAIND_AVAILABLE: - raise ModuleNotFoundError("The megaind module is not available.") - megaind.setOd(*args, **kwargs) + raise NotImplementedError("Sequent hardware not implemented.") From 5cabebc40c5eefa87932a0bc52c7bd0bfd1445d3 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Thu, 22 Aug 2024 17:30:32 -0700 Subject: [PATCH 13/15] Allow export of cast_string_to_type --- python/lsst/ts/vent/controller/__init__.py | 2 +- python/lsst/ts/vent/controller/dispatcher.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/lsst/ts/vent/controller/__init__.py b/python/lsst/ts/vent/controller/__init__.py index 8308a16..e38c43d 100644 --- a/python/lsst/ts/vent/controller/__init__.py +++ b/python/lsst/ts/vent/controller/__init__.py @@ -26,4 +26,4 @@ from .config import Config from .controller import Controller -from .dispatcher import Dispatcher +from .dispatcher import Dispatcher, cast_string_to_type diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index bb05af1..01c4da0 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -29,7 +29,7 @@ from .controller import Controller, VentGateState -def _cast_string_to_type(new_type: type, value: str): +def cast_string_to_type(new_type: type, value: str): """Converts the value string to the specified type. In the case of boolean, "True" or "T" or "1" is ``True`` and everything else is ``False``. Other cases are handled by cast. @@ -150,7 +150,7 @@ async def read_and_dispatch(self) -> None: try: # Convert the arguments to their expected type. - args = [_cast_string_to_type(t, arg) for t, arg in zip(types, args)] + args = [cast_string_to_type(t, arg) for t, arg in zip(types, args)] # Call the method with the specified arguments. await getattr(self, command)(*args) # Send back a success response. From 81078d362cd826baef15148bca58f1ff392a3472 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Fri, 23 Aug 2024 09:06:21 -0700 Subject: [PATCH 14/15] Additional docstrings and type hinting --- python/lsst/ts/vent/controller/controller.py | 35 +++++++++++++++++--- python/lsst/ts/vent/controller/dispatcher.py | 5 ++- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/python/lsst/ts/vent/controller/controller.py b/python/lsst/ts/vent/controller/controller.py index c74e1a0..9bcd06c 100644 --- a/python/lsst/ts/vent/controller/controller.py +++ b/python/lsst/ts/vent/controller/controller.py @@ -275,7 +275,7 @@ async def get_drive_state(self) -> FanDriveState: Returns ------- - FanDriveState + `FanDriveState` The current fan drive state based on the contents of the IPAE register (described as "IPar Status" in the Schneider Electric ATV320 manual). @@ -430,10 +430,24 @@ def vent_state(self, vent_number: int) -> VentGateState: case _: return VentGateState.FAULT - def get_opto_ch(self, *args, **kwargs) -> int: + def get_opto_ch(self, stack_number: int, channel_number: int) -> int: """Calls hardware I/O or a simulated substitute depending whether the class was instantiated with simulate = True. + Parameters + ---------- + stack_number: int + The hardware stack number for the I/O card. + + channel_number: int + The I/O channel number to read. + + Returns + ------- + int + If the I/O input channel is high, returns 1, or, + if the I/O input channel is low, returns 0. + Raises ------ AssertionError @@ -445,14 +459,25 @@ def get_opto_ch(self, *args, **kwargs) -> int: assert self.connected if self.simulator is not None: - return self.simulator.get_opto_ch(*args, **kwargs) + return self.simulator.get_opto_ch(stack_number, channel_number) else: raise NotImplementedError("Sequent hardware not implemented.") - def set_od(self, *args, **kwargs) -> None: + def set_od(self, stack_number: int, channel_number: int, value: int) -> None: """Calls harware I/O or a simulated substitute depending whether the class was instantiated with simulate = True. + Parameters + ---------- + stack_number : int + The hardware stack number for the I/O card. + + channel_number: int + The I/O channel number to write. + + value: int + The value to send to the I/O output, 1 for high or 0 for low. + Raises ------ AssertionError @@ -464,6 +489,6 @@ def set_od(self, *args, **kwargs) -> None: assert self.connected if self.simulator is not None: - self.simulator.set_od(*args, **kwargs) + self.simulator.set_od(stack_number, channel_number, value) else: raise NotImplementedError("Sequent hardware not implemented.") diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 01c4da0..3fc4e41 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -23,13 +23,16 @@ import json import logging import traceback +from typing import Type, TypeVar from lsst.ts import tcpip, utils from .controller import Controller, VentGateState +T = TypeVar("T", bool, int, float, str) -def cast_string_to_type(new_type: type, value: str): + +def cast_string_to_type(new_type: Type[T], value: str) -> T: """Converts the value string to the specified type. In the case of boolean, "True" or "T" or "1" is ``True`` and everything else is ``False``. Other cases are handled by cast. From 40d5d5922c3cc5dd37468afa98a300c0b6620b90 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Fri, 23 Aug 2024 09:09:50 -0700 Subject: [PATCH 15/15] Docstring documentation of cast_string_to_type return value --- python/lsst/ts/vent/controller/dispatcher.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/python/lsst/ts/vent/controller/dispatcher.py b/python/lsst/ts/vent/controller/dispatcher.py index 3fc4e41..ccb6055 100644 --- a/python/lsst/ts/vent/controller/dispatcher.py +++ b/python/lsst/ts/vent/controller/dispatcher.py @@ -45,6 +45,11 @@ def cast_string_to_type(new_type: Type[T], value: str) -> T: value: str The value to be converted. + Returns + ------- + T + The value represented in the string, converted to the specfied type. + Raises ------ ValueError