fix: Use QMetaObject.invokeMethod for TCP acquisition GUI state update#463
Merged
hongquanli merged 1 commit intomasterfrom Jan 20, 2026
Merged
fix: Use QMetaObject.invokeMethod for TCP acquisition GUI state update#463hongquanli merged 1 commit intomasterfrom
hongquanli merged 1 commit intomasterfrom
Conversation
hongquanli
force-pushed
the
fix/tcp-acquisition-gui-state
branch
from
b27c14d to
d9d5719
Compare
The previous implementation used QTimer.singleShot(0, callback) with threading.Event.wait() which caused 5-second timeouts because the Qt event loop couldn't process callbacks while the TCP thread was blocked. A signal-based approach was attempted but caused race conditions where run_acquisition() was called before the Qt event loop processed the queued signal, resulting in napari layers being initialized with wrong scale parameters (BF/fluorescence size mismatch in display). The fix uses QMetaObject.invokeMethod with Qt.BlockingQueuedConnection, which is Qt's recommended approach for cross-thread synchronous method calls. This blocks the TCP thread until the method completes on the main thread, ensuring the GUI is fully updated before acquisition starts. This fixes: - NDViewer not receiving the dataset path during TCP acquisitions - Progress bar not showing during TCP acquisitions - Widget controls not being grayed out during TCP acquisitions - BF/fluorescence size mismatch in napari multichannel display Changes: - Use QMetaObject.invokeMethod instead of signal emission - Add @slot decorator to set_acquisition_running_state for Qt invocation - Add error handling: check invokeMethod return value and catch exceptions - Add try-except in slot (Qt silently swallows exceptions in queued connections) - Document why different patterns are used for different methods Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
hongquanli
force-pushed
the
fix/tcp-acquisition-gui-state
branch
from
d9d5719 to
4dea665
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes TCP acquisition GUI state updates by replacing the previous QTimer.singleShot + threading.Event pattern with Qt's recommended QMetaObject.invokeMethod with Qt.BlockingQueuedConnection for synchronous cross-thread method calls.
Changes:
- Replaced asynchronous GUI update pattern with synchronous Qt blocking connection to prevent race conditions
- Added
@Slotdecorator and exception handling toset_acquisition_running_statemethod in WellplateMultiPointWidget - Updated documentation to clarify the different patterns used for GUI updates
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| software/control/widgets.py | Added @slot decorator to set_acquisition_running_state, added exception handling with explicit logging, and updated documentation |
| software/control/microscope_control_server.py | Replaced QTimer.singleShot pattern with QMetaObject.invokeMethod using BlockingQueuedConnection, added new Qt imports, and improved error handling and documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
969
to
+1010
| def _set_gui_acquisition_state(self, yaml_data, is_running: bool) -> None: | ||
| """Update GUI widget state to reflect acquisition running/stopped. | ||
|
|
||
| Uses QTimer.singleShot with threading.Event to ensure the GUI update | ||
| completes before returning, matching the pattern in _update_gui_from_yaml. | ||
| Uses QMetaObject.invokeMethod with Qt.BlockingQueuedConnection to ensure | ||
| the GUI update completes on the main thread before this method returns. | ||
| This is Qt's recommended approach for cross-thread synchronous method calls. | ||
|
|
||
| Note: This uses a different pattern than _update_gui_from_yaml because this method | ||
| MUST complete before run_acquisition() is called, otherwise napari layers get | ||
| initialized with wrong scale parameters (race condition). The QTimer.singleShot | ||
| pattern used elsewhere doesn't guarantee completion before the next line executes. | ||
| """ | ||
| if not QT_AVAILABLE: | ||
| return | ||
|
|
||
| widget = self._get_widget_for_type(yaml_data.widget_type) | ||
| if not widget: | ||
| if widget is None: | ||
| self._log.warning(f"Cannot update GUI state: No widget found for type '{yaml_data.widget_type}'") | ||
| return | ||
|
|
||
| if not hasattr(widget, "set_acquisition_running_state"): | ||
| self._log.warning(f"Widget {type(widget).__name__} lacks set_acquisition_running_state method") | ||
| return | ||
|
|
||
| # Use threading.Event to wait for GUI update to complete | ||
| gui_update_complete = threading.Event() | ||
|
|
||
| def update_state(): | ||
| try: | ||
| widget.set_acquisition_running_state(is_running, yaml_data.nz, yaml_data.delta_z_um) | ||
| except Exception as e: | ||
| self._log.error(f"Failed to update GUI acquisition state: {e}") | ||
| finally: | ||
| gui_update_complete.set() | ||
|
|
||
| # Schedule on Qt main thread | ||
| QTimer.singleShot(0, update_state) | ||
|
|
||
| # Wait for completion | ||
| if not gui_update_complete.wait(timeout=5.0): | ||
| self._log.warning("GUI acquisition state update timed out after 5 seconds") | ||
| # Use BlockingQueuedConnection to call method on main thread and wait for completion | ||
| # This ensures GUI is fully updated before run_acquisition() is called | ||
| try: | ||
| success = QMetaObject.invokeMethod( | ||
| widget, | ||
| "set_acquisition_running_state", | ||
| Qt.BlockingQueuedConnection, | ||
| Q_ARG(bool, is_running), | ||
| Q_ARG(int, yaml_data.nz), | ||
| Q_ARG(float, yaml_data.delta_z_um), | ||
| ) | ||
| if not success: | ||
| self._log.error( | ||
| f"QMetaObject.invokeMethod failed to invoke set_acquisition_running_state " | ||
| f"on {type(widget).__name__}" | ||
| ) | ||
| except Exception as e: | ||
| self._log.error(f"Failed to update GUI acquisition state: {e}") |
There was a problem hiding this comment.
The _set_gui_acquisition_state method calls set_acquisition_running_state on widgets of type "flexible" and "wellplate". However, only WellplateMultiPointWidget has this method implemented - FlexibleMultiPointWidget does not. This will cause the method to log a warning and silently fail for flexible widget acquisitions. The set_acquisition_running_state method should be added to FlexibleMultiPointWidget to match the implementation in WellplateMultiPointWidget.
Contributor
Author
There was a problem hiding this comment.
[Claude Code] False positive - TCP command explicitly rejects flexible widget acquisitions at line 1176-1182 with if yaml_data.widget_type \!= "wellplate": raise ValueError(...). So _set_gui_acquisition_state will never be called for FlexibleMultiPointWidget via TCP.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
QMetaObject.invokeMethodwithQt.BlockingQueuedConnectionProblem
The previous implementation caused:
Solution
Use
QMetaObject.invokeMethodwithQt.BlockingQueuedConnectionwhich blocks the TCP thread until the method completes on the main Qt thread.Test plan
scripts/run_acquisition.py🤖 Generated with Claude Code