fix(kit): preserve off-catalog values in settings-list choice editors
This commit is contained in:
@@ -117,16 +117,44 @@ class SettingsDelegate(InstrumentDelegate):
|
||||
choices = index.data(CHOICES_ROLE)
|
||||
if choices:
|
||||
combo = QComboBox(parent)
|
||||
combo.addItems(list(choices))
|
||||
labels = list(choices)
|
||||
raws = [str(r) for r in (index.data(CHOICE_RAWS_ROLE) or [])]
|
||||
raw = index.data(RAW_ROLE)
|
||||
text = str(index.data() or "")
|
||||
if raw is not None and str(raw) not in raws:
|
||||
# Off-catalog device value (catalog/firmware drift on real
|
||||
# hardware): prepend it so it stays visible/selectable and an
|
||||
# open-then-click-away commit is a no-op — never a silent write
|
||||
# of the first catalog choice. Its raw is the value itself, so
|
||||
# re-committing it writes nothing new (see setModelData).
|
||||
labels.insert(0, text or str(raw))
|
||||
raws.insert(0, str(raw))
|
||||
elif raw is None and text and text not in labels:
|
||||
# RAW_ROLE not yet set but the display text is off-catalog.
|
||||
labels.insert(0, text)
|
||||
raws.insert(0, text)
|
||||
combo.addItems(labels)
|
||||
# The *effective* raw list (may include the off-catalog entry),
|
||||
# stashed on the editor so setEditorData/setModelData index into the
|
||||
# exact list this combo was built from — reading CHOICE_RAWS_ROLE
|
||||
# there would desync currentIndex from the prepended entry.
|
||||
combo._raws = raws
|
||||
return combo
|
||||
return super().createEditor(parent, option, index)
|
||||
|
||||
@staticmethod
|
||||
def _editor_raws(editor: QComboBox, index) -> list[str]:
|
||||
raws = getattr(editor, "_raws", None)
|
||||
if raws is None: # combo not built by createEditor above
|
||||
raws = [str(r) for r in (index.data(CHOICE_RAWS_ROLE) or [])]
|
||||
return raws
|
||||
|
||||
def setEditorData(self, editor, index) -> None: # noqa: N802 (Qt)
|
||||
if isinstance(editor, QComboBox):
|
||||
raws = index.data(CHOICE_RAWS_ROLE) or []
|
||||
raws = self._editor_raws(editor, index)
|
||||
raw = index.data(RAW_ROLE)
|
||||
if raw in raws:
|
||||
editor.setCurrentIndex(raws.index(raw))
|
||||
if raw is not None and str(raw) in raws:
|
||||
editor.setCurrentIndex(raws.index(str(raw)))
|
||||
else:
|
||||
# fall back to display-text match (e.g. RAW_ROLE not yet set)
|
||||
pos = editor.findText(str(index.data() or ""))
|
||||
@@ -137,7 +165,7 @@ class SettingsDelegate(InstrumentDelegate):
|
||||
|
||||
def setModelData(self, editor, model, index) -> None: # noqa: N802 (Qt)
|
||||
if isinstance(editor, QComboBox):
|
||||
raws = index.data(CHOICE_RAWS_ROLE) or []
|
||||
raws = self._editor_raws(editor, index)
|
||||
i = editor.currentIndex()
|
||||
raw = raws[i] if 0 <= i < len(raws) else editor.currentText()
|
||||
new_text = editor.currentText()
|
||||
@@ -147,12 +175,18 @@ class SettingsDelegate(InstrumentDelegate):
|
||||
# Stash the raw without signalling; we rely on itemChanged from
|
||||
# model.setData(text) below as the single emission. But when two
|
||||
# choices share the same display label Qt skips the signal (text
|
||||
# unchanged), so we emit itemChanged manually in that case.
|
||||
# unchanged), so we emit itemChanged manually — only when the raw
|
||||
# actually changed: committing an unchanged selection (e.g.
|
||||
# opening the editor on an off-catalog value and clicking away)
|
||||
# must not write anything to the hardware.
|
||||
prev = item.data(RAW_ROLE)
|
||||
was = view.blockSignals(True)
|
||||
item.setData(RAW_ROLE, raw)
|
||||
same_text = item.text() == new_text
|
||||
view.blockSignals(was)
|
||||
if same_text:
|
||||
if prev is not None and str(prev) == str(raw):
|
||||
return # nothing changed → no emission, no device write
|
||||
# Text won't change → Qt will swallow the signal; emit directly.
|
||||
view.itemChanged.emit(item)
|
||||
return
|
||||
|
||||
@@ -94,6 +94,48 @@ def test_duplicate_display_labels_do_not_alias(qtbot):
|
||||
assert edits == [("x", "0")] # exactly one emission, correct raw
|
||||
|
||||
|
||||
def test_off_catalog_value_displayed_and_commit_without_change_is_a_noop(qtbot):
|
||||
"""Hardware safety: catalog/firmware drift must never turn a click-away into a write."""
|
||||
sl = SettingsList()
|
||||
qtbot.addWidget(sl)
|
||||
sl.add_choice("mode", "Reporting mode", {"0": "Off", "1": "Hourly", "2": "Daily"})
|
||||
sl.set_value("mode", "7") # off-catalog device value
|
||||
item = sl.table.item(sl._rows["mode"].row, 1)
|
||||
assert item.text() == "7" # (a) visible, not silently lost
|
||||
|
||||
edits = []
|
||||
sl.settingEdited.connect(lambda k, v: edits.append((k, v)))
|
||||
index = sl.table.model().index(sl._rows["mode"].row, 1)
|
||||
editor = sl.delegate.createEditor(sl.table, None, index)
|
||||
sl.delegate.setEditorData(editor, index)
|
||||
# (b) the off-catalog entry is in the dropdown and is current — NOT catalog index 0
|
||||
assert [editor.itemText(i) for i in range(editor.count())] == [
|
||||
"7", "Off", "Hourly", "Daily"
|
||||
]
|
||||
assert editor.currentIndex() == 0
|
||||
assert editor.currentText() == "7"
|
||||
# (c) open editor → click away (commit without change): no emission, no write
|
||||
sl.delegate.setModelData(editor, sl.table.model(), index)
|
||||
assert edits == []
|
||||
assert sl.value("mode") == "7" # raw untouched
|
||||
|
||||
|
||||
def test_off_catalog_then_real_choice_emits_exactly_that_raw(qtbot):
|
||||
sl = SettingsList()
|
||||
qtbot.addWidget(sl)
|
||||
sl.add_choice("mode", "Reporting mode", {"0": "Off", "1": "Hourly", "2": "Daily"})
|
||||
sl.set_value("mode", "7") # off-catalog device value
|
||||
edits = []
|
||||
sl.settingEdited.connect(lambda k, v: edits.append((k, v)))
|
||||
index = sl.table.model().index(sl._rows["mode"].row, 1)
|
||||
editor = sl.delegate.createEditor(sl.table, None, index)
|
||||
sl.delegate.setEditorData(editor, index)
|
||||
editor.setCurrentIndex(2) # "Hourly" (shifted by the prepend)
|
||||
sl.delegate.setModelData(editor, sl.table.model(), index)
|
||||
assert edits == [("mode", "1")] # (d) one emission, the catalog raw
|
||||
assert sl.value("mode") == "1"
|
||||
|
||||
|
||||
def test_choice_cells_render_right_aligned_mono(qtbot):
|
||||
from cim_suite.core.ui.kit import KIND_ROLE, CellKind
|
||||
|
||||
|
||||
Reference in New Issue
Block a user