diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index e397aad7..f2e786c7 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -36,6 +36,7 @@ DrifterConfig, Expedition, Location, + SensorConfig, ShipConfig, ShipUnderwaterSTConfig, Waypoint, @@ -76,6 +77,14 @@ def log_exception_to_file( f.write("\n") +def _default_sensors(config_class) -> list: + """List of SensorConfig instances from the config class's sensors default_factory.""" + sensors_field = config_class.model_fields.get("sensors") + if sensors_field is None or sensors_field.default_factory is None: + return [] + return sensors_field.default_factory() + + DEFAULT_TS_CONFIG = {"period_minutes": 5.0} DEFAULT_ADCP_CONFIG = { @@ -83,6 +92,7 @@ def log_exception_to_file( "period_minutes": 5.0, } + INSTRUMENT_FIELDS = { "adcp_config": { "class": ADCPConfig, @@ -91,6 +101,7 @@ def log_exception_to_file( {"name": "num_bins"}, {"name": "period", "minutes": True}, ], + # underway instrument, so active/inactive is controlled by the on/off toggle, not the schedule (therefore no instrument_type key) }, "ship_underwater_st_config": { "class": ShipUnderwaterSTConfig, @@ -98,10 +109,12 @@ def log_exception_to_file( "attributes": [ {"name": "period", "minutes": True}, ], + # underway instrument, so active/inactive is controlled by the on/off toggle, not the schedule (therefore no instrument_type key) }, "ctd_config": { "class": CTDConfig, "title": "CTD", + "instrument_type": InstrumentType.CTD, "attributes": [ {"name": "max_depth_meter"}, {"name": "min_depth_meter"}, @@ -111,6 +124,7 @@ def log_exception_to_file( "xbt_config": { "class": XBTConfig, "title": "XBT", + "instrument_type": InstrumentType.XBT, "attributes": [ {"name": "min_depth_meter"}, {"name": "max_depth_meter"}, @@ -121,6 +135,7 @@ def log_exception_to_file( "argo_float_config": { "class": ArgoFloatConfig, "title": "Argo Float", + "instrument_type": InstrumentType.ARGO_FLOAT, "attributes": [ {"name": "min_depth_meter"}, {"name": "max_depth_meter"}, @@ -135,6 +150,7 @@ def log_exception_to_file( "drifter_config": { "class": DrifterConfig, "title": "Drifter", + "instrument_type": InstrumentType.DRIFTER, "attributes": [ {"name": "depth_meter"}, {"name": "lifetime", "days": True}, @@ -265,7 +281,7 @@ def compose(self) -> ComposeResult: ## SECTION: "Instrument Configurations"" with Collapsible( - title="[b]Instrument Configurations[/b] (advanced users only)", + title="[b]Instrument Configurations[/b]", collapsed=True, ): for instrument_name, info in INSTRUMENT_FIELDS.items(): @@ -339,6 +355,34 @@ def compose(self) -> ComposeResult: id=f"validation-failure-label-{instrument_name}_{attr}", classes="-hidden validation-failure", ) + # sensor toggles, derived from the config class's sensors default_factory + default_sensor_configs = _default_sensors(config_class) + if default_sensor_configs: + yield Label("[b]Sensors:[/b]", markup=True) + # which sensors are currently active + if config_instance and hasattr( + config_instance, "sensors" + ): + active_sensor_types = { + sc.sensor_type + for sc in config_instance.sensors + if sc.enabled + } + else: + # if no config loaded yet, default all sensors on + active_sensor_types = { + sc.sensor_type for sc in default_sensor_configs + } + for sc in default_sensor_configs: + sensor_id = f"{instrument_name}_sensor_{sc.sensor_type.value}" + with Horizontal(classes="sensor-toggle-row"): + yield Label( + f" {sc.sensor_type.value.replace('_', ' ').title()}:" + ) + yield Switch( + value=sc.sensor_type in active_sensor_types, + id=sensor_id, + ) ## 2) SCHEDULE EDITOR @@ -393,6 +437,8 @@ def save_changes(self) -> bool: self._update_schedule() self.expedition.to_yaml(self.path.joinpath(EXPEDITION)) return True + except UserError: + raise except Exception as e: log_exception_to_file( e, @@ -449,6 +495,41 @@ def _update_instrument_configs(self): kwargs["max_depth_meter"] = -1000.0 else: kwargs["max_depth_meter"] = -150.0 + # collect sensor toggles + default_sensor_configs = _default_sensors(config_class) + if default_sensor_configs: + sensors = [ + SensorConfig(sensor_type=sc.sensor_type) + for sc in default_sensor_configs + if self.query_one( + f"#{instrument_name}_sensor_{sc.sensor_type.value}", Switch + ).value + ] + if not sensors: + # for schedule-based instruments, only raise if actually used in a waypoint + # for underway, this is handled by the on/off toggle + instrument_type = info.get("instrument_type") + is_active = instrument_type is None or any( + instrument_type + in ( + wp.instrument + if isinstance(wp.instrument, list) + else [wp.instrument] + ) + for wp in self.expedition.schedule.waypoints + if wp.instrument + ) + if is_active: + title = info.get( + "title", instrument_name.replace("_", " ").title() + ) + raise UserError( + f"'{title}' has no sensors selected. " + f"At least one sensor must be enabled for each active instrument." + ) + kwargs["sensors"] = ( + sensors if sensors else _default_sensors(config_class) + ) setattr( self.expedition.instruments_config, instrument_name, @@ -1256,6 +1337,18 @@ class PlanApp(App): margin: 0 1; } + .sensor-toggle-row { + height: auto; + margin: 0; + padding: 0; + } + + .sensor-toggle-row Label { + width: 24; + margin-top: 1; + color: $text-muted; + } + .year-select { width: 20; } diff --git a/src/virtualship/models/__init__.py b/src/virtualship/models/__init__.py index 71f24211..dd4b2bf1 100644 --- a/src/virtualship/models/__init__.py +++ b/src/virtualship/models/__init__.py @@ -9,6 +9,7 @@ Expedition, InstrumentsConfig, Schedule, + SensorConfig, ShipConfig, ShipUnderwaterSTConfig, Waypoint, @@ -23,6 +24,7 @@ __all__ = [ # noqa: RUF022 "Location", "Schedule", + "SensorConfig", "ShipConfig", "Waypoint", "ArgoFloatConfig", diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index f0522cde..29459223 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -1,20 +1,20 @@ -import os -import shutil -import tempfile from datetime import datetime from pathlib import Path from unittest.mock import MagicMock import pytest import yaml -from textual.widgets import Button, Collapsible, Input +from textual.widgets import Button, Collapsible, Input, Switch -from virtualship.cli._plan import ExpeditionEditor, PlanApp +from virtualship.cli._plan import ExpeditionEditor, PlanApp, _default_sensors +from virtualship.instruments.sensors import SensorType from virtualship.models import ( + CTDConfig, Expedition, InstrumentsConfig, Location, Schedule, + SensorConfig, Waypoint, ) from virtualship.utils import EXPEDITION, get_example_expedition @@ -24,6 +24,24 @@ NEW_LON = "0.015" +def _make_expedition( + tmpdir: Path, + waypoints: list, + instruments_config=None, +) -> None: + """Write a minimal expedition YAML.""" + if instruments_config is None: + instruments_config = InstrumentsConfig.model_validate( + yaml.safe_load(get_example_expedition()).get("instruments_config") + ) + ship_config = yaml.safe_load(get_example_expedition()).get("ship_config") + Expedition( + schedule=Schedule(waypoints=waypoints), + instruments_config=instruments_config, + ship_config=ship_config, + ).to_yaml(tmpdir / EXPEDITION) + + async def simulate_input(pilot, box, new_value): """Simulate inputs to the UI.""" box.focus() @@ -35,15 +53,26 @@ async def simulate_input(pilot, box, new_value): await pilot.pause(0.05) +async def _expand_instrument_configs( + expedition_editor, pilot, instrument_title: str | None = None +): + """Expand the outer 'Instrument Configurations' collapsible and the instrument-specific inner one.""" + for coll in expedition_editor.query(Collapsible): + if "Instrument Configurations" in (coll.title or ""): + coll.collapsed = False + await pilot.pause() + break + if instrument_title is not None: + for coll in expedition_editor.query(Collapsible): + if instrument_title in (coll.title or ""): + coll.collapsed = False + await pilot.pause() + break + + @pytest.mark.asyncio -async def test_UI_changes(): +async def test_UI_changes(tmp_path): """Test making changes to UI inputs and saving to YAML (simulated botton presses and typing inputs).""" - tmpdir = Path(tempfile.mkdtemp()) - - instruments_config = InstrumentsConfig.model_validate( - yaml.safe_load(get_example_expedition()).get("instruments_config") - ) - ship_config = yaml.safe_load(get_example_expedition()).get("ship_config") waypoints = [ Waypoint( location=Location(0, 0), @@ -61,15 +90,9 @@ async def test_UI_changes(): instrument=["CTD"], ), ] - expedition = Expedition( - schedule=Schedule(waypoints=waypoints), - instruments_config=instruments_config, - ship_config=ship_config, - ) + _make_expedition(tmp_path, waypoints) - expedition.to_yaml(tmpdir / EXPEDITION) - - app = PlanApp(path=tmpdir) + app = PlanApp(path=tmp_path) async with app.run_test(size=(120, 100)) as pilot: await pilot.pause(0.5) @@ -133,8 +156,7 @@ async def test_UI_changes(): ) # verify changes to speed, lat, lon in saved YAML - expedition_path = os.path.join(tmpdir, EXPEDITION) - with open(expedition_path) as f: + with open(tmp_path / EXPEDITION) as f: saved_expedition = yaml.safe_load(f) assert saved_expedition["ship_config"]["ship_speed_knots"] == float(NEW_SPEED) @@ -148,45 +170,22 @@ async def test_UI_changes(): args, _ = plan_screen.notify.call_args assert "*** Error saving changes ***" in args[0] - # cleanup - shutil.rmtree(tmpdir) - @pytest.mark.asyncio -async def test_UI_opens_with_null_time_and_instrument(): +async def test_UI_opens_with_null_time_and_instrument(tmp_path): """Test that the UI opens correctly when waypoints have time: null and instrument: null.""" - tmpdir = Path(tempfile.mkdtemp()) - - instruments_config = InstrumentsConfig.model_validate( - yaml.safe_load(get_example_expedition()).get("instruments_config") - ) - ship_config = yaml.safe_load(get_example_expedition()).get("ship_config") waypoints = [ Waypoint( location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0), instrument=["CTD"], ), - Waypoint( - location=Location(0.01, 0.01), - time=None, - instrument=None, - ), - Waypoint( - location=Location(0.02, 0.02), - time=None, - instrument=None, - ), + Waypoint(location=Location(0.01, 0.01), time=None, instrument=None), + Waypoint(location=Location(0.02, 0.02), time=None, instrument=None), ] - expedition = Expedition( - schedule=Schedule(waypoints=waypoints), - instruments_config=instruments_config, - ship_config=ship_config, - ) - - expedition.to_yaml(tmpdir / EXPEDITION) + _make_expedition(tmp_path, waypoints) - app = PlanApp(path=tmpdir) + app = PlanApp(path=tmp_path) async with app.run_test(size=(120, 100)) as pilot: await pilot.pause(0.5) @@ -197,5 +196,184 @@ async def test_UI_opens_with_null_time_and_instrument(): # verify the app opened without errors by checking the editor loaded assert expedition_editor is not None - # cleanup - shutil.rmtree(tmpdir) + +def test_default_sensors_returns_declared_defaults(): + """_default_sensors() should mirror the config class's default sensor list.""" + ctd_sensors = _default_sensors(CTDConfig) + types = {sc.sensor_type for sc in ctd_sensors} + assert SensorType.TEMPERATURE in types + assert SensorType.SALINITY in types + + +@pytest.mark.asyncio +async def test_sensor_toggle_saved_to_yaml(tmp_path): + """Toggling a sensor switch off and saving should persist only the enabled sensors.""" + _make_expedition( + tmp_path, + [ + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 0, 0, 0), + instrument=["CTD"], + ), + Waypoint( + location=Location(0.01, 0.01), + time=datetime(2022, 1, 1, 1, 0, 0), + instrument=None, + ), + ], + ) + + app = PlanApp(path=tmp_path) + async with app.run_test(size=(160, 120)) as pilot: + await pilot.pause(0.5) + plan_screen = pilot.app.screen + plan_screen.notify = MagicMock() + expedition_editor = plan_screen.query_one(ExpeditionEditor) + await _expand_instrument_configs(expedition_editor, pilot, "CTD") + + # turn off SALINITY on CTD + sal_switch = expedition_editor.query_one("#ctd_config_sensor_SALINITY", Switch) + await pilot.click(sal_switch) + await pilot.pause(0.2) + + await pilot.click(plan_screen.query_one("#save_button", Button)) + await pilot.pause(0.5) + + plan_screen.notify.assert_called_once_with( + "Changes saved successfully", severity="information", timeout=20 + ) + + with open(tmp_path / EXPEDITION) as f: + saved = yaml.safe_load(f) + saved_sensors = saved["instruments_config"]["ctd_config"]["sensors"] + assert "SALINITY" not in saved_sensors + assert "TEMPERATURE" in saved_sensors + + +@pytest.mark.asyncio +async def test_deselecting_all_sensors_on_active_instrument_blocks_save(tmp_path): + """Removing all sensors from an instrument used in the schedule should show an error.""" + _make_expedition( + tmp_path, + [ + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 0, 0, 0), + instrument=["XBT"], + ), + Waypoint( + location=Location(0.01, 0.01), + time=datetime(2022, 1, 1, 1, 0, 0), + instrument=None, + ), + ], + ) + + app = PlanApp(path=tmp_path) + async with app.run_test(size=(160, 120)) as pilot: + await pilot.pause(0.5) + plan_screen = pilot.app.screen + plan_screen.notify = MagicMock() + expedition_editor = plan_screen.query_one(ExpeditionEditor) + await _expand_instrument_configs(expedition_editor, pilot, "XBT") + + # XBT only has TEMPERATURE, deselect it + temp_switch = expedition_editor.query_one( + "#xbt_config_sensor_TEMPERATURE", Switch + ) + await pilot.click(temp_switch) + await pilot.pause(0.2) + + await pilot.click(plan_screen.query_one("#save_button", Button)) + await pilot.pause(0.5) + + args, _ = plan_screen.notify.call_args + assert "*** Error saving changes ***" in args[0] + assert "XBT" in args[0] + + +@pytest.mark.asyncio +async def test_deselecting_all_sensors_on_inactive_instrument(tmp_path): + """Removing all sensors from an instrument NOT in the schedule should not block saving.""" + # only CTD waypoints, XBT is inactive + _make_expedition( + tmp_path, + [ + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 0, 0, 0), + instrument=["CTD"], + ), + Waypoint( + location=Location(0.01, 0.01), + time=datetime(2022, 1, 1, 1, 0, 0), + instrument=None, + ), + ], + ) + + app = PlanApp(path=tmp_path) + async with app.run_test(size=(160, 120)) as pilot: + await pilot.pause(0.5) + plan_screen = pilot.app.screen + plan_screen.notify = MagicMock() + expedition_editor = plan_screen.query_one(ExpeditionEditor) + await _expand_instrument_configs(expedition_editor, pilot, "XBT") + + # deselect XBT's only sensor even though XBT is not in the schedule + temp_switch = expedition_editor.query_one( + "#xbt_config_sensor_TEMPERATURE", Switch + ) + await pilot.click(temp_switch) + await pilot.pause(0.2) + + await pilot.click(plan_screen.query_one("#save_button", Button)) + await pilot.pause(0.5) + + plan_screen.notify.assert_called_once_with( + "Changes saved successfully", severity="information", timeout=20 + ) + + +@pytest.mark.asyncio +async def test_sensor_initial_state_reflects_config(tmp_path): + """Sensor switches should reflect the enabled sensors already stored in the config.""" + ctd_config = CTDConfig( + stationkeeping_time_minutes=50, + min_depth_meter=-11.0, + max_depth_meter=-2000.0, + sensors=[SensorConfig(sensor_type=SensorType.TEMPERATURE)], + ) + instruments_config = InstrumentsConfig.model_validate( + yaml.safe_load(get_example_expedition()).get("instruments_config") + ) + instruments_config.ctd_config = ctd_config + _make_expedition( + tmp_path, + [ + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 0, 0, 0), + instrument=["CTD"], + ), + Waypoint( + location=Location(0.01, 0.01), + time=datetime(2022, 1, 1, 1, 0, 0), + instrument=None, + ), + ], + instruments_config, + ) + + app = PlanApp(path=tmp_path) + async with app.run_test(size=(160, 120)) as pilot: + await pilot.pause(0.5) + expedition_editor = pilot.app.screen.query_one(ExpeditionEditor) + + temp_switch = expedition_editor.query_one( + "#ctd_config_sensor_TEMPERATURE", Switch + ) + sal_switch = expedition_editor.query_one("#ctd_config_sensor_SALINITY", Switch) + assert temp_switch.value is True, "TEMPERATURE should be ON" + assert sal_switch.value is False, "SALINITY should be OFF (not in saved config)"