From b3beed4ed34f1b74bd3b429264d556b708be3417 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Wed, 11 Mar 2026 14:28:44 +0100 Subject: [PATCH 1/2] Relax deeplabcut-live pins to >=1.1 (#59) Change strict pins for deeplabcut-live from ==1.1 to >=1.1 in pyproject.toml (base deps, [pytorch] extra, and [tf] extra). This relaxes exact version locking to allow newer releases while keeping 1.1 as the minimum required version. --- pyproject.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 02e8a96..1c1c153 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ classifiers = [ ] dependencies = [ "cv2-enumerate-cameras", - "deeplabcut-live==1.1", + "deeplabcut-live>=1.1", "matplotlib", "numpy", "opencv-python", @@ -52,7 +52,7 @@ dev = [ ] gentl = [ "harvesters" ] pytorch = [ - "deeplabcut-live[pytorch]==1.1", # includes timm and scipy + "deeplabcut-live[pytorch]>=1.1", # includes timm and scipy ] test = [ "hypothesis>=6", @@ -64,7 +64,7 @@ test = [ "tox-gh-actions", ] tf = [ - "deeplabcut-live[tf]==1.1", + "deeplabcut-live[tf]>=1.1", ] [project.scripts] dlclivegui = "dlclivegui:main" From 3951b339136a1d913b54b21a1b82e7be31599936 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Fri, 13 Mar 2026 13:24:56 +0100 Subject: [PATCH 2/2] Fix camera config UI responsiveness issues (#58) * Reset working settings on show; allow camera edits Add showEvent to reset the cleanup guard and rebuild the working settings from the latest accepted settings, then repopulate the dialog. Allow removing/moving active cameras even while a scan is running and adjust preview button enablement logic. Auto-select the first active camera when populating the list and ensure multi_camera_settings is updated from the working copy before emitting settings_changed on apply. * Add GUI e2e tests for camera dialog Add two end-to-end GUI tests for camera configuration dialog to prevent regressions: - test_remove_active_camera_works_while_scan_running: Verifies that removing the active camera still works while a discovery scan is running. The test slows CameraFactory.detect_cameras via monkeypatch to keep the scan running, ensures the remove button is enabled during scan, removes the selected camera, and cleans up by cancelling the scan. - test_ok_updates_internal_multicamera_settings: Ensures that after adding a second camera and accepting the dialog (OK), the settings_changed signal emits the updated MultiCameraSettings and the dialog's internal _multi_camera_settings is updated to match the accepted settings. These tests guard against regressions where scan-running state blocked structure edits (remove/move) and where dialog acceptance did not update the dialog's internal settings. * Standardize preview checks & validate camera items Introduce a helper to detect live previews and replace scattered direct PreviewState checks with it. Add validation for selected detected-camera items (show a warning if invalid). Normalize camera backend id casing when formatting labels. Remove a redundant try/except around cleanup guard reset and always reset _cleanup_done on show. Drop an extra in-place settings assignment in the apply flow and remove a now-unneeded call to _populate_from_settings on show. These changes tidy preview control, improve robustness for camera selection, and unify camera identity handling. * Simplify comments in camera config tests Clean up and shorten comments in tests/gui/camera_config/test_cam_dialog_e2e.py: remove parenthetical notes about expected failures tied to implementation details and replace them with concise, direct expectations. No functional test logic changed. * Update camera_config_dialog.py * Improve camera dialog validation & workers Refine CameraConfigDialog behavior to prevent races and invalid states: only clean up the scan worker when it's not running; disable the Add Camera button unless the selected item is a DetectedCamera; improve probe worker cancellation and waiting to avoid concurrent probes; emit the multi-camera model copy instead of a deepcopy when settings change. Add _enabled_count_with and enforce MAX_CAMERAS when enabling a camera and before closing the dialog. Clamp crop coordinates and only apply crop when the rectangle is valid to avoid invalid-frame cropping. Small comment/organization tweaks included. * Sync selection and improve spinbox Enter handling Ensure selection state is synchronized after initialization by calling _populate_from_settings and a new _post_init_sync_selection to set _current_edit_index. Install event filters on camera numeric spinboxes and their lineEdits, and improve Enter-key handling so interpretText() is invoked on the correct spinbox (whether the event comes from the spinbox or its lineEdit) before applying camera settings. Introduce _selected_detected_camera helper to centralize retrieval of the selected detected camera and use it to control the Add button enablement; simplify scan-related UI enable/disable logic and remove duplicated state handling. Miscellaneous cleanup and minor refactors in camera_config_dialog.py. * Normalize camera ID and improve scan cleanup Normalize dlc_camera_id values (split on ':' and lowercase backend part, fallback to lowercasing whole string) and refresh camera labels accordingly. Improve camera discovery shutdown: set scan state to CANCELING when stopping, avoid immediate cleanup if the scan thread is still running, and route worker finished to a new handler (_on_scan_thread_finished) that performs cleanup and sets state to IDLE. Also stop calling _populate_from_settings() when opening the camera dialog to avoid overwriting unsaved changes; let the dialog refresh itself when shown. * Cache camera scans and add settings setter Introduce caching for camera discovery to avoid unnecessary rescans by adding _last_scan_backend and _has_scan_results and a _maybe_refresh_available_cameras(force=False) helper that only calls _refresh_available_cameras when backend or cache state requires it. Refactor population logic: add _populate_active_list_from_working to fill the active cameras list (preserving selection optionally) and simplify _populate_from_settings to use it. Add public set_settings(...) to update the dialog with new MultiCameraSettings and optional dlc_camera_id without destroying unsaved UI state. Improve scan worker cleanup logic and add a debug message to clarify deferred cleanup. Update backend-change handling to invalidate cache, and update scan result handlers to set cache flags appropriately. Minor UI tweaks: remove a stray currentRow() call, ensure button states are updated, and switch main window to call set_settings(...) when reusing the dialog so the dialog manages its own refresh behavior. * Improve scan worker cleanup and error logging Replace a bare exception handler when installing event filters with specific (AttributeError, RuntimeError) handling and log a warning so failures are not silently swallowed. Enhance _on_scan_thread_finished to ignore finished signals from stale workers, attempt deleteLater() on old senders, and only transition the active worker to IDLE to avoid race conditions affecting scan state. * Update test_cam_dialog_e2e.py * Fix cleanup guards and scan state transitions Introduce explicit cleanup flags and tighten cleanup logic to avoid races and double-run teardown. Add _cleanp_requested and _cleanup_completed, replace uses of the old _cleanup_done flag, and only mark cleanup completed when no scan/preview/probe worker is active. Also always transition the scan state to IDLE when cleaning up. Update the e2e test to wait for scans to finish earlier and reduce related timeouts to reflect the more deterministic teardown behavior. * Update camera_config_dialog.py --- .../gui/camera_config/camera_config_dialog.py | 297 +++++++++++++++--- dlclivegui/gui/main_window.py | 4 +- .../gui/camera_config/test_cam_dialog_e2e.py | 95 ++++++ 3 files changed, 343 insertions(+), 53 deletions(-) diff --git a/dlclivegui/gui/camera_config/camera_config_dialog.py b/dlclivegui/gui/camera_config/camera_config_dialog.py index 5f2caff..4c94fb7 100644 --- a/dlclivegui/gui/camera_config/camera_config_dialog.py +++ b/dlclivegui/gui/camera_config/camera_config_dialog.py @@ -48,6 +48,9 @@ def __init__( self.setWindowTitle("Configure Cameras") self.setMinimumSize(960, 720) + self._cleanup_requested = False + self._cleanup_completed = False + self._dlc_camera_id: str | None = None # self.dlc_camera_id: str | None = None # Actual/working camera settings @@ -70,9 +73,20 @@ def __init__( self._settings_scroll: QScrollArea | None = None self._settings_scroll_contents: QWidget | None = None + # Scan cache + self._last_scan_backend: str | None = None + self._has_scan_results: bool = False + self._setup_ui() - self._populate_from_settings() self._connect_signals() + self._populate_from_settings() + self._post_init_sync_selection() + + def _post_init_sync_selection(self) -> None: + """Ensure current selection state is reflected in _current_edit_index.""" + row = self.active_cameras_list.currentRow() + if row >= 0: + self._on_active_camera_selected(row) @property def dlc_camera_id(self) -> str | None: @@ -81,10 +95,31 @@ def dlc_camera_id(self) -> str | None: @dlc_camera_id.setter def dlc_camera_id(self, value: str | None) -> None: - """Set the currently selected DLC camera ID.""" - self._dlc_camera_id = value + if not value: + self._dlc_camera_id = None + else: + try: + b, idx = value.split(":", 1) + self._dlc_camera_id = f"{b.lower()}:{idx}" + except ValueError: + # fallback: lowercase entire string + self._dlc_camera_id = value.lower() self._refresh_camera_labels() + def showEvent(self, event): + super().showEvent(event) + # Reset cleanup guard so close cleanup runs for each session + self._cleanup_completed = False + + # Rebuild the working copy from the latest “accepted” settings + self._working_settings = self._multi_camera_settings.model_copy(deep=True) + self._current_edit_index = None + + self._populate_active_list_from_working(keep_selection=True) + self._post_init_sync_selection() + + self._maybe_refresh_available_cameras(force=False) + # Maintain overlay geometry when resizing def resizeEvent(self, event): super().resizeEvent(event) @@ -117,7 +152,7 @@ def eventFilter(self, obj, event): # Intercept Enter in FPS and crop spinboxes if event.type() == QEvent.KeyPress and isinstance(event, QKeyEvent): if event.key() in (Qt.Key_Return, Qt.Key_Enter): - if obj in ( + spinboxes = ( self.cam_fps, self.cam_width, self.cam_height, @@ -127,15 +162,22 @@ def eventFilter(self, obj, event): self.cam_crop_y0, self.cam_crop_x1, self.cam_crop_y1, - ): - # Commit any pending text → value + ) + + def _matches(sb): + return obj is sb or obj is getattr(sb, "lineEdit", lambda: None)() + + if any(_matches(sb) for sb in spinboxes): try: - obj.interpretText() + # If event came from lineEdit, interpretText still belongs to the spinbox + for sb in spinboxes: + if _matches(sb): + sb.interpretText() + break except Exception: pass - # Apply settings to persist crop/FPS to CameraSettings + self._apply_camera_settings() - # Consume so OK isn't triggered return True return super().eventFilter(obj, event) @@ -153,9 +195,9 @@ def reject(self) -> None: def _on_close_cleanup(self) -> None: """Stop preview, cancel workers, and reset scan UI. Safe to call multiple times.""" # Guard to avoid running twice if closeEvent + reject/accept both run - if getattr(self, "_cleanup_done", False): + if getattr(self, "_cleanup_completed", False): return - self._cleanup_done = True + self._cleanup_requested = True # Stop preview (loader + backend + timer) try: @@ -171,9 +213,15 @@ def _on_close_cleanup(self) -> None: except Exception: pass # Keep this short to reduce UI freeze + self._set_scan_state(CameraScanState.CANCELING, message="Canceling discovery…") sw.wait(300) - self._set_scan_state(CameraScanState.IDLE) - self._cleanup_scan_worker() + if sw.isRunning(): + LOGGER.debug("Cleanup: scan worker still running; deferring worker cleanup to finished()") + elif self._scan_worker is sw: + # Worker has stopped; safe to perform scan-worker-specific cleanup now + self._cleanup_scan_worker() + elif self._scan_worker and not self._scan_worker.isRunning(): + self._cleanup_scan_worker() # Cancel probe worker pw = getattr(self, "_probe_worker", None) @@ -211,11 +259,36 @@ def _on_close_cleanup(self) -> None: except Exception: pass + if ( + not self._is_scan_running() + and not self._is_preview_live() + and not (self._probe_worker and self._probe_worker.isRunning()) + ): + self._cleanup_completed = True + # ------------------------------- # UI setup # ------------------------------- def _setup_ui(self) -> None: setup_camera_config_dialog_ui(self) + for sb in ( + self.cam_fps, + self.cam_width, + self.cam_height, + self.cam_exposure, + self.cam_gain, + self.cam_crop_x0, + self.cam_crop_y0, + self.cam_crop_x1, + self.cam_crop_y1, + ): + try: + sb.installEventFilter(self) + le = sb.lineEdit() + if le is not None: + le.installEventFilter(self) + except (AttributeError, RuntimeError) as exc: + LOGGER.warning("Failed to install event filter on %s: %s", sb.objectName(), exc) def _position_scan_overlay(self) -> None: """Position scan overlay to cover the available_cameras_list area.""" @@ -288,6 +361,9 @@ def _mark_dirty(*_args): # ------------------------------- # UI state updates # ------------------------------- + def _is_preview_live(self) -> bool: + return self._preview.state in (PreviewState.ACTIVE, PreviewState.LOADING) + def _set_apply_dirty(self, dirty: bool) -> None: """Visually mark Apply Settings button as 'dirty' (pending edits).""" if dirty: @@ -304,15 +380,15 @@ def _update_button_states(self) -> None: active_row = self.active_cameras_list.currentRow() has_active_selection = active_row >= 0 - allow_structure_edits = has_active_selection and not scan_running - self.remove_camera_btn.setEnabled(allow_structure_edits) - self.move_up_btn.setEnabled(allow_structure_edits and active_row > 0) - self.move_down_btn.setEnabled(allow_structure_edits and active_row < self.active_cameras_list.count() - 1) - # During loading, preview button becomes "Cancel Loading" + # Allow removing/moving active cameras even during scanning + self.remove_camera_btn.setEnabled(has_active_selection) + self.move_up_btn.setEnabled(has_active_selection and active_row > 0) + self.move_down_btn.setEnabled(has_active_selection and active_row < self.active_cameras_list.count() - 1) + self.preview_btn.setEnabled(has_active_selection or self._preview.state == PreviewState.LOADING) - available_row = self.available_cameras_list.currentRow() - self.add_camera_btn.setEnabled(available_row >= 0 and not scan_running) + + self.add_camera_btn.setEnabled((self._selected_detected_camera() is not None) and not scan_running) def _sync_preview_ui(self) -> None: """Update buttons/overlays based on preview state only.""" @@ -377,10 +453,20 @@ def _refresh_camera_labels(self) -> None: def _format_camera_label(self, cam: CameraSettings, index: int = -1) -> str: status = "✓" if cam.enabled else "○" - this_id = f"{cam.backend}:{cam.index}" + this_id = f"{(cam.backend or '').lower()}:{cam.index}" dlc_indicator = " [DLC]" if this_id == self._dlc_camera_id and cam.enabled else "" return f"{status} {cam.name} [{cam.backend}:{cam.index}]{dlc_indicator}" + def _selected_detected_camera(self) -> DetectedCamera | None: + row = self.available_cameras_list.currentRow() + if row < 0: + return None + item = self.available_cameras_list.item(row) + if not item: + return None + detected = item.data(Qt.ItemDataRole.UserRole) + return detected if isinstance(detected, DetectedCamera) else None + def _update_active_list_item(self, row: int, cam: CameraSettings) -> None: """Refresh the active camera list row text and color.""" item = self.active_cameras_list.item(row) @@ -454,6 +540,8 @@ def _append_status(self, text: str) -> None: # Camera discovery and probing # ------------------------------- def _on_backend_changed(self, _index: int) -> None: + self._has_scan_results = False + self._last_scan_backend = None self._refresh_available_cameras() def _is_scan_running(self) -> bool: @@ -465,7 +553,6 @@ def _is_scan_running(self) -> bool: def _set_scan_state(self, state: CameraScanState, message: str | None = None) -> None: """Single source of truth for scan-related UI controls.""" self._scan_state = state - scanning = state in (CameraScanState.RUNNING, CameraScanState.CANCELING) # Overlay message @@ -486,10 +573,8 @@ def _set_scan_state(self, state: CameraScanState, message: str | None = None) -> # Disable discovery inputs while scanning self.backend_combo.setEnabled(not scanning) self.refresh_btn.setEnabled(not scanning) - # Available list + add flow blocked while scanning (structure edits disallowed) self.available_cameras_list.setEnabled(not scanning) - self.add_camera_btn.setEnabled(False if scanning else (self.available_cameras_list.currentRow() >= 0)) self._update_button_states() @@ -536,7 +621,7 @@ def _refresh_available_cameras(self) -> None: w.canceled.connect(self._on_scan_canceled) # Cleanup only - w.finished.connect(self._cleanup_scan_worker) + w.finished.connect(self._on_scan_thread_finished) self.scan_started.emit(f"Scanning {backend} cameras…") w.start() @@ -549,6 +634,21 @@ def _on_scan_progress(self, msg: str) -> None: return self._show_scan_overlay(msg or "Discovering cameras…") + def _on_scan_thread_finished(self) -> None: + sender = self.sender() + # Ignore finished signals from old workers; only clean up the active one. + if sender is not None and sender is not self._scan_worker: + LOGGER.debug("[Scan] Ignoring finished from old worker") + # Make sure the old worker can be cleaned up by Qt. + try: + sender.deleteLater() + except AttributeError: + pass + return + + self._cleanup_scan_worker() + self._set_scan_state(CameraScanState.IDLE) + def _on_scan_result(self, cams: list) -> None: if self.sender() is not self._scan_worker: LOGGER.debug("[Scan] Ignoring result from old worker: %d cameras", len(cams) if cams else 0) @@ -559,6 +659,8 @@ def _on_scan_result(self, cams: list) -> None: # Apply results to UI first (stability guarantee) self._detected_cameras = cams or [] self.available_cameras_list.clear() + self._has_scan_results = True + self._last_scan_backend = self._current_backend_key() if not self._detected_cameras: placeholder = QListWidgetItem("No cameras detected.") @@ -588,6 +690,8 @@ def _on_scan_error(self, msg: str) -> None: placeholder = QListWidgetItem("Scan failed.") placeholder.setFlags(Qt.ItemIsEnabled) self.available_cameras_list.addItem(placeholder) + self._has_scan_results = False + self._last_scan_backend = self._current_backend_key() self._finish_scan("error") @@ -609,6 +713,8 @@ def request_scan_cancel(self) -> None: placeholder = QListWidgetItem("Scan canceled.") placeholder.setFlags(Qt.ItemIsEnabled) self.available_cameras_list.addItem(placeholder) + self._has_scan_results = True # keep any results that did arrive, even if cancel requested + self._last_scan_backend = self._current_backend_key() if w is None or not w.isRunning(): self._finish_scan("cancel") @@ -630,7 +736,8 @@ def _on_available_camera_selected(self, row: int) -> None: if self._scan_worker and self._scan_worker.isRunning(): self.add_camera_btn.setEnabled(False) return - self.add_camera_btn.setEnabled(row >= 0 and not self._is_scan_running()) + detected = self._selected_detected_camera() + self.add_camera_btn.setEnabled(isinstance(detected, DetectedCamera)) def _on_available_camera_double_clicked(self, item: QListWidgetItem) -> None: if self._is_scan_running(): @@ -676,7 +783,7 @@ def _on_active_camera_selected(self, row: int) -> None: return # Stop any running preview when selection changes - if self._preview.state in (PreviewState.ACTIVE, PreviewState.LOADING): + if self._is_preview_live(): self._stop_preview() self._current_edit_index = row @@ -712,6 +819,9 @@ def _add_selected_camera(self) -> None: return item = self.available_cameras_list.item(row) detected = item.data(Qt.ItemDataRole.UserRole) + if not isinstance(detected, DetectedCamera): + QMessageBox.warning(self, "Invalid Selection", "Selected item is not a valid camera.") + return # make sure this is to lower for comparison against camera_identity_key backend = (self.backend_combo.currentData() or "opencv").lower() @@ -751,6 +861,8 @@ def _add_selected_camera(self) -> None: self._start_probe_for_camera(new_cam) def _remove_selected_camera(self) -> None: + if self._is_preview_live(): + self._stop_preview() if not self._commit_pending_edits(reason="before removing a camera"): return row = self.active_cameras_list.currentRow() @@ -765,6 +877,8 @@ def _remove_selected_camera(self) -> None: self._update_button_states() def _move_camera_up(self) -> None: + if self._is_preview_live(): + self._stop_preview() if not self._commit_pending_edits(reason="before reordering cameras"): return row = self.active_cameras_list.currentRow() @@ -778,6 +892,8 @@ def _move_camera_up(self) -> None: self._refresh_camera_labels() def _move_camera_down(self) -> None: + if self._is_preview_live(): + self._stop_preview() if not self._commit_pending_edits(reason="before reordering cameras"): return row = self.active_cameras_list.currentRow() @@ -905,6 +1021,14 @@ def _commit_pending_edits(self, *, reason: str = "") -> bool: ) return False + def _enabled_count_with(self, row: int, new_enabled: bool) -> int: + count = 0 + for i, cam in enumerate(self._working_settings.cameras): + enabled = new_enabled if i == row else bool(cam.enabled) + if enabled: + count += 1 + return count + def _apply_camera_settings(self) -> bool: try: for sb in ( @@ -932,10 +1056,15 @@ def _apply_camera_settings(self) -> bool: current_model = self._working_settings.cameras[row] new_model = self._build_model_from_form(current_model) - diff = CameraSettings.check_diff(current_model, new_model) + if bool(new_model.enabled): + if self._enabled_count_with(row, True) > self.MAX_CAMERAS: + QMessageBox.warning( + self, "Maximum Cameras", f"Maximum of {self.MAX_CAMERAS} active cameras allowed." + ) + self.cam_enabled_checkbox.setChecked(bool(current_model.enabled)) + return False - self._working_settings.cameras[row] = new_model - self._update_active_list_item(row, new_model) + diff = CameraSettings.check_diff(current_model, new_model) LOGGER.debug( "[Apply] backend=%s idx=%s changes=%s", @@ -1004,19 +1133,66 @@ def _clear_settings_form(self) -> None: self.apply_settings_btn.setEnabled(False) self.reset_settings_btn.setEnabled(False) + def set_settings(self, settings: MultiCameraSettings, *, dlc_camera_id: str | None = None) -> None: + self._multi_camera_settings = settings or MultiCameraSettings(cameras=[]) + self._working_settings = self._multi_camera_settings.model_copy(deep=True) + self._current_edit_index = None + + if dlc_camera_id is not None: + self.dlc_camera_id = dlc_camera_id + + self._populate_active_list_from_working(keep_selection=True) + self._post_init_sync_selection() + self._maybe_refresh_available_cameras() + def _populate_from_settings(self) -> None: """Populate the dialog from existing settings.""" - self.active_cameras_list.clear() - for i, cam in enumerate(self._working_settings.cameras): - item = QListWidgetItem(self._format_camera_label(cam, i)) - item.setData(Qt.ItemDataRole.UserRole, cam) - if not cam.enabled: - item.setForeground(Qt.GlobalColor.gray) - self.active_cameras_list.addItem(item) - - self._refresh_available_cameras() + self._populate_active_list_from_working(keep_selection=True) self._update_button_states() + def _populate_active_list_from_working(self, *, keep_selection: bool = True) -> None: + """Populate only the active cameras list from _working_settings (no scanning).""" + prev_row = self.active_cameras_list.currentRow() if keep_selection else -1 + + self.active_cameras_list.blockSignals(True) + try: + self.active_cameras_list.clear() + for i, cam in enumerate(self._working_settings.cameras): + item = QListWidgetItem(self._format_camera_label(cam, i)) + item.setData(Qt.ItemDataRole.UserRole, cam) + if not cam.enabled: + item.setForeground(Qt.GlobalColor.gray) + self.active_cameras_list.addItem(item) + finally: + self.active_cameras_list.blockSignals(False) + + # restore selection if possible + if self.active_cameras_list.count() > 0: + if keep_selection and 0 <= prev_row < self.active_cameras_list.count(): + self.active_cameras_list.setCurrentRow(prev_row) + else: + self.active_cameras_list.setCurrentRow(0) + + def _current_backend_key(self) -> str: + return (self.backend_combo.currentData() or self.backend_combo.currentText().split()[0] or "opencv").lower() + + def _maybe_refresh_available_cameras(self, *, force: bool = False) -> None: + """Refresh available list only when needed (backend changed, no cache, or forced).""" + backend = self._current_backend_key() + + needs_scan = ( + force + or not self._has_scan_results + or (self._last_scan_backend is None) + or (backend != self._last_scan_backend) + or (self.available_cameras_list.count() == 0) # defensive: list got cleared + ) + if needs_scan: + self._refresh_available_cameras() + else: + # No scan; just ensure Add button state is consistent + self._update_button_states() + def _reset_selected_camera(self, *, clear_backend_cache: bool = False) -> None: """Reset the selected camera by probing device defaults and applying them to requested values.""" if self._current_edit_index is None: @@ -1026,7 +1202,7 @@ def _reset_selected_camera(self, *, clear_backend_cache: bool = False) -> None: return # Stop preview to avoid fighting an open capture - if self._preview.state in (PreviewState.ACTIVE, PreviewState.LOADING): + if self._is_preview_live(): self._stop_preview() cam = self._working_settings.cameras[row] @@ -1065,6 +1241,9 @@ def _on_ok_clicked(self) -> None: # Auto-apply pending edits before saving if not self._commit_pending_edits(reason="before going back to the main window"): return + if len(self._working_settings.get_active_cameras()) > self.MAX_CAMERAS: + QMessageBox.warning(self, "Maximum Cameras", f"Maximum of {self.MAX_CAMERAS} active cameras allowed.") + return try: if self.apply_settings_btn.isEnabled(): self._append_status("[OK button] Auto-applying pending settings before closing dialog.") @@ -1076,13 +1255,14 @@ def _on_ok_clicked(self) -> None: if self._working_settings.cameras and not active: QMessageBox.warning(self, "No Active Cameras", "Please enable at least one camera or remove all cameras.") return - self.settings_changed.emit(copy.deepcopy(self._working_settings)) + self._multi_camera_settings = self._working_settings.model_copy(deep=True) + self.settings_changed.emit(self._multi_camera_settings) self._on_close_cleanup() self.accept() # ------------------------------- - # Probe (device telemetry) management + # Probe management # ------------------------------- def _start_probe_for_camera(self, cam: CameraSettings, *, apply_to_requested: bool = False) -> None: @@ -1092,9 +1272,18 @@ def _start_probe_for_camera(self, cam: CameraSettings, *, apply_to_requested: bo requested width/height/fps with detected device values. """ # Don’t probe if preview is active/loading - if self._preview.state in (PreviewState.ACTIVE, PreviewState.LOADING): + if self._is_preview_live(): return + pw = getattr(self, "_probe_worker", None) + if pw and pw.isRunning(): + try: + pw.request_cancel() + except Exception: + pass + pw.wait(200) + self._probe_worker = None + # Track probe intent self._probe_apply_to_requested = bool(apply_to_requested) self._probe_target_row = int(self._current_edit_index) if self._current_edit_index is not None else None @@ -1339,7 +1528,7 @@ def _start_preview(self) -> None: """Start camera preview asynchronously (no UI freeze).""" if not self._commit_pending_edits(reason="before starting preview"): return - if self._preview.state in (PreviewState.ACTIVE, PreviewState.LOADING): + if self._is_preview_live(): return row = self._current_edit_index @@ -1587,13 +1776,21 @@ def _update_preview(self) -> None: rotation = self.cam_rotation.currentData() frame = apply_rotation(frame, rotation) - # Apply crop if set in the form (real-time from UI) + # Compute crop with clamping h, w = frame.shape[:2] - x0 = self.cam_crop_x0.value() - y0 = self.cam_crop_y0.value() - x1 = self.cam_crop_x1.value() or w - y1 = self.cam_crop_y1.value() or h - frame = apply_crop(frame, x0, y0, x1, y1) + x0 = max(0, min(self.cam_crop_x0.value(), w)) + y0 = max(0, min(self.cam_crop_y0.value(), h)) + x1_val = self.cam_crop_x1.value() + y1_val = self.cam_crop_y1.value() + x1 = max(0, min(x1_val if x1_val > 0 else w, w)) + y1 = max(0, min(y1_val if y1_val > 0 else h, h)) + + # Only apply if valid rectangle; otherwise skip crop + if x1 > x0 and y1 > y0: + frame = apply_crop(frame, x0, y0, x1, y1) + else: + # Optional: show a status once, not every frame + pass # Resize to fit preview label frame = resize_to_fit(frame, max_w=400, max_h=300) diff --git a/dlclivegui/gui/main_window.py b/dlclivegui/gui/main_window.py index 380eda0..0e3beb0 100644 --- a/dlclivegui/gui/main_window.py +++ b/dlclivegui/gui/main_window.py @@ -1192,9 +1192,7 @@ def _open_camera_config_dialog(self) -> None: self._cam_dialog = CameraConfigDialog(self, self._config.multi_camera) self._cam_dialog.settings_changed.connect(self._on_multi_camera_settings_changed) else: - # Refresh its UI from current settings when reopened - self._cam_dialog._populate_from_settings() - self._cam_dialog.dlc_camera_id = self._inference_camera_id + self._cam_dialog.set_settings(self._config.multi_camera, dlc_camera_id=self._inference_camera_id) self._cam_dialog.show() self._cam_dialog.raise_() diff --git a/tests/gui/camera_config/test_cam_dialog_e2e.py b/tests/gui/camera_config/test_cam_dialog_e2e.py index 49867b8..df1c357 100644 --- a/tests/gui/camera_config/test_cam_dialog_e2e.py +++ b/tests/gui/camera_config/test_cam_dialog_e2e.py @@ -120,6 +120,7 @@ def dialog(qtbot, patch_detect_cameras): d = CameraConfigDialog(None, s) qtbot.addWidget(d) d.show() + qtbot.waitUntil(lambda: not d._is_scan_running(), timeout=2000) qtbot.waitExposed(d) yield d @@ -424,3 +425,97 @@ def slow_run(self): qtbot.waitUntil(lambda: dialog._preview.loader is None and dialog._preview.state == PreviewState.IDLE, timeout=2000) assert dialog._preview.backend is None + + +@pytest.mark.gui +def test_remove_active_camera_works_while_scan_running(dialog, qtbot, monkeypatch): + """ + Regression test for: + - 'When coming back to camera config after choosing a camera, it cannot be removed' + Root cause: scan_running disabled structure edits (Remove/Move). + Expected: Remove works even while discovery scan is running. + """ + + # Slow down camera detection so scan stays RUNNING long enough for interaction + def slow_detect(backend, max_devices=10, should_cancel=None, progress_cb=None, **kwargs): + for i in range(50): + if should_cancel and should_cancel(): + break + if progress_cb: + progress_cb(f"Scanning… {i}") + time.sleep(0.02) + return [ + DetectedCamera(index=0, label=f"{backend}-X"), + DetectedCamera(index=1, label=f"{backend}-Y"), + ] + + monkeypatch.setattr(CameraFactory, "detect_cameras", staticmethod(slow_detect)) + + # Ensure an active row is selected + dialog.active_cameras_list.setCurrentRow(0) + qtbot.waitUntil(lambda: dialog.active_cameras_list.currentRow() == 0, timeout=1000) + + initial_active = dialog.active_cameras_list.count() + initial_model = len(dialog._working_settings.cameras) + assert initial_active == initial_model == 1 + + # Trigger scan; wait until scan controls indicate it's running + qtbot.mouseClick(dialog.refresh_btn, Qt.LeftButton) + qtbot.waitUntil(lambda: dialog._is_scan_running(), timeout=1000) + qtbot.waitUntil(lambda: dialog.scan_cancel_btn.isVisible(), timeout=1000) + + # Remove button should be enabled even during scan + qtbot.waitUntil(lambda: dialog.remove_camera_btn.isEnabled(), timeout=1000) + + # Remove the selected active camera during scan + qtbot.mouseClick(dialog.remove_camera_btn, Qt.LeftButton) + + assert dialog.active_cameras_list.count() == initial_active - 1 + assert len(dialog._working_settings.cameras) == initial_model - 1 + + # Clean up: cancel scan so teardown doesn't hang waiting for scan completion + if dialog.scan_cancel_btn.isVisible() and dialog.scan_cancel_btn.isEnabled(): + qtbot.mouseClick(dialog.scan_cancel_btn, Qt.LeftButton) + + qtbot.waitUntil(lambda: not dialog._is_scan_running(), timeout=3000) + + +@pytest.mark.gui +def test_ok_updates_internal_multicamera_settings(dialog, qtbot): + """ + Regression test for: + - 'adding another camera and hitting OK does not add the new extra camera' + when caller reads dialog._multi_camera_settings after closing. + + Expected: + - OK emits updated settings + - dialog._multi_camera_settings is updated to match accepted settings + """ + + # Ensure backend combo matches the active camera backend, so duplicate logic behaves consistently + _select_backend_for_active_cam(dialog, cam_row=0) + + # Scan and add a non-duplicate camera (index 1) + _run_scan_and_wait(dialog, qtbot, timeout=2000) + dialog.available_cameras_list.setCurrentRow(1) + qtbot.mouseClick(dialog.add_camera_btn, Qt.LeftButton) + + qtbot.waitUntil(lambda: dialog.active_cameras_list.count() == 2, timeout=1000) + assert len(dialog._working_settings.cameras) == 2 + + # Click OK and capture emitted settings + with qtbot.waitSignal(dialog.settings_changed, timeout=2000) as sig: + qtbot.mouseClick(dialog.ok_btn, Qt.LeftButton) + + emitted = sig.args[0] + assert isinstance(emitted, MultiCameraSettings) + assert len(emitted.cameras) == 2 + + # Check: internal source-of-truth must match accepted state + assert dialog._multi_camera_settings is not None + assert len(dialog._multi_camera_settings.cameras) == 2 + + # Optional: ensure camera identities match (names/index/backend) + assert [(c.backend, int(c.index)) for c in dialog._multi_camera_settings.cameras] == [ + (c.backend, int(c.index)) for c in emitted.cameras + ]