diff --git a/CHANGELOG.md b/CHANGELOG.md index 9855518..c73ed55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Next version + +### ✨ Improved + +* [#32](https://github.com/sdss/lvmecp/pull/32) Major refactor of the `Modbus` and `ModbusRegister` classes. The main change is that the performance has been greatly improved, with `Modbus.get_all()` going from taking ~0.6 seconds to under 0.1. The register and coil blocks are now read completely, in chunks as large as the device will accept, as opposed to before, when we would read each variable with one read command (although the connection was not closed in between). Note that several methods and variables have been renamed; see the PR for details. + + ## 1.0.2 - December 27, 2024 ### ✨ Improved diff --git a/python/lvmecp/actor/actor.py b/python/lvmecp/actor/actor.py index 10047ff..1a89997 100644 --- a/python/lvmecp/actor/actor.py +++ b/python/lvmecp/actor/actor.py @@ -60,8 +60,6 @@ def __init__( else: self.plc = plc - self.semaphore = asyncio.Semaphore(5) - self._emit_status_task: asyncio.Task | None = None self._monitor_dome_task: asyncio.Task | None = None @@ -71,8 +69,6 @@ def __init__( self._engineering_mode_duration: float | None = None self._engineering_mode_task: asyncio.Task | None = None - self._last_heartbeat: float | None = None - self.running: bool = False async def start(self, **kwargs): @@ -81,6 +77,8 @@ async def start(self, **kwargs): await super().start(**kwargs) self.running = True + await self.plc.read_all_registers(use_cache=False) + # Start PLC modules now that the actor is running. This prevents the modules # trying to broadcast messages before the actor is ready. await self.plc.start_modules() @@ -185,9 +183,7 @@ async def emit_heartbeat(self): """Emits a heartbeat to the PLC.""" self.log.debug("Emitting heartbeat to the PLC.") - await self.plc.modbus["hb_set"].set(True) - - self._last_heartbeat = time.time() + await self.plc.modbus["hb_set"].write(True) async def _check_internal(self): return await super()._check_internal() diff --git a/python/lvmecp/actor/commands/dome.py b/python/lvmecp/actor/commands/dome.py index 63242d3..d036aca 100644 --- a/python/lvmecp/actor/commands/dome.py +++ b/python/lvmecp/actor/commands/dome.py @@ -74,7 +74,12 @@ async def close(command: ECPCommand, force=False): async def status(command: ECPCommand): """Returns the status of the dome.""" - status = await command.actor.plc.dome.update(use_cache=False) + status = await command.actor.plc.dome.update( + use_cache=False, + force_output=True, + command=command, + ) + if status is None: return command.fail("Failed retrieving dome status.") @@ -84,7 +89,7 @@ async def status(command: ECPCommand): if status & DomeStatus.POSITION_UNKNOWN: command.warning("Dome position is unknown!!!") - return command.finish(dome_open=bool(status & DomeStatus.OPEN)) + return command.finish() @dome.command() @@ -101,9 +106,7 @@ async def stop(command: ECPCommand): async def reset(command: ECPCommand, force=False): """Resets dome error state.""" - try: - await command.actor.plc.dome.reset() - except DomeError as err: - return command.fail(err) + command.warning("Resetting dome error state.") + await command.actor.plc.dome.reset() return command.finish() diff --git a/python/lvmecp/actor/commands/engineering.py b/python/lvmecp/actor/commands/engineering.py index 200f485..4d321b0 100644 --- a/python/lvmecp/actor/commands/engineering.py +++ b/python/lvmecp/actor/commands/engineering.py @@ -18,7 +18,28 @@ if TYPE_CHECKING: - from lvmecp.actor import ECPCommand + from lvmecp.actor import ECPActor, ECPCommand + + +async def get_eng_mode_status(actor: ECPActor) -> dict: + enabled = actor.is_engineering_mode_enabled() + started_at = actor._engineering_mode_started_at + duration = actor._engineering_mode_duration + + registers = await actor.plc.read_all_registers(use_cache=True) + + if duration is None or started_at is None: + ends_at = None + else: + ends_at = started_at + duration + + return { + "enabled": enabled, + "started_at": timestamp_to_iso(started_at), + "ends_at": timestamp_to_iso(ends_at), + "software_override": registers["engineering_mode_software"], + "hardware_override": registers["engineering_mode_hardware"], + } @parser.group(name="engineering-mode") @@ -36,12 +57,32 @@ def engineering_mode(): help="Timeout for the engineering mode. " "If not passed, the default timeout is used.", ) -async def enable(command: ECPCommand, timeout: float | None = None): +@click.option( + "--hardware-override", + is_flag=True, + help="Sets the hardware override flag.", +) +@click.option( + "--software-override", + is_flag=True, + help="Sets the software override flag.", +) +async def enable( + command: ECPCommand, + timeout: float | None = None, + hardware_override: bool = False, + software_override: bool = False, +): """Enables the engineering mode.""" await command.actor.engineering_mode(True, timeout=timeout) - return command.finish(engineering_mode=True) + if hardware_override: + await command.actor.plc.modbus.write_register("engineering_mode_hardware", True) + if software_override: + await command.actor.plc.modbus.write_register("engineering_mode_software", True) + + return command.finish(engineering_mode=await get_eng_mode_status(command.actor)) @engineering_mode.command() @@ -50,26 +91,14 @@ async def disable(command: ECPCommand): await command.actor.engineering_mode(False) - return command.finish(engineering_mode=False) + await command.actor.plc.modbus.write_register("engineering_mode_hardware", False) + await command.actor.plc.modbus.write_register("engineering_mode_software", False) + + return command.finish(engineering_mode=await get_eng_mode_status(command.actor)) @engineering_mode.command() async def status(command: ECPCommand): """Returns the status of the engineering mode.""" - enabled = command.actor.is_engineering_mode_enabled() - started_at = command.actor._engineering_mode_started_at - duration = command.actor._engineering_mode_duration - - if duration is None or started_at is None: - ends_at = None - else: - ends_at = started_at + duration - - return command.finish( - engineering_mode={ - "enabled": enabled, - "started_at": timestamp_to_iso(started_at), - "ends_at": timestamp_to_iso(ends_at), - } - ) + return command.finish(engineering_mode=await get_eng_mode_status(command.actor)) diff --git a/python/lvmecp/actor/commands/modbus.py b/python/lvmecp/actor/commands/modbus.py new file mode 100644 index 0000000..abbb631 --- /dev/null +++ b/python/lvmecp/actor/commands/modbus.py @@ -0,0 +1,178 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# +# @Author: José Sánchez-Gallego (gallegoj@uw.edu) +# @Date: 2024-12-28 +# @Filename: modbus.py +# @License: BSD 3-clause (http://www.opensource.org/licenses/BSD-3-Clause) + +from __future__ import annotations + +import asyncio + +from typing import TYPE_CHECKING, Literal + +import click + +from lvmecp.modbus import ModbusRegister + +from . import parser + + +if TYPE_CHECKING: + from lvmecp.actor import ECPCommand + from lvmecp.modbus import RegisterModes + + +def get_register( + command: ECPCommand, + address_or_name: str, + register_type: RegisterModes | None = None, + allow_unknown: bool = False, +) -> ModbusRegister | Literal[False]: + """Returns a register from an address or name.""" + + plc = command.actor.plc + + register: ModbusRegister | None = None + address: int | None = None + + try: + address = int(address_or_name) # type: ignore + + if isinstance(address, int) and not register_type: + command.fail("When passing an address, --register-type must be specified.") + return False + + for _, reg in plc.modbus.items(): + if reg.address == address: + register = reg + break + except ValueError: + register = plc.modbus.get(address_or_name) + address = register.address if register else None + + if register is None: + if allow_unknown and address is not None and register_type: + return ModbusRegister( + command.actor.plc.modbus, + name=f"{register_type}_{address}", + address=address, + mode=register_type, + count=1, + readonly=False, + ) + + command.fail(f"Register {address_or_name!r} not found.") + return False + + return register + + +@parser.group() +def modbus(): + """Low-level access to the PLC Modbus variables.""" + + pass + + +@modbus.command() +@click.argument("address", metavar="ADDRESS|NAME") +@click.option( + "--register-type", + type=click.Choice(["coil", "holding_register"]), + default=None, + help="The type of register to read. Required if an address is passed.", +) +@click.option( + "--allow-unknown", + is_flag=True, + help="Allow unknown registers. Requires specifying an address.", +) +async def read( + command: ECPCommand, + address: str, + register_type: Literal["coil", "holding_register"] | None = None, + allow_unknown: bool = False, +): + """Reads a Modbus register.""" + + if not ( + register := get_register( + command, + address, + register_type=register_type, + allow_unknown=allow_unknown, + ) + ): + return False + + value = await register.read(use_cache=False) + + return command.finish( + register={ + "name": register.name, + "address": register.address, + "value": value, + } + ) + + +@modbus.command() +@click.argument("address", metavar="ADDRESS|NAME") +@click.argument("value", type=int) +@click.option( + "--register-type", + type=click.Choice(["coil", "holding_register"]), + default=None, + help="The type of register to read. Required if an address is passed.", +) +@click.option( + "--allow-unknown", + is_flag=True, + help="Allow unknown registers. Requires specifying an address.", +) +async def write( + command: ECPCommand, + address: str, + value: int, + register_type: Literal["coil", "holding_register"] | None = None, + allow_unknown: bool = False, +): + """Writes a value to a Modbus register.""" + + if not ( + register := get_register( + command, + address, + register_type=register_type, + allow_unknown=allow_unknown, + ) + ): + return False + + name = register.name + + if register.readonly: + return command.fail(f"Register {name!r} is read-only.") + + if register.mode == "coil": + value = bool(int(value)) + else: + value = int(value) + + try: + await register.write(value) + except Exception as err: + return command.fail(f"Error writing to register {name!r}: {err!r}") + + await asyncio.sleep(0.5) + new_value = await register.read(use_cache=False) + + return command.finish( + register={ + "name": name, + "address": register.address, + "value": new_value, + } + ) diff --git a/python/lvmecp/actor/commands/status.py b/python/lvmecp/actor/commands/status.py index 2f771cf..b4f1173 100644 --- a/python/lvmecp/actor/commands/status.py +++ b/python/lvmecp/actor/commands/status.py @@ -26,19 +26,27 @@ @parser.command() @click.option("--no-registers", is_flag=True, help="Does not output registers.") -async def status(command: ECPCommand, no_registers: bool = False): +@click.option("--no-cache", is_flag=True, help="Ignores the internal cache.") +async def status( + command: ECPCommand, + no_registers: bool = False, + no_cache: bool = False, +): """Returns the enclosure status.""" plc = command.actor.plc if no_registers is False: - async with command.actor.semaphore: - command.info(registers=(await plc.read_all_registers(use_cache=False))) + command.info(registers=(await plc.read_all_registers(use_cache=not no_cache))) modules: list[PLCModule] = [plc.dome, plc.safety, plc.lights] await asyncio.gather( *[ - module.update(force_output=True, command=command, use_cache=True) + module.update( + force_output=True, + command=command, + use_cache=True, + ) for module in modules ] ) @@ -48,6 +56,6 @@ async def status(command: ECPCommand, no_registers: bool = False): o2_percent_spectrograph=plc.safety.o2_level_spectrograph, ) - command.info(last_heartbeat_set=timestamp_to_iso(command.actor._last_heartbeat)) + command.info(heartbeat_ack=timestamp_to_iso(plc.safety.last_heartbeat_ack)) return command.finish() diff --git a/python/lvmecp/dome.py b/python/lvmecp/dome.py index 29c006b..894248c 100644 --- a/python/lvmecp/dome.py +++ b/python/lvmecp/dome.py @@ -13,6 +13,7 @@ from time import time from types import SimpleNamespace +import numpy from astropy.time import Time from lvmopstools.ephemeris import get_ephemeris_summary @@ -22,6 +23,9 @@ from lvmecp.module import PLCModule +MOVE_CHECK_INTERVAL: float = 0.5 + + class DomeController(PLCModule[DomeStatus]): """Controller for the rolling dome.""" @@ -42,10 +46,9 @@ async def _update_internal(self, use_cache: bool = True, **kwargs): assert self.flag new_status = self.flag(0) - if dome_status.drive_state: - new_status |= self.flag.DRIVE_AVAILABLE - else: - new_status |= self.flag.NODRIVE + # The variable that would determine if the drive is available is not + # does not exist anymore, so we assume it is. + new_status |= self.flag.DRIVE_AVAILABLE if dome_status.drive_enabled: new_status |= self.flag.DRIVE_ENABLED @@ -57,12 +60,6 @@ async def _update_internal(self, use_cache: bool = True, **kwargs): else: new_status |= self.flag.MOTOR_CLOSING - if dome_status.drive_brake: - new_status |= self.flag.BRAKE_ENABLED - - # if dome_status.overcurrent: - # new_status |= self.flag.OVERCURRENT - if dome_status.dome_open is True: new_status |= self.flag.OPEN elif dome_status.dome_closed is True: @@ -73,12 +70,24 @@ async def _update_internal(self, use_cache: bool = True, **kwargs): if new_status.value == 0: new_status = self.flag(self.flag.__unknown__) - return new_status + if dome_status.dome_open: + percent_open = 1 + elif dome_status.dome_closed: + percent_open = 0 + else: + full_open = config["dome.full_open_mm"] + percent_open = numpy.clip(dome_status.dome_position / full_open, 0, 1) + + extra_info = { + "dome_percent_open": round(float(percent_open) * 100, 1), + } + + return new_status, extra_info async def set_direction(self, open: bool): """Sets the motor direction (`True` means open, `False` close).""" - await self.modbus["drive_direction"].set(open) + await self.modbus["drive_direction"].write(open) await self.update(use_cache=False) async def _move(self, open: bool, force: bool = False): @@ -117,22 +126,24 @@ async def _move(self, open: bool, force: bool = False): warnings.warn("Dome already at position, but forcing.", ECPWarning) log.debug("Setting motor_direction.") - await self.modbus["motor_direction"].set(open) + await self.modbus["motor_direction"].write(open) - await asyncio.sleep(0.5) + await asyncio.sleep(0.1) log.debug("Setting drive_enabled.") - await self.modbus["drive_enabled"].set(True) + await self.modbus["drive_enabled"].write(True) - await asyncio.sleep(0.5) + await asyncio.sleep(0.1) last_enabled: float = 0.0 while True: # Still moving. - await asyncio.sleep(2) + await asyncio.sleep(MOVE_CHECK_INTERVAL) + + drive_enabled = await self.modbus["drive_enabled"].read(use_cache=False) - drive_enabled = await self.modbus["drive_enabled"].get() - move_done = await self.modbus["dome_open" if open else "dome_closed"].get() + move_done_register = self.modbus["dome_open" if open else "dome_closed"] + move_done = await move_done_register.read(use_cache=False) if drive_enabled: last_enabled = time() @@ -174,15 +185,15 @@ async def stop(self): if not drive_enabled: return - await self.plc.modbus["drive_enabled"].set(False) + await self.plc.modbus["drive_enabled"].write(False) await self.update(use_cache=False) async def reset(self): """Resets the roll-off error state.""" - await self.modbus["rolloff_error_reset"].set(1) - await asyncio.sleep(1) + await self.modbus["dome_error_reset"].write(True) + await asyncio.sleep(0.5) def is_allowed(self): """Returns whether the dome is allowed to move.""" diff --git a/python/lvmecp/etc/lvmecp.yml b/python/lvmecp/etc/lvmecp.yml index aa1d34f..3fa4c78 100644 --- a/python/lvmecp/etc/lvmecp.yml +++ b/python/lvmecp/etc/lvmecp.yml @@ -1,181 +1,268 @@ modbus: host: 10.8.38.51 port: 502 - cache_timeout: 0.5 + cache_timeout: 1 registers: door_locked: address: 0 group: safety + mode: coil + readonly: true door_closed: address: 1 group: safety + mode: coil + readonly: true local: address: 2 group: safety + mode: coil + readonly: true e_status: - address: 200 + address: 199 group: safety + mode: coil + readonly: true e_stop: - address: 199 + address: 200 group: safety + mode: coil + readonly: false e_relay_reset: address: 201 group: safety + mode: coil + readonly: false cr_new: address: 234 group: lights + mode: coil + readonly: false cr_status: address: 334 group: lights + mode: coil + readonly: true ur_new: address: 235 group: lights + mode: coil + readonly: false ur_status: address: 335 group: lights + mode: coil + readonly: true sr_new: address: 236 group: lights + mode: coil + readonly: false sr_status: address: 336 group: lights + mode: coil + readonly: true uma_new: address: 237 group: lights + mode: coil + readonly: false uma_status: address: 337 group: lights + mode: coil + readonly: true tb_new: address: 238 group: lights + mode: coil + readonly: false tb_status: address: 338 group: lights + mode: coil + readonly: true tr_new: address: 239 group: lights + mode: coil + readonly: false tr_status: address: 339 group: lights + mode: coil + readonly: true drive_enabled: address: 99 group: dome - drive_state: - address: 100 - group: dome + mode: coil + readonly: false motor_direction: address: 101 group: dome - drive_brake: - address: 102 - group: dome + mode: coil + readonly: false ne_limit: address: 104 group: dome + mode: coil + readonly: true se_limit: address: 105 group: dome + mode: coil + readonly: true nw_limit: address: 106 group: dome + mode: coil + readonly: true sw_limit: address: 107 group: dome + mode: coil + readonly: true dome_closed: address: 108 group: dome + mode: coil + readonly: true dome_open: address: 109 group: dome - # overcurrent: - # address: 109 - # group: dome - drive_velocity1: - address: 103 - mode: holding_register + mode: coil + readonly: true + dome_lockout: + address: 110 group: dome - drive_velocity2: - address: 104 - mode: holding_register + mode: coil + readonly: true + dome_error: + address: 111 group: dome - open_timeout: - address: 119 - mode: holding_register + mode: coil + readonly: true + dome_error_reset: + address: 112 group: dome - close_timeout: - address: 120 + mode: coil + readonly: false + dome_counter: + address: 149 mode: holding_register group: dome - drive_current: - address: 129 + readonly: true + dome_position: + address: 150 mode: holding_register group: dome - dome_counter: - address: 149 + readonly: true + dome_speed: + address: 151 mode: holding_register group: dome + readonly: true dome_status1: address: 410 mode: holding_register group: dome + readonly: true dome_status2: address: 411 mode: holding_register group: dome - rolloff_lockout: - address: 110 + readonly: true + dome_set_frequency: + address: 412 + mode: holding_register group: dome - rolloff_error: - address: 111 + readonly: true + dome_output_frequency: + address: 413 + mode: holding_register group: dome - rolloff_error_reset: - address: 112 + readonly: true + dome_output_current: + address: 129 + mode: holding_register group: dome + readonly: true + dome_output_voltage: + address: 416 + mode: holding_register + group: dome + readonly: true + dome_motor_current_rpm: + address: 417 + mode: holding_register + group: dome + readonly: true + dome_present_fault_record: + address: 399 + mode: holding_register + group: dome + readonly: true oxygen_read_utilities_room: address: 599 mode: holding_register group: safety + readonly: true oxygen_read_spectrograph_room: address: 600 mode: holding_register group: safety - oxygen_mode_utilities_room: + readonly: true + oxygen_error_code_utilities_room: address: 601 mode: holding_register group: safety - oxygen_mode_spectrograph_room: + readonly: true + oxygen_error_code_spectrograph_room: address: 602 mode: holding_register group: safety + readonly: true hb_set: address: 599 mode: coil + readonly: false group: safety hb_ack: address: 600 mode: coil + readonly: true group: safety hb_error: address: 601 mode: coil + readonly: true group: safety rain_sensor_alarm: address: 699 mode: coil + readonly: true group: safety - rain_sensor_counter: + rain_sensor_count: address: 699 mode: holding_register + readonly: true group: safety - -safety: - override_local_mode: False - o2_threshold: 19.5 - -dome: - daytime_allowed: false - daytime_tolerance: 600 - anti_flap_tolerance: [3, 600] + rain_sensor_countdown: + address: 700 + mode: holding_register + readonly: true + group: safety + engineering_mode_hardware: + address: 899 + mode: coil + readonly: false + group: engineering_mode + engineering_mode_software: + address: 900 + mode: coil + readonly: false + group: engineering_mode hvac: host: 10.8.38.49 @@ -187,139 +274,249 @@ hvac: mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_flowmeter_ahu_spectrograph: address: 2 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_exterior_humidity: address: 4 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_humidity_utilities_room: address: 6 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_humidity_spectrograph_room: address: 8 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_humidity_telescope_platform: address: 10 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_water_pressure_inlet_circuit_1: address: 12 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_water_pressure_inlet_circuit_2: address: 14 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_exterior_temperature: address: 16 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_utilities_room_temperature: address: 18 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_spectrograph_room_temperature: address: 20 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_telescope_platform_temperature: address: 22 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_chiller_in_water_temperature: address: 24 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_ahu_in_water_temperature: address: 26 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_ahu_injection_temperature: address: 28 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_ahu_return_temperature: address: 30 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_chiller_out_temperature: address: 32 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_ahu_out_temperature: address: 34 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_ahu_cold_valve: address: 36 mode: holding_register count: 2 decoder: float_32bit + readonly: true hvac_system_status: address: 0 mode: coil + readonly: true hvac_ahu_filter_status: address: 1 mode: coil + readonly: true hvac_vin_1_air_pressure: address: 2 mode: coil + readonly: true hvac_vin_2_air_pressure: address: 3 mode: coil + readonly: true hvac_roll_off_roof_position: address: 4 mode: coil + readonly: true hvac_status_damper_1: address: 5 mode: coil + readonly: true hvac_status_damper_2: address: 6 mode: coil + readonly: true hvac_ahu_heater_1: address: 7 mode: coil + readonly: true hvac_start_stop_chiller: address: 8 mode: coil + readonly: true hvac_start_stop_vin_1: address: 9 mode: coil + readonly: true hvac_start_stop_vin_2: address: 10 mode: coil + readonly: true hvac_start_stop_water_pump: address: 11 mode: coil + readonly: true hvac_start_stop_ahu: address: 12 mode: coil + readonly: true + +simulator: + host: 127.0.0.1 + port: 5020 + overrides: + door_locked: true + door_closed: true + dome_closed: true + events: + ur_new: + on_value: 1 + then: + register: ur_status + action: toggle + reset_trigger: true + cr_new: + on_value: 1 + then: + register: cr_status + action: toggle + reset_trigger: true + sr_new: + on_value: 1 + then: + register: sr_status + action: toggle + reset_trigger: true + uma_new: + on_value: 1 + then: + register: uma_status + action: toggle + reset_trigger: true + tb_new: + on_value: 1 + then: + register: tb_status + action: toggle + reset_trigger: true + tr_new: + on_value: 1 + then: + register: tr_status + action: toggle + reset_trigger: true + hb_set: + on_value: 1 + then: + register: hb_ack + action: set + reset_trigger: true + e_stop: + on_value: 1 + then: + register: e_status + action: set + reset_trigger: true + e_relay_reset: + on_value: 1 + then: + register: e_status + action: reset + reset_trigger: true + dome_error_reset: + on_value: 1 + then: + register: dome_error + action: reset + reset_trigger: true engineering_mode: default_duration: 300 +safety: + o2_threshold: 19.5 + +dome: + daytime_allowed: false + daytime_tolerance: 600 + anti_flap_tolerance: [3, 600] + full_open_mm: 9480 + actor: name: lvmecp host: localhost diff --git a/python/lvmecp/etc/schema.json b/python/lvmecp/etc/schema.json index 742403d..76d4af8 100644 --- a/python/lvmecp/etc/schema.json +++ b/python/lvmecp/etc/schema.json @@ -18,14 +18,29 @@ } } }, - "lights": { - "type": "string" - }, + "lights": { "type": "string" }, + "lights_labels": { "type": "string" }, + "dome_percent_open": { "type": "number" }, "o2_percent_utilities": { "type": "number" }, "o2_percent_spectrograph": { "type": "number" }, - "last_heartbeat_set": { + "heartbeat_ack": { "oneOf": [{ "type": "string" }, { "type": "null" }] }, + "register": { + "type": "object", + "properties": { + "name": { "type": "string" }, + "address": { "type": "number" }, + "value": { + "oneOf": [ + { "type": "boolean" }, + { "type": "number" }, + { "type": "null" } + ] + } + }, + "required": ["name", "value"] + }, "engineering_mode": { "type": "object", "properties": { @@ -35,10 +50,12 @@ }, "ends_at": { "oneOf": [{ "type": "string" }, { "type": "null" }] - } + }, + "software_override": { "type": "boolean" }, + "hardware_override": { "type": "boolean" } }, "required": ["enabled", "started_at", "ends_at"] } }, - "additionalProperties": true + "additionalProperties": false } diff --git a/python/lvmecp/hvac.py b/python/lvmecp/hvac.py index 42e6331..8559555 100644 --- a/python/lvmecp/hvac.py +++ b/python/lvmecp/hvac.py @@ -23,9 +23,9 @@ class HVACController(PLCModule): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.status: dict[str, float | bool | None] = {} + self.status: dict[str, int | bool] = {} async def _update_internal(self, **kwargs): """Update status.""" - self.status = await self.modbus.get_all() + self.status = await self.modbus.read_all() diff --git a/python/lvmecp/lights.py b/python/lvmecp/lights.py index ff22082..5ecc37a 100644 --- a/python/lvmecp/lights.py +++ b/python/lvmecp/lights.py @@ -43,7 +43,7 @@ class LightsController(PLCModule): flag = LightStatus interval = 30.0 - async def _update_internal(self, use_cache: bool = True): + async def _update_internal(self, use_cache: bool = True, **kwargs): """Update status.""" assert self.flag is not None @@ -123,7 +123,7 @@ async def toggle(self, light: str): code = self.get_code(light) log.debug(f"Toggling light {code}.") - await self.modbus[f"{code}_new"].set(True) + await self.modbus[f"{code}_new"].write(True) await asyncio.sleep(0.5) await self.update(use_cache=False) diff --git a/python/lvmecp/maskbits.py b/python/lvmecp/maskbits.py index 967cf8f..4d226dd 100644 --- a/python/lvmecp/maskbits.py +++ b/python/lvmecp/maskbits.py @@ -54,6 +54,9 @@ class SafetyStatus(Maskbit): O2_SENSOR_SR_ALARM = 0x400 # Spec room O2_SENSOR_SR_FAULT = 0x800 RAIN_SENSOR_ALARM = 0x1000 + E_STOP = 0x2000 + DOME_LOCKED = 0x4000 + DOME_ERROR = 0x8000 UNKNOWN = 0x100000 diff --git a/python/lvmecp/modbus.py b/python/lvmecp/modbus.py index 3c7c94e..c2e0b63 100644 --- a/python/lvmecp/modbus.py +++ b/python/lvmecp/modbus.py @@ -12,8 +12,9 @@ import pathlib from time import time -from typing import cast +from typing import Literal, Sequence +from lvmopstools.retrier import Retrier from pymodbus.client.tcp import AsyncModbusTcpClient from pymodbus.constants import Endian from pymodbus.payload import BinaryPayloadDecoder @@ -24,10 +25,15 @@ from lvmecp import config as lvmecp_config from lvmecp import log from lvmecp.exceptions import ECPError +from lvmecp.tools import TimedCacheDict MAX_RETRIES = 3 -TIMEOUT = 10.0 +MAX_COUNT_HR = 100 +CONNECTION_TIMEOUT = 10.0 + + +RegisterModes = Literal["coil", "holding_register", "discrete_input", "input_register"] class ModbusRegister: @@ -47,6 +53,8 @@ class ModbusRegister: or ``input_register``. group A grouping key for registers. + readonly + Whether the register is read-only. """ @@ -55,10 +63,11 @@ def __init__( modbus: Modbus, name: str, address: int, - mode: str = "coil", + mode: RegisterModes = "coil", count: int = 1, group: str | None = None, decoder: str | None = None, + readonly: bool = True, ): self.modbus = modbus self.client = modbus.client @@ -69,18 +78,11 @@ def __init__( self.count = count self.group = group self.decoder = decoder + self.readonly = readonly - self._last_value: int | float = 0 - self._last_seen: float = 0 - - async def _get_internal(self, use_cache: bool = True): + async def _read_internal(self): """Return the value of the modbus register.""" - cache_timeout = self.modbus.cache_timeout - last_seen_interval = time() - self._last_seen - if use_cache and last_seen_interval < cache_timeout: - return self._last_value - if self.mode == "coil": func = self.client.read_coils elif self.mode == "holding_register": @@ -92,21 +94,14 @@ async def _get_internal(self, use_cache: bool = True): else: raise ValueError(f"Invalid block mode {self.mode!r}.") - if self.client.connected: + async with self.modbus: resp = await func( self.address, count=self.count, slave=self.modbus.slave, - ) # type: ignore - else: - async with self.modbus: - resp = await func( - self.address, - count=self.count, - slave=self.modbus.slave, - ) # type: ignore - - if resp.function_code > 0x80: + ) + + if resp.isError(): raise ValueError( f"Invalid response for element " f"{self.name!r}: 0x{resp.function_code:02X}." @@ -119,87 +114,76 @@ async def _get_internal(self, use_cache: bool = True): registers = resp.registers value = registers[0 : self.count] if self.count > 1 else registers[0] - if self.decoder is not None: - if self.decoder == "float_32bit": - bin_payload = BinaryPayloadDecoder.fromRegisters( - value, - byteorder=Endian.BIG, - wordorder=Endian.LITTLE, - ) - value = round(bin_payload.decode_32bit_float(), 3) - else: - raise ValueError(f"Unknown decoder {self.decoder}") + value = self.decode(value) if not isinstance(value, (int, float)): raise ValueError(f"Invalid type for {self.name!r} response.") - self._last_value = value - self._last_seen = time() + return value + + def decode(self, value: int | bool | list[int | bool]): + """Decodes the raw value from the register.""" + + if self.decoder is not None: + if self.decoder == "float_32bit": + bin_payload = BinaryPayloadDecoder.fromRegisters( + value, + byteorder=Endian.BIG, + wordorder=Endian.LITTLE, + ) + value = round(bin_payload.decode_32bit_float(), 3) + else: + raise ValueError(f"Unknown decoder {self.decoder}") return value - async def get(self, open_connection: bool = True, use_cache: bool = True): - """Return the value of the modbus register. Implements retry.""" + @Retrier(max_attempts=MAX_RETRIES, delay=0.5, max_delay=2.0) + async def read(self, use_cache: bool = True): + """Return the value of the modbus register. - for ntries in range(1, MAX_RETRIES + 1): - # If we need to open the connection, use the Modbus context - # and call ourselves recursively with open_connection=False - # (at that point it will be open). - if open_connection: - await self.modbus.connect() + Parameters + ---------- + use_cache + Whether to use the cache to retrieve the value. If the cache is not + available, or the value is not in the cache, the register will be read. + This function does not set the cache after reading the register. - if not self.modbus.client or not self.modbus.client.connected: - raise ConnectionError("Not connected to modbus server.") + """ - try: - return await self._get_internal(use_cache=use_cache) - except Exception: - if ntries >= MAX_RETRIES: - raise + if use_cache: + cache = self.modbus.register_cache + if self.name in cache and (value := cache[self.name]) is not None: + return value - await asyncio.sleep(0.5) - finally: - if open_connection: - await self.modbus.disconnect() + return await self._read_internal() - async def set(self, value: int | bool): + @Retrier(max_attempts=MAX_RETRIES, delay=0.5, max_delay=2.0) + async def write(self, value: int | bool): """Sets the value of the register.""" - for ntries in range(1, MAX_RETRIES + 1): - # Always open the connection. - async with self.modbus: - if self.mode == "coil": - func = self.client.write_coil - elif self.mode == "holding_register": - func = self.client.write_register - elif self.mode == "discrete_input" or self.mode == "input_register": - raise ValueError(f"Block of mode {self.mode!r} is read-only.") - else: - raise ValueError(f"Invalid block mode {self.mode!r}.") - - try: - if self.client.connected: - resp = await func(self.address, value) # type: ignore - else: - async with self.modbus: - resp = await func(self.address, value) # type: ignore - - if resp.function_code > 0x80: - raise ECPError( - f"Invalid response for element " - f"{self.name!r}: 0x{resp.function_code:02X}." - ) - else: - self._last_value = int(value) - self._last_seen = time() + if self.readonly: + raise ECPError(f"Register {self.name!r} is read-only.") - return + if self.mode == "coil": + func = self.client.write_coil + elif self.mode == "holding_register": + func = self.client.write_register + elif self.mode == "discrete_input" or self.mode == "input_register": + raise ValueError(f"Block of mode {self.mode!r} is read-only.") + else: + raise ValueError(f"Invalid block mode {self.mode!r}.") - except Exception as err: - if ntries >= MAX_RETRIES: - raise ECPError(f"Failed setting {self.name!r}: {err}") + # Always open the connection. + async with self.modbus: + resp = await func(self.address, value) # type: ignore - await asyncio.sleep(0.5) + if resp.isError(): + raise ECPError( + f"Invalid response for element " + f"{self.name!r}: 0x{resp.function_code:02X}." + ) + else: + self.modbus.register_cache[self.name] = value class Modbus(dict[str, ModbusRegister]): @@ -232,40 +216,42 @@ def __init__(self, config: dict | pathlib.Path | str | None = None): self.port = self.config["port"] self.slave = self.config.get("slave", 0) - # Cache results so that very close calls to get_all() don't need to - # open a connection and read the registers. - self.cache_timeout = self.config.get("cache_timeout", 0.5) - self._register_cache: dict[str, int | float | None] = {} - self._register_last_seen: float = 0 + # Cache results so that very close calls to get_all() + # don't need to open a connection and read the registers. + self.cache_timeout = self.config.get("cache_timeout", 1) + self.register_cache = TimedCacheDict(self.cache_timeout, mode="null") + # Modbus client. self.client = AsyncModbusTcpClient(self.host, port=self.port) + # Lock to allow up to 5 concurrent connections. self.lock = asyncio.Lock() self._lock_release_task: asyncio.Task | None = None - register_data = self.config["registers"] + # Create the internal dictionary of registers registers = { name: ModbusRegister( self, name, - elem["address"], - mode=elem.get("mode", "coil"), - group=elem.get("group", None), - count=elem.get("count", 1), - decoder=elem.get("decoder", None), + register["address"], + mode=register.get("mode", "coil"), + group=register.get("group", None), + count=register.get("count", 1), + decoder=register.get("decoder", None), + readonly=register.get("readonly", True), ) - for name, elem in register_data.items() + for name, register in self.config["registers"].items() } dict.__init__(self, registers) - for name, elem in registers.items(): - setattr(self, name, elem) + for name, register in registers.items(): + setattr(self, name, register) async def connect(self): """Connects to the client.""" try: - await asyncio.wait_for(self.lock.acquire(), TIMEOUT) + await asyncio.wait_for(self.lock.acquire(), CONNECTION_TIMEOUT) except asyncio.TimeoutError: raise RuntimeError("Timed out waiting for lock to be released.") @@ -319,50 +305,98 @@ async def __aexit__(self, exc_type, exc, tb): async def unlock_on_timeout(self): """Removes the lock after an amount of time.""" - await asyncio.sleep(TIMEOUT) - if self.lock.locked(): - self.lock.release() - - async def get_all(self, use_cache: bool = True): - """Returns a dictionary with all the registers.""" + await asyncio.sleep(CONNECTION_TIMEOUT) + await self.disconnect() - if use_cache and time() - self._register_last_seen < self.cache_timeout: - if None not in self._register_cache.values(): - return self._register_cache + async def read_all(self, use_cache: bool = True) -> dict[str, int | bool]: + """Returns a dictionary with all the registers and sets the cache.""" - names = results = [] + if use_cache: + cache_times = self.register_cache._cache_time.values() + oldest_cache = min(cache_times) if len(cache_times) > 0 else 0 + if time() - oldest_cache < self.cache_timeout: + return self.register_cache.freeze() + # With this PLC it is more efficient to read the entire coil and + # holding register blocks in one go, and then parse the results, as + # opposed to reading each register individually. async with self: - names = [name for name in self] - tasks = [ - elem.get(open_connection=False, use_cache=False) - for elem in self.values() - ] + for mode in ["coil", "holding_register"]: + mode_count = max( + register.address + register.count + for register in self.values() + if register.mode == mode + ) + + if mode == "coil": + func = self.client.read_coils + elif mode == "holding_register": + func = self.client.read_holding_registers + else: + raise ValueError(f"Invalid mode {mode!r}.") - results = await asyncio.gather(*tasks, return_exceptions=True) + data: list[int | bool] = [] - if any([isinstance(result, Exception) for result in results]): - for ii, result in enumerate(results): - if isinstance(result, Exception): - log.warning(f"Failed retrieving value for {names[ii]!r}") - results[ii] = None + if mode == "coil": + # For coils we can read the entire block. + resp = await func(0, count=1023, slave=self.slave) - registers = cast( - dict[str, int | float | None], - {names[ii]: results[ii] for ii in range(len(names))}, - ) + if resp.isError(): + raise ValueError( + f"Invalid response for block {mode!r}: " + f"0x{resp.function_code:02X}." + ) + + data += resp.bits + + else: + # For holding registers we are limited to reading MAX_COUNT_HR + # at once. We iterate to get all the data. + n_reads = mode_count // MAX_COUNT_HR + 1 + + for nn in range(n_reads): + resp = await func( + nn * MAX_COUNT_HR, + count=MAX_COUNT_HR, + slave=self.slave, + ) + + if resp.isError(): + raise ValueError( + f"Invalid response for block {mode!r}: " + f"0x{resp.function_code:02X}." + ) + + data += resp.registers + + for name, register in self.items(): + if register.mode != mode: + continue + + value = data[register.address : register.address + register.count] + value = register.decode(value) + + if isinstance(value, Sequence) and len(value) == 1: + value = value[0] - self._register_cache = registers - self._register_last_seen = time() + self.register_cache[name] = value + + registers = self.register_cache.freeze() + + # Just to double check that none of the values have expired, we loop over the + # dictionary and if necessary we read the register again. + for name, value in registers.items(): + if value is None: + registers[name] = await self[name].read(use_cache=False) return registers async def read_group(self, group: str, use_cache: bool = True): """Returns a dictionary of all read registers that match a ``group``.""" - registers = await self.get_all(use_cache=use_cache) + registers = await self.read_all(use_cache=use_cache) - group_registers = {} + group_registers: dict[str, int | bool] = {} for name in self: register = self[name] if register.group is not None and register.group == group: @@ -372,3 +406,21 @@ async def read_group(self, group: str, use_cache: bool = True): group_registers[name] = registers[name] return group_registers + + async def read_register(self, register: str, use_cache: bool = True) -> int | bool: + """Reads a register.""" + + if register not in self: + raise ValueError(f"Register {register!r} not found.") + + return await self[register].read(use_cache=use_cache) + + async def write_register(self, register: str, value: int | bool): + """Writes a value to a register.""" + + assert isinstance(register, str) + + if register not in self: + raise ValueError(f"Register {register!r} not found.") + + await self[register].write(value) diff --git a/python/lvmecp/module.py b/python/lvmecp/module.py index e80e307..0f3a82e 100644 --- a/python/lvmecp/module.py +++ b/python/lvmecp/module.py @@ -11,7 +11,16 @@ import abc import asyncio -from typing import TYPE_CHECKING, Callable, Coroutine, Generic, Type, TypeVar +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Coroutine, + Generic, + Sequence, + Type, + TypeVar, +) from lvmecp import log from lvmecp.tools import cancel_tasks_by_name @@ -38,7 +47,7 @@ def __init__( modbus: Modbus | None = None, interval: float | None = None, start: bool = True, - notifier: Callable[[int, str], Callable | Coroutine] | None = None, + notifier: Callable[[int, str, dict], Callable | Coroutine] | None = None, ): self.name = name self.plc = plc @@ -80,7 +89,10 @@ async def _status_loop(self): await asyncio.sleep(self._interval) @abc.abstractmethod - async def _update_internal(self, **kwargs) -> Flag_co: + async def _update_internal( + self, + **kwargs, + ) -> Flag_co | tuple[Flag_co, dict[str, Any]]: """Determines the new module flag status.""" pass @@ -94,14 +106,23 @@ async def update( """Refreshes the module status.""" try: - new_status = await self._update_internal(use_cache=use_cache) + internal_output = await self._update_internal(use_cache=use_cache) + if isinstance(internal_output, Sequence): + new_status, extra_info = internal_output + else: + new_status, extra_info = internal_output, {} except Exception as err: log.warning(f"{self.name}: failed updating status: {err}") new_status = self.flag(self.flag.__unknown__) if self.flag else None + extra_info = {} # Only notify if the status has changed. - if new_status != self.status or force_output: - await self.notify_status(new_status, **notifier_kwargs) + if (new_status != self.status and not extra_info) or force_output: + await self.notify_status( + new_status, + extra_keywords=extra_info, + **notifier_kwargs, + ) self.status = new_status @@ -110,6 +131,7 @@ async def update( async def notify_status( self, status: Flag_co | None = None, + extra_keywords: dict[str, Any] = {}, wait: bool = False, **kwargs, ): @@ -123,7 +145,7 @@ async def notify_status( return if asyncio.iscoroutinefunction(self.notifier): - coro = self.notifier(status.value, str(status), **kwargs) + coro = self.notifier(status.value, str(status), extra_keywords, **kwargs) if wait: await coro else: @@ -137,5 +159,6 @@ async def notify_status( self.notifier, status.value, str(status), + extra_keywords, **kwargs, ) diff --git a/python/lvmecp/plc.py b/python/lvmecp/plc.py index 36e4ad1..9a4c4e9 100644 --- a/python/lvmecp/plc.py +++ b/python/lvmecp/plc.py @@ -10,7 +10,7 @@ import asyncio -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from lvmecp.hvac import HVACController from lvmecp.modbus import Modbus @@ -32,15 +32,23 @@ def create_actor_notifier( use_hex: bool = True, labels_suffix="_labels", level="d", + allow_broadcasts: bool = False, ): """Generate a notifier function for a keyword.""" - async def notifier(value: int, labels: str, command: Command | None = None): + async def notifier( + value: int, + labels: str, + extra_keywords: dict[str, Any] = {}, + command: Command | None = None, + ): message = { keyword: value if use_hex is False else hex(value), f"{keyword}{labels_suffix}": labels, } - if command is None and actor: + message.update(extra_keywords) + + if command is None and actor and allow_broadcasts: # Allow for 3 seconds for broadcast. This is needed because the PLC # starts before the actor and for the first message the exchange is # not yet available. @@ -114,7 +122,9 @@ async def start_modules(self): async def read_all_registers(self, use_cache: bool = True): """Reads all the connected registers and returns a dictionary.""" - registers = await self.modbus.get_all(use_cache=use_cache) - registers.update(await self.hvac_modbus.get_all(use_cache=use_cache)) + registers_plc, registers_hvac = await asyncio.gather( + self.modbus.read_all(use_cache=use_cache), + self.hvac_modbus.read_all(use_cache=use_cache), + ) - return registers + return registers_plc | registers_hvac diff --git a/python/lvmecp/safety.py b/python/lvmecp/safety.py index 37137f7..9a35147 100644 --- a/python/lvmecp/safety.py +++ b/python/lvmecp/safety.py @@ -9,6 +9,7 @@ from __future__ import annotations import math +import time from types import SimpleNamespace from lvmecp.maskbits import SafetyStatus @@ -27,6 +28,8 @@ def __init__(self, *args, **kwargs): self.o2_level_utilities: float = math.nan self.o2_level_spectrograph: float = math.nan + self.last_heartbeat_ack: float | None = None + async def _update_internal(self, use_cache: bool = True, **kwargs): assert self.flag is not None @@ -51,33 +54,41 @@ async def _update_internal(self, use_cache: bool = True, **kwargs): self.o2_level_utilities = safety_status.oxygen_read_utilities_room / 10.0 if self.o2_level_utilities < self.plc.config["safety"]["o2_threshold"]: new_status |= self.flag.O2_SENSOR_UR_ALARM - if safety_status.oxygen_mode_utilities_room == 8: + if safety_status.oxygen_error_code_utilities_room == 8: new_status |= self.flag.O2_SENSOR_UR_FAULT # Spectrograph room O2 sensor self.o2_level_spectrograph = safety_status.oxygen_read_spectrograph_room / 10.0 if self.o2_level_spectrograph < self.plc.config["safety"]["o2_threshold"]: new_status |= self.flag.O2_SENSOR_SR_ALARM - if safety_status.oxygen_mode_spectrograph_room == 8: + if safety_status.oxygen_error_code_spectrograph_room == 8: new_status |= self.flag.O2_SENSOR_SR_FAULT # Rain sensor if safety_status.rain_sensor_alarm: new_status |= self.flag.RAIN_SENSOR_ALARM + # E-stop + if safety_status.e_status: + new_status |= self.flag.E_STOP + + # Dome lockout and error + if await self.plc.modbus["dome_lockout"].read(): + new_status |= self.flag.DOME_LOCKED + if await self.plc.modbus["dome_error"].read(): + new_status |= self.flag.DOME_ERROR + if new_status.value == 0: new_status = self.flag(self.flag.__unknown__) + if await self.plc.modbus["hb_ack"].read(): + self.last_heartbeat_ack = time.time() + return new_status async def is_remote(self): """Returns `True` if NOT in local mode (i.e., safe to operate remotely).""" - safety_config = self.plc.config.get("safety", {}) - override_local = safety_config.get("override_local_mode", False) - if override_local: - return True - await self.update() assert self.status is not None and self.flag is not None diff --git a/python/lvmecp/simulator.py b/python/lvmecp/simulator.py index 85e5000..f8f1995 100644 --- a/python/lvmecp/simulator.py +++ b/python/lvmecp/simulator.py @@ -9,10 +9,9 @@ from __future__ import annotations import asyncio -from contextlib import suppress from copy import deepcopy -from typing import ClassVar, cast +from typing import Any, cast from pymodbus.datastore import ( ModbusServerContext, @@ -21,6 +20,8 @@ ) from pymodbus.server import ServerAsyncStop, StartAsyncTcpServer +from sdsstools.utils import cancel_task + from lvmecp import config @@ -30,24 +31,21 @@ class Simulator: """A modbus simulator for a PLC controller.""" - OVERRIDES: ClassVar[dict[str, int]] = {} - def __init__( self, registers: dict, - address: str = "127.0.0.1", + host: str = "127.0.0.1", port: int = 5020, - overrides={}, + overrides: dict[str, int | bool] = {}, + events: dict[str, dict[str, Any]] = {}, ): - self.address = address + self.host = host self.port = port self.registers = deepcopy(registers) - self.overrides = Simulator.OVERRIDES.copy() - self.overrides.update(overrides) - - self.current_values: dict[str, list[int]] = {} + self.overrides = overrides + self.events = events self.context: ModbusServerContext | None = None self.slave_context: ModbusSlaveContext @@ -58,10 +56,10 @@ def __init__( self.__task: asyncio.Task | None = None def reset(self): - di = {} - co = {} - hr = {} - ir = {} + di = {address: 0 for address in range(0, 1024)} + co = {address: 0 for address in range(0, 1024)} + hr = {address: 0 for address in range(0, 1024)} + ir = {address: 0 for address in range(0, 1024)} for register in self.registers: mode = self.registers[register].get("mode", "coil") @@ -79,13 +77,6 @@ def reset(self): else: raise ValueError(f"Invalid mode {mode!r} for register {register!r}.") - code = 1 if mode == "coil" else 3 - self.current_values[register.lower()] = [ - address, - code, - value, - ] - self.slave_context = ModbusSlaveContext( di=ModbusSparseDataBlock(di), co=ModbusSparseDataBlock(co), @@ -102,69 +93,97 @@ async def start(self, monitor_interval: float = 0.01): self.__task = asyncio.create_task(self._monitor_context(monitor_interval)) - await StartAsyncTcpServer( - self.context, - address=(self.address, self.port), - ) + await StartAsyncTcpServer(self.context, address=(self.host, self.port)) async def stop(self): """Stops the simulator.""" await ServerAsyncStop() - if self.__task: - self.__task.cancel() - with suppress(asyncio.CancelledError): - await self.__task - - self.__task = None + self.__task = await cancel_task(self.__task) def __del__(self): if self.__task: self.__task.cancel() - async def _monitor_context(self, interval: float): - """Monitor the context.""" + def get_register_data(self, register: str): + """Returns the data for a register.""" + + register_data = self.registers[register] + + address = register_data["address"] + mode = register_data["mode"] - async def set_value(register: str, new_value: int, delay: float = 0): - if delay > 0: - await asyncio.sleep(delay) + if mode == "coil": + code = 1 + elif mode == "holding_register": + code = 3 + else: + raise ValueError(f"Invalid mode {mode!r} for register {register!r}.") - address, code, current_value = self.current_values[register] + return { + "name": register, + "address": address, + "mode": mode, + "code": code, + "value": int(self.slave_context.getValues(code, address, 1)[0]), + } - context.setValues(code, address, [new_value]) - self.current_values[register][2] = new_value + async def _monitor_context(self, interval: float): + """Monitor the context.""" assert self.context context = cast(ModbusSlaveContext, self.context[0]) while True: - for register in self.current_values: - address, code, current_value = self.current_values[register] - new_value = int(context.getValues(code, address, count=1)[0]) - - if new_value == current_value: + for trigger_register, event_data in self.events.items(): + on_value: int | bool | None = event_data.get("on_value", None) + if on_value is None: continue - self.current_values[register][2] = new_value + trigger_data = self.get_register_data(trigger_register) - if register.endswith("_new"): - # For lights. When we change the value of the XX_new - # register the light is switched and XX_status changes value. - status_name = register.replace("_new", "_status") - asyncio.create_task(set_value(status_name, new_value, 0.0)) + if trigger_data["value"] != on_value: + continue - elif register == "e_stop": - if new_value == 1: - asyncio.create_task(set_value("e_status", 1, 0.0)) - asyncio.create_task(set_value("e_reset", 0, 0.0)) + then = event_data["then"] + + then_register_data = self.get_register_data(then["register"]) + if then["action"] == "toggle": + context.setValues( + then_register_data["code"], + then_register_data["address"], + [not then_register_data["value"]], + ) + elif then["action"] == "set": + context.setValues( + then_register_data["code"], + then_register_data["address"], + [1], + ) + elif then["action"] == "reset": + context.setValues( + then_register_data["code"], + then_register_data["address"], + [0], + ) + else: + continue - elif register == "e_reset": - if new_value == 1: - asyncio.create_task(set_value("e_status", 0, 0.0)) - asyncio.create_task(set_value("e_stop", 0, 0.0)) + if then.get("reset_trigger", True): + context.setValues( + trigger_data["code"], + trigger_data["address"], + [0], + ) await asyncio.sleep(interval) -plc_simulator = Simulator(config["modbus"]["registers"]) +plc_simulator = Simulator( + config["modbus"]["registers"], + host=config["simulator"]["host"], + port=config["simulator"]["port"], + overrides=config["simulator"]["overrides"], + events=config["simulator"]["events"], +) diff --git a/python/lvmecp/tools.py b/python/lvmecp/tools.py index a8512c4..04d5b9e 100644 --- a/python/lvmecp/tools.py +++ b/python/lvmecp/tools.py @@ -9,10 +9,11 @@ from __future__ import annotations import asyncio +import time from contextlib import suppress from datetime import datetime, timezone -from typing import Any, Callable, Coroutine +from typing import Any, Callable, Coroutine, Literal __all__ = ["loop_coro", "cancel_tasks_by_name", "timestamp_to_iso"] @@ -60,3 +61,50 @@ def timestamp_to_iso(ts: float | None, timespec: str = "seconds") -> str | None: .isoformat(timespec=timespec) .replace("+00:00", "Z") ) + + +class TimedCacheDict(dict): + """A dictionary that caches values for a certain amount of time. + + Parameters + ---------- + timeout + The timeout in seconds for the cache. + mode + The mode for the cache. If ``delete``, the key will be deleted + after the timeout. If ``null``, the value will be set to ``None``. + + """ + + def __init__(self, timeout: float, mode: Literal["delete", "null"] = "delete"): + self.timeout = timeout + self.mode = mode + + self._cache_time: dict[str, float] = {} + + super().__init__() + + def freeze(self): + """Returns a non-cached version of the dictionary.""" + + return dict(self) + + def __getitem__(self, key: str): + try: + if time.time() - self._cache_time[key] > self.timeout: + if self.mode == "delete": + del self[key] + else: + self[key] = None + except KeyError: + pass + + return super().__getitem__(key) + + def __setitem__(self, key: str, value): + self._cache_time[key] = time.time() + super().__setitem__(key, value) + + def __delitem__(self, key: str): + del self._cache_time[key] + super().__delitem__(key) diff --git a/tests/conftest.py b/tests/conftest.py index cc09be9..1b96171 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,13 +13,18 @@ from contextlib import suppress from copy import deepcopy +from typing import cast + import pytest +import pytest_mock +from pymodbus.datastore import ModbusSlaveContext from clu.testing import setup_test_actor import lvmecp from lvmecp import config from lvmecp.actor import ECPActor +from lvmecp.modbus import Modbus from lvmecp.simulator import Simulator, plc_simulator @@ -38,7 +43,14 @@ async def simulator(): @pytest.fixture() -async def actor(simulator: Simulator, mocker): +def context(simulator: Simulator) -> ModbusSlaveContext: + assert simulator.context + + return cast(ModbusSlaveContext, simulator.context[0]) + + +@pytest.fixture() +def test_config(): ecp_config = deepcopy(config) del ecp_config["actor"]["log_dir"] @@ -49,9 +61,25 @@ async def actor(simulator: Simulator, mocker): schema_path = ecp_config["actor"]["schema"] ecp_config["actor"]["schema"] = os.path.dirname(lvmecp.__file__) + "/" + schema_path - _actor = ECPActor.from_config(ecp_config) + yield ecp_config + - mocker.patch.object(_actor.plc.hvac.modbus, "get_all", return_value={}) +@pytest.fixture() +async def modbus(simulator: Simulator, test_config: dict): + _modbus = Modbus(test_config["modbus"]) + + yield _modbus + + +@pytest.fixture() +async def actor( + simulator: Simulator, + mocker: pytest_mock.MockerFixture, + test_config: dict, +): + _actor = ECPActor.from_config(test_config) + + mocker.patch.object(_actor.plc.hvac.modbus, "read_all", return_value={}) _actor = await setup_test_actor(_actor) # type: ignore _actor.connection.connection = mocker.MagicMock(spec={"is_closed": False}) diff --git a/tests/test_command_dome.py b/tests/test_command_dome.py index 7d74211..6639a60 100644 --- a/tests/test_command_dome.py +++ b/tests/test_command_dome.py @@ -21,12 +21,81 @@ if TYPE_CHECKING: + from pymodbus.datastore import ModbusSlaveContext from pytest_mock import MockerFixture from lvmecp.actor import ECPActor -async def test_command_dome_open(actor: ECPActor, mocker: MockerFixture): +@pytest.mark.parametrize("open", [True, False]) +async def test_command_dome_status( + context: ModbusSlaveContext, + actor: ECPActor, + open: bool, +): + if open: + address = actor.plc.modbus["dome_open"].address + else: + address = actor.plc.modbus["dome_closed"].address + + context.setValues(1, address, [1]) + + cmd = await actor.invoke_mock_command("dome status") + await cmd + + assert cmd.status.did_succeed + + +async def test_command_dome_moving(context: ModbusSlaveContext, actor: ECPActor): + address = actor.plc.modbus["drive_enabled"].address + + context.setValues(1, address, [1]) + + cmd = await actor.invoke_mock_command("dome status") + await cmd + + assert cmd.status.did_succeed + text = cmd.replies.get("text") + assert text == "Dome is moving!!!" + + +async def test_command_dome_position_unknown( + context: ModbusSlaveContext, + actor: ECPActor, +): + context.setValues(1, actor.plc.modbus["dome_closed"].address, [0]) + + cmd = await actor.invoke_mock_command("dome status") + await cmd + + assert cmd.status.did_succeed + text = cmd.replies.get("text") + assert text == "Dome position is unknown!!!" + + +async def test_command_dome_stop(actor: ECPActor): + await actor.plc.modbus["drive_enabled"].write(1) + + cmd = await actor.invoke_mock_command("dome stop") + await cmd + + assert cmd.status.did_succeed + + assert (await actor.plc.modbus["drive_enabled"].read(use_cache=False)) == 0 + + +async def test_command_dome_reset(context: ModbusSlaveContext, actor: ECPActor): + context.setValues(1, actor.plc.modbus["dome_error"].address, [1]) + + cmd = await actor.invoke_mock_command("dome reset") + await cmd + + assert cmd.status.did_succeed + + assert (await actor.plc.modbus["dome_error"].read(use_cache=False)) == 0 + + +async def test_command_dome_open_mock(actor: ECPActor, mocker: MockerFixture): mocker.patch.object(actor.plc.dome, "is_daytime", return_value=False) mocker.patch.object(actor.plc.dome, "_move", return_value=True) @@ -38,6 +107,35 @@ async def test_command_dome_open(actor: ECPActor, mocker: MockerFixture): assert cmd.status.did_succeed +async def test_command_dome_open( + actor: ECPActor, + context: ModbusSlaveContext, + mocker: MockerFixture, +): + async def open_with_delay(): + context.setValues(1, actor.plc.modbus["dome_closed"].address, [0]) + + await asyncio.sleep(0.3) + + context.setValues(1, actor.plc.modbus["dome_open"].address, [1]) + context.setValues(1, actor.plc.modbus["drive_enabled"].address, [0]) + + mocker.patch.object(actor.plc.dome, "is_daytime", return_value=False) + mocker.patch.object(lvmecp.dome, "MOVE_CHECK_INTERVAL", 0.1) + + cmd = await actor.invoke_mock_command("dome open") + + await asyncio.sleep(0.1) + asyncio.create_task(open_with_delay()) + + await cmd + + assert cmd.status.did_succeed + + assert (await actor.plc.modbus["drive_enabled"].read(use_cache=False)) == 0 + assert (await actor.plc.modbus["dome_open"].read(use_cache=False)) == 1 + + async def test_command_dome_close(actor: ECPActor, mocker: MockerFixture): mocker.patch.object(actor.plc.dome, "_move", return_value=True) diff --git a/tests/test_command_heartbeat.py b/tests/test_command_heartbeat.py index 43e1ea5..2626724 100644 --- a/tests/test_command_heartbeat.py +++ b/tests/test_command_heartbeat.py @@ -18,7 +18,7 @@ async def test_command_heartbeat(actor: ECPActor, mocker: MockerFixture): - hb_set_mock = mocker.patch.object(actor.plc.modbus["hb_set"], "set") + hb_set_mock = mocker.patch.object(actor.plc.modbus["hb_set"], "write") cmd = await actor.invoke_mock_command("heartbeat") await cmd @@ -29,7 +29,7 @@ async def test_command_heartbeat(actor: ECPActor, mocker: MockerFixture): async def test_command_heartbeat_fails(actor: ECPActor, mocker: MockerFixture): - mocker.patch.object(actor.plc.modbus["hb_set"], "set", side_effect=Exception) + mocker.patch.object(actor.plc.modbus["hb_set"], "write", side_effect=Exception) cmd = await actor.invoke_mock_command("heartbeat") await cmd diff --git a/tests/test_command_modbus.py b/tests/test_command_modbus.py new file mode 100644 index 0000000..824d052 --- /dev/null +++ b/tests/test_command_modbus.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# +# @Author: José Sánchez-Gallego (gallegoj@uw.edu) +# @Date: 2024-12-28 +# @Filename: test_command_modbus.py +# @License: BSD 3-clause (http://www.opensource.org/licenses/BSD-3-Clause) + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from pytest_mock import MockerFixture + + +if TYPE_CHECKING: + from pymodbus.datastore import ModbusSlaveContext + + from lvmecp.actor import ECPActor + + +async def test_command_modbus_read(actor: ECPActor): + read_cmd = await actor.invoke_mock_command("modbus read door_locked") + await read_cmd + + assert read_cmd.status.did_succeed + assert read_cmd.replies.get("register")["value"] + + +async def test_command_modbus_read_address(actor: ECPActor): + read_cmd = await actor.invoke_mock_command("modbus read --register-type coil 1") + await read_cmd + + assert read_cmd.status.did_succeed + + assert read_cmd.replies.get("register")["name"] == "door_closed" + assert read_cmd.replies.get("register")["value"] + + +async def test_command_modbus_read_address_no_register_type(actor: ECPActor): + read_cmd = await actor.invoke_mock_command("modbus read 1") + await read_cmd + + assert read_cmd.status.did_fail + + assert "--register-type must be specified" in read_cmd.replies.get("error") + + +async def test_command_modbus_read_bad_register(actor: ECPActor): + read_cmd = await actor.invoke_mock_command("modbus read bad_register") + await read_cmd + + assert read_cmd.status.did_fail + + assert "not found" in read_cmd.replies.get("error") + + +async def test_modbus_write_register(actor: ECPActor): + pre_value = await actor.plc.modbus["motor_direction"].read(use_cache=False) + assert pre_value == 0 + + write_cmd = await actor.invoke_mock_command("modbus write motor_direction 1") + await write_cmd + + assert write_cmd.status.did_succeed + + new_value = await actor.plc.modbus["motor_direction"].read(use_cache=False) + assert new_value == 1 + + +async def test_modbus_write_register_readonly(actor: ECPActor): + write_cmd = await actor.invoke_mock_command("modbus write door_locked 1") + await write_cmd + + assert write_cmd.status.did_fail + assert "is read-only" in write_cmd.replies.get("error") + + +async def test_modbus_write_register_fails(actor: ECPActor, mocker: MockerFixture): + mocker.patch.object( + actor.plc.modbus["motor_direction"], + "write", + side_effect=ValueError("cannot write"), + ) + + write_cmd = await actor.invoke_mock_command("modbus write motor_direction 1") + await write_cmd + + assert write_cmd.status.did_fail + assert "cannot write" in write_cmd.replies.get("error") + + +async def test_modbus_write_unknown_register( + context: ModbusSlaveContext, + actor: ECPActor, +): + cmd = await actor.invoke_mock_command( + "modbus write --register-type coil --allow-unknown 999 1" + ) + await cmd + + assert cmd.status.did_succeed + + register = cmd.replies.get("register") + assert register == {"name": "coil_999", "address": 999, "value": True} diff --git a/tests/test_modbus.py b/tests/test_modbus.py new file mode 100644 index 0000000..e093a08 --- /dev/null +++ b/tests/test_modbus.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# +# @Author: José Sánchez-Gallego (gallegoj@uw.edu) +# @Date: 2024-12-29 +# @Filename: test_modbus.py +# @License: BSD 3-clause (http://www.opensource.org/licenses/BSD-3-Clause) + +from __future__ import annotations + +import asyncio + +from typing import TYPE_CHECKING, cast + +import pytest +from pytest_mock import MockerFixture + +import lvmecp.modbus +from lvmecp.modbus import ModbusRegister, RegisterModes + + +if TYPE_CHECKING: + from pymodbus.datastore import ModbusSlaveContext + + from lvmecp.modbus import Modbus + + +async def test_modbus_read(modbus: Modbus): + resp = await modbus.read_register("door_locked") + assert resp == 1 + + +@pytest.mark.parametrize( + "register_type", + ["coil", "discrete_input", "input_register", "holding_register"], +) +async def test_modbus_register_read( + context: ModbusSlaveContext, + modbus: Modbus, + register_type: str, +): + is_discrete = register_type in ["coil", "discrete_input"] + + write_func_code = 1 + if register_type == "discrete_input": + write_func_code = 2 + elif register_type == "holding_register": + write_func_code = 3 + elif register_type == "input_register": + write_func_code = 4 + + context.setValues(write_func_code, 99, [1 if is_discrete else 101]) + + register = ModbusRegister( + modbus, + name="test_register", + address=99, + mode=cast(RegisterModes, register_type), + count=1, + readonly=True, + ) + + resp = await register.read(use_cache=False) + assert resp == (1 if is_discrete else 101) + + +async def test_modbus_register_read_decoder_float_32bit( + context: ModbusSlaveContext, + modbus: Modbus, +): + context.setValues(3, 99, [4000, 16000]) + + register = ModbusRegister( + modbus, + name="test_register", + address=99, + mode="holding_register", + count=2, + readonly=True, + decoder="float_32bit", + ) + + resp = await register.read(use_cache=False) + + assert resp == 0.250 + + +@pytest.mark.parametrize("register_type", ["coil", "holding_register"]) +async def test_modbus_register_write( + context: ModbusSlaveContext, + modbus: Modbus, + register_type: str, +): + is_discrete = register_type in ["coil", "discrete_input"] + write_func_code = 1 if is_discrete else 3 + + register = ModbusRegister( + modbus, + name="test_register", + address=99, + mode=cast(RegisterModes, register_type), + count=1, + readonly=False, + ) + + await register.write(1 if is_discrete else 101) + + value = context.getValues(write_func_code, 99) + assert value[0] == (1 if is_discrete else 101) + + +@pytest.mark.parametrize("register_type", ["discrete_input", "input_register"]) +async def test_modbus_register_write_on_readonly(modbus: Modbus, register_type: str): + register = ModbusRegister( + modbus, + name="test_register", + address=99, + mode=cast(RegisterModes, register_type), + count=1, + readonly=False, + ) + + with pytest.raises(ValueError) as err: + await register.write(1) + + assert "is read-only" in str(err.value) + + +async def test_modbus_connection_fails(modbus: Modbus, mocker: MockerFixture): + mocker.patch.object(modbus.client, "connect", side_effect=ConnectionError) + + with pytest.raises(ConnectionError): + await modbus.read_register("door_locked") + + +async def test_modbus_connection_timeouts(modbus: Modbus, mocker: MockerFixture): + mocker.patch.object(modbus.client, "connect", side_effect=asyncio.TimeoutError) + + with pytest.raises(ConnectionError): + await modbus.read_register("door_locked") + + +async def test_modbus_lock_release(modbus: Modbus, mocker: MockerFixture): + mocker.patch.object(lvmecp.modbus, "CONNECTION_TIMEOUT", 0.1) + + async with modbus: + assert modbus.client.connected + assert modbus.lock.locked() + + await asyncio.sleep(0.2) + + assert not modbus.client.connected + assert not modbus.lock.locked()