refactor: change CameraController to inherit from QObject and manage threading in CameraManager

core/camera/camera_controller.py

@@ -1,49 +1,51 @@
-from PySide6.QtCore import QObject, QThread, QTimer, Signal, Slot, QMutex, QMutexLocker
+from PySide6.QtCore import QObject, QTimer, Signal, Slot, QMutex, QMutexLocker
 from PySide6.QtGui import QImage, QPixmap
 import cv2
 
 from .base_camera import BaseCamera
 
 
-class CameraController(QThread):
+class CameraController(QObject):
+    """
+    A QObject worker for handling camera operations in a separate thread.
+    This object should be moved to a QThread.
+    """
     frame_ready = Signal(QPixmap)
     photo_ready = Signal(QPixmap)
     error_occurred = Signal(str)
-    _enable_timer = Signal(bool)
-
 
     def __init__(self, parent: QObject | None = None) -> None:
         super().__init__(parent)
-        self.camera = None
+        self.camera: BaseCamera | None = None
-        self.timer = None
+        self.timer: QTimer | None = None
         self.fps = 15
         self.is_streaming = False
         self.is_connected = False
-
         self._camera_mutex = QMutex()
-        self.start()
 
-
+    @Slot()
-    def run(self) -> None:
+    def run(self):
+        """
+        Initializes resources in the worker thread.
+        This should be connected to the QThread.started signal.
+        """
         self.timer = QTimer()
         self.timer.timeout.connect(self._update_frame)
-        self._enable_timer.connect(self._set_timer)
-        self.exec()
- 
-    def stop(self):
-        self.stop_camera()
-        self.quit()
-        self.wait()
 
+    @Slot(BaseCamera, int)
     def set_camera(self, camera: BaseCamera, fps: int = 15) -> None:
         with QMutexLocker(self._camera_mutex):
+            if self.is_streaming:
                 self.stop_stream()
+            if self.is_connected:
                 self.stop_camera()
 
             self.camera = camera
             self.fps = fps
 
+    @Slot()
     def start_camera(self) -> None:
+        with QMutexLocker(self._camera_mutex):
             if self.camera is None or self.is_connected:
                 return
 
@@ -53,63 +55,56 @@ class CameraController(QThread):
                 self.is_connected = False
                 self.error_occurred.emit(self.camera.get_error_msg())
 
+    @Slot()
     def stop_camera(self) -> None:
+        with QMutexLocker(self._camera_mutex):
             if self.is_streaming:
                 self.stop_stream()
 
-        if self.camera is not None:
+            if self.camera is not None and self.is_connected:
                 self.camera.disconnect()
 
             self.is_connected = False
 
+    @Slot()
     def start_stream(self):
-        if not self.is_connected:
+        with QMutexLocker(self._camera_mutex):
+            if not self.is_connected or self.is_streaming or self.timer is None:
                 return
 
-        if self.is_streaming:
-            return
-
-        if self.timer:
             self.is_streaming = True
-            # self.timer.start()
+            self.timer.setInterval(int(1000 / self.fps))
-            self._enable_timer.emit(True)
+            self.timer.start()
 
+    @Slot()
     def stop_stream(self) -> None:
-        if self.is_streaming:
+        with QMutexLocker(self._camera_mutex):
+            if not self.is_streaming or self.timer is None:
+                return
+            
             self.is_streaming = False
-            if self.timer:
+            self.timer.stop()
-                # self.timer.stop()
-                self._enable_timer.emit(False)
 
     def _update_frame(self) -> None:
+        # This method is called by the timer, which is in the same thread.
+        # A mutex is still good practice for accessing the shared camera object.
         with QMutexLocker(self._camera_mutex):
-            if self.camera is None or not self.is_connected:
+            if self.camera is None or not self.is_connected or not self.is_streaming:
-                return
-        
-            if not self.is_streaming:
                 return
         
             ret, frame = self.camera.get_frame()
 
             if not ret:
-                self.error_occurred.emit(self.camera.get_error_msg())
+                error_msg = self.camera.get_error_msg()
+                if error_msg:
+                    self.error_occurred.emit(error_msg)
                 return
 
         if frame is not None:
+            # Process the frame and emit it.
             rgb_image = cv2.cvtColor(frame, cv2.COLOR_BGR2RGB)
             h, w, ch = rgb_image.shape
             qimg = QImage(rgb_image.data, w, h, ch * w, QImage.Format.Format_RGB888)
             pixmap = QPixmap.fromImage(qimg)
-
             self.frame_ready.emit(pixmap)
             
-    def _set_timer(self, enable: bool):
-        if not self.timer:
-            return
-        
-        if enable:
-            self.timer.setInterval(int(1000 / self.fps))
-            self.timer.start()
-        else:
-            self.timer.stop()
-            

core/camera/camera_manager.py

@@ -1,4 +1,4 @@
-from PySide6.QtCore import QObject, Signal, QRunnable, QThreadPool
+from PySide6.QtCore import QObject, Signal, QRunnable, QThreadPool, QThread
 from PySide6.QtGui import QPixmap
 
 from .camera_controller import CameraController
