pymmcore-plus / pymmcore-widgets

A set of Qt-based widgets onto the pymmcore-plus model
https://pymmcore-plus.github.io/pymmcore-widgets
Other
12 stars 7 forks source link

Device name with "-" crashes GroupPresetWidget #363

Closed wl-stepp closed 1 month ago

wl-stepp commented 1 month ago

Description

Trying to update a PresetConfig from the Widget by added another Preset to Channel. Crashes because one of our devices is called "pE-800" and the "-" makes the widget think it's called "pE" only. This works in the Java version however. I think there might be a step where the "-" is used to separate device and setting?

What I Did

We can replicate with a minimal system:

from pymmcore_widgets import GroupPresetTableWidget
from pymmcore_plus import CMMCorePlus
from qtpy.QtWidgets import QApplication

app = QApplication([])
mmcore = CMMCorePlus()
mmcore.loadSystemConfiguration('./MMConfig_error.cfg')

group_presets = GroupPresetTableWidget(mmcore=mmcore)
group_presets.show()

app.exec_()

Error when trying to change a preset in ErrorChannel, config attached.

Traceback (most recent call last):
  File "C:\Control_2\pymmcore-widgets\src\pymmcore_widgets\_group_preset_widget\_group_preset_table_widget.py", line 227, in _on_new_group_preset
    self._populate_table()
  File "C:\Control_2\pymmcore-widgets\src\pymmcore_widgets\_group_preset_widget\_group_preset_table_widget.py", line 246, in _populate_table
    wdg = self._create_group_widget(group)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Control_2\pymmcore-widgets\src\pymmcore_widgets\_group_preset_widget\_group_preset_table_widget.py", line 281, in _create_group_widget
    return PropertyWidget(device, prop)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Control_2\pymmcore-widgets\src\pymmcore_widgets\_property_widget.py", line 312, in __init__
    raise ValueError(f"Device not loaded: {device_label!r}")
ValueError: Device not loaded: 'DCam'

MMConfig_error.zip

tlambert03 commented 1 month ago

good catch and thanks for the issue. this should be easy to fix

tlambert03 commented 1 month ago

this is fixed now. side note: there's a lot to be desired in the ConfigGroup editing widgets. The current ones are mostly a direct porting of the MMStudio interface (and they were made early in the process and contain a lot of code duplication). So some variant of #307 or #338 will likely replace these widgets in the future.