diff --git a/software/control/_def.py b/software/control/_def.py index 182a0683e..53dbbc736 100644 --- a/software/control/_def.py +++ b/software/control/_def.py @@ -116,7 +116,7 @@ def convert_to_var(option: Union[str, "TriggerMode"]) -> "TriggerMode": class Acquisition: NUMBER_OF_FOVS_PER_AF = 3 - IMAGE_FORMAT = "bmp" + IMAGE_FORMAT = "tiff" IMAGE_DISPLAY_SCALING_FACTOR = 0.3 DX = 0.9 DY = 0.9 diff --git a/software/control/core/core.py b/software/control/core/core.py index ad1fa2526..0fd46cf3d 100644 --- a/software/control/core/core.py +++ b/software/control/core/core.py @@ -32,13 +32,15 @@ from typing import List, Tuple, Optional, Dict, Any, Callable, TypeVar -from queue import Queue +from queue import Queue, Empty, Full from threading import Thread, Lock from pathlib import Path from datetime import datetime +from dataclasses import dataclass from enum import Enum from control.models import AcquisitionChannel import time +import csv import itertools import json import math @@ -93,6 +95,23 @@ def set_display_resolution_scaling(self, display_resolution_scaling): self._handler.set_display_resolution_scaling(display_resolution_scaling) +log = squid.logging.get_logger(__name__) + + +@dataclass +class _DefaultRecordingChannel: + """Stand-in for an AcquisitionChannel when no live channel is set during recording. + + save_image() and return_pseudo_colored_image() only consult `.name`; the other + fields keep frames.csv rows uniform when no channel is available. + """ + + name: str = "live" + exposure_time: float = 0.0 + analog_gain: float = 0.0 + illumination_intensity: float = 0.0 + + class ImageSaver(QObject): stop_recording = Signal() @@ -110,53 +129,92 @@ def __init__(self, image_format=Acquisition.IMAGE_FORMAT): self.counter = 0 self.recording_start_time = 0 self.recording_time_limit = -1 + self._channel_provider: Optional[Callable[[], Optional[Any]]] = None + self._csv_file = None + self._csv_writer = None + self._dropped_count = 0 + self._last_queue_full_warning_ts = 0.0 + + def set_channel_provider(self, provider): + """Register a callable returning the active live channel (or None). + + Called per-frame from the saver thread. Cross-thread reads of the + channel reference are GIL-atomic, so no lock is needed; the GUI thread + just swaps the reference when the user toggles channels. + """ + self._channel_provider = provider def process_queue(self): while True: - # stop the thread if stop signal is received if self.stop_signal_received: return - # process the queue try: - [image, frame_ID, timestamp] = self.queue.get(timeout=0.1) - self.image_lock.acquire(True) - folder_ID = int(self.counter / self.max_num_image_per_folder) - file_ID = int(self.counter % self.max_num_image_per_folder) - # create a new folder - if file_ID == 0: - utils.ensure_directory_exists(os.path.join(self.base_path, self.experiment_ID, str(folder_ID))) - - if image.dtype == np.uint16: - # need to use tiff when saving 16 bit images - saving_path = os.path.join( - self.base_path, self.experiment_ID, str(folder_ID), str(file_ID) + "_" + str(frame_ID) + ".tiff" - ) - iio.imwrite(saving_path, image) - else: - saving_path = os.path.join( - self.base_path, - self.experiment_ID, - str(folder_ID), - str(file_ID) + "_" + str(frame_ID) + "." + self.image_format, + image, frame_id, timestamp = self.queue.get(timeout=0.1) + except Empty: + continue + try: + with self.image_lock: + folder_id = self.counter // self.max_num_image_per_folder + file_id = self.counter % self.max_num_image_per_folder + save_dir = os.path.join(self.base_path, self.experiment_ID, str(folder_id)) + if file_id == 0: + utils.ensure_directory_exists(save_dir) + + channel = ( + self._channel_provider() if self._channel_provider else None + ) or _DefaultRecordingChannel() + # Use get_image_filepath for channel-aware naming consistency with the + # multipoint pipeline, but write directly with cv2/iio for speed — + # save_image's imageio.imwrite path is ~10x slower than cv2.imwrite for + # uint8 BMPs and turns the saver into the FPS bottleneck. + saving_path = utils_acquisition.get_image_filepath( + save_dir, str(file_id), channel.name, image.dtype ) - cv2.imwrite(saving_path, image) - - self.counter = self.counter + 1 + if image.dtype == np.uint16: + iio.imwrite(saving_path, image) + else: + cv2.imwrite(saving_path, image) + + if self._csv_writer is not None: + rel_path = os.path.relpath(saving_path, os.path.join(self.base_path, self.experiment_ID)) + self._csv_writer.writerow( + [ + frame_id, + datetime.fromtimestamp(timestamp).isoformat(), + getattr(channel, "name", "live"), + getattr(channel, "exposure_time", 0.0), + getattr(channel, "analog_gain", 0.0), + getattr(channel, "illumination_intensity", 0.0), + rel_path, + ] + ) + # No per-row flush: it's an fsync per frame on most filesystems + # and was the second-largest contributor to the FPS regression. + # stop_experiment() flushes via close(). + + self.counter += 1 + except OSError as e: + log.error(f"Writer fatal error: {e}; stopping recording") + self.stop_recording.emit() + except Exception as e: + log.warning(f"Failed to write frame {frame_id}: {e}") + finally: self.queue.task_done() - self.image_lock.release() - except: - pass - def enqueue(self, image, frame_ID, timestamp): + def enqueue(self, image, frame_id, timestamp): try: - self.queue.put_nowait([image, frame_ID, timestamp]) - if (self.recording_time_limit > 0) and ( - time.time() - self.recording_start_time >= self.recording_time_limit - ): - self.stop_recording.emit() - # when using self.queue.put(str_), program can be slowed down despite multithreading because of the block and the GIL - except: - print("imageSaver queue is full, image discarded") + self.queue.put_nowait([image, frame_id, timestamp]) + except Full: + self._dropped_count += 1 + now = time.time() + if now - self._last_queue_full_warning_ts >= 1.0: + log.warning(f"Image queue full; frame {frame_id} dropped") + self._last_queue_full_warning_ts = now + return + + if self.recording_time_limit > 0 and time.time() - self.recording_start_time >= self.recording_time_limit: + log.info(f"Auto-stopping: time limit reached ({self.recording_time_limit}s)") + self.stop_recording.emit() def set_base_path(self, path): self.base_path = path @@ -165,23 +223,65 @@ def set_recording_time_limit(self, time_limit): self.recording_time_limit = time_limit def start_new_experiment(self, experiment_ID, add_timestamp=True): + # Defensively close any prior recording in case the caller didn't. + self.stop_experiment() + if add_timestamp: - # generate unique experiment ID self.experiment_ID = experiment_ID + "_" + datetime.now().strftime("%Y-%m-%d_%H-%M-%S.%f") else: self.experiment_ID = experiment_ID self.recording_start_time = time.time() - # create a new folder - try: - utils.ensure_directory_exists(os.path.join(self.base_path, self.experiment_ID)) - # to do: save configuration - except: - pass - # reset the counter self.counter = 0 + self._dropped_count = 0 - def close(self): + experiment_dir = os.path.join(self.base_path, self.experiment_ID) + try: + utils.ensure_directory_exists(experiment_dir) + except Exception as e: + log.error(f"Failed to create experiment directory {experiment_dir}: {e}") + raise + + csv_path = os.path.join(experiment_dir, "frames.csv") + self._csv_file = open(csv_path, "w", newline="") + self._csv_writer = csv.writer(self._csv_file) + self._csv_writer.writerow( + ["frame_id", "timestamp_iso", "channel", "exposure_ms", "gain", "illumination_intensity", "file"] + ) + self._csv_file.flush() + + log.info(f"Recording started: id={self.experiment_ID}, dir={experiment_dir}") + if self._channel_provider is None: + log.warning("channel_provider not set; frames tagged with default 'live' channel") + + def stop_experiment(self): + """Finalize the current recording: drain the queue, close frames.csv, log summary. + + Called by the widget on Stop and on time-limit auto-stop. Idempotent. + Drains the queue first so any buffered frames land in the current + experiment dir before experiment_ID is cleared — without that, the + saver thread races and writes to a non-existent directory. + """ + # Block until the saver thread has processed every buffered frame. + # Safe because the caller has already stopped the streamHandler, so no + # new items can be enqueued; queue.task_done() runs in finally so a + # write exception cannot leave us hanging here. self.queue.join() + if self._csv_file is not None: + try: + self._csv_file.close() + finally: + self._csv_file = None + self._csv_writer = None + if self.experiment_ID: + duration = time.time() - self.recording_start_time + log.info( + f"Recording stopped: frames_saved={self.counter}, " + f"dropped={self._dropped_count}, duration={duration:.1f}s" + ) + self.experiment_ID = "" + + def close(self): + self.stop_experiment() self.stop_signal_received = True self.thread.join() diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index 07414e56e..a7cd22b04 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -888,7 +888,11 @@ def load_widgets(self): self.emission_filter_wheel, self.liveController, config_repo=self.microscope.config_repo ) - self.recordingControlWidget = widgets.RecordingWidget(self.streamHandler, self.imageSaver) + self.recordingControlWidget = widgets.RecordingWidget( + self.streamHandler, + self.imageSaver, + channel_provider=lambda: self.liveController.currentConfiguration, + ) self.wellplateFormatWidget = widgets.WellplateFormatWidget( self.stage, self.navigationViewer, self.streamHandler, self.liveController ) diff --git a/software/control/widgets.py b/software/control/widgets.py index 52323c74e..4877da185 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -4410,11 +4410,11 @@ def update_displacement_um_display(self, displacement=None): class RecordingWidget(QFrame): - def __init__(self, streamHandler, imageSaver, main=None, *args, **kwargs): + def __init__(self, streamHandler, imageSaver, main=None, channel_provider=None, *args, **kwargs): super().__init__(*args, **kwargs) - self.imageSaver = imageSaver # for saving path control + self.imageSaver = imageSaver self.streamHandler = streamHandler - self.base_path_is_set = False + self._channel_provider = channel_provider self.add_components() self.setFrameStyle(QFrame.Panel | QFrame.Raised) @@ -4425,8 +4425,6 @@ def add_components(self): self.lineEdit_savingDir = QLineEdit() self.lineEdit_savingDir.setReadOnly(True) - self.lineEdit_savingDir.setText("Choose a base saving directory") - self.lineEdit_savingDir.setText(DEFAULT_SAVING_PATH) self.imageSaver.set_base_path(DEFAULT_SAVING_PATH) @@ -4485,26 +4483,24 @@ def add_components(self): self.imageSaver.stop_recording.connect(self.stop_recording) def set_saving_dir(self): - dialog = QFileDialog() - save_dir_base = dialog.getExistingDirectory(None, "Select Folder") + save_dir_base = QFileDialog.getExistingDirectory(None, "Select Folder") + if not save_dir_base: + return self.imageSaver.set_base_path(save_dir_base) self.lineEdit_savingDir.setText(save_dir_base) - self.base_path_is_set = True def toggle_recording(self, pressed): - if self.base_path_is_set == False: - self.btn_record.setChecked(False) - msg = QMessageBox() - msg.setText("Please choose base saving directory first") - msg.exec_() - return if pressed: self.lineEdit_experimentID.setEnabled(False) self.btn_setSavingDir.setEnabled(False) + if self._channel_provider is not None: + self.imageSaver.set_channel_provider(self._channel_provider) self.imageSaver.start_new_experiment(self.lineEdit_experimentID.text()) self.streamHandler.start_recording() else: self.streamHandler.stop_recording() + self.imageSaver.stop_experiment() + self.imageSaver.set_channel_provider(None) self.lineEdit_experimentID.setEnabled(True) self.btn_setSavingDir.setEnabled(True) @@ -4513,6 +4509,8 @@ def stop_recording(self): self.lineEdit_experimentID.setEnabled(True) self.btn_record.setChecked(False) self.streamHandler.stop_recording() + self.imageSaver.stop_experiment() + self.imageSaver.set_channel_provider(None) self.btn_setSavingDir.setEnabled(True) diff --git a/software/tests/control/test_def.py b/software/tests/control/test_def.py index e658db6f9..b04945dd7 100644 --- a/software/tests/control/test_def.py +++ b/software/tests/control/test_def.py @@ -227,3 +227,16 @@ def test_unrecognized_values_default_to_false(self): assert self._parse_sim_setting("fasle") is False assert self._parse_sim_setting("simualte") is False assert self._parse_sim_setting("invalid") is False + + +def test_default_image_format_is_tiff(): + """The in-code default for Acquisition.IMAGE_FORMAT is 'tiff' (was 'bmp'). + + INI files with [ACQUISITION] image_format=... override this at module + load time, so we assert against the source rather than the runtime value. + """ + import inspect + import control._def + + source = inspect.getsource(control._def.Acquisition) + assert 'IMAGE_FORMAT = "tiff"' in source diff --git a/software/tests/control/test_widgets.py b/software/tests/control/test_widgets.py index 7fc5ae99f..9c8c0c29a 100644 --- a/software/tests/control/test_widgets.py +++ b/software/tests/control/test_widgets.py @@ -1,13 +1,20 @@ import logging import os import tempfile +from pathlib import Path from unittest.mock import MagicMock, Mock, patch +import numpy as np import pytest +from qtpy.QtCore import Qt import control._def import control.microscope -from control.widgets import check_ram_available_with_error_dialog, NDViewerTab, SurfacePlotWidget +import control.widgets +from control.core import core as core_module +from control.widgets import check_ram_available_with_error_dialog, NDViewerTab, RecordingWidget, SurfacePlotWidget +from squid.abc import CameraFrame, CameraFrameFormat +from squid.config import CameraPixelFormat import tests.control.test_stubs as ts @@ -2356,6 +2363,231 @@ def test_expand_button_shows_only_dropped_when_single_message(self, warning_widg assert widget.btn_expand.isVisible() +# ============================================================================ +# RecordingWidget Tests +# ============================================================================ + + +@pytest.fixture +def recording_widget(qtbot, tmp_path, monkeypatch): + # `widgets.py` imports DEFAULT_SAVING_PATH via `from control._def import *`, which binds a + # separate name in the widgets module — both bindings must be patched. + monkeypatch.setattr(control._def, "DEFAULT_SAVING_PATH", str(tmp_path)) + monkeypatch.setattr(control.widgets, "DEFAULT_SAVING_PATH", str(tmp_path)) + + stream_handler = core_module.QtStreamHandler(accept_new_frame_fn=lambda: True) + image_saver = core_module.ImageSaver() + stream_handler.packet_image_to_write.connect(image_saver.enqueue) + + widget = RecordingWidget(stream_handler, image_saver) + qtbot.addWidget(widget) + try: + yield widget, stream_handler, image_saver + finally: + image_saver.close() + + +class TestRecordingWidget: + """Regression tests for the legacy single-camera RecordingWidget.""" + + def test_default_saving_path_applied_on_construction(self, recording_widget, tmp_path): + widget, _, image_saver = recording_widget + assert widget.lineEdit_savingDir.text() == str(tmp_path) + assert image_saver.base_path == str(tmp_path) + + def test_channel_change_mid_recording_reflected_in_filenames(self, qtbot, recording_widget): + widget, _, image_saver = recording_widget + + class _Stub: + def __init__(self, name): + self.name = name + self.exposure_time = 10.0 + self.analog_gain = 1.0 + self.illumination_intensity = 50.0 + + state = {"channel": _Stub("BF LED matrix full")} + image_saver.set_channel_provider(lambda: state["channel"]) + + widget.lineEdit_experimentID.setText("exp") + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + + img = np.zeros((8, 8), dtype=np.uint8) + image_saver.enqueue(img, 0, 0.0) + qtbot.waitUntil(lambda: image_saver.counter == 1, timeout=2000) + + state["channel"] = _Stub("488 nm") + image_saver.enqueue(img, 1, 1.0) + qtbot.waitUntil(lambda: image_saver.counter == 2, timeout=2000) + + exp_dir = Path(image_saver.base_path) / image_saver.experiment_ID + names = sorted(p.name for p in exp_dir.rglob("*") if p.suffix in (".tiff", ".bmp")) + assert any("BF_LED_matrix_full" in n for n in names), f"No BF file: {names}" + assert any("488_nm" in n for n in names), f"No 488 file: {names}" + + def test_widget_constructed_with_channel_provider(self, qtbot, tmp_path, monkeypatch): + """RecordingWidget accepts channel_provider kwarg and registers it on Record-press.""" + monkeypatch.setattr(control._def, "DEFAULT_SAVING_PATH", str(tmp_path)) + monkeypatch.setattr(control.widgets, "DEFAULT_SAVING_PATH", str(tmp_path)) + + stream_handler = core_module.QtStreamHandler(accept_new_frame_fn=lambda: True) + image_saver = core_module.ImageSaver() + stream_handler.packet_image_to_write.connect(image_saver.enqueue) + + sentinel = object() + provider = lambda: sentinel # noqa: E731 + + widget = RecordingWidget(stream_handler, image_saver, channel_provider=provider) + qtbot.addWidget(widget) + try: + widget.lineEdit_experimentID.setText("exp") + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + assert image_saver._channel_provider is provider + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + assert image_saver._channel_provider is None + finally: + image_saver.close() + + def test_close_does_not_hang_when_write_raises(self, qtbot, recording_widget): + """A write exception must still mark the queue item done; otherwise close() hangs on join().""" + widget, _, image_saver = recording_widget + widget.lineEdit_experimentID.setText("exp") + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + + with patch("control.utils_acquisition.save_image", side_effect=RuntimeError("boom")): + image_saver.enqueue(np.zeros((4, 4), dtype=np.uint8), 0, 0.0) + qtbot.waitUntil(lambda: image_saver.queue.empty(), timeout=2000) + + def test_recording_lifecycle_finalizes_csv_and_starts_fresh(self, qtbot, recording_widget, caplog): + """Stopping a recording closes frames.csv, logs the summary, and a subsequent + recording opens a brand-new CSV (no file-descriptor leak across sessions).""" + widget, _, image_saver = recording_widget + + widget.lineEdit_experimentID.setText("first") + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + first_csv = image_saver._csv_file + assert first_csv is not None and not first_csv.closed + + image_saver.enqueue(np.zeros((4, 4), dtype=np.uint8), 0, 0.0) + qtbot.waitUntil(lambda: image_saver.counter == 1, timeout=2000) + + with caplog.at_level(logging.INFO): + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + + assert first_csv.closed, "Stopping should close frames.csv" + assert image_saver._csv_file is None + assert any("Recording stopped" in rec.message and "frames_saved=1" in rec.message for rec in caplog.records) + + widget.lineEdit_experimentID.setText("second") + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + second_csv = image_saver._csv_file + assert second_csv is not None and second_csv is not first_csv + assert not second_csv.closed + + def test_directory_creation_failure_raises_and_logs(self, qtbot, recording_widget, caplog): + _, _, image_saver = recording_widget + with patch("control.utils.ensure_directory_exists", side_effect=OSError("mock disk error")): + with caplog.at_level(logging.ERROR): + with pytest.raises(OSError): + image_saver.start_new_experiment("doomed") + assert any("Failed to create experiment directory" in rec.message for rec in caplog.records) + + def test_queue_full_logs_warning_and_increments_dropped_count(self, qtbot, recording_widget, caplog): + widget, _, image_saver = recording_widget + widget.lineEdit_experimentID.setText("exp") + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + + # Block the saver thread by holding image_lock so the queue can't drain. + img = np.zeros((8, 8), dtype=np.uint8) + with image_saver.image_lock: + with caplog.at_level(logging.WARNING): + for frame_id in range(40): + image_saver.enqueue(img, frame_id, 0.0) + + assert image_saver._dropped_count > 0 + assert any("queue full" in rec.message.lower() for rec in caplog.records) + + def test_frames_csv_written_with_metadata(self, qtbot, recording_widget): + widget, _, image_saver = recording_widget + + class _Stub: + name = "488 nm" + exposure_time = 25.0 + analog_gain = 1.5 + illumination_intensity = 80.0 + + image_saver.set_channel_provider(lambda: _Stub()) + + widget.lineEdit_experimentID.setText("exp") + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + exp_id = image_saver.experiment_ID # stop_experiment will clear it + + # Enqueue directly so streamHandler's save-FPS throttle doesn't drop frames. + img = np.zeros((16, 16), dtype=np.uint8) + for i in range(3): + image_saver.enqueue(img, i * 30, 1000.0 + i) + + qtbot.waitUntil(lambda: image_saver.counter == 3, timeout=2000) + # Stop the recording so frames.csv is flushed and closed. + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + + csv_path = Path(image_saver.base_path) / exp_id / "frames.csv" + assert csv_path.is_file() + + import csv as _csv + + with csv_path.open() as f: + rows = list(_csv.DictReader(f)) + assert len(rows) == 3 + assert [r["frame_id"] for r in rows] == ["0", "30", "60"] + assert rows[0]["channel"] == "488 nm" + assert float(rows[0]["exposure_ms"]) == 25.0 + assert float(rows[0]["gain"]) == 1.5 + assert float(rows[0]["illumination_intensity"]) == 80.0 + assert rows[0]["file"].endswith(".tiff") or rows[0]["file"].endswith(".bmp") + + def test_record_without_browse_starts_recording(self, qtbot, recording_widget): + widget, _, image_saver = recording_widget + widget.lineEdit_experimentID.setText("exp") + + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + + assert widget.btn_record.isChecked() + assert image_saver.experiment_ID.startswith("exp_") + assert not widget.lineEdit_experimentID.isEnabled() + assert not widget.btn_setSavingDir.isEnabled() + + def test_browse_cancel_preserves_existing_path(self, recording_widget, tmp_path): + widget, _, image_saver = recording_widget + with patch("qtpy.QtWidgets.QFileDialog.getExistingDirectory", return_value=""): + widget.set_saving_dir() + assert image_saver.base_path == str(tmp_path) + assert widget.lineEdit_savingDir.text() == str(tmp_path) + + def test_frame_is_persisted_to_disk(self, qtbot, recording_widget): + widget, stream_handler, image_saver = recording_widget + widget.lineEdit_experimentID.setText("exp") + widget.entry_saveFPS.setValue(1000) + qtbot.mouseClick(widget.btn_record, Qt.LeftButton) + + frame = CameraFrame( + frame_id=0, + timestamp=0.0, + frame=np.zeros((32, 32), dtype=np.uint8), + frame_format=CameraFrameFormat.RAW, + frame_pixel_format=CameraPixelFormat.MONO8, + ) + stream_handler.get_frame_callback()(frame) + + qtbot.waitUntil(lambda: image_saver.counter > 0, timeout=2000) + + exp_dir = Path(image_saver.base_path) / image_saver.experiment_ID + files = [p for p in exp_dir.rglob("*") if p.is_file()] + assert files, f"No image saved under {exp_dir}" + # No channel provider was set, so save_image gets the default "live" sentinel. + # Filename comes from utils_acquisition.get_image_filepath: _. + assert any("live" in p.stem for p in files), f"Expected 'live' in filename; got {[p.name for p in files]}" + + class TestWarningErrorWidgetErrorExemptionWithDroppedCount: """Tests for error exemption interacting with dropped count."""