@@ -55,30 +55,50 @@ class CameraManager(QObject):
     Zarządza wszystkimi operacjami związanymi z kamerami,
     stanowiąc fasadę dla reszty aplikacji.
     """
+    # --- Public API Signals ---
     frame_ready = Signal(QPixmap)
     error_occurred = Signal(str)
-    
     detection_started = Signal()
     cameras_detected = Signal(list)
-
     camera_started = Signal()
     camera_stopped = Signal()
 
+    # --- Internal signals to communicate with worker thread ---
+    _request_set_camera = Signal(BaseCamera, int)
+    _request_start_camera = Signal()
+    _request_stop_camera = Signal()
+    _request_start_stream = Signal()
+    _request_stop_stream = Signal()
+
     def __init__(self, parent: QObject | None = None) -> None:
         super().__init__(parent)
-        self._camera_controller = CameraController()
-        self._detected_cameras: list[dict] = []
-        self._active_camera: BaseCamera | None = None
-        self._active_camera_info: dict | None = None
-        self.thread_pool = QThreadPool.globalInstance()
         
+        self._camera_thread = QThread()
+        self._camera_controller = CameraController()
+        self._camera_controller.moveToThread(self._camera_thread)
+
+        # --- Connections ---
+        # Connect signals from controller to be re-emitted by manager
         self._camera_controller.frame_ready.connect(self.frame_ready)
         self._camera_controller.error_occurred.connect(self.error_occurred)
         
+        # Connect internal requests to controller slots
+        self._request_set_camera.connect(self._camera_controller.set_camera)
+        self._request_start_camera.connect(self._camera_controller.start_camera)
+        self._request_stop_camera.connect(self._camera_controller.stop_camera)
+        self._request_start_stream.connect(self._camera_controller.start_stream)
+        self._request_stop_stream.connect(self._camera_controller.stop_stream)
+
+        # Connect thread management
+        self._camera_thread.started.connect(self._camera_controller.run)
+        
+        self._camera_thread.start()
+
+        self._detected_cameras: list[dict] = []
+        self._active_camera_info: dict | None = None
+        self.thread_pool = QThreadPool.globalInstance() # For detection worker
+
     def detect_cameras(self) -> None:
-        """
-        Rozpoczyna asynchroniczne wykrywanie kamer w osobnym wątku.
-        """
         self.detection_started.emit()
         worker = CameraDetectionWorker()
         worker.signals.finished.connect(self._on_detection_finished)
@@ -86,9 +106,6 @@ class CameraManager(QObject):
         self.thread_pool.start(worker)
 
     def _on_detection_finished(self, detected_cameras: list):
-        """
-        Slot wywoływany po zakończeniu pracy workera wykrywającego kamery.
-        """
         self._detected_cameras = detected_cameras
         self.cameras_detected.emit(self._detected_cameras)
 
@@ -96,8 +113,10 @@ class CameraManager(QObject):
         return self._detected_cameras
 
     def start_camera(self, camera_id: str, fps: int = 15) -> None:
-        """Uruchamia wybraną kamerę."""
+        if self._active_camera_info and self._active_camera_info['id'] == camera_id:
-        if self._active_camera:
+            return
+
+        if self._active_camera_info:
             self.stop_camera()
 
         camera_info = next((c for c in self._detected_cameras if c['id'] == camera_id), None)
@@ -109,34 +128,28 @@ class CameraManager(QObject):
         camera_type = camera_info['type']
         camera_index = camera_info['index']
 
+        camera_instance: BaseCamera | None = None
         if camera_type == "gphoto":
-            self._active_camera = GPhotoCamera()
+            camera_instance = GPhotoCamera()
         elif camera_type == "opencv":
-            self._active_camera = OpenCvCamera()
+            camera_instance = OpenCvCamera()
         else:
             self.error_occurred.emit(f"Nieznany typ kamery: {camera_type}")
             return
         
         self._active_camera_info = camera_info
         
-        self._camera_controller.set_camera(self._active_camera, fps)
+        # Emit signals to trigger slots in the worker thread
-        self._camera_controller.start_camera()
+        self._request_set_camera.emit(camera_instance, fps)
-        
+        self._request_start_camera.emit()
-        # Trzeba sprawdzić, czy połączenie się udało
+        self._request_start_stream.emit()
-        if self._camera_controller.is_connected:
-            self._camera_controller.start_stream()
         self.camera_started.emit()
-        else:
-            # Błąd został już wyemitowany przez CameraController
-            self._active_camera = None
-            self._active_camera_info = None
-
 
     def stop_camera(self) -> None:
-        """Zatrzymuje aktywną kamerę."""
+        if self._active_camera_info:
-        if self._active_camera:
+            # Emit signals to trigger slots in the worker thread
-            self._camera_controller.stop_camera()
+            self._request_stop_stream.emit()
-            self._active_camera = None
+            self._request_stop_camera.emit()
             self._active_camera_info = None
             self.camera_stopped.emit()
 
@@ -144,6 +157,9 @@ class CameraManager(QObject):
         return self._active_camera_info
 
     def shutdown(self) -> None:
-        """Zamyka kontroler kamery i jego wątek."""
+        if self._camera_thread.isRunning():
-        self.stop_camera()
+            self._camera_thread.quit()
-        self._camera_controller.stop()
+            if not self._camera_thread.wait(5000):
+                print("Camera thread did not finish gracefully, terminating.")
+                self._camera_thread.terminate()
+                self._camera_thread.wait()