From 95e628cc2353cffbe1f3e97e8786b8488f944613 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 23 Oct 2024 19:29:04 +0900 Subject: [PATCH 01/28] Added remote update function and minimally working --- miniscope_io/cli/main.py | 2 + miniscope_io/cli/update.py | 26 ++++ miniscope_io/device_update.py | 245 +++++++++++++++++++--------------- miniscope_io/plots/headers.py | 2 +- 4 files changed, 166 insertions(+), 109 deletions(-) create mode 100644 miniscope_io/cli/update.py diff --git a/miniscope_io/cli/main.py b/miniscope_io/cli/main.py index 7392b165..9fc7fc2e 100644 --- a/miniscope_io/cli/main.py +++ b/miniscope_io/cli/main.py @@ -5,6 +5,7 @@ import click from miniscope_io.cli.stream import stream +from miniscope_io.cli.update import update @click.group() @@ -18,3 +19,4 @@ def cli(ctx: click.Context) -> None: cli.add_command(stream) +cli.add_command(update) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py new file mode 100644 index 00000000..8f25b3ce --- /dev/null +++ b/miniscope_io/cli/update.py @@ -0,0 +1,26 @@ +""" +CLI for updating device over IR or UART. +""" + +import click + +from miniscope_io.device_update import DevUpdate + + +@click.command() +@click.option("-p", "--port", + required=False, + help="Serial port to connect to. Needed if multiple FTDI devices are connected.") +@click.option( + "-t", + "--target", + required=True, + type=click.Choice(["LED", "GAIN"]), + help="Target to update" + ) +@click.option("-v", "--value", required=True, type=int, help="Value to set") +def update(port: str, target: str, value: int) -> None: + """ + Update device configuration. + """ + DevUpdate(port=port, target=target, value=value) \ No newline at end of file diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 255de749..4dcd2660 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -7,138 +7,167 @@ """ -import argparse -import sys import time +from enum import Enum +from typing import Optional -import numpy as np import serial +import serial.tools.list_ports +from pydantic import BaseModel, ConfigDict, field_validator, model_validator -from miniscope_io import init_logger +from miniscope_io.logging import init_logger -# Parsers for update LED -updateDeviceParser = argparse.ArgumentParser("updateDevice") -updateDeviceParser.add_argument("port", help="serial port") -updateDeviceParser.add_argument("baudrate", help="baudrate") -updateDeviceParser.add_argument("module", help="module to update") -updateDeviceParser.add_argument("value", help="LED value") +logger = init_logger(name="device_update", level="DEBUG") +class UpdateTarget(Enum): + """Targets to update.""" + LED = 0 + GAIN = 1 -def updateDevice() -> None: +class DevUpdateCommand(BaseModel): """ - Script to update hardware settings over a generic UART-USB converter. - This script currently supports updating the excitation LED brightness and - electrical wetting lens driver gain. + Command to update device configuration. + """ + + port: str + target: UpdateTarget + value: int + + model_config = ConfigDict(arbitrary_types_allowed=True) + + @model_validator(mode="after") + def validate_values(cls, values: dict) -> dict: + """ + Validate values based on target. + """ + target = values.target + value = values.value + + if target == UpdateTarget.LED: + assert 0 <= value <= 100, "For LED, value must be between 0 and 100" + elif target == UpdateTarget.GAIN: + assert 0 <= value <= 255, "For GAIN, value must be between 0 and 255" + return values + + @field_validator("port") + def validate_port(cls, value: str)->str: + """ + Validate port. + + Args: + value: Port to validate. + + Returns: + Validated port. + + Raises: + ValueError: If no serial ports found or port not found. + """ + portlist = list(serial.tools.list_ports.comports()) + + if len(portlist) == 0: + raise ValueError("No serial ports found") + if value not in [port.device for port in portlist]: + raise ValueError(f"Port {value} not found") + return value + + @field_validator("target", mode="before") + def validate_target(cls, value: str) -> UpdateTarget: + """ + Validate and convert target string to UpdateTarget Enum type. + + Args: + value (str): Target to validate. + + Returns: + UpdateTarget: Validated target as UpdateTarget. + + Raises: + ValueError: If target not found. + """ + try: + return UpdateTarget[value] + except KeyError as e: + raise ValueError(f"Target {value} not found.") from e + +def DevUpdate( + target: str, + value: int, + port: Optional[str] = None, +) -> None: + """ + IR-based update of device configuration. .. note:: Not tested after separating from stream_daq.py. - Examples - -------- - >>> updateDevice [COM port] [baudrate] [module] [value] + Args: + port: Serial port to which the device is connected. + target: What to update on the device (e.g., LED or GAIN). + value: Value to which the target should be updated. - ..todo:: - Test to see if changing package structure broke anything. + Returns: + None """ - logger = init_logger("streamDaq") - - args = updateDeviceParser.parse_args() - moduleList = ["LED", "EWL"] - - ledMAX = 100 - ledMIN = 0 - - ewlMAX = 255 - ewlMIN = 0 - - ledDeviceTag = 0 # 2-bits each for now - ewlDeviceTag = 1 # 2-bits each for now - deviceTagPos = 4 - preamblePos = 6 - - Preamble = [2, 1] # 2-bits each for now - - uartPayload = 4 - uartRepeat = 5 - uartTimeGap = 0.01 - - try: - assert len(vars(args)) == 4 - except AssertionError as msg: - logger.exception("Usage: updateDevice [COM port] [baudrate] [module] [value]") - raise msg + if port: + logger.info(f"Using port {port}") + command = DevUpdateCommand(port=port, target=target, value=value) + else: + ftdi_port_list = find_ftdi_device() + if len(ftdi_port_list) == 0: + raise ValueError("No FTDI devices found.") + if len(ftdi_port_list) > 1: + raise ValueError("Multiple FTDI devices found. Please specify the port.") + if len(ftdi_port_list) == 1: + port = ftdi_port_list[0] + logger.info(f"Using port {port}") + + command = DevUpdateCommand(port=port, target=target, value=value) + logger.info(f"Updating {target} to {value} on port {port}") + + #Header to indicate target/value. This should be a bit pattern that is unlikely to be the value. + target_mask = 0b01110000 + value_mask = 0b10000000 + reset_byte = 0b00000000 try: - comport = str(args.port) - except (ValueError, IndexError) as e: + serial_port = serial.Serial(port=command.port, baudrate=2400, timeout=5, stopbits=2) + except Exception as e: logger.exception(e) raise e + logger.info("Open serial port") try: - baudrate = int(args.baudrate) - except (ValueError, IndexError) as e: - logger.exception(e) - raise e + target_command = command.target.value + target_mask + serial_port.write(target_command.to_bytes(1, 'big')) + logger.debug(f"Target {command.target}; command: {bin(target_command)}") + time.sleep(0.1) - try: - module = str(args.module) - assert module in moduleList - except AssertionError as msg: - err_str = "Available modules:\n" - for module in moduleList: - err_str += "\t" + module + "\n" - logger.exception(err_str) - raise msg + value_command = command.value + value_mask + serial_port.write(value_command.to_bytes(1, 'big')) + logger.debug(f"Value {command.value}; command: {bin(value_command)}") + time.sleep(0.1) - try: - value = int(args.value) - except Exception as e: - logger.exception("Value needs to be an integer") - raise e + serial_port.write(reset_byte.to_bytes(1, "big")) - try: - if module == "LED": - assert value <= ledMAX and value >= ledMIN - if module == "EWL": - assert value <= ewlMAX and value >= ewlMIN - except AssertionError as msg: - if module == "LED": - logger.exception("LED value need to be a integer within 0-100") - if module == "EWL": - logger.exception("EWL value need to be an integer within 0-255") - raise msg - - if module == "LED": - deviceTag = ledDeviceTag << deviceTagPos - elif module == "EWL": - deviceTag = ewlDeviceTag << deviceTagPos - - command = [0, 0] - - command[0] = int( - Preamble[0] * 2**preamblePos + deviceTag + np.floor(value / (2**uartPayload)) - ).to_bytes(1, "big") - command[1] = int( - Preamble[1] * 2**preamblePos + deviceTag + value % (2**uartPayload) - ).to_bytes(1, "big") - - # set up serial port - try: - serial_port = serial.Serial(port=comport, baudrate=baudrate, timeout=5, stopbits=1) - except Exception as e: - logger.exception(e) - raise e - logger.info("Open serial port") + finally: + serial_port.close() + logger.info("Closed serial port") - for uartCommand in command: - for _ in range(uartRepeat): - # read UART data until preamble and put into queue - serial_port.write(uartCommand) - time.sleep(uartTimeGap) - serial_port.close() - logger.info("\t" + module + ": " + str(value)) - logger.info("Close serial port") - sys.exit(1) +def find_ftdi_device() -> list: + """ + Find FTDI devices connected to the computer. + """ + FTDI_VENDOR_ID = 0x0403 + FTDI_PRODUCT_ID = 0x6001 + ports = serial.tools.list_ports.comports() + ftdi_ports = [] + + for port in ports: + if port.vid == FTDI_VENDOR_ID and port.pid == FTDI_PRODUCT_ID: + ftdi_ports.append(port.device) + + return ftdi_ports \ No newline at end of file diff --git a/miniscope_io/plots/headers.py b/miniscope_io/plots/headers.py index 8082c288..71ba1d28 100644 --- a/miniscope_io/plots/headers.py +++ b/miniscope_io/plots/headers.py @@ -153,7 +153,7 @@ def _init_plot( plt.ion() fig: plt.Figure axes: np.ndarray[Any, np.dtype[plt.Axes]] - fig, axes = plt.subplots(len(self.header_keys), 1, figsize=(6, len(self.header_keys) * 3)) + fig, axes = plt.subplots(len(self.header_keys), 1, figsize=(6, len(self.header_keys) * 2)) axes = np.array(axes).reshape(-1) # Ensure axes is an array # Initialize line objects From e88e20833ca22e1ee225d63c9e9842ed41819ab6 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Thu, 24 Oct 2024 14:31:25 +0900 Subject: [PATCH 02/28] Formatting --- miniscope_io/cli/update.py | 19 +++++++++---------- miniscope_io/device_update.py | 33 +++++++++++++++++++-------------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index 8f25b3ce..0ba1eb7a 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -8,19 +8,18 @@ @click.command() -@click.option("-p", "--port", - required=False, - help="Serial port to connect to. Needed if multiple FTDI devices are connected.") @click.option( - "-t", - "--target", - required=True, - type=click.Choice(["LED", "GAIN"]), - help="Target to update" - ) + "-p", + "--port", + required=False, + help="Serial port to connect to. Needed if multiple FTDI devices are connected.", +) +@click.option( + "-t", "--target", required=True, type=click.Choice(["LED", "GAIN"]), help="Target to update" +) @click.option("-v", "--value", required=True, type=int, help="Value to set") def update(port: str, target: str, value: int) -> None: """ Update device configuration. """ - DevUpdate(port=port, target=target, value=value) \ No newline at end of file + DevUpdate(port=port, target=target, value=value) diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 4dcd2660..896bd5e6 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -19,11 +19,14 @@ logger = init_logger(name="device_update", level="DEBUG") + class UpdateTarget(Enum): """Targets to update.""" + LED = 0 GAIN = 1 + class DevUpdateCommand(BaseModel): """ Command to update device configuration. @@ -50,7 +53,7 @@ def validate_values(cls, values: dict) -> dict: return values @field_validator("port") - def validate_port(cls, value: str)->str: + def validate_port(cls, value: str) -> str: """ Validate port. @@ -90,10 +93,11 @@ def validate_target(cls, value: str) -> UpdateTarget: except KeyError as e: raise ValueError(f"Target {value} not found.") from e + def DevUpdate( - target: str, - value: int, - port: Optional[str] = None, + target: str, + value: int, + port: Optional[str] = None, ) -> None: """ IR-based update of device configuration. @@ -123,14 +127,15 @@ def DevUpdate( if len(ftdi_port_list) == 1: port = ftdi_port_list[0] logger.info(f"Using port {port}") - + command = DevUpdateCommand(port=port, target=target, value=value) logger.info(f"Updating {target} to {value} on port {port}") - #Header to indicate target/value. This should be a bit pattern that is unlikely to be the value. - target_mask = 0b01110000 - value_mask = 0b10000000 - reset_byte = 0b00000000 + # Header to indicate target/value. + # This should be a bit pattern that is unlikely to be the value. + target_mask = 0b01110000 + value_mask = 0b10000000 + reset_byte = 0b00000000 try: serial_port = serial.Serial(port=command.port, baudrate=2400, timeout=5, stopbits=2) @@ -141,12 +146,12 @@ def DevUpdate( try: target_command = command.target.value + target_mask - serial_port.write(target_command.to_bytes(1, 'big')) + serial_port.write(target_command.to_bytes(1, "big")) logger.debug(f"Target {command.target}; command: {bin(target_command)}") time.sleep(0.1) value_command = command.value + value_mask - serial_port.write(value_command.to_bytes(1, 'big')) + serial_port.write(value_command.to_bytes(1, "big")) logger.debug(f"Value {command.value}; command: {bin(value_command)}") time.sleep(0.1) @@ -165,9 +170,9 @@ def find_ftdi_device() -> list: FTDI_PRODUCT_ID = 0x6001 ports = serial.tools.list_ports.comports() ftdi_ports = [] - + for port in ports: if port.vid == FTDI_VENDOR_ID and port.pid == FTDI_PRODUCT_ID: ftdi_ports.append(port.device) - - return ftdi_ports \ No newline at end of file + + return ftdi_ports From ada51cbcf74269f1e9b60cc5e485a3cd4933578e Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sun, 27 Oct 2024 18:40:42 +0900 Subject: [PATCH 03/28] ROI/gain/LED value remote update prototype working --- miniscope_io/cli/update.py | 6 +++++- miniscope_io/device_update.py | 13 +++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index 0ba1eb7a..d17e56c1 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -15,7 +15,11 @@ help="Serial port to connect to. Needed if multiple FTDI devices are connected.", ) @click.option( - "-t", "--target", required=True, type=click.Choice(["LED", "GAIN"]), help="Target to update" + "-t", + "--target", + required=True, + type=click.Choice(["LED", "GAIN", "ROI_X", "ROI_Y"]), + help="Target to update" ) @click.option("-v", "--value", required=True, type=int, help="Value to set") def update(port: str, target: str, value: int) -> None: diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 896bd5e6..c8df2c04 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -11,6 +11,7 @@ from enum import Enum from typing import Optional +import numpy as np import serial import serial.tools.list_ports from pydantic import BaseModel, ConfigDict, field_validator, model_validator @@ -25,7 +26,11 @@ class UpdateTarget(Enum): LED = 0 GAIN = 1 - + ROI_X = 2 + ROI_Y = 3 + ROI_WIDTH = 4 # not implemented + ROI_HEIGHT = 5 # not implemented + EWL = 6 # not implemented class DevUpdateCommand(BaseModel): """ @@ -49,7 +54,7 @@ def validate_values(cls, values: dict) -> dict: if target == UpdateTarget.LED: assert 0 <= value <= 100, "For LED, value must be between 0 and 100" elif target == UpdateTarget.GAIN: - assert 0 <= value <= 255, "For GAIN, value must be between 0 and 255" + assert value in [1, 2, 4], "For GAIN, value must be 1, 2, or 4" return values @field_validator("port") @@ -134,7 +139,7 @@ def DevUpdate( # Header to indicate target/value. # This should be a bit pattern that is unlikely to be the value. target_mask = 0b01110000 - value_mask = 0b10000000 + value_mask = 0b00000000 reset_byte = 0b00000000 try: @@ -150,7 +155,7 @@ def DevUpdate( logger.debug(f"Target {command.target}; command: {bin(target_command)}") time.sleep(0.1) - value_command = command.value + value_mask + value_command = (command.value + value_mask) & 0xFF serial_port.write(value_command.to_bytes(1, "big")) logger.debug(f"Value {command.value}; command: {bin(value_command)}") time.sleep(0.1) From e705b69ae31164617cf003cea0013a1d161a17cc Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sun, 27 Oct 2024 19:27:26 +0900 Subject: [PATCH 04/28] allow multi-word value transfer with headers --- miniscope_io/device_update.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index c8df2c04..8281bd14 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -138,8 +138,11 @@ def DevUpdate( # Header to indicate target/value. # This should be a bit pattern that is unlikely to be the value. - target_mask = 0b01110000 - value_mask = 0b00000000 + target_mask = 0b11000000 + LSB_header = 0b01000000 + MSB_header = 0b10000000 + LSB_value_mask = 0b000000111111 #value below 12-bit + MSB_value_mask = 0b111111000000 #value below 12-bit reset_byte = 0b00000000 try: @@ -155,9 +158,14 @@ def DevUpdate( logger.debug(f"Target {command.target}; command: {bin(target_command)}") time.sleep(0.1) - value_command = (command.value + value_mask) & 0xFF - serial_port.write(value_command.to_bytes(1, "big")) - logger.debug(f"Value {command.value}; command: {bin(value_command)}") + value_LSB_command = ((command.value & LSB_value_mask) + LSB_header) & 0xFF + serial_port.write(value_LSB_command.to_bytes(1, "big")) + logger.debug(f"Value {command.value}; command_LSB: {bin(value_LSB_command)}") + time.sleep(0.1) + + value_MSB_command = (((command.value & MSB_value_mask) >> 6) + MSB_header) & 0xFF + serial_port.write(value_MSB_command.to_bytes(1, "big")) + logger.debug(f"Value {command.value}; command_MSB: {bin(value_MSB_command)}") time.sleep(0.1) serial_port.write(reset_byte.to_bytes(1, "big")) From 7ad77adb445134ef009462e6db227f02bcf57aec Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sun, 27 Oct 2024 20:28:38 +0900 Subject: [PATCH 05/28] device ID-based over-the-air update --- miniscope_io/cli/update.py | 14 +++++++++++--- miniscope_io/device_update.py | 36 +++++++++++++++++++++-------------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index d17e56c1..f17aef0d 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -14,16 +14,24 @@ required=False, help="Serial port to connect to. Needed if multiple FTDI devices are connected.", ) +@click.option( + "-i", + "--device_id", + required=False, + default=0, + type=int, + help="ID of the device to update. 0 will update all devices.", +) @click.option( "-t", "--target", required=True, type=click.Choice(["LED", "GAIN", "ROI_X", "ROI_Y"]), - help="Target to update" + help="Target to update", ) @click.option("-v", "--value", required=True, type=int, help="Value to set") -def update(port: str, target: str, value: int) -> None: +def update(port: str, target: str, value: int, device_id: int) -> None: """ Update device configuration. """ - DevUpdate(port=port, target=target, value=value) + DevUpdate(port=port, target=target, value=value, device_id=device_id) diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 8281bd14..cff20ddd 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -11,7 +11,6 @@ from enum import Enum from typing import Optional -import numpy as np import serial import serial.tools.list_ports from pydantic import BaseModel, ConfigDict, field_validator, model_validator @@ -28,15 +27,17 @@ class UpdateTarget(Enum): GAIN = 1 ROI_X = 2 ROI_Y = 3 - ROI_WIDTH = 4 # not implemented - ROI_HEIGHT = 5 # not implemented - EWL = 6 # not implemented + ROI_WIDTH = 4 # not implemented + ROI_HEIGHT = 5 # not implemented + EWL = 6 # not implemented + class DevUpdateCommand(BaseModel): """ Command to update device configuration. """ + device_id: int port: str target: UpdateTarget value: int @@ -102,6 +103,7 @@ def validate_target(cls, value: str) -> UpdateTarget: def DevUpdate( target: str, value: int, + device_id: int, port: Optional[str] = None, ) -> None: """ @@ -112,6 +114,7 @@ def DevUpdate( Not tested after separating from stream_daq.py. Args: + device_id: ID of the device. 0 will update all devices. port: Serial port to which the device is connected. target: What to update on the device (e.g., LED or GAIN). value: Value to which the target should be updated. @@ -122,7 +125,6 @@ def DevUpdate( if port: logger.info(f"Using port {port}") - command = DevUpdateCommand(port=port, target=target, value=value) else: ftdi_port_list = find_ftdi_device() if len(ftdi_port_list) == 0: @@ -133,17 +135,18 @@ def DevUpdate( port = ftdi_port_list[0] logger.info(f"Using port {port}") - command = DevUpdateCommand(port=port, target=target, value=value) + command = DevUpdateCommand(device_id=device_id, port=port, target=target, value=value) logger.info(f"Updating {target} to {value} on port {port}") # Header to indicate target/value. # This should be a bit pattern that is unlikely to be the value. - target_mask = 0b11000000 + id_header = 0b00000000 + target_header = 0b11000000 LSB_header = 0b01000000 MSB_header = 0b10000000 - LSB_value_mask = 0b000000111111 #value below 12-bit - MSB_value_mask = 0b111111000000 #value below 12-bit - reset_byte = 0b00000000 + LSB_value_mask = 0b000000111111 # value below 12-bit + MSB_value_mask = 0b111111000000 # value below 12-bit + reset_byte = 0b11111111 try: serial_port = serial.Serial(port=command.port, baudrate=2400, timeout=5, stopbits=2) @@ -153,19 +156,24 @@ def DevUpdate( logger.info("Open serial port") try: - target_command = command.target.value + target_mask + id_command = (command.device_id + id_header) & 0xFF + serial_port.write(id_command.to_bytes(1, "big")) + logger.debug(f"Command: {format(id_command, '08b')}; Device ID: {command.device_id}") + time.sleep(0.1) + + target_command = (command.target.value + target_header) & 0xFF serial_port.write(target_command.to_bytes(1, "big")) - logger.debug(f"Target {command.target}; command: {bin(target_command)}") + logger.debug(f"Command: {format(target_command, '08b')}; Target: {command.target.name}") time.sleep(0.1) value_LSB_command = ((command.value & LSB_value_mask) + LSB_header) & 0xFF serial_port.write(value_LSB_command.to_bytes(1, "big")) - logger.debug(f"Value {command.value}; command_LSB: {bin(value_LSB_command)}") + logger.debug(f"Command: {format(value_LSB_command, '08b')}; Value: {command.value} (LSB)") time.sleep(0.1) value_MSB_command = (((command.value & MSB_value_mask) >> 6) + MSB_header) & 0xFF serial_port.write(value_MSB_command.to_bytes(1, "big")) - logger.debug(f"Value {command.value}; command_MSB: {bin(value_MSB_command)}") + logger.debug(f"Command: {format(value_MSB_command, '08b')}; Value: {command.value} (MSB)") time.sleep(0.1) serial_port.write(reset_byte.to_bytes(1, "big")) From afdbc9796a568c0d207f34238329ddaaaae8dc6b Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Fri, 1 Nov 2024 18:40:09 +0900 Subject: [PATCH 06/28] Add device commands (mainly for restart) --- miniscope_io/cli/update.py | 44 ++++++++++++++++++++++++++++------- miniscope_io/device_update.py | 5 +++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index f17aef0d..20a800d3 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -4,8 +4,7 @@ import click -from miniscope_io.device_update import DevUpdate - +from miniscope_io.device_update import DevUpdate, DeviceCommand @click.command() @click.option( @@ -25,13 +24,42 @@ @click.option( "-t", "--target", - required=True, + required=False, type=click.Choice(["LED", "GAIN", "ROI_X", "ROI_Y"]), - help="Target to update", + help="Target to update. Cannot be used with --restart.", +) +@click.option( + "-v", + "--value", + required=False, + type=int, + help="Value to set. Must be used with --target and cannot be used with --restart.", +) +@click.option( + "--restart", + is_flag=True, + type=bool, + help="Restart the device. Cannot be used with --target or --value." ) -@click.option("-v", "--value", required=True, type=int, help="Value to set") -def update(port: str, target: str, value: int, device_id: int) -> None: +def update(port: str, target: str, value: int, device_id: int, restart: bool) -> None: """ - Update device configuration. + Update device configuration or restart it. """ - DevUpdate(port=port, target=target, value=value, device_id=device_id) + + # Check mutual exclusivity + if (target and not value) or (value and not target): + raise click.UsageError("Both --target and --value are required if one is specified.") + + if (target or value) and restart: + raise click.UsageError("Options --target/--value and --restart cannot be used together.") + + if target and value: + DevUpdate(port=port, target=target, value=value, device_id=device_id) + elif restart: + DevUpdate(port=port, + target='DEVICE', + value=DeviceCommand.RESTART, + device_id=device_id + ) + else: + raise click.UsageError("Either --target with --value or --restart must be specified.") \ No newline at end of file diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index cff20ddd..01eb1304 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -19,6 +19,9 @@ logger = init_logger(name="device_update", level="DEBUG") +class DeviceCommand(Enum): + """Commands for device.""" + RESTART = 200 class UpdateTarget(Enum): """Targets to update.""" @@ -30,7 +33,7 @@ class UpdateTarget(Enum): ROI_WIDTH = 4 # not implemented ROI_HEIGHT = 5 # not implemented EWL = 6 # not implemented - + DEVICE = 99 # for device commands class DevUpdateCommand(BaseModel): """ From a560a4814ec05f6476cc200af668b2ac4666c8ce Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Fri, 1 Nov 2024 18:54:06 +0900 Subject: [PATCH 07/28] Isolate models in a separate file --- miniscope_io/cli/update.py | 15 ++--- miniscope_io/device_update.py | 93 +-------------------------- miniscope_io/models/devupdate.py | 105 +++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 100 deletions(-) create mode 100644 miniscope_io/models/devupdate.py diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index 20a800d3..f2e8168e 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -4,7 +4,8 @@ import click -from miniscope_io.device_update import DevUpdate, DeviceCommand +from miniscope_io.device_update import DeviceCommand, DevUpdate + @click.command() @click.option( @@ -39,7 +40,7 @@ "--restart", is_flag=True, type=bool, - help="Restart the device. Cannot be used with --target or --value." + help="Restart the device. Cannot be used with --target or --value.", ) def update(port: str, target: str, value: int, device_id: int, restart: bool) -> None: """ @@ -49,17 +50,13 @@ def update(port: str, target: str, value: int, device_id: int, restart: bool) -> # Check mutual exclusivity if (target and not value) or (value and not target): raise click.UsageError("Both --target and --value are required if one is specified.") - + if (target or value) and restart: raise click.UsageError("Options --target/--value and --restart cannot be used together.") if target and value: DevUpdate(port=port, target=target, value=value, device_id=device_id) elif restart: - DevUpdate(port=port, - target='DEVICE', - value=DeviceCommand.RESTART, - device_id=device_id - ) + DevUpdate(port=port, target="DEVICE", value=DeviceCommand.RESTART, device_id=device_id) else: - raise click.UsageError("Either --target with --value or --restart must be specified.") \ No newline at end of file + raise click.UsageError("Either --target with --value or --restart must be specified.") diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 01eb1304..33cffac7 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -1,107 +1,18 @@ """ -Update miniscope device configuration. - -.. todo:: - - What kind of devices does this apply to? - +Update miniscope device configuration, such as LED, GAIN, etc. """ import time -from enum import Enum from typing import Optional import serial import serial.tools.list_ports -from pydantic import BaseModel, ConfigDict, field_validator, model_validator from miniscope_io.logging import init_logger +from miniscope_io.models.devupdate import DevUpdateCommand logger = init_logger(name="device_update", level="DEBUG") -class DeviceCommand(Enum): - """Commands for device.""" - RESTART = 200 - -class UpdateTarget(Enum): - """Targets to update.""" - - LED = 0 - GAIN = 1 - ROI_X = 2 - ROI_Y = 3 - ROI_WIDTH = 4 # not implemented - ROI_HEIGHT = 5 # not implemented - EWL = 6 # not implemented - DEVICE = 99 # for device commands - -class DevUpdateCommand(BaseModel): - """ - Command to update device configuration. - """ - - device_id: int - port: str - target: UpdateTarget - value: int - - model_config = ConfigDict(arbitrary_types_allowed=True) - - @model_validator(mode="after") - def validate_values(cls, values: dict) -> dict: - """ - Validate values based on target. - """ - target = values.target - value = values.value - - if target == UpdateTarget.LED: - assert 0 <= value <= 100, "For LED, value must be between 0 and 100" - elif target == UpdateTarget.GAIN: - assert value in [1, 2, 4], "For GAIN, value must be 1, 2, or 4" - return values - - @field_validator("port") - def validate_port(cls, value: str) -> str: - """ - Validate port. - - Args: - value: Port to validate. - - Returns: - Validated port. - - Raises: - ValueError: If no serial ports found or port not found. - """ - portlist = list(serial.tools.list_ports.comports()) - - if len(portlist) == 0: - raise ValueError("No serial ports found") - if value not in [port.device for port in portlist]: - raise ValueError(f"Port {value} not found") - return value - - @field_validator("target", mode="before") - def validate_target(cls, value: str) -> UpdateTarget: - """ - Validate and convert target string to UpdateTarget Enum type. - - Args: - value (str): Target to validate. - - Returns: - UpdateTarget: Validated target as UpdateTarget. - - Raises: - ValueError: If target not found. - """ - try: - return UpdateTarget[value] - except KeyError as e: - raise ValueError(f"Target {value} not found.") from e - def DevUpdate( target: str, diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py new file mode 100644 index 00000000..2a4d4617 --- /dev/null +++ b/miniscope_io/models/devupdate.py @@ -0,0 +1,105 @@ +""" +Models for device update commands. +""" + +from enum import Enum + +import serial.tools.list_ports +from pydantic import BaseModel, ConfigDict, field_validator, model_validator + + +class DeviceCommand(Enum): + """Commands for device.""" + + RESTART = 200 + + +class UpdateTarget(Enum): + """Targets to update.""" + + LED = 0 + GAIN = 1 + ROI_X = 2 + ROI_Y = 3 + ROI_WIDTH = 4 # not implemented + ROI_HEIGHT = 5 # not implemented + EWL = 6 # not implemented + DEVICE = 99 # for device commands + + +class DevUpdateCommand(BaseModel): + """ + Command to update device configuration. + """ + + device_id: int + port: str + target: UpdateTarget + value: int + + model_config = ConfigDict(arbitrary_types_allowed=True) + + @model_validator(mode="after") + def validate_values(cls, values: dict) -> dict: + """ + Validate values based on target. + """ + target = values.target + value = values.value + + if target == UpdateTarget.LED: + assert 0 <= value <= 100, "For LED, value must be between 0 and 100" + elif target == UpdateTarget.GAIN: + assert value in [1, 2, 4], "For GAIN, value must be 1, 2, or 4" + elif target == UpdateTarget.DEVICE: + assert value in DeviceCommand.__members__.values() + elif ( + target == UpdateTarget.ROI_X + or target == UpdateTarget.ROI_Y + or target == UpdateTarget.ROI_WIDTH + or target == UpdateTarget.ROI_HEIGHT + or target == UpdateTarget.EWL + ): + pass # Need to implement + return values + + @field_validator("port") + def validate_port(cls, value: str) -> str: + """ + Validate port. + + Args: + value: Port to validate. + + Returns: + Validated port. + + Raises: + ValueError: If no serial ports found or port not found. + """ + portlist = list(serial.tools.list_ports.comports()) + + if len(portlist) == 0: + raise ValueError("No serial ports found") + if value not in [port.device for port in portlist]: + raise ValueError(f"Port {value} not found") + return value + + @field_validator("target", mode="before") + def validate_target(cls, value: str) -> UpdateTarget: + """ + Validate and convert target string to UpdateTarget Enum type. + + Args: + value (str): Target to validate. + + Returns: + UpdateTarget: Validated target as UpdateTarget. + + Raises: + ValueError: If target not found. + """ + try: + return UpdateTarget[value] + except KeyError as e: + raise ValueError(f"Target {value} not found.") from e From 7ac453e64397bf43bc728c3289e3f030ded4ab0c Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Fri, 1 Nov 2024 19:00:16 +0900 Subject: [PATCH 08/28] add devupdate model tests --- tests/test_models/test_model_devupdate.py | 48 +++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 tests/test_models/test_model_devupdate.py diff --git a/tests/test_models/test_model_devupdate.py b/tests/test_models/test_model_devupdate.py new file mode 100644 index 00000000..31a5e9b7 --- /dev/null +++ b/tests/test_models/test_model_devupdate.py @@ -0,0 +1,48 @@ +import pytest +from unittest.mock import patch +from pydantic import ValidationError + +from miniscope_io.models.devupdate import DevUpdateCommand, UpdateTarget, DeviceCommand + +def mock_comports(): + class Port: + def __init__(self, device): + self.device = device + + return [Port("COM1"), Port("COM2")] + +@pytest.fixture +def mock_serial_ports(): + with patch('serial.tools.list_ports.comports', side_effect=mock_comports): + yield + +def test_valid_led_update(mock_serial_ports): + cmd = DevUpdateCommand(device_id=1, port="COM1", target="LED", value=50) + assert cmd.target == UpdateTarget.LED + assert cmd.value == 50 + +def test_valid_gain_update(mock_serial_ports): + cmd = DevUpdateCommand(device_id=1, port="COM2", target="GAIN", value=2) + assert cmd.target == UpdateTarget.GAIN + assert cmd.value == 2 + +def test_invalid_led_value(mock_serial_ports): + with pytest.raises(ValidationError): + DevUpdateCommand(device_id=1, port="COM1", target="LED", value=150) + +def test_invalid_gain_value(mock_serial_ports): + with pytest.raises(ValidationError): + DevUpdateCommand(device_id=1, port="COM1", target="GAIN", value=3) + +def test_invalid_target(mock_serial_ports): + with pytest.raises(ValueError): + DevUpdateCommand(device_id=1, port="COM1", target="FAKEDEVICE", value=10) + +def test_invalid_port(): + with patch('serial.tools.list_ports.comports', return_value=mock_comports()): + with pytest.raises(ValidationError): + DevUpdateCommand(device_id=1, port="COM3", target="LED", value=50) + +def test_device_command(mock_serial_ports): + cmd = DevUpdateCommand(device_id=1, port="COM2", target="DEVICE", value=DeviceCommand.RESTART.value) + assert cmd.value == DeviceCommand.RESTART.value \ No newline at end of file From c1ea61a157fb958be8fd6a10d83563aeaae75c74 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Fri, 1 Nov 2024 19:15:20 +0900 Subject: [PATCH 09/28] Fix model tests --- miniscope_io/cli/update.py | 8 ++++++-- miniscope_io/device_update.py | 2 +- miniscope_io/models/devupdate.py | 2 +- tests/test_models/test_model_devupdate.py | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index f2e8168e..eddc52e7 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -4,7 +4,8 @@ import click -from miniscope_io.device_update import DeviceCommand, DevUpdate +from miniscope_io.device_update import DevUpdate +from miniscope_io.models.devupdate import DeviceCommand @click.command() @@ -57,6 +58,9 @@ def update(port: str, target: str, value: int, device_id: int, restart: bool) -> if target and value: DevUpdate(port=port, target=target, value=value, device_id=device_id) elif restart: - DevUpdate(port=port, target="DEVICE", value=DeviceCommand.RESTART, device_id=device_id) + DevUpdate(port=port, + target="DEVICE", + value=DeviceCommand.RESTART.value, + device_id=device_id) else: raise click.UsageError("Either --target with --value or --restart must be specified.") diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 33cffac7..7bc37e0a 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -11,7 +11,7 @@ from miniscope_io.logging import init_logger from miniscope_io.models.devupdate import DevUpdateCommand -logger = init_logger(name="device_update", level="DEBUG") +logger = init_logger(name="device_update", level="INFO") def DevUpdate( diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index 2a4d4617..a3b2f39c 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -52,7 +52,7 @@ def validate_values(cls, values: dict) -> dict: elif target == UpdateTarget.GAIN: assert value in [1, 2, 4], "For GAIN, value must be 1, 2, or 4" elif target == UpdateTarget.DEVICE: - assert value in DeviceCommand.__members__.values() + assert value in [DeviceCommand.RESTART.value], "For DEVICE, value must be in [200]" elif ( target == UpdateTarget.ROI_X or target == UpdateTarget.ROI_Y diff --git a/tests/test_models/test_model_devupdate.py b/tests/test_models/test_model_devupdate.py index 31a5e9b7..1feb24a4 100644 --- a/tests/test_models/test_model_devupdate.py +++ b/tests/test_models/test_model_devupdate.py @@ -45,4 +45,4 @@ def test_invalid_port(): def test_device_command(mock_serial_ports): cmd = DevUpdateCommand(device_id=1, port="COM2", target="DEVICE", value=DeviceCommand.RESTART.value) - assert cmd.value == DeviceCommand.RESTART.value \ No newline at end of file + assert cmd.value == int(DeviceCommand.RESTART.value) \ No newline at end of file From 8782cd2fd012d37f0894288a2f4f564149c763b4 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Fri, 1 Nov 2024 19:30:31 +0900 Subject: [PATCH 10/28] update format --- miniscope_io/cli/update.py | 7 +++---- tests/test_models/test_model_devupdate.py | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index eddc52e7..49c66f3b 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -58,9 +58,8 @@ def update(port: str, target: str, value: int, device_id: int, restart: bool) -> if target and value: DevUpdate(port=port, target=target, value=value, device_id=device_id) elif restart: - DevUpdate(port=port, - target="DEVICE", - value=DeviceCommand.RESTART.value, - device_id=device_id) + DevUpdate( + port=port, target="DEVICE", value=DeviceCommand.RESTART.value, device_id=device_id + ) else: raise click.UsageError("Either --target with --value or --restart must be specified.") diff --git a/tests/test_models/test_model_devupdate.py b/tests/test_models/test_model_devupdate.py index 1feb24a4..31a5e9b7 100644 --- a/tests/test_models/test_model_devupdate.py +++ b/tests/test_models/test_model_devupdate.py @@ -45,4 +45,4 @@ def test_invalid_port(): def test_device_command(mock_serial_ports): cmd = DevUpdateCommand(device_id=1, port="COM2", target="DEVICE", value=DeviceCommand.RESTART.value) - assert cmd.value == int(DeviceCommand.RESTART.value) \ No newline at end of file + assert cmd.value == DeviceCommand.RESTART.value \ No newline at end of file From 9fff132c50cb30d3c036d83d1916e3b7106166a4 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Fri, 1 Nov 2024 19:52:48 +0900 Subject: [PATCH 11/28] moved cmd constants to model file --- miniscope_io/device_update.py | 27 +++++++++++---------------- miniscope_io/models/devupdate.py | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 7bc37e0a..61bb3db6 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -9,7 +9,7 @@ import serial.tools.list_ports from miniscope_io.logging import init_logger -from miniscope_io.models.devupdate import DevUpdateCommand +from miniscope_io.models.devupdate import CommandDefinitions, DevUpdateCommand logger = init_logger(name="device_update", level="INFO") @@ -52,16 +52,6 @@ def DevUpdate( command = DevUpdateCommand(device_id=device_id, port=port, target=target, value=value) logger.info(f"Updating {target} to {value} on port {port}") - # Header to indicate target/value. - # This should be a bit pattern that is unlikely to be the value. - id_header = 0b00000000 - target_header = 0b11000000 - LSB_header = 0b01000000 - MSB_header = 0b10000000 - LSB_value_mask = 0b000000111111 # value below 12-bit - MSB_value_mask = 0b111111000000 # value below 12-bit - reset_byte = 0b11111111 - try: serial_port = serial.Serial(port=command.port, baudrate=2400, timeout=5, stopbits=2) except Exception as e: @@ -70,27 +60,32 @@ def DevUpdate( logger.info("Open serial port") try: - id_command = (command.device_id + id_header) & 0xFF + id_command = (command.device_id + CommandDefinitions.id_header) & 0xFF serial_port.write(id_command.to_bytes(1, "big")) logger.debug(f"Command: {format(id_command, '08b')}; Device ID: {command.device_id}") time.sleep(0.1) - target_command = (command.target.value + target_header) & 0xFF + target_command = (command.target.value + CommandDefinitions.target_header) & 0xFF serial_port.write(target_command.to_bytes(1, "big")) logger.debug(f"Command: {format(target_command, '08b')}; Target: {command.target.name}") time.sleep(0.1) - value_LSB_command = ((command.value & LSB_value_mask) + LSB_header) & 0xFF + value_LSB_command = ( + (command.value & CommandDefinitions.LSB_value_mask) + CommandDefinitions.LSB_header + ) & 0xFF serial_port.write(value_LSB_command.to_bytes(1, "big")) logger.debug(f"Command: {format(value_LSB_command, '08b')}; Value: {command.value} (LSB)") time.sleep(0.1) - value_MSB_command = (((command.value & MSB_value_mask) >> 6) + MSB_header) & 0xFF + value_MSB_command = ( + ((command.value & CommandDefinitions.MSB_value_mask) >> 6) + + CommandDefinitions.MSB_header + ) & 0xFF serial_port.write(value_MSB_command.to_bytes(1, "big")) logger.debug(f"Command: {format(value_MSB_command, '08b')}; Value: {command.value} (MSB)") time.sleep(0.1) - serial_port.write(reset_byte.to_bytes(1, "big")) + serial_port.write(CommandDefinitions.reset_byte.to_bytes(1, "big")) finally: serial_port.close() diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index a3b2f39c..4bcf5abd 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -14,6 +14,22 @@ class DeviceCommand(Enum): RESTART = 200 +class CommandDefinitions(BaseModel): + """ + Definitions of Bit masks and headers for commands. + """ + + # Header to indicate target/value. + # It probably won't be used in other places so defined here. + id_header = 0b00000000 + target_header = 0b11000000 + LSB_header = 0b01000000 + MSB_header = 0b10000000 + LSB_value_mask = 0b000000111111 # value below 12-bit + MSB_value_mask = 0b111111000000 # value below 12-bit + reset_byte = 0b11111111 + + class UpdateTarget(Enum): """Targets to update.""" From 3fe11088f2b85a35896365dd5a9f914b7866254f Mon Sep 17 00:00:00 2001 From: t-sasatani Date: Sat, 2 Nov 2024 00:27:49 +0900 Subject: [PATCH 12/28] fix definitions --- miniscope_io/device_update.py | 23 ++++++++++------------- miniscope_io/models/devupdate.py | 4 ++-- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 61bb3db6..fffd9d1d 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -9,7 +9,7 @@ import serial.tools.list_ports from miniscope_io.logging import init_logger -from miniscope_io.models.devupdate import CommandDefinitions, DevUpdateCommand +from miniscope_io.models.devupdate import DevUpdateCommand, UpdateCommandDefinitions logger = init_logger(name="device_update", level="INFO") @@ -21,16 +21,12 @@ def DevUpdate( port: Optional[str] = None, ) -> None: """ - IR-based update of device configuration. - - .. note:: - - Not tested after separating from stream_daq.py. + Remote update of device configuration. Args: device_id: ID of the device. 0 will update all devices. port: Serial port to which the device is connected. - target: What to update on the device (e.g., LED or GAIN). + target: What to update on the device (e.g., LED, GAIN). value: Value to which the target should be updated. Returns: @@ -60,32 +56,33 @@ def DevUpdate( logger.info("Open serial port") try: - id_command = (command.device_id + CommandDefinitions.id_header) & 0xFF + id_command = (command.device_id + UpdateCommandDefinitions.id_header) & 0xFF serial_port.write(id_command.to_bytes(1, "big")) logger.debug(f"Command: {format(id_command, '08b')}; Device ID: {command.device_id}") time.sleep(0.1) - target_command = (command.target.value + CommandDefinitions.target_header) & 0xFF + target_command = (command.target.value + UpdateCommandDefinitions.target_header) & 0xFF serial_port.write(target_command.to_bytes(1, "big")) logger.debug(f"Command: {format(target_command, '08b')}; Target: {command.target.name}") time.sleep(0.1) value_LSB_command = ( - (command.value & CommandDefinitions.LSB_value_mask) + CommandDefinitions.LSB_header + (command.value & UpdateCommandDefinitions.LSB_value_mask) + + UpdateCommandDefinitions.LSB_header ) & 0xFF serial_port.write(value_LSB_command.to_bytes(1, "big")) logger.debug(f"Command: {format(value_LSB_command, '08b')}; Value: {command.value} (LSB)") time.sleep(0.1) value_MSB_command = ( - ((command.value & CommandDefinitions.MSB_value_mask) >> 6) - + CommandDefinitions.MSB_header + ((command.value & UpdateCommandDefinitions.MSB_value_mask) >> 6) + + UpdateCommandDefinitions.MSB_header ) & 0xFF serial_port.write(value_MSB_command.to_bytes(1, "big")) logger.debug(f"Command: {format(value_MSB_command, '08b')}; Value: {command.value} (MSB)") time.sleep(0.1) - serial_port.write(CommandDefinitions.reset_byte.to_bytes(1, "big")) + serial_port.write(UpdateCommandDefinitions.reset_byte.to_bytes(1, "big")) finally: serial_port.close() diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index 4bcf5abd..c591ad79 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -14,9 +14,9 @@ class DeviceCommand(Enum): RESTART = 200 -class CommandDefinitions(BaseModel): +class UpdateCommandDefinitions: """ - Definitions of Bit masks and headers for commands. + Definitions of Bit masks and headers for remote update commands. """ # Header to indicate target/value. From 3bbed78918da370baab0394a761ee913c451299e Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sat, 2 Nov 2024 19:04:39 +0900 Subject: [PATCH 13/28] Fix remote reboot command --- miniscope_io/cli/update.py | 12 +++++------- miniscope_io/models/devupdate.py | 8 +++++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index 49c66f3b..557fbdc4 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -38,12 +38,12 @@ help="Value to set. Must be used with --target and cannot be used with --restart.", ) @click.option( - "--restart", + "--reboot", is_flag=True, type=bool, help="Restart the device. Cannot be used with --target or --value.", ) -def update(port: str, target: str, value: int, device_id: int, restart: bool) -> None: +def update(port: str, target: str, value: int, device_id: int, reboot: bool) -> None: """ Update device configuration or restart it. """ @@ -52,14 +52,12 @@ def update(port: str, target: str, value: int, device_id: int, restart: bool) -> if (target and not value) or (value and not target): raise click.UsageError("Both --target and --value are required if one is specified.") - if (target or value) and restart: + if (target or value) and reboot: raise click.UsageError("Options --target/--value and --restart cannot be used together.") if target and value: DevUpdate(port=port, target=target, value=value, device_id=device_id) - elif restart: - DevUpdate( - port=port, target="DEVICE", value=DeviceCommand.RESTART.value, device_id=device_id - ) + elif reboot: + DevUpdate(port=port, target="DEVICE", value=DeviceCommand.REBOOT.value, device_id=device_id) else: raise click.UsageError("Either --target with --value or --restart must be specified.") diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index c591ad79..f4a5e9a4 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -11,7 +11,7 @@ class DeviceCommand(Enum): """Commands for device.""" - RESTART = 200 + REBOOT = 200 class UpdateCommandDefinitions: @@ -31,7 +31,9 @@ class UpdateCommandDefinitions: class UpdateTarget(Enum): - """Targets to update.""" + """ + Targets to update. Needs to be under 6-bit. + """ LED = 0 GAIN = 1 @@ -40,7 +42,7 @@ class UpdateTarget(Enum): ROI_WIDTH = 4 # not implemented ROI_HEIGHT = 5 # not implemented EWL = 6 # not implemented - DEVICE = 99 # for device commands + DEVICE = 50 # for device commands class DevUpdateCommand(BaseModel): From 41b50fe82df12369cf7de1c583e9b2b76efe9c77 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sat, 2 Nov 2024 19:21:12 +0900 Subject: [PATCH 14/28] minor fix for model and modeltest --- miniscope_io/models/devupdate.py | 2 +- tests/test_models/test_model_devupdate.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index f4a5e9a4..be54fe05 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -70,7 +70,7 @@ def validate_values(cls, values: dict) -> dict: elif target == UpdateTarget.GAIN: assert value in [1, 2, 4], "For GAIN, value must be 1, 2, or 4" elif target == UpdateTarget.DEVICE: - assert value in [DeviceCommand.RESTART.value], "For DEVICE, value must be in [200]" + assert value in [DeviceCommand.REBOOT.value], "For DEVICE, value must be in [200]" elif ( target == UpdateTarget.ROI_X or target == UpdateTarget.ROI_Y diff --git a/tests/test_models/test_model_devupdate.py b/tests/test_models/test_model_devupdate.py index 31a5e9b7..f5a8e8ef 100644 --- a/tests/test_models/test_model_devupdate.py +++ b/tests/test_models/test_model_devupdate.py @@ -44,5 +44,5 @@ def test_invalid_port(): DevUpdateCommand(device_id=1, port="COM3", target="LED", value=50) def test_device_command(mock_serial_ports): - cmd = DevUpdateCommand(device_id=1, port="COM2", target="DEVICE", value=DeviceCommand.RESTART.value) - assert cmd.value == DeviceCommand.RESTART.value \ No newline at end of file + cmd = DevUpdateCommand(device_id=1, port="COM2", target="DEVICE", value=DeviceCommand.REBOOT.value) + assert cmd.value == DeviceCommand.REBOOT.value \ No newline at end of file From d4212a2f7be93a7833f2978fe88913de35629aa3 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sat, 2 Nov 2024 21:02:23 +0900 Subject: [PATCH 15/28] bundle ota update with yaml --- miniscope_io/cli/update.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index 557fbdc4..718a16d5 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -3,6 +3,7 @@ """ import click +import yaml from miniscope_io.device_update import DevUpdate from miniscope_io.models.devupdate import DeviceCommand @@ -37,13 +38,20 @@ type=int, help="Value to set. Must be used with --target and cannot be used with --restart.", ) +@click.option( + "-c", + "--config", + required=False, + type=click.Path(exists=True, dir_okay=False), + help="YAML file with configuration to update. Specify target and value pairs in the file.", +) @click.option( "--reboot", is_flag=True, type=bool, help="Restart the device. Cannot be used with --target or --value.", ) -def update(port: str, target: str, value: int, device_id: int, reboot: bool) -> None: +def update(port: str, target: str, value: int, device_id: int, reboot: bool, config: str) -> None: """ Update device configuration or restart it. """ @@ -52,11 +60,17 @@ def update(port: str, target: str, value: int, device_id: int, reboot: bool) -> if (target and not value) or (value and not target): raise click.UsageError("Both --target and --value are required if one is specified.") - if (target or value) and reboot: - raise click.UsageError("Options --target/--value and --restart cannot be used together.") - + if ((target or value) and reboot) or (config and reboot) or (config and (target or value)): + raise click.UsageError( + "Options --target/--value and --restart" " and --config are mutually exclusive." + ) if target and value: DevUpdate(port=port, target=target, value=value, device_id=device_id) + elif config: + with open(config) as f: + config_file = yaml.safe_load(f) + for key, value in config_file: + DevUpdate(port=port, target=key, value=value, device_id=device_id) elif reboot: DevUpdate(port=port, target="DEVICE", value=DeviceCommand.REBOOT.value, device_id=device_id) else: From 70d806a45e445f5e422257b8778a99f5202b32e8 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sat, 2 Nov 2024 21:46:17 +0900 Subject: [PATCH 16/28] Add tests for device update --- tests/test_device_update.py | 110 ++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 tests/test_device_update.py diff --git a/tests/test_device_update.py b/tests/test_device_update.py new file mode 100644 index 00000000..6e6fc171 --- /dev/null +++ b/tests/test_device_update.py @@ -0,0 +1,110 @@ +import pytest +from pydantic import ValidationError +from unittest.mock import MagicMock, patch, call +from miniscope_io.models.devupdate import UpdateCommandDefinitions, UpdateTarget +from miniscope_io.device_update import DevUpdate, find_ftdi_device + + +@patch("miniscope_io.device_update.serial.tools.list_ports.comports") +@patch("miniscope_io.device_update.serial.Serial") +def test_devupdate_with_device_connected(mock_serial, mock_comports): + """ + Test DevUpdate function with a device connected. + """ + mock_serial_instance = mock_serial.return_value + mock_comports.return_value = [ + MagicMock(vid=0x0403, pid=0x6001, device="COM3"), + MagicMock(vid=0x1234, pid=0x5678, device="COM4"), + ] + target = "LED" + value = 2 + device_id = 1 + port = "COM3" + + DevUpdate(target, value, device_id, port) + + mock_serial.assert_called_once_with(port=port, baudrate=2400, timeout=5, stopbits=2) + + id_command = (device_id + UpdateCommandDefinitions.id_header) & 0xFF + target_command = (UpdateTarget.LED.value + UpdateCommandDefinitions.target_header) & 0xFF + value_LSB_command = ( + (value & UpdateCommandDefinitions.LSB_value_mask) + UpdateCommandDefinitions.LSB_header + ) & 0xFF + value_MSB_command = ( + ((value & UpdateCommandDefinitions.MSB_value_mask) >> 6) + UpdateCommandDefinitions.MSB_header + ) & 0xFF + reset_command = UpdateCommandDefinitions.reset_byte + + expected_calls = [ + call(id_command.to_bytes(1, 'big')), + call(target_command.to_bytes(1, 'big')), + call(value_LSB_command.to_bytes(1, 'big')), + call(value_MSB_command.to_bytes(1, 'big')), + call(reset_command.to_bytes(1, 'big')), + ] + + assert mock_serial_instance.write.call_count == len(expected_calls) + mock_serial_instance.write.assert_has_calls(expected_calls, any_order=False) + +@patch("miniscope_io.device_update.serial.tools.list_ports.comports") +def test_devupdate_without_device_connected(mock_comports): + """ + Test DevUpdate function without a device connected. + """ + mock_comports.return_value = [ + ] + target = "GAIN" + value = 2 + device_id = 0 + + with pytest.raises(ValueError, match="No FTDI devices found."): + DevUpdate(target, value, device_id) + +@patch("miniscope_io.device_update.serial.tools.list_ports.comports") +def test_find_ftdi_device(mock_comports): + """ + Test find_ftdi_device function. + """ + mock_comports.return_value = [ + MagicMock(vid=0x0403, pid=0x6001, device="COM3"), + MagicMock(vid=0x1234, pid=0x5678, device="COM4"), + ] + + result = find_ftdi_device() + + assert result == ["COM3"] + +@patch("miniscope_io.models.devupdate.serial.tools.list_ports.comports") +def test_invalid_target_raises_error(mock_comports): + """ + Test that an invalid target raises an error. + """ + mock_comports.return_value = [ + MagicMock(vid=0x0403, pid=0x6001, device="COM3"), + MagicMock(vid=0x1234, pid=0x5678, device="COM4"), + ] + + target = "RANDOM_STRING" + value = 50 + device_id = 1 + port = "COM3" + + with pytest.raises(ValidationError, match="Target RANDOM_STRING not found"): + DevUpdate(target, value, device_id, port) + +@patch("miniscope_io.models.devupdate.serial.tools.list_ports.comports") +def test_invalid_led_value_raises_error(mock_comports): + """ + Test that an invalid LED value raises an error. + """ + mock_comports.return_value = [ + MagicMock(vid=0x0403, pid=0x6001, device="COM3"), + MagicMock(vid=0x1234, pid=0x5678, device="COM4"), + ] + target = "LED" + value = 150 # LED value should be between 0 and 100 + device_id = 1 + port = "COM3" + + with pytest.raises(ValidationError, match="For LED, value must be between 0 and 100"): + DevUpdate(target, value, device_id, port) \ No newline at end of file From 212df470351ee1b9b03bbdd4b9c2159e212a804b Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sat, 2 Nov 2024 22:02:14 +0900 Subject: [PATCH 17/28] Add controller docs (mostly just placeholder) --- docs/device/update_controller.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 docs/device/update_controller.md diff --git a/docs/device/update_controller.md b/docs/device/update_controller.md new file mode 100644 index 00000000..c9c70c92 --- /dev/null +++ b/docs/device/update_controller.md @@ -0,0 +1,7 @@ +# Update controller + +**Under Construction:** This section will be populated when these devices are released. + +This page will document equipments needed to use the `miniscope_io.device_update` module (or `mio update` interface.) +## prerequisite +- A custom FTDI chip based IR transmitter (details will be released soon) \ No newline at end of file From 37210086b31aeb7fe48daa752e0be587b5ca4142 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sat, 2 Nov 2024 22:17:18 +0900 Subject: [PATCH 18/28] update docs index, add update device tests --- docs/index.md | 1 + tests/test_device_update.py | 88 ++++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index 07c26b55..6c83bc76 100644 --- a/docs/index.md +++ b/docs/index.md @@ -15,6 +15,7 @@ cli/main :maxdepth: 1 device/test_device +device/update_controller ``` ```{toctree} diff --git a/tests/test_device_update.py b/tests/test_device_update.py index 6e6fc171..23e63e99 100644 --- a/tests/test_device_update.py +++ b/tests/test_device_update.py @@ -1,4 +1,5 @@ import pytest +import serial from pydantic import ValidationError from unittest.mock import MagicMock, patch, call from miniscope_io.models.devupdate import UpdateCommandDefinitions, UpdateTarget @@ -107,4 +108,89 @@ def test_invalid_led_value_raises_error(mock_comports): port = "COM3" with pytest.raises(ValidationError, match="For LED, value must be between 0 and 100"): - DevUpdate(target, value, device_id, port) \ No newline at end of file + DevUpdate(target, value, device_id, port) + + +@patch("miniscope_io.device_update.serial.tools.list_ports.comports") +def test_devupdate_with_multiple_ftdi_devices(mock_comports): + """ + Test that multiple FTDI devices raise an error. + """ + mock_comports.return_value = [ + MagicMock(vid=0x0403, pid=0x6001, device="COM1"), + MagicMock(vid=0x0403, pid=0x6001, device="COM2"), + ] + + target = "GAIN" + value = 5 + device_id = 1 + + with pytest.raises(ValueError, match="Multiple FTDI devices found. Please specify the port."): + DevUpdate(target, value, device_id) + +@patch("miniscope_io.device_update.serial.Serial") +def test_devupdate_serial_exception_handling(mock_serial): + """ + Test exception handling when serial port cannot be opened. + """ + mock_serial.side_effect = serial.SerialException("Serial port error") + + target = "LED" + value = 50 + device_id = 1 + port = "COM3" + + with pytest.raises(ValidationError): + DevUpdate(target, value, device_id, port) + +@patch("miniscope_io.device_update.serial.tools.list_ports.comports") +def test_specified_port_not_ftdi_device(mock_comports): + """ + Test with a specified port not corresponding to an FTDI device. + """ + mock_comports.return_value = [ + MagicMock(vid=None, pid=None, device="COM3") + ] + + target = "GAIN" + value = 10 + device_id = 1 + + with pytest.raises(ValueError, match="No FTDI devices found."): + DevUpdate(target, value, device_id) + +@patch("miniscope_io.models.devupdate.serial.tools.list_ports.comports") +@patch("miniscope_io.device_update.serial.Serial") +def test_devupdate_command_execution_order(mock_serial, mock_comports): + """ + Test that commands are executed in the correct order. + """ + mock_serial_instance = mock_serial.return_value + + mock_comports.return_value = [ + MagicMock(vid=0x0403, pid=0x6001, device="COM3"), + MagicMock(vid=0x1234, pid=0x5678, device="COM4"), + ] + target = "GAIN" + value = 2 + device_id = 1 + port = "COM3" + + DevUpdate(target, value, device_id, port) + + id_command = (device_id + UpdateCommandDefinitions.id_header) & 0xFF + target_command = (UpdateTarget.GAIN.value + UpdateCommandDefinitions.target_header) & 0xFF + value_LSB_command = ((value & UpdateCommandDefinitions.LSB_value_mask) + UpdateCommandDefinitions.LSB_header) & 0xFF + value_MSB_command = (((value & UpdateCommandDefinitions.MSB_value_mask) >> 6) + UpdateCommandDefinitions.MSB_header) & 0xFF + reset_command = UpdateCommandDefinitions.reset_byte + + expected_calls = [ + call(id_command.to_bytes(1, 'big')), + call(target_command.to_bytes(1, 'big')), + call(value_LSB_command.to_bytes(1, 'big')), + call(value_MSB_command.to_bytes(1, 'big')), + call(reset_command.to_bytes(1, 'big')), + ] + + assert mock_serial_instance.write.call_count == len(expected_calls) + mock_serial_instance.write.assert_has_calls(expected_calls, any_order=False) \ No newline at end of file From a5bceb03d89c586ed1660548a23a413173711ce1 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Sat, 2 Nov 2024 22:21:14 +0900 Subject: [PATCH 19/28] remove duplicate functions --- tests/test_device_update.py | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/tests/test_device_update.py b/tests/test_device_update.py index 23e63e99..712b85c9 100644 --- a/tests/test_device_update.py +++ b/tests/test_device_update.py @@ -158,39 +158,3 @@ def test_specified_port_not_ftdi_device(mock_comports): with pytest.raises(ValueError, match="No FTDI devices found."): DevUpdate(target, value, device_id) - -@patch("miniscope_io.models.devupdate.serial.tools.list_ports.comports") -@patch("miniscope_io.device_update.serial.Serial") -def test_devupdate_command_execution_order(mock_serial, mock_comports): - """ - Test that commands are executed in the correct order. - """ - mock_serial_instance = mock_serial.return_value - - mock_comports.return_value = [ - MagicMock(vid=0x0403, pid=0x6001, device="COM3"), - MagicMock(vid=0x1234, pid=0x5678, device="COM4"), - ] - target = "GAIN" - value = 2 - device_id = 1 - port = "COM3" - - DevUpdate(target, value, device_id, port) - - id_command = (device_id + UpdateCommandDefinitions.id_header) & 0xFF - target_command = (UpdateTarget.GAIN.value + UpdateCommandDefinitions.target_header) & 0xFF - value_LSB_command = ((value & UpdateCommandDefinitions.LSB_value_mask) + UpdateCommandDefinitions.LSB_header) & 0xFF - value_MSB_command = (((value & UpdateCommandDefinitions.MSB_value_mask) >> 6) + UpdateCommandDefinitions.MSB_header) & 0xFF - reset_command = UpdateCommandDefinitions.reset_byte - - expected_calls = [ - call(id_command.to_bytes(1, 'big')), - call(target_command.to_bytes(1, 'big')), - call(value_LSB_command.to_bytes(1, 'big')), - call(value_MSB_command.to_bytes(1, 'big')), - call(reset_command.to_bytes(1, 'big')), - ] - - assert mock_serial_instance.write.call_count == len(expected_calls) - mock_serial_instance.write.assert_has_calls(expected_calls, any_order=False) \ No newline at end of file From b9138143a693e5590eabbf6259bcd84456b85606 Mon Sep 17 00:00:00 2001 From: sneakers-the-rat Date: Tue, 5 Nov 2024 19:45:06 -0800 Subject: [PATCH 20/28] add pyserial to intersphinx mapping --- docs/conf.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index 4aba3156..17452239 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -11,7 +11,7 @@ from unittest.mock import Mock # Mock _ok module -sys.modules['_ok'] = Mock() +sys.modules["_ok"] = Mock() project = "miniscope-io" copyright = "2023, Jonny" @@ -49,6 +49,7 @@ "numpy": ("https://numpy.org/doc/stable/", None), "pandas": ("https://pandas.pydata.org/docs/", None), "rich": ("https://rich.readthedocs.io/en/stable/", None), + "pyserial": ("https://pyserial.readthedocs.io/en/stable/", None), } # ---------- From 1ed6e464feea46b89f745061b1ae33a0ab035a26 Mon Sep 17 00:00:00 2001 From: Takuya Sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 6 Nov 2024 14:20:16 +0800 Subject: [PATCH 21/28] Specify return type of FTDI devices Co-authored-by: Jonny Saunders --- miniscope_io/device_update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index fffd9d1d..1efaf959 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -89,7 +89,7 @@ def DevUpdate( logger.info("Closed serial port") -def find_ftdi_device() -> list: +def find_ftdi_device() -> list[str]: """ Find FTDI devices connected to the computer. """ From d9543f8f682bd371a3401a9413e83833491df772 Mon Sep 17 00:00:00 2001 From: Takuya Sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 6 Nov 2024 14:27:05 +0800 Subject: [PATCH 22/28] Update command key validation Co-authored-by: Jonny Saunders --- miniscope_io/models/devupdate.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index be54fe05..f534e6ab 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -71,14 +71,10 @@ def validate_values(cls, values: dict) -> dict: assert value in [1, 2, 4], "For GAIN, value must be 1, 2, or 4" elif target == UpdateTarget.DEVICE: assert value in [DeviceCommand.REBOOT.value], "For DEVICE, value must be in [200]" - elif ( - target == UpdateTarget.ROI_X - or target == UpdateTarget.ROI_Y - or target == UpdateTarget.ROI_WIDTH - or target == UpdateTarget.ROI_HEIGHT - or target == UpdateTarget.EWL - ): - pass # Need to implement + elif target in UpdateTarget: + raise NotImplementedError() + else: + raise ValueError(f"{target} is not a valid update target, need an instance of UpdateTarget") return values @field_validator("port") From a0116b6821d3b17a337ffe848d2658605c49d582 Mon Sep 17 00:00:00 2001 From: Takuya Sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 6 Nov 2024 14:27:54 +0800 Subject: [PATCH 23/28] Update update target not found error message Co-authored-by: Jonny Saunders --- miniscope_io/models/devupdate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index f534e6ab..d232d4d2 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -116,4 +116,4 @@ def validate_target(cls, value: str) -> UpdateTarget: try: return UpdateTarget[value] except KeyError as e: - raise ValueError(f"Target {value} not found.") from e + raise ValueError(f"Target {value} not found, must be a member of UpdateTarget: {list(UpdateTarget.__members__.keys())}.") from e From 8214b60919019ebb37ad8fa32aa25ce7eb7207f7 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 6 Nov 2024 17:47:55 +0800 Subject: [PATCH 24/28] separate device command, specify update device as key, snake case --- miniscope_io/cli/main.py | 3 +- miniscope_io/cli/update.py | 73 +++++++++++++++++++++----------- miniscope_io/device_update.py | 2 +- miniscope_io/models/devupdate.py | 9 +++- tests/test_device_update.py | 20 ++++----- 5 files changed, 69 insertions(+), 38 deletions(-) diff --git a/miniscope_io/cli/main.py b/miniscope_io/cli/main.py index 9fc7fc2e..68d0fabb 100644 --- a/miniscope_io/cli/main.py +++ b/miniscope_io/cli/main.py @@ -5,7 +5,7 @@ import click from miniscope_io.cli.stream import stream -from miniscope_io.cli.update import update +from miniscope_io.cli.update import device, update @click.group() @@ -20,3 +20,4 @@ def cli(ctx: click.Context) -> None: cli.add_command(stream) cli.add_command(update) +cli.add_command(device) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index 718a16d5..6995c02a 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -5,7 +5,7 @@ import click import yaml -from miniscope_io.device_update import DevUpdate +from miniscope_io.device_update import device_update from miniscope_io.models.devupdate import DeviceCommand @@ -25,53 +25,78 @@ help="ID of the device to update. 0 will update all devices.", ) @click.option( - "-t", - "--target", + "-k", + "--key", required=False, type=click.Choice(["LED", "GAIN", "ROI_X", "ROI_Y"]), - help="Target to update. Cannot be used with --restart.", + help="key to update. Cannot be used with --restart.", ) @click.option( "-v", "--value", required=False, type=int, - help="Value to set. Must be used with --target and cannot be used with --restart.", + help="Value to set. Must be used with --key and cannot be used with --restart.", ) @click.option( "-c", "--config", required=False, type=click.Path(exists=True, dir_okay=False), - help="YAML file with configuration to update. Specify target and value pairs in the file.", + help="YAML file with configuration to update. Specify key and value pairs in the file.", ) -@click.option( - "--reboot", - is_flag=True, - type=bool, - help="Restart the device. Cannot be used with --target or --value.", -) -def update(port: str, target: str, value: int, device_id: int, reboot: bool, config: str) -> None: +def update(port: str, key: str, value: int, device_id: int, config: str) -> None: """ - Update device configuration or restart it. + Update device configuration. """ # Check mutual exclusivity - if (target and not value) or (value and not target): - raise click.UsageError("Both --target and --value are required if one is specified.") + if (key and not value) or (value and not key): + raise click.UsageError("Both --key and --value are required if one is specified.") - if ((target or value) and reboot) or (config and reboot) or (config and (target or value)): + if config and (key or value): raise click.UsageError( - "Options --target/--value and --restart" " and --config are mutually exclusive." + "Options --key/--value and --restart" " and --config are mutually exclusive." ) - if target and value: - DevUpdate(port=port, target=target, value=value, device_id=device_id) + if key and value: + device_update(port=port, key=key, value=value, device_id=device_id) elif config: with open(config) as f: config_file = yaml.safe_load(f) for key, value in config_file: - DevUpdate(port=port, target=key, value=value, device_id=device_id) - elif reboot: - DevUpdate(port=port, target="DEVICE", value=DeviceCommand.REBOOT.value, device_id=device_id) + device_update(port=port, key=key, value=value, device_id=device_id) + else: + raise click.UsageError("Either --key with --value or --restart must be specified.") + + +@click.command() +@click.option( + "-p", + "--port", + required=False, + help="Serial port to connect to. Needed if multiple FTDI devices are connected.", +) +@click.option( + "-i", + "--device_id", + required=False, + default=0, + type=int, + help="ID of the device to update. 0 will update all devices.", +) +@click.option( + "--reboot", + is_flag=True, + type=bool, + help="Restart the device. Cannot be used with --key or --value.", +) +def device(port: str, device_id: int, reboot: bool) -> None: + """ + Send device commands (e.g., reboot) + """ + + # Check mutual exclusivity + if reboot: + device_update(port=port, key="DEVICE", value=DeviceCommand.REBOOT.value, device_id=device_id) else: - raise click.UsageError("Either --target with --value or --restart must be specified.") + raise click.UsageError("Only --reboot is currently implemented.") diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 1efaf959..3d1f29de 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -14,7 +14,7 @@ logger = init_logger(name="device_update", level="INFO") -def DevUpdate( +def device_update( target: str, value: int, device_id: int, diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index d232d4d2..aedfcd2e 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -74,7 +74,9 @@ def validate_values(cls, values: dict) -> dict: elif target in UpdateTarget: raise NotImplementedError() else: - raise ValueError(f"{target} is not a valid update target, need an instance of UpdateTarget") + raise ValueError( + f"{target} is not a valid update target," "need an instance of UpdateTarget" + ) return values @field_validator("port") @@ -116,4 +118,7 @@ def validate_target(cls, value: str) -> UpdateTarget: try: return UpdateTarget[value] except KeyError as e: - raise ValueError(f"Target {value} not found, must be a member of UpdateTarget: {list(UpdateTarget.__members__.keys())}.") from e + raise ValueError( + f"Target {value} not found, must be a member of UpdateTarget:" + f" {list(UpdateTarget.__members__.keys())}." + ) from e diff --git a/tests/test_device_update.py b/tests/test_device_update.py index 712b85c9..f6d55329 100644 --- a/tests/test_device_update.py +++ b/tests/test_device_update.py @@ -3,14 +3,14 @@ from pydantic import ValidationError from unittest.mock import MagicMock, patch, call from miniscope_io.models.devupdate import UpdateCommandDefinitions, UpdateTarget -from miniscope_io.device_update import DevUpdate, find_ftdi_device +from miniscope_io.device_update import device_update, find_ftdi_device @patch("miniscope_io.device_update.serial.tools.list_ports.comports") @patch("miniscope_io.device_update.serial.Serial") def test_devupdate_with_device_connected(mock_serial, mock_comports): """ - Test DevUpdate function with a device connected. + Test device_update function with a device connected. """ mock_serial_instance = mock_serial.return_value mock_comports.return_value = [ @@ -22,7 +22,7 @@ def test_devupdate_with_device_connected(mock_serial, mock_comports): device_id = 1 port = "COM3" - DevUpdate(target, value, device_id, port) + device_update(target, value, device_id, port) mock_serial.assert_called_once_with(port=port, baudrate=2400, timeout=5, stopbits=2) @@ -50,7 +50,7 @@ def test_devupdate_with_device_connected(mock_serial, mock_comports): @patch("miniscope_io.device_update.serial.tools.list_ports.comports") def test_devupdate_without_device_connected(mock_comports): """ - Test DevUpdate function without a device connected. + Test device_update function without a device connected. """ mock_comports.return_value = [ ] @@ -59,7 +59,7 @@ def test_devupdate_without_device_connected(mock_comports): device_id = 0 with pytest.raises(ValueError, match="No FTDI devices found."): - DevUpdate(target, value, device_id) + device_update(target, value, device_id) @patch("miniscope_io.device_update.serial.tools.list_ports.comports") def test_find_ftdi_device(mock_comports): @@ -91,7 +91,7 @@ def test_invalid_target_raises_error(mock_comports): port = "COM3" with pytest.raises(ValidationError, match="Target RANDOM_STRING not found"): - DevUpdate(target, value, device_id, port) + device_update(target, value, device_id, port) @patch("miniscope_io.models.devupdate.serial.tools.list_ports.comports") def test_invalid_led_value_raises_error(mock_comports): @@ -108,7 +108,7 @@ def test_invalid_led_value_raises_error(mock_comports): port = "COM3" with pytest.raises(ValidationError, match="For LED, value must be between 0 and 100"): - DevUpdate(target, value, device_id, port) + device_update(target, value, device_id, port) @patch("miniscope_io.device_update.serial.tools.list_ports.comports") @@ -126,7 +126,7 @@ def test_devupdate_with_multiple_ftdi_devices(mock_comports): device_id = 1 with pytest.raises(ValueError, match="Multiple FTDI devices found. Please specify the port."): - DevUpdate(target, value, device_id) + device_update(target, value, device_id) @patch("miniscope_io.device_update.serial.Serial") def test_devupdate_serial_exception_handling(mock_serial): @@ -141,7 +141,7 @@ def test_devupdate_serial_exception_handling(mock_serial): port = "COM3" with pytest.raises(ValidationError): - DevUpdate(target, value, device_id, port) + device_update(target, value, device_id, port) @patch("miniscope_io.device_update.serial.tools.list_ports.comports") def test_specified_port_not_ftdi_device(mock_comports): @@ -157,4 +157,4 @@ def test_specified_port_not_ftdi_device(mock_comports): device_id = 1 with pytest.raises(ValueError, match="No FTDI devices found."): - DevUpdate(target, value, device_id) + device_update(target, value, device_id) From fa1abfbb182f0cade8498c2c4a735110a1493cec Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 6 Nov 2024 17:54:51 +0800 Subject: [PATCH 25/28] move FTDI constants to module, change UpdateTarget type --- miniscope_io/cli/update.py | 4 +++- miniscope_io/device_update.py | 4 ++-- miniscope_io/models/devupdate.py | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index 6995c02a..30605335 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -97,6 +97,8 @@ def device(port: str, device_id: int, reboot: bool) -> None: # Check mutual exclusivity if reboot: - device_update(port=port, key="DEVICE", value=DeviceCommand.REBOOT.value, device_id=device_id) + device_update( + port=port, key="DEVICE", value=DeviceCommand.REBOOT.value, device_id=device_id + ) else: raise click.UsageError("Only --reboot is currently implemented.") diff --git a/miniscope_io/device_update.py b/miniscope_io/device_update.py index 3d1f29de..9680794e 100644 --- a/miniscope_io/device_update.py +++ b/miniscope_io/device_update.py @@ -12,6 +12,8 @@ from miniscope_io.models.devupdate import DevUpdateCommand, UpdateCommandDefinitions logger = init_logger(name="device_update", level="INFO") +FTDI_VENDOR_ID = 0x0403 +FTDI_PRODUCT_ID = 0x6001 def device_update( @@ -93,8 +95,6 @@ def find_ftdi_device() -> list[str]: """ Find FTDI devices connected to the computer. """ - FTDI_VENDOR_ID = 0x0403 - FTDI_PRODUCT_ID = 0x6001 ports = serial.tools.list_ports.comports() ftdi_ports = [] diff --git a/miniscope_io/models/devupdate.py b/miniscope_io/models/devupdate.py index aedfcd2e..a432a838 100644 --- a/miniscope_io/models/devupdate.py +++ b/miniscope_io/models/devupdate.py @@ -30,7 +30,7 @@ class UpdateCommandDefinitions: reset_byte = 0b11111111 -class UpdateTarget(Enum): +class UpdateTarget(int, Enum): """ Targets to update. Needs to be under 6-bit. """ From 4db38cf695f48587669db9afa2e813c02a3fc3e7 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 6 Nov 2024 18:13:42 +0800 Subject: [PATCH 26/28] change update config to batch (name) --- miniscope_io/cli/update.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index 30605335..e4ba5a56 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -39,13 +39,13 @@ help="Value to set. Must be used with --key and cannot be used with --restart.", ) @click.option( - "-c", - "--config", + "-b", + "--batch", required=False, type=click.Path(exists=True, dir_okay=False), - help="YAML file with configuration to update. Specify key and value pairs in the file.", + help="YAML file that works as a batch file to update. Specify key and value pairs in the file.", ) -def update(port: str, key: str, value: int, device_id: int, config: str) -> None: +def update(port: str, key: str, value: int, device_id: int, batch: str) -> None: """ Update device configuration. """ @@ -54,16 +54,16 @@ def update(port: str, key: str, value: int, device_id: int, config: str) -> None if (key and not value) or (value and not key): raise click.UsageError("Both --key and --value are required if one is specified.") - if config and (key or value): + if batch and (key or value): raise click.UsageError( - "Options --key/--value and --restart" " and --config are mutually exclusive." + "Options --key/--value and --restart" " and --batch are mutually exclusive." ) if key and value: device_update(port=port, key=key, value=value, device_id=device_id) - elif config: - with open(config) as f: - config_file = yaml.safe_load(f) - for key, value in config_file: + elif batch: + with open(batch) as f: + batch_file = yaml.safe_load(f) + for key, value in batch_file: device_update(port=port, key=key, value=value, device_id=device_id) else: raise click.UsageError("Either --key with --value or --restart must be specified.") From ef6d1db428be8888f9a01e2882589367c2e058d3 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 6 Nov 2024 18:23:55 +0800 Subject: [PATCH 27/28] add experimental feature note --- miniscope_io/cli/update.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/miniscope_io/cli/update.py b/miniscope_io/cli/update.py index e4ba5a56..b809a446 100644 --- a/miniscope_io/cli/update.py +++ b/miniscope_io/cli/update.py @@ -22,7 +22,7 @@ required=False, default=0, type=int, - help="ID of the device to update. 0 will update all devices.", + help="[EXPERIMENTAL FEATURE] ID of the device to update. 0 (default) will update all devices.", ) @click.option( "-k", @@ -43,7 +43,10 @@ "--batch", required=False, type=click.Path(exists=True, dir_okay=False), - help="YAML file that works as a batch file to update. Specify key and value pairs in the file.", + help=( + "[EXPERIMENTAL FEATURE] YAML file that works as a batch file to update." + "Specify key and value pairs in the file." + ), ) def update(port: str, key: str, value: int, device_id: int, batch: str) -> None: """ @@ -82,7 +85,7 @@ def update(port: str, key: str, value: int, device_id: int, batch: str) -> None: required=False, default=0, type=int, - help="ID of the device to update. 0 will update all devices.", + help="[EXPERIMENTAL FEATURE] ID of the device to update. 0 (default) will update all devices.", ) @click.option( "--reboot", From e3bba77fa57fd9d43b713686176118bb732bb380 Mon Sep 17 00:00:00 2001 From: t-sasatani <33111879+t-sasatani@users.noreply.github.com> Date: Wed, 6 Nov 2024 22:49:08 +0800 Subject: [PATCH 28/28] make update test with parameterized fixtures --- tests/test_device_update.py | 103 ++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 47 deletions(-) diff --git a/tests/test_device_update.py b/tests/test_device_update.py index f6d55329..ca1da6d2 100644 --- a/tests/test_device_update.py +++ b/tests/test_device_update.py @@ -5,18 +5,24 @@ from miniscope_io.models.devupdate import UpdateCommandDefinitions, UpdateTarget from miniscope_io.device_update import device_update, find_ftdi_device - -@patch("miniscope_io.device_update.serial.tools.list_ports.comports") -@patch("miniscope_io.device_update.serial.Serial") -def test_devupdate_with_device_connected(mock_serial, mock_comports): +@pytest.fixture +def mock_serial_fixture(request): + device_list = request.param + with patch('serial.Serial') as mock_serial, patch('serial.tools.list_ports.comports') as mock_comports: + mock_serial_instance = mock_serial.return_value + mock_comports.return_value = [MagicMock(vid=device['vid'], pid=device['pid'], device=device['device']) + for device in device_list] + yield mock_serial, mock_comports, mock_serial_instance + +@pytest.mark.parametrize('mock_serial_fixture', [ + [{'vid': 0x0403, 'pid': 0x6001, 'device': 'COM3'}, + {'vid': 0x0111, 'pid': 0x6111, 'device': 'COM2'}], +], indirect=True) +def test_devupdate_with_device_connected(mock_serial_fixture): """ Test device_update function with a device connected. """ - mock_serial_instance = mock_serial.return_value - mock_comports.return_value = [ - MagicMock(vid=0x0403, pid=0x6001, device="COM3"), - MagicMock(vid=0x1234, pid=0x5678, device="COM4"), - ] + mock_serial, mock_comports, mock_serial_instance = mock_serial_fixture target = "LED" value = 2 device_id = 1 @@ -47,13 +53,15 @@ def test_devupdate_with_device_connected(mock_serial, mock_comports): assert mock_serial_instance.write.call_count == len(expected_calls) mock_serial_instance.write.assert_has_calls(expected_calls, any_order=False) -@patch("miniscope_io.device_update.serial.tools.list_ports.comports") -def test_devupdate_without_device_connected(mock_comports): + +@pytest.mark.parametrize('mock_serial_fixture', [ + [], +], indirect=True) +def test_devupdate_without_device_connected(mock_serial_fixture): """ Test device_update function without a device connected. """ - mock_comports.return_value = [ - ] + target = "GAIN" value = 2 device_id = 0 @@ -61,29 +69,26 @@ def test_devupdate_without_device_connected(mock_comports): with pytest.raises(ValueError, match="No FTDI devices found."): device_update(target, value, device_id) -@patch("miniscope_io.device_update.serial.tools.list_ports.comports") -def test_find_ftdi_device(mock_comports): +@pytest.mark.parametrize('mock_serial_fixture', [ + [{'vid': 0x0403, 'pid': 0x6001, 'device': 'COM3'}, + {'vid': 0x0111, 'pid': 0x6111, 'device': 'COM2'}], +], indirect=True) +def test_find_ftdi_device(mock_serial_fixture): """ Test find_ftdi_device function. """ - mock_comports.return_value = [ - MagicMock(vid=0x0403, pid=0x6001, device="COM3"), - MagicMock(vid=0x1234, pid=0x5678, device="COM4"), - ] - result = find_ftdi_device() assert result == ["COM3"] -@patch("miniscope_io.models.devupdate.serial.tools.list_ports.comports") -def test_invalid_target_raises_error(mock_comports): +@pytest.mark.parametrize('mock_serial_fixture', [ + [{'vid': 0x0403, 'pid': 0x6001, 'device': 'COM3'}, + {'vid': 0x0111, 'pid': 0x6111, 'device': 'COM2'}], +], indirect=True) +def test_invalid_target_raises_error(mock_serial_fixture): """ Test that an invalid target raises an error. """ - mock_comports.return_value = [ - MagicMock(vid=0x0403, pid=0x6001, device="COM3"), - MagicMock(vid=0x1234, pid=0x5678, device="COM4"), - ] target = "RANDOM_STRING" value = 50 @@ -93,15 +98,16 @@ def test_invalid_target_raises_error(mock_comports): with pytest.raises(ValidationError, match="Target RANDOM_STRING not found"): device_update(target, value, device_id, port) -@patch("miniscope_io.models.devupdate.serial.tools.list_ports.comports") -def test_invalid_led_value_raises_error(mock_comports): +@pytest.mark.parametrize('mock_serial_fixture', [ + [{'vid': 0x0403, 'pid': 0x6001, 'device': 'COM3'}, + {'vid': 0x0111, 'pid': 0x6111, 'device': 'COM2'}], +], indirect=True) +def test_invalid_led_value_raises_error(mock_serial_fixture): """ Test that an invalid LED value raises an error. """ - mock_comports.return_value = [ - MagicMock(vid=0x0403, pid=0x6001, device="COM3"), - MagicMock(vid=0x1234, pid=0x5678, device="COM4"), - ] + mock_serial, mock_comports, mock_serial_instance = mock_serial_fixture + target = "LED" value = 150 # LED value should be between 0 and 100 device_id = 1 @@ -110,16 +116,14 @@ def test_invalid_led_value_raises_error(mock_comports): with pytest.raises(ValidationError, match="For LED, value must be between 0 and 100"): device_update(target, value, device_id, port) - -@patch("miniscope_io.device_update.serial.tools.list_ports.comports") -def test_devupdate_with_multiple_ftdi_devices(mock_comports): +@pytest.mark.parametrize('mock_serial_fixture', [ + [{'vid': 0x0403, 'pid': 0x6001, 'device': 'COM3'}, + {'vid': 0x0403, 'pid': 0x6001, 'device': 'COM2'}], +], indirect=True) +def test_devupdate_with_multiple_ftdi_devices(mock_serial_fixture): """ Test that multiple FTDI devices raise an error. """ - mock_comports.return_value = [ - MagicMock(vid=0x0403, pid=0x6001, device="COM1"), - MagicMock(vid=0x0403, pid=0x6001, device="COM2"), - ] target = "GAIN" value = 5 @@ -128,11 +132,16 @@ def test_devupdate_with_multiple_ftdi_devices(mock_comports): with pytest.raises(ValueError, match="Multiple FTDI devices found. Please specify the port."): device_update(target, value, device_id) -@patch("miniscope_io.device_update.serial.Serial") -def test_devupdate_serial_exception_handling(mock_serial): +@pytest.mark.parametrize('mock_serial_fixture', [ + [{'vid': 0x0403, 'pid': 0x6001, 'device': 'COM3'}], +], indirect=True) +def test_devupdate_serial_exception_handling(mock_serial_fixture): """ Test exception handling when serial port cannot be opened. + Might be too obvious so it might be better to re-think this test. """ + mock_serial, mock_comports, mock_serial_instance = mock_serial_fixture + mock_serial.side_effect = serial.SerialException("Serial port error") target = "LED" @@ -140,17 +149,17 @@ def test_devupdate_serial_exception_handling(mock_serial): device_id = 1 port = "COM3" - with pytest.raises(ValidationError): + with pytest.raises(serial.SerialException): device_update(target, value, device_id, port) -@patch("miniscope_io.device_update.serial.tools.list_ports.comports") -def test_specified_port_not_ftdi_device(mock_comports): +@pytest.mark.parametrize('mock_serial_fixture', [ + [{'vid': 0x0413, 'pid': 0x6111, 'device': 'COM2'}], +], indirect=True) +def test_specified_port_not_ftdi_device(mock_serial_fixture): """ Test with a specified port not corresponding to an FTDI device. """ - mock_comports.return_value = [ - MagicMock(vid=None, pid=None, device="COM3") - ] + mock_serial, mock_comports, mock_serial_instance = mock_serial_fixture target = "GAIN" value = 10