fix(iomodbus): own Export menu via shared engine; fixes dialog teardown crash
2a39bfe folded the xlsx exports into the Catalog menu with hand-rolled handlers
(collect_sheet/collect_all/save_sheets_dialog + bound-method connections),
which broke two things:
1. Teardown crash (tests/iomodbus/test_ui_smoke.py::test_calibration_dialog_
computes): pytest-qt holds only weakrefs to registered widgets. The shared
add_export_actions closures (capturing parent) were the only strong,
C++-anchored reference keeping the MainWindow's Python wrapper alive past
the test frame; with only weakly-stored bound-method slots left, the window
wrapper died by refcount at function exit, shiboken deleted the ownerless
C++ QMainWindow, and the cascade deleted the parented CalibrationDialog's
C++ object - while the dialog's wrapper survived in qframelesswindow's
windowEffect reference cycle, so qtbot teardown closed a dead C++ object
(RuntimeError: Internal C++ object already deleted). Verified by weakref/
shiboken6.isValid experiments with and without a window-anchoring closure.
2. Wrong menu: spreadsheet export of live grid data is not a device-catalog
(IOModbus.txt) file operation.
Rework per the Instrument spec: data export gets its own "Export" QToolButton
menu between the Refresh/poll group and the Catalog menu (separator-delimited),
built by the shared add_export_menu_actions engine - restoring the empty-export
notice and the window-anchoring closures. The Catalog menu is back to Import
Devices / Export Devices / Supported Devices. Both menus keep tooltips visible.
Toolbar sizeHint: 1190px themed offscreen (1019px untheme) vs 1240px budget.
Tests: tests/iomodbus 145 passed; full suite 1012 passed, zero errors.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -34,11 +34,7 @@ from PySide6.QtWidgets import (
|
||||
|
||||
from cim_suite.core.ui.chrome import ChromeDialog, StatusFooter
|
||||
from cim_suite.core.ui.copy_menu import enable_copy_in
|
||||
from cim_suite.core.ui.export_action import (
|
||||
collect_all,
|
||||
collect_sheet,
|
||||
save_sheets_dialog,
|
||||
)
|
||||
from cim_suite.core.ui.export_action import add_export_menu_actions
|
||||
from cim_suite.core.ui.kit import ConnectionChip, InstrumentTabWidget
|
||||
|
||||
from .. import config
|
||||
@@ -313,8 +309,28 @@ class MainWindow(QMainWindow):
|
||||
tb.addAction(self.act_log)
|
||||
|
||||
tb.addSeparator()
|
||||
# The legacy &Catalog menubar folds into the toolbar (§5.2: single row).
|
||||
# Data-export actions also live here (file-level operations; keeps the toolbar narrow).
|
||||
# Data export (.xlsx) — the shared engine pair, behind an "Export ▾" menu
|
||||
# button instead of two plain toolbar buttons (BL-DS5: with IBM Plex loaded
|
||||
# the flat buttons push sizeHint past the 1240px budget).
|
||||
export_btn = QToolButton(tb)
|
||||
export_btn.setText("Export ▾")
|
||||
self.export_menu = QMenu(export_btn)
|
||||
self.export_menu.setToolTipsVisible(True) # QMenu hides action tooltips by default
|
||||
self.act_export_tab, self.act_export_all = add_export_menu_actions(
|
||||
self.export_menu, self.tabs, self, self._export_metadata,
|
||||
file_prefix="IOModbus",
|
||||
last_dir=config.export_dir,
|
||||
remember_dir=config.set_export_dir,
|
||||
)
|
||||
self.act_export_tab.setToolTip("Export the active data tab to an Excel workbook.")
|
||||
self.act_export_all.setToolTip("Export all data tabs to a single Excel workbook.")
|
||||
export_btn.setMenu(self.export_menu)
|
||||
export_btn.setPopupMode(QToolButton.ToolButtonPopupMode.InstantPopup)
|
||||
tb.addWidget(export_btn)
|
||||
|
||||
tb.addSeparator()
|
||||
# The legacy &Catalog menubar folds into the toolbar (§5.2: single row) —
|
||||
# device-catalog file operations (IOModbus.txt) only.
|
||||
menu_btn = QToolButton(tb)
|
||||
menu_btn.setText("Catalog ▾")
|
||||
self.catalog_menu = QMenu(menu_btn)
|
||||
@@ -328,14 +344,6 @@ class MainWindow(QMainWindow):
|
||||
self.catalog_menu.addSeparator()
|
||||
self.act_supported = self.catalog_menu.addAction("Supported Devices…")
|
||||
self.act_supported.triggered.connect(self._show_supported_devices)
|
||||
self.catalog_menu.addSeparator()
|
||||
# Export data — placed here rather than as separate toolbar buttons (BL-DS5: toolbar fit).
|
||||
self.act_export_tab = self.catalog_menu.addAction("Export This Tab…")
|
||||
self.act_export_tab.setToolTip("Export the active data tab to an Excel workbook.")
|
||||
self.act_export_tab.triggered.connect(self._export_this_tab)
|
||||
self.act_export_all = self.catalog_menu.addAction("Export All Tabs…")
|
||||
self.act_export_all.setToolTip("Export all data tabs to a single Excel workbook.")
|
||||
self.act_export_all.triggered.connect(self._export_all_tabs)
|
||||
menu_btn.setMenu(self.catalog_menu)
|
||||
menu_btn.setPopupMode(QToolButton.ToolButtonPopupMode.InstantPopup)
|
||||
tb.addWidget(menu_btn)
|
||||
@@ -379,24 +387,6 @@ class MainWindow(QMainWindow):
|
||||
if self._connector is not None:
|
||||
self._connector(dlg.selected_port(), dlg.selected_baud())
|
||||
|
||||
def _export_this_tab(self) -> None:
|
||||
meta = self._export_metadata()
|
||||
idx = self.tabs.currentIndex()
|
||||
sheet = collect_sheet(self.tabs, idx, meta)
|
||||
label = self.tabs.tabText(idx)
|
||||
if sheet:
|
||||
save_sheets_dialog(
|
||||
self, f"IOModbus_{label}.xlsx", [sheet],
|
||||
last_dir=config.export_dir, remember_dir=config.set_export_dir,
|
||||
)
|
||||
|
||||
def _export_all_tabs(self) -> None:
|
||||
sheets = collect_all(self.tabs, self._export_metadata())
|
||||
save_sheets_dialog(
|
||||
self, "IOModbus_AllTabs.xlsx", sheets,
|
||||
last_dir=config.export_dir, remember_dir=config.set_export_dir,
|
||||
)
|
||||
|
||||
def _scan(self) -> None:
|
||||
start, end = parse_address_range(self.addr_edit.text())
|
||||
self._ctrl.scan(start, end)
|
||||
|
||||
@@ -30,13 +30,20 @@ def test_chip_tracks_connection(window):
|
||||
|
||||
|
||||
def test_catalog_menu_replaces_the_menubar(window):
|
||||
# Catalog ▾ is device-catalog file operations (IOModbus.txt) only.
|
||||
labels = [a.text() for a in window.catalog_menu.actions() if a.text()]
|
||||
assert any("Import Devices" in t for t in labels)
|
||||
assert any("Export Devices" in t for t in labels)
|
||||
assert any("Supported Devices" in t for t in labels)
|
||||
assert labels == ["Import Devices…", "Export Devices…", "Supported Devices…"]
|
||||
assert window.menuBar().actions() == [] # the old menubar is gone
|
||||
|
||||
|
||||
def test_export_menu_holds_the_data_exports(window):
|
||||
# Spreadsheet export lives in its own Export ▾ menu, via the shared core engine.
|
||||
labels = [a.text() for a in window.export_menu.actions() if a.text()]
|
||||
assert labels == ["Export This Tab…", "Export All Tabs…"]
|
||||
assert window.act_export_tab.text() == "Export This Tab…"
|
||||
assert window.act_export_all.text() == "Export All Tabs…"
|
||||
|
||||
|
||||
def test_disconnect_stops_controller(window, monkeypatch):
|
||||
stopped = []
|
||||
monkeypatch.setattr(window._ctrl, "stop", lambda: stopped.append(True))
|
||||
|
||||
Reference in New Issue
Block a user