Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Over The Air device config feature #48

Merged
merged 29 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
95e628c
Added remote update function and minimally working
t-sasatani Oct 23, 2024
e88e208
Formatting
t-sasatani Oct 24, 2024
ada51cb
ROI/gain/LED value remote update prototype working
t-sasatani Oct 27, 2024
e705b69
allow multi-word value transfer with headers
t-sasatani Oct 27, 2024
7ad77ad
device ID-based over-the-air update
t-sasatani Oct 27, 2024
afdbc97
Add device commands (mainly for restart)
t-sasatani Nov 1, 2024
a560a48
Isolate models in a separate file
t-sasatani Nov 1, 2024
7ac453e
add devupdate model tests
t-sasatani Nov 1, 2024
c1ea61a
Fix model tests
t-sasatani Nov 1, 2024
8782cd2
update format
t-sasatani Nov 1, 2024
9fff132
moved cmd constants to model file
t-sasatani Nov 1, 2024
3fe1108
fix definitions
t-sasatani Nov 1, 2024
3bbed78
Fix remote reboot command
t-sasatani Nov 2, 2024
41b50fe
minor fix for model and modeltest
t-sasatani Nov 2, 2024
d4212a2
bundle ota update with yaml
t-sasatani Nov 2, 2024
70d806a
Add tests for device update
t-sasatani Nov 2, 2024
212df47
Add controller docs (mostly just placeholder)
t-sasatani Nov 2, 2024
3721008
update docs index, add update device tests
t-sasatani Nov 2, 2024
a5bceb0
remove duplicate functions
t-sasatani Nov 2, 2024
b913814
add pyserial to intersphinx mapping
sneakers-the-rat Nov 6, 2024
1ed6e46
Specify return type of FTDI devices
t-sasatani Nov 6, 2024
d9543f8
Update command key validation
t-sasatani Nov 6, 2024
a0116b6
Update update target not found error message
t-sasatani Nov 6, 2024
8214b60
separate device command, specify update device as key, snake case
t-sasatani Nov 6, 2024
fa1abfb
move FTDI constants to module, change UpdateTarget type
t-sasatani Nov 6, 2024
4db38cf
change update config to batch (name)
t-sasatani Nov 6, 2024
ef6d1db
add experimental feature note
t-sasatani Nov 6, 2024
e3bba77
make update test with parameterized fixtures
t-sasatani Nov 6, 2024
7e0a836
Merge branch 'main' into feature_ir_update
t-sasatani Nov 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion miniscope_io/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -20,3 +20,4 @@ def cli(ctx: click.Context) -> None:

cli.add_command(stream)
cli.add_command(update)
cli.add_command(device)
75 changes: 51 additions & 24 deletions miniscope_io/cli/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -25,53 +25,80 @@
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.")
6 changes: 3 additions & 3 deletions miniscope_io/device_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
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 DevUpdate(
def device_update(
target: str,
value: int,
device_id: int,
Expand Down Expand Up @@ -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 = []

Expand Down
11 changes: 8 additions & 3 deletions miniscope_io/models/devupdate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down Expand Up @@ -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"
Copy link
Collaborator

@sneakers-the-rat sneakers-the-rat Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this happens to me literally all the time where i want to split a line for aesthetic reasons and then black is like "no you should not have split that line." it borders on malicious compliance and i imagine black having a smile on its face every time it does this like "you were the one who set the line width!"

Copy link
Collaborator Author

@t-sasatani t-sasatani Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was fully trusting black and wasn't even paying attention to the changes it made lol. Didn't know it was making my code ugly!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it is literally equivalent to the interpreter after parsing (?), but at the same time there is something bad about ("start " "end") vs "start end" that feels like the machines are taking revenge when they make it happen.

)
return values

@field_validator("port")
Expand Down Expand Up @@ -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
20 changes: 10 additions & 10 deletions tests/test_device_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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)

Expand Down Expand Up @@ -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 = [
]
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand All @@ -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")
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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)
Loading