From 8c1cb4d54c31d4234f081b568e2e66eb8313b90a Mon Sep 17 00:00:00 2001 From: fdrgsp Date: Tue, 17 Mar 2026 15:25:29 -0400 Subject: [PATCH 1/6] feat: update wip --- .../hcwizard/config_wizard.py | 40 ++++----- src/pymmcore_widgets/hcwizard/devices_page.py | 90 +++++++++++++++++-- tests/test_config_wizard.py | 9 +- 3 files changed, 104 insertions(+), 35 deletions(-) diff --git a/src/pymmcore_widgets/hcwizard/config_wizard.py b/src/pymmcore_widgets/hcwizard/config_wizard.py index 5a270dc37..f8f16af59 100644 --- a/src/pymmcore_widgets/hcwizard/config_wizard.py +++ b/src/pymmcore_widgets/hcwizard/config_wizard.py @@ -1,5 +1,6 @@ from __future__ import annotations +import os from pathlib import Path from typing import TYPE_CHECKING @@ -7,7 +8,6 @@ from pymmcore_plus.model import Microscope from qtpy.QtCore import QSize from qtpy.QtWidgets import ( - QFileDialog, QLabel, QMessageBox, QVBoxLayout, @@ -49,6 +49,7 @@ def __init__( ): super().__init__(parent) self._core = core or CMMCorePlus.instance() + self._original_config = self._core.systemConfigurationFile() or "" self._model = Microscope() self._model.load_available_devices(self._core) self.setWizardStyle(QWizard.WizardStyle.ModernStyle) @@ -98,27 +99,14 @@ def closeEvent(self, event: QCloseEvent | None) -> None: if self._model.is_dirty(): answer = QMessageBox.question( self, - "Save changes?", - "Would you like to save your changes before exiting?", - QMessageBox.StandardButton.Save - | QMessageBox.StandardButton.Discard - | QMessageBox.StandardButton.Cancel, + "Discard changes?", + "You have unsaved changes. Discard and close?", + QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No, ) - if answer == QMessageBox.StandardButton.Cancel: + if answer == QMessageBox.StandardButton.No: event.ignore() return - elif answer == QMessageBox.StandardButton.Save: - (fname, _) = QFileDialog.getSaveFileName( - self, "Select Destination", "", "Config Files (*.cfg)" - ) - if fname: - self.setField(DEST_CONFIG, fname) - self.accept() - else: - event.ignore() - return - else: - self.reject() + self.reject() super().closeEvent(event) def accept(self) -> None: @@ -129,11 +117,17 @@ def accept(self) -> None: super().accept() def reject(self) -> None: - """Reject the wizard and reload the prior configuration.""" + """Reject the wizard and restore the prior core state.""" super().reject() - last_config_file = self._core.systemConfigurationFile() - if last_config_file is not None: - self._core.loadSystemConfiguration(last_config_file) + try: + self._core.unloadAllDevices() + except Exception: + pass + if self._original_config and os.path.isfile(self._original_config): + try: + self._core.loadSystemConfiguration(self._original_config) + except Exception: + pass def _update_step(self, current_index: int) -> None: """Change text on the left when the page changes.""" diff --git a/src/pymmcore_widgets/hcwizard/devices_page.py b/src/pymmcore_widgets/hcwizard/devices_page.py index bad40847e..561ae1add 100644 --- a/src/pymmcore_widgets/hcwizard/devices_page.py +++ b/src/pymmcore_widgets/hcwizard/devices_page.py @@ -224,6 +224,13 @@ def _edit_selected_device(self) -> None: ) idx = self._model.devices.index(device) self._model.devices[idx] = dev + + # If a hub was re-initialized, rediscover peripherals + if dev.device_type == DeviceType.Hub: + self._model.load_available_devices(self._core) + dlg2 = PeripheralSetupDlg(dev, self._model, self._core, self) + dlg2.exec() + self.rebuild_table() @@ -459,13 +466,82 @@ def __init__(self, model: Microscope, core: CMMCorePlus): def initializePage(self) -> None: """Called to prepare the page just before it is shown.""" - err = {} - # TODO: there are errors that occur outside of this call that could also be - # shown in the tooltip above... - self._model.initialize( - self._core, on_fail=lambda d, e: err.update({d.name: str(e)}) - ) + err = self._two_pass_initialize() self._model.mark_clean() self.current.rebuild_table(err) self.available.rebuild_table() - return + + def _two_pass_initialize(self) -> dict[str, str]: + """Initialize devices using a two-pass approach. + + Pass 1: Serial ports, then hub devices (discovering peripherals after + each hub). Pass 2: Remaining devices with parent labels set before + initialization. + """ + err: dict[str, str] = {} + seen: set[str] = set() + + serial_devs: list[Device] = [] + hub_devs: list[Device] = [] + other_devs: list[Device] = [] + + for device in (*self._model.assigned_com_ports, *self._model.devices): + if device.name in seen or device.device_type == DeviceType.Core: + continue + seen.add(device.name) + if device.device_type == DeviceType.Serial: + serial_devs.append(device) + elif device.device_type == DeviceType.Hub: + hub_devs.append(device) + else: + other_devs.append(device) + + # Pass 1a: Initialize serial ports + for device in serial_devs: + try: + device.initialize(self._core, reload=True, apply_pre_init=True) + except Exception as e: + err[device.name] = str(e) + + # Pass 1b: Initialize hub devices, discover peripherals after each + for device in hub_devs: + try: + device.initialize(self._core, reload=True, apply_pre_init=True) + except Exception as e: + err[device.name] = str(e) + # Reload available devices so hub children become discoverable + self._model.load_available_devices(self._core) + + # Pass 2: Initialize remaining devices with parent label set before init + for device in other_devs: + try: + device.load(self._core, reload=True) + if device.parent_label: + self._core.setParentLabel(device.name, device.parent_label) + for prop in device.properties: + if prop.is_pre_init: + prop.apply_to_core(self._core, then_update=False) + self._core.initializeDevice(device.name) + device.initialized = True + device.apply_to_core(self._core) + except Exception as e: + err[device.name] = str(e) + + return err + + def validatePage(self) -> bool: + """Validate the page when the user clicks Next. + + Refreshes parent hub references from core for devices that may have had + their parent set during hub initialization but not in the config file. + """ + for dev in self._model.devices: + if dev.name not in self._core.getLoadedDevices(): + continue + try: + parent = self._core.getParentLabel(dev.name) + if parent and not dev.parent_label: + dev.parent_label = parent + except RuntimeError: + pass + return super().validatePage() # type: ignore diff --git a/tests/test_config_wizard.py b/tests/test_config_wizard.py index bb4280545..f47235398 100644 --- a/tests/test_config_wizard.py +++ b/tests/test_config_wizard.py @@ -14,7 +14,6 @@ from pymmcore_widgets.hcwizard._peripheral_setup_dialog import PeripheralSetupDlg from pymmcore_widgets.hcwizard.config_wizard import ( ConfigWizard, - QFileDialog, QMessageBox, ) from pymmcore_widgets.hcwizard.finish_page import DEST_CONFIG @@ -67,12 +66,12 @@ def test_config_wizard(global_mmcore: CMMCorePlus, qtbot, tmp_path: Path): assert st1 == st2 wiz._model.devices.pop() + # Closing with unsaved changes and confirming discard should reject with patch.object( - QMessageBox, "question", lambda *_: QMessageBox.StandardButton.Save + QMessageBox, "question", lambda *_: QMessageBox.StandardButton.Yes ): - with patch.object(QFileDialog, "getSaveFileName", lambda *_: (str(out), "")): - with qtbot.waitSignal(wiz.accepted): - wiz.closeEvent(QCloseEvent()) + with qtbot.waitSignal(wiz.rejected): + wiz.closeEvent(QCloseEvent()) def test_config_wizard_rejection(global_mmcore: CMMCorePlus, qtbot, tmp_path: Path): From ecef7ba8d16dc5df105ce73a1a31ee80525dabae Mon Sep 17 00:00:00 2001 From: fdrgsp Date: Sat, 21 Mar 2026 12:30:16 -0400 Subject: [PATCH 2/6] feat: implement configuration cleanup and device reload in ConfigWizard --- .../hcwizard/config_wizard.py | 63 ++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/src/pymmcore_widgets/hcwizard/config_wizard.py b/src/pymmcore_widgets/hcwizard/config_wizard.py index f8f16af59..f1699feea 100644 --- a/src/pymmcore_widgets/hcwizard/config_wizard.py +++ b/src/pymmcore_widgets/hcwizard/config_wizard.py @@ -1,10 +1,11 @@ from __future__ import annotations +import logging import os from pathlib import Path from typing import TYPE_CHECKING -from pymmcore_plus import CMMCorePlus +from pymmcore_plus import CMMCorePlus, Keyword from pymmcore_plus.model import Microscope from qtpy.QtCore import QSize from qtpy.QtWidgets import ( @@ -25,6 +26,8 @@ if TYPE_CHECKING: from qtpy.QtGui import QCloseEvent +logger = logging.getLogger(__name__) + class ConfigWizard(QWizard): """Hardware Configuration Wizard for Micro-Manager. @@ -113,7 +116,25 @@ def accept(self) -> None: """Accept the wizard and save the configuration to a file.""" dest = self.field(DEST_CONFIG) dest_path = Path(dest) + + # Remove stale config entries referencing devices no longer in the model. + # Matches Java's MicroscopeModel.checkConfigurations(). + self._check_configurations() + self._model.save(dest_path) + + # Unload all devices and reload from the saved file so that the core + # state cleanly matches the file on disk. This matches the Java + # ConfigMenu.runHardwareWizard() post-wizard reload step. + try: + self._core.unloadAllDevices() + except Exception: + logger.exception("Failed to unload devices after save") + try: + self._core.loadSystemConfiguration(str(dest_path)) + except Exception: + logger.exception("Failed to reload saved configuration") + super().accept() def reject(self) -> None: @@ -129,6 +150,46 @@ def reject(self) -> None: except Exception: pass + def _check_configurations(self) -> None: + """Remove stale settings from config groups and pixel size presets. + + Mirrors Java MicroscopeModel.checkConfigurations(): for every config + group / pixel-size preset, drop any Setting whose device_name does not + match a device currently in the model. Empty presets and empty groups + are removed entirely. + """ + device_names = {d.name for d in self._model.devices} + device_names.add(Keyword.CoreDevice.value) + + # --- config groups --- + groups_to_remove: list[str] = [] + for group in self._model.config_groups.values(): + presets_to_remove: list[str] = [] + for preset in group.presets.values(): + preset.settings = [ + s for s in preset.settings if s.device_name in device_names + ] + if not preset.settings: + presets_to_remove.append(preset.name) + for name in presets_to_remove: + del group.presets[name] + if not group.presets: + groups_to_remove.append(group.name) + for name in groups_to_remove: + del self._model.config_groups[name] + + # --- pixel size presets --- + px = self._model.pixel_size_group + px_presets_to_remove: list[str] = [] + for preset in px.presets.values(): + preset.settings = [ + s for s in preset.settings if s.device_name in device_names + ] + if not preset.settings: + px_presets_to_remove.append(preset.name) + for name in px_presets_to_remove: + del px.presets[name] + def _update_step(self, current_index: int) -> None: """Change text on the left when the page changes.""" for i, label in enumerate(self.step_labels): From 8332ee08e75fac11e1178035bab5eeae74f790cc Mon Sep 17 00:00:00 2001 From: fdrgsp Date: Sat, 21 Mar 2026 17:02:40 -0400 Subject: [PATCH 3/6] feat: read state labels + focus directions from hardware --- src/pymmcore_widgets/hcwizard/labels_page.py | 30 +++++++ src/pymmcore_widgets/hcwizard/roles_page.py | 90 +++++++++++++++++--- 2 files changed, 109 insertions(+), 11 deletions(-) diff --git a/src/pymmcore_widgets/hcwizard/labels_page.py b/src/pymmcore_widgets/hcwizard/labels_page.py index 2b1ca7ba5..3056c85cb 100644 --- a/src/pymmcore_widgets/hcwizard/labels_page.py +++ b/src/pymmcore_widgets/hcwizard/labels_page.py @@ -1,3 +1,4 @@ +import logging from typing import cast from pymmcore_plus import CMMCorePlus, DeviceType @@ -7,6 +8,7 @@ QComboBox, QHBoxLayout, QLabel, + QPushButton, QTableWidget, QTableWidgetItem, QVBoxLayout, @@ -15,6 +17,8 @@ from ._base_page import ConfigWizardPage +logger = logging.getLogger(__name__) + class _LabelTable(QTableWidget): def __init__(self, model: Microscope): @@ -71,9 +75,16 @@ def __init__(self, model: Microscope, core: CMMCorePlus): self.dev_combo = QComboBox() self.dev_combo.currentTextChanged.connect(self.labels_table.rebuild) + self._read_hw_btn = QPushButton("Read from Hardware") + self._read_hw_btn.setToolTip( + "Read the current state labels from the hardware device" + ) + self._read_hw_btn.clicked.connect(self._read_labels_from_hardware) + row = QHBoxLayout() row.addWidget(QLabel("Device:")) row.addWidget(self.dev_combo, 1) + row.addWidget(self._read_hw_btn) layout = QVBoxLayout(self) layout.addLayout(row) @@ -93,3 +104,22 @@ def initializePage(self) -> None: self.labels_table.rebuild(self.dev_combo.currentText()) super().initializePage() + + def _read_labels_from_hardware(self) -> None: + """Read state labels from the hardware device and update the table.""" + dev_name = self.dev_combo.currentText() + if not dev_name: + return + + try: + hw_labels = self._core.getStateLabels(dev_name) + except Exception: + logger.exception("Failed to read labels from hardware for %s", dev_name) + return + + dev = self._model.get_device(dev_name) + for i, label in enumerate(hw_labels): + if i < len(dev.labels): + dev.set_label(i, label) + + self.labels_table.rebuild(dev_name) diff --git a/src/pymmcore_widgets/hcwizard/roles_page.py b/src/pymmcore_widgets/hcwizard/roles_page.py index a18161bc5..d2b9297f5 100644 --- a/src/pymmcore_widgets/hcwizard/roles_page.py +++ b/src/pymmcore_widgets/hcwizard/roles_page.py @@ -1,10 +1,24 @@ -from pymmcore_plus import CMMCorePlus, DeviceType, Keyword +from pymmcore_plus import CMMCorePlus, DeviceType, FocusDirection, Keyword from pymmcore_plus.model import Microscope -from qtpy.QtWidgets import QCheckBox, QComboBox, QFormLayout +from qtpy.QtWidgets import ( + QCheckBox, + QComboBox, + QFormLayout, + QGroupBox, + QScrollArea, + QVBoxLayout, +) from superqt.utils import signals_blocked from ._base_page import ConfigWizardPage +_FOCUS_DIR_OPTIONS = ["Unknown", "Toward Sample", "Away From Sample"] +_FOCUS_DIR_VALUES = [ + FocusDirection.Unknown, + FocusDirection.TowardSample, + FocusDirection.AwayFromSample, +] + class RolesPage(ConfigWizardPage): """Page for selecting default devices and auto-shutter setting.""" @@ -24,18 +38,36 @@ def __init__(self, model: Microscope, core: CMMCorePlus): self.auto_shutter_checkbox = QCheckBox() self.auto_shutter_checkbox.stateChanged.connect(self._on_auto_shutter_changed) - # TODO: focus directions - layout = QFormLayout(self) - layout.setFieldGrowthPolicy(QFormLayout.FieldGrowthPolicy.AllNonFixedFieldsGrow) - layout.addRow("Default Camera", self.camera_combo) - layout.addRow("Default Shutter", self.shutter_combo) - layout.addRow("Default Focus Stage", self.focus_combo) - layout.addRow("Use auto-shutter", self.auto_shutter_checkbox) + # focus direction combos, keyed by stage device name + self._focus_dir_combos: dict[str, QComboBox] = {} + + layout = QVBoxLayout(self) + + roles_form = QFormLayout() + roles_form.setFieldGrowthPolicy( + QFormLayout.FieldGrowthPolicy.AllNonFixedFieldsGrow + ) + roles_form.addRow("Default Camera", self.camera_combo) + roles_form.addRow("Default Shutter", self.shutter_combo) + roles_form.addRow("Default Focus Stage", self.focus_combo) + roles_form.addRow("Use auto-shutter", self.auto_shutter_checkbox) + layout.addLayout(roles_form) + + # focus direction section + self._focus_dir_group = QGroupBox("Stage Focus Directions") + self._focus_dir_layout = QFormLayout() + self._focus_dir_layout.setFieldGrowthPolicy( + QFormLayout.FieldGrowthPolicy.AllNonFixedFieldsGrow + ) + self._focus_dir_group.setLayout(self._focus_dir_layout) + + scroll = QScrollArea() + scroll.setWidgetResizable(True) + scroll.setWidget(self._focus_dir_group) + layout.addWidget(scroll) def initializePage(self) -> None: """Called to prepare the page just before it is shown.""" - # try/catch - # reset and populate the combo boxes with available devices with signals_blocked(self.camera_combo): self.camera_combo.clear() @@ -84,6 +116,9 @@ def initializePage(self) -> None: if stages and not self.focus_combo.currentText(): self.focus_combo.setCurrentText(stages[0]) + # rebuild focus direction combos for all stage devices + self._rebuild_focus_directions() + super().initializePage() def _on_camera_changed(self, text: str) -> None: @@ -98,3 +133,36 @@ def _on_focus_changed(self, text: str) -> None: def _on_auto_shutter_changed(self, state: int) -> None: val = "1" if bool(state) else "0" self._model.core_device.set_property(Keyword.CoreAutoShutter, val) + + # --- focus direction --- + + def _rebuild_focus_directions(self) -> None: + """Rebuild the focus direction combo boxes for all stage devices.""" + # clear existing combos + self._focus_dir_combos.clear() + while self._focus_dir_layout.rowCount(): + self._focus_dir_layout.removeRow(0) + + stage_devices = list(self._model.filter_devices(device_type=DeviceType.Stage)) + self._focus_dir_group.setVisible(bool(stage_devices)) + + for dev in stage_devices: + combo = QComboBox() + combo.addItems(_FOCUS_DIR_OPTIONS) + # read current direction from model + try: + idx = _FOCUS_DIR_VALUES.index(dev.focus_direction) + except ValueError: + idx = 0 + combo.setCurrentIndex(idx) + combo.currentIndexChanged.connect( + lambda index, d=dev: self._on_focus_direction_changed(d.name, index) + ) + self._focus_dir_combos[dev.name] = combo + self._focus_dir_layout.addRow(dev.name, combo) + + def _on_focus_direction_changed(self, dev_name: str, index: int) -> None: + direction = _FOCUS_DIR_VALUES[index] + dev = self._model.get_device(dev_name) + dev.focus_direction = direction + self._core.setFocusDirection(dev_name, direction) From e68072a6207bedf4c2982701e3ec4e2f2ee8ca36 Mon Sep 17 00:00:00 2001 From: fdrgsp Date: Sat, 21 Mar 2026 18:45:47 -0400 Subject: [PATCH 4/6] add tests --- .../hcwizard/config_wizard.py | 8 +- src/pymmcore_widgets/hcwizard/labels_page.py | 2 +- src/pymmcore_widgets/hcwizard/roles_page.py | 2 +- tests/test_config_wizard.py | 294 +++++++++++++++++- 4 files changed, 299 insertions(+), 7 deletions(-) diff --git a/src/pymmcore_widgets/hcwizard/config_wizard.py b/src/pymmcore_widgets/hcwizard/config_wizard.py index f1699feea..46bfa4164 100644 --- a/src/pymmcore_widgets/hcwizard/config_wizard.py +++ b/src/pymmcore_widgets/hcwizard/config_wizard.py @@ -128,11 +128,11 @@ def accept(self) -> None: # ConfigMenu.runHardwareWizard() post-wizard reload step. try: self._core.unloadAllDevices() - except Exception: + except Exception: # pragma: no cover logger.exception("Failed to unload devices after save") try: self._core.loadSystemConfiguration(str(dest_path)) - except Exception: + except Exception: # pragma: no cover logger.exception("Failed to reload saved configuration") super().accept() @@ -142,12 +142,12 @@ def reject(self) -> None: super().reject() try: self._core.unloadAllDevices() - except Exception: + except Exception: # pragma: no cover pass if self._original_config and os.path.isfile(self._original_config): try: self._core.loadSystemConfiguration(self._original_config) - except Exception: + except Exception: # pragma: no cover pass def _check_configurations(self) -> None: diff --git a/src/pymmcore_widgets/hcwizard/labels_page.py b/src/pymmcore_widgets/hcwizard/labels_page.py index 3056c85cb..c55cc8ff8 100644 --- a/src/pymmcore_widgets/hcwizard/labels_page.py +++ b/src/pymmcore_widgets/hcwizard/labels_page.py @@ -113,7 +113,7 @@ def _read_labels_from_hardware(self) -> None: try: hw_labels = self._core.getStateLabels(dev_name) - except Exception: + except Exception: # pragma: no cover logger.exception("Failed to read labels from hardware for %s", dev_name) return diff --git a/src/pymmcore_widgets/hcwizard/roles_page.py b/src/pymmcore_widgets/hcwizard/roles_page.py index d2b9297f5..7c307f0f3 100644 --- a/src/pymmcore_widgets/hcwizard/roles_page.py +++ b/src/pymmcore_widgets/hcwizard/roles_page.py @@ -152,7 +152,7 @@ def _rebuild_focus_directions(self) -> None: # read current direction from model try: idx = _FOCUS_DIR_VALUES.index(dev.focus_direction) - except ValueError: + except ValueError: # pragma: no cover idx = 0 combo.setCurrentIndex(idx) combo.currentIndexChanged.connect( diff --git a/tests/test_config_wizard.py b/tests/test_config_wizard.py index f47235398..e43f50fc9 100644 --- a/tests/test_config_wizard.py +++ b/tests/test_config_wizard.py @@ -5,7 +5,9 @@ from unittest.mock import patch import pytest -from pymmcore_plus.model import Microscope +from pymmcore_plus import FocusDirection, Keyword +from pymmcore_plus.model import Microscope, Setting +from pymmcore_plus.model._config_group import ConfigGroup, ConfigPreset from qtpy.QtCore import Qt from qtpy.QtGui import QCloseEvent, QFocusEvent @@ -17,6 +19,8 @@ QMessageBox, ) from pymmcore_widgets.hcwizard.finish_page import DEST_CONFIG +from pymmcore_widgets.hcwizard.labels_page import LabelsPage +from pymmcore_widgets.hcwizard.roles_page import RolesPage if TYPE_CHECKING: from pymmcore_plus import CMMCorePlus @@ -178,3 +182,291 @@ def test_peripheral_setup_dialog(qtbot, global_mmcore: CMMCorePlus): assert list(dlg.selectedPeripherals()) dlg.accept() assert "SomeDevice" in global_mmcore.getLoadedDevices() + + +# ---- _check_configurations ---- + + +def _make_wizard_with_loaded_config( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> ConfigWizard: + """Create a wizard and navigate past intro to load the config into the model.""" + wiz = ConfigWizard(str(TEST_CONFIG), global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + wiz.next() # loads config into model via IntroPage + return wiz + + +def test_check_configurations_removes_stale_settings( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """Stale settings referencing removed devices are cleaned up.""" + wiz = _make_wizard_with_loaded_config(global_mmcore, qtbot) + + stale = Setting( + device_name="NonExistentDevice", property_name="Foo", property_value="Bar" + ) + valid = Setting(device_name="Camera", property_name="Binning", property_value="1") + group = ConfigGroup(name="TestGroup") + group.presets["mixed"] = ConfigPreset(name="mixed", settings=[valid, stale]) + group.presets["all_stale"] = ConfigPreset(name="all_stale", settings=[stale]) + wiz._model.config_groups["TestGroup"] = group + + wiz._check_configurations() + + # "mixed" keeps only the valid setting; "all_stale" is removed + assert "mixed" in wiz._model.config_groups["TestGroup"].presets + mixed = wiz._model.config_groups["TestGroup"].presets["mixed"] + assert all(s.device_name != "NonExistentDevice" for s in mixed.settings) + assert "all_stale" not in wiz._model.config_groups["TestGroup"].presets + + +def test_check_configurations_removes_empty_groups( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """Groups left with no presets are removed entirely.""" + wiz = _make_wizard_with_loaded_config(global_mmcore, qtbot) + + stale = Setting(device_name="GoneDevice", property_name="X", property_value="1") + group = ConfigGroup(name="EmptyGroup") + group.presets["only"] = ConfigPreset(name="only", settings=[stale]) + wiz._model.config_groups["EmptyGroup"] = group + + wiz._check_configurations() + + assert "EmptyGroup" not in wiz._model.config_groups + + +def test_check_configurations_cleans_pixel_size_presets( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """Pixel size presets with stale device refs are cleaned up.""" + wiz = _make_wizard_with_loaded_config(global_mmcore, qtbot) + + stale = Setting(device_name="RemovedDev", property_name="Label", property_value="X") + px = wiz._model.pixel_size_group + px.presets["stale_res"] = ConfigPreset(name="stale_res", settings=[stale]) + + wiz._check_configurations() + + assert "stale_res" not in px.presets + + +def test_check_configurations_keeps_core_device_settings( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """Settings referencing the Core device are preserved.""" + wiz = _make_wizard_with_loaded_config(global_mmcore, qtbot) + + core_setting = Setting( + device_name=Keyword.CoreDevice.value, + property_name="ChannelGroup", + property_value="Channel", + ) + group = ConfigGroup(name="CoreGroup") + group.presets["p"] = ConfigPreset(name="p", settings=[core_setting]) + wiz._model.config_groups["CoreGroup"] = group + + wiz._check_configurations() + + assert "CoreGroup" in wiz._model.config_groups + assert len(wiz._model.config_groups["CoreGroup"].presets["p"].settings) == 1 + + +# ---- accept / reject / closeEvent ---- + + +def test_accept_saves_and_reloads( + global_mmcore: CMMCorePlus, qtbot: QtBot, tmp_path: Path +) -> None: + """Accept saves the config, unloads, then reloads from file.""" + wiz = ConfigWizard(str(TEST_CONFIG), global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + for _ in range(5): + wiz.next() + + out = tmp_path / "out.cfg" + wiz.setField(DEST_CONFIG, str(out)) + wiz.accept() + + assert out.exists() + # Core should have been reloaded from the saved file + assert global_mmcore.systemConfigurationFile() == str(out) + + +def test_close_event_no_changes_does_not_prompt( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """Closing when clean should not show a dialog.""" + wiz = ConfigWizard("", core=global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + + with patch.object(QMessageBox, "question") as mock_q: + wiz.closeEvent(QCloseEvent()) + mock_q.assert_not_called() + + +def test_close_event_cancel_discard(global_mmcore: CMMCorePlus, qtbot: QtBot) -> None: + """Declining discard on close should ignore the event.""" + wiz = _make_wizard_with_loaded_config(global_mmcore, qtbot) + wiz._model.devices.pop() # make dirty + + event = QCloseEvent() + with patch.object( + QMessageBox, "question", lambda *_: QMessageBox.StandardButton.No + ): + wiz.closeEvent(event) + assert not event.isAccepted() + + +# ---- RolesPage focus directions ---- + + +def test_roles_page_focus_directions(global_mmcore: CMMCorePlus, qtbot: QtBot) -> None: + """Focus direction combos are built for stage devices and update model.""" + wiz = ConfigWizard(str(TEST_CONFIG), global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + + roles_page = wiz.page(2) + assert isinstance(roles_page, RolesPage) + + # Navigate to the roles page + wiz.next() # devices + wiz.next() # roles + + # Should have combos for stage devices (Z and Z1 from test config) + assert roles_page._focus_dir_group.isVisible() + assert len(roles_page._focus_dir_combos) >= 1 + + # Change a focus direction and verify it updates the model + stage_name = next(iter(roles_page._focus_dir_combos)) + combo = roles_page._focus_dir_combos[stage_name] + combo.setCurrentIndex(1) # "Toward Sample" + + dev = wiz._model.get_device(stage_name) + assert dev.focus_direction == FocusDirection.TowardSample + + combo.setCurrentIndex(2) # "Away From Sample" + assert dev.focus_direction == FocusDirection.AwayFromSample + + +def test_roles_page_no_stages_hides_group( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """Focus direction group is hidden when no stages exist.""" + global_mmcore.loadSystemConfiguration() + wiz = ConfigWizard("", core=global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + + roles_page = wiz.page(2) + assert isinstance(roles_page, RolesPage) + + wiz.next() # devices (empty) + wiz.next() # roles + + assert not roles_page._focus_dir_group.isVisible() + assert len(roles_page._focus_dir_combos) == 0 + + +# ---- LabelsPage read from hardware ---- + + +def test_labels_page_read_from_hardware( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """Read from Hardware button updates labels from core state.""" + wiz = ConfigWizard(str(TEST_CONFIG), global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + + labels_page = wiz.page(4) + assert isinstance(labels_page, LabelsPage) + + # Navigate to labels page + for _ in range(4): + wiz.next() + + # Should have state devices in the combo + assert labels_page.dev_combo.count() > 0 + dev_name = labels_page.dev_combo.currentText() + assert dev_name + + # Modify a label in the model to differ from hardware + dev = wiz._model.get_device(dev_name) + dev.set_label(0, "MODIFIED_LABEL") + assert dev.labels[0] == "MODIFIED_LABEL" + + # Click "Read from Hardware" to restore from core + labels_page._read_labels_from_hardware() + + # The label should now match what the hardware reports + hw_labels = global_mmcore.getStateLabels(dev_name) + assert dev.labels[0] == hw_labels[0] + + +def test_labels_page_read_from_hardware_no_device( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """Read from Hardware does nothing when no device is selected.""" + global_mmcore.loadSystemConfiguration() + wiz = ConfigWizard("", core=global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + + labels_page = wiz.page(4) + assert isinstance(labels_page, LabelsPage) + + # No devices loaded, combo should be empty + for _ in range(4): + wiz.next() + + assert labels_page.dev_combo.currentText() == "" + # Should not raise + labels_page._read_labels_from_hardware() + + +# ---- DevicesPage two-pass initialize & validatePage ---- + + +def test_two_pass_initialize(global_mmcore: CMMCorePlus, qtbot: QtBot) -> None: + """Two-pass initialization loads config and initializes devices.""" + wiz = ConfigWizard(str(TEST_CONFIG), global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + wiz.next() # triggers initializePage on DevicesPage + + dev_page = wiz.page(1) + assert isinstance(dev_page, devices_page.DevicesPage) + + # Devices should be populated after initialization + assert len(dev_page._model.devices) > 0 + # Verify table was built + assert dev_page.current.table.rowCount() > 0 + + +def test_validate_page_syncs_parent_labels( + global_mmcore: CMMCorePlus, qtbot: QtBot +) -> None: + """validatePage should sync parent labels from core to model.""" + wiz = ConfigWizard(str(TEST_CONFIG), global_mmcore) + qtbot.addWidget(wiz) + wiz.show() + wiz.next() # DevicesPage init + + dev_page = wiz.page(1) + assert isinstance(dev_page, devices_page.DevicesPage) + + # Clear a parent_label in the model but leave the core reference intact + camera_dev = wiz._model.get_device("Camera") + assert camera_dev.parent_label # should be "DHub" from config + camera_dev.parent_label = "" + + dev_page.validatePage() + + # Should have been restored from core + assert camera_dev.parent_label == "DHub" From 0f2a9f33c9880cb10de82d78deeeb54a2734c89a Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Tue, 24 Mar 2026 16:28:31 -0400 Subject: [PATCH 5/6] fix: update device visibility handling in ComTable and adjust PropTable height calculation --- src/pymmcore_widgets/hcwizard/_dev_setup_dialog.py | 2 ++ .../hcwizard/_simple_prop_table.py | 14 ++++---------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/pymmcore_widgets/hcwizard/_dev_setup_dialog.py b/src/pymmcore_widgets/hcwizard/_dev_setup_dialog.py index 375c6d7b5..d937bbe71 100644 --- a/src/pymmcore_widgets/hcwizard/_dev_setup_dialog.py +++ b/src/pymmcore_widgets/hcwizard/_dev_setup_dialog.py @@ -362,9 +362,11 @@ def rebuild_port(self, port_dev_name: str, port_library_name: str = "") -> None: self._port_dev_name = port_dev_name if port_dev_name not in self._core.getLoadedDevices(): if not port_library_name: + self.hide() return self._core.loadDevice(port_dev_name, port_library_name, port_dev_name) prop_names = self._core.getDevicePropertyNames(port_dev_name) + self.show() return super().rebuild([(port_dev_name, p) for p in prop_names]) diff --git a/src/pymmcore_widgets/hcwizard/_simple_prop_table.py b/src/pymmcore_widgets/hcwizard/_simple_prop_table.py index 427efd3ef..432747ed4 100644 --- a/src/pymmcore_widgets/hcwizard/_simple_prop_table.py +++ b/src/pymmcore_widgets/hcwizard/_simple_prop_table.py @@ -5,7 +5,7 @@ from typing import TYPE_CHECKING from pymmcore_plus import CMMCorePlus, Keyword -from qtpy.QtCore import QSize, Signal +from qtpy.QtCore import Signal from qtpy.QtWidgets import QComboBox, QTableWidget, QTableWidgetItem, QWidget from pymmcore_widgets.device_properties._property_widget import PropertyWidget @@ -54,15 +54,6 @@ def __init__(self, core: CMMCorePlus, parent: QWidget | None = None) -> None: self.verticalHeader().setDefaultSectionSize(24) self.setColumnWidth(0, 200) - def sizeHint(self) -> QSize: - """Return a size hint that accounts for the number of rows.""" - hint = super().sizeHint() - if self.rowCount() > 0: - header_h = self.horizontalHeader().height() - rows_h = sum(self.rowHeight(r) for r in range(self.rowCount())) - hint.setHeight(header_h + rows_h + 2) - return hint - def iterRows(self) -> Iterator[tuple[str, str]]: """Iterate over rows, yielding (prop_name, prop_value).""" for r in range(self.rowCount()): @@ -98,3 +89,6 @@ def rebuild( device, prop_name, mmcore=self._core, connect_core=False ) self.setCellWidget(i, 1, wdg) + header_h = self.horizontalHeader().height() + rows_h = self.verticalHeader().defaultSectionSize() * self.rowCount() + self.setFixedHeight(header_h + rows_h + 2) From e61ac2dfdd5eaacdf94b5fb8e8ddf14b9b25bf8a Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 25 Mar 2026 09:20:40 -0400 Subject: [PATCH 6/6] fix wizard load and init --- src/pymmcore_widgets/hcwizard/devices_page.py | 73 +++++++++++++++---- 1 file changed, 58 insertions(+), 15 deletions(-) diff --git a/src/pymmcore_widgets/hcwizard/devices_page.py b/src/pymmcore_widgets/hcwizard/devices_page.py index 561ae1add..970ed54fd 100644 --- a/src/pymmcore_widgets/hcwizard/devices_page.py +++ b/src/pymmcore_widgets/hcwizard/devices_page.py @@ -474,21 +474,55 @@ def initializePage(self) -> None: def _two_pass_initialize(self) -> dict[str, str]: """Initialize devices using a two-pass approach. - Pass 1: Serial ports, then hub devices (discovering peripherals after - each hub). Pass 2: Remaining devices with parent labels set before - initialization. + First load ALL devices into core (without initializing), read their + actual device types from core, then initialize in the correct order: + serial ports -> hubs (with peripheral discovery) -> remaining devices. """ err: dict[str, str] = {} - seen: set[str] = set() + # --- Load phase --- + # Load all devices into core without initializing so we can query + # their real device types. Also load any assigned COM ports that + # are not already in the device list (matching Java's loading of + # all available serial ports). + loaded: set[str] = set() + for device in self._model.devices: + if device.device_type == DeviceType.Core: + continue + try: + device.load(self._core, reload=True) + loaded.add(device.name) + except Exception as e: + err[device.name] = str(e) + + # Set parent labels (requires all devices to be loaded) + for device in self._model.devices: + if device.name not in loaded: + continue + if device.parent_label: + try: + self._core.setParentLabel(device.name, device.parent_label) + except Exception: + pass + + # Read actual device types from core so we can categorise correctly. + # Config-file parsing leaves most devices as DeviceType.Any. + for device in self._model.devices: + if device.name not in loaded: + continue + try: + device.device_type = self._core.getDeviceType(device.name) + except Exception: + pass + + # --- Categorise phase --- serial_devs: list[Device] = [] hub_devs: list[Device] = [] other_devs: list[Device] = [] - for device in (*self._model.assigned_com_ports, *self._model.devices): - if device.name in seen or device.device_type == DeviceType.Core: + for device in self._model.devices: + if device.device_type == DeviceType.Core or device.name not in loaded: continue - seen.add(device.name) if device.device_type == DeviceType.Serial: serial_devs.append(device) elif device.device_type == DeviceType.Hub: @@ -496,28 +530,37 @@ def _two_pass_initialize(self) -> dict[str, str]: else: other_devs.append(device) - # Pass 1a: Initialize serial ports + # --- Initialize phase --- + + # Phase 1a: Initialize serial ports for device in serial_devs: try: - device.initialize(self._core, reload=True, apply_pre_init=True) + for prop in device.properties: + if prop.is_pre_init: + prop.apply_to_core(self._core, then_update=False) + self._core.initializeDevice(device.name) + device.initialized = True + device.apply_to_core(self._core) except Exception as e: err[device.name] = str(e) - # Pass 1b: Initialize hub devices, discover peripherals after each + # Phase 1b: Initialize hub devices, discover peripherals after each for device in hub_devs: try: - device.initialize(self._core, reload=True, apply_pre_init=True) + for prop in device.properties: + if prop.is_pre_init: + prop.apply_to_core(self._core, then_update=False) + self._core.initializeDevice(device.name) + device.initialized = True + device.apply_to_core(self._core) except Exception as e: err[device.name] = str(e) # Reload available devices so hub children become discoverable self._model.load_available_devices(self._core) - # Pass 2: Initialize remaining devices with parent label set before init + # Phase 2: Initialize remaining devices for device in other_devs: try: - device.load(self._core, reload=True) - if device.parent_label: - self._core.setParentLabel(device.name, device.parent_label) for prop in device.properties: if prop.is_pre_init: prop.apply_to_core(self._core, then_update=False)