From 75ca73f148a975e69e42727228dcc375aab31681 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Wed, 18 Feb 2026 15:09:40 +0000 Subject: [PATCH 001/104] Decrease target brightness for both cameras as its pre-gamma --- .../things/camera/picamera_recalibrate_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index a11b6c91..71e247d3 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -85,7 +85,7 @@ IMX219_SENSOR_INFO = SensorInfo( unpacked_pixel_format="SBGGR10", bit_depth=10, blacklevel=64, - default_target_white_level=700, + default_target_white_level=300, short_pause=0.2, long_pause=0.5, ) @@ -95,7 +95,7 @@ IMX477_SENSOR_INFO = SensorInfo( unpacked_pixel_format="SBGGR12", bit_depth=12, blacklevel=256, - default_target_white_level=2800, + default_target_white_level=1200, short_pause=0.2, long_pause=1.0, ) From e324014a81d17031c5aa7f23aab1050c9cebf366 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Wed, 18 Feb 2026 16:00:38 +0000 Subject: [PATCH 002/104] Increase target exposure to 40% --- .../things/camera/picamera_recalibrate_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 71e247d3..77175f11 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -85,7 +85,7 @@ IMX219_SENSOR_INFO = SensorInfo( unpacked_pixel_format="SBGGR10", bit_depth=10, blacklevel=64, - default_target_white_level=300, + default_target_white_level=400, short_pause=0.2, long_pause=0.5, ) @@ -95,7 +95,7 @@ IMX477_SENSOR_INFO = SensorInfo( unpacked_pixel_format="SBGGR12", bit_depth=12, blacklevel=256, - default_target_white_level=1200, + default_target_white_level=1600, short_pause=0.2, long_pause=1.0, ) From 84f30320226226a365f0672756c914a75a24b019 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Tue, 3 Mar 2026 14:52:29 +0000 Subject: [PATCH 003/104] Update docstrings to explain lower brightness --- .../things/camera/picamera.py | 3 ++- .../things/camera/picamera_recalibrate_utils.py | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 40ad6bda..29afc35b 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -640,7 +640,8 @@ class StreamingPiCamera2(BaseCamera): :param target_white_level: Raw target white level, this should be an integer within the range set by the bit-depth of the camera sensor (10-bit for PiCamera v2, 12 Bit for Picamera HQ. If None the default will be used for - the current sensor. This is approximately 70% saturated. + the current sensor. This is approximately 40% saturated, but after gamma + curve is applied, the pixel values will have a value around 200. :param percentile: The percentile to use instead of maximum. Default 99.9. When calculating the brightest pixel, a percentile is used rather than the maximum in order to be robust to a small number of noisy/bright pixels. diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 77175f11..0a5637ab 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -120,10 +120,11 @@ def adjust_shutter_and_gain_from_raw( :param camera: A Picamera2 object. :param target_white_level: The raw value we aim for, the raw value of the brightest pixels should be approximately this bright. The value to set depends on the - sensor bit depth. We recommend values of 700 for 10-bit sensors and 2800 for - 12-bit sensors. This is about 70% of saturated once the blacklevel is + sensor bit depth. We recommend values of 400 for 10-bit sensors and 1600 for + 12-bit sensors. This is about 40% of saturated once the blacklevel is subtracted. The maximum possible value depends on the sensor bit depth, the - sensor blacklevel and the tolerance argument. + sensor blacklevel and the tolerance argument. While this only uses 40% of the + sensor range, after gamma this corresponds to pixel value ~200. :param max_iterations: We will terminate once we perform this many iterations, whether or not we converge. More than 10 shouldn't happen. :param tolerance: How close to the target value we consider "done". Expressed as a From 5fab6825e257498b37d4a5dcc7c590429d4453e3 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 4 Mar 2026 22:47:01 +0000 Subject: [PATCH 004/104] Update picamera tests --- picamera_coverage.zip | Bin 54038 -> 54038 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/picamera_coverage.zip b/picamera_coverage.zip index bd21c4667099fa1b3fb6feaff98cfd967c3581fb..4a6c18565efa1d51a3ede16982b80d3e405a658b 100644 GIT binary patch delta 311 zcmbQXjCtBJW}yIYW)=|!5D?pt64Rc|xp$+`gFa=68xM~Q%Q5gQ;Y?R&@L}L@%=g+q z)#1<5{YpLz?7EB&8{3&Di}pvVelu416Ucmkfo%=rod;|W>JKt7uz?I?U}j*DU|=g? zHu!NXe)Hu1_@;i86-Wnx;gv1w9DsKT zn44G_m{?jG86{d6f;CzCiB9IcJ)UsKaL9LBFL&SonV;{lwR*DCg^L;{28k(VNhygb<|!75DXB>*X$EE{1}P> Date: Thu, 5 Mar 2026 13:03:17 +0000 Subject: [PATCH 005/104] Add forbidden windows commands to stitching and utilities to check for file name sanitation. Unit tests also added --- .../stitching.py | 32 +++++++- .../utilities.py | 81 ++++++++++++++++++- tests/unit_tests/test_stitching_validation.py | 49 +++++++++++ tests/unit_tests/test_utilities.py | 71 ++++++++++++++++ 4 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 tests/unit_tests/test_stitching_validation.py create mode 100644 tests/unit_tests/test_utilities.py diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index 1f077ec6..0f6eb5e7 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/src/openflexure_microscope_server/stitching.py @@ -48,18 +48,46 @@ class StitcherValidationError(RuntimeError): """The stitcher received values that it deems unsafe to create a command from.""" +# A list of commands that are forbidden in any part of a generated CLI command. +# This provides defense-in-depth against trying to execute arbitrary shells +# or elevation tools. +_FORBIDDEN_COMMANDS = { + "sudo", + "sh", + "bash", + "python", + "python3", + "perl", + "ruby", + "php", + "nc", + "netcat", + "curl", + "wget", +} + + def validate_command(cmd: list[str]) -> None: - """Validate that the command only characters that are allowed in a path. + """Validate that the command only contains characters that are allowed in a path. The values in the commands should be numbers, commandline flags, paths, and executables. All of these should be allowed by ``make_path_safe``. + This also checks against a blacklist of forbidden commands for defense-in-depth. + :raises StitcherValidationError: if any element in the command is not safe. """ for element in cmd: + # Check against forbidden commands (case-insensitive) + if element.lower() in _FORBIDDEN_COMMANDS: + raise StitcherValidationError( + f"Invalid stiching command: Forbidden element '{element}' detected." + ) + + # Ensure characters are safe for a path component if element != make_path_safe(element): raise StitcherValidationError( - "Invalid stiching command: Contains unsafe characters." + f"Invalid stiching command: Element '{element}' contains unsafe characters." ) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index d6df4df9..c93bf6ed 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -83,6 +83,32 @@ _POSIX_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-/]") # Matches anything that isn't a-z, A-Z, 0-9, _, ., - _NAME_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-]") +# Windows reserved names (case-insensitive) +_WINDOWS_RESERVED_NAMES = { + "CON", + "PRN", + "AUX", + "NUL", + "COM1", + "COM2", + "COM3", + "COM4", + "COM5", + "COM6", + "COM7", + "COM8", + "COM9", + "LPT1", + "LPT2", + "LPT3", + "LPT4", + "LPT5", + "LPT6", + "LPT7", + "LPT8", + "LPT9", +} + def make_path_safe(unsafe_path_string: str) -> str: """Replace unsafe characters in a file path with underscores, preserving separators. @@ -94,16 +120,41 @@ def make_path_safe(unsafe_path_string: str) -> str: including path separators (forward slash on POSIX, and forward slash/backslash/colon on Windows). + Additionally, it sanitises each path component for Windows reserved names and + trailing dots/spaces to ensure maximum cross-platform compatibility. + :param unsafe_path_string: The original path string to sanitise. :returns: A version of the input string safe to use as a file path. """ + # Split by separators first to sanitise components independently + components = re.split(r"([/\\])", unsafe_path_string) + sanitised_components = [] + unsafe_character_pattern = ( _WINDOWS_UNSAFE_PATTERN if sys.platform.startswith("win") else _POSIX_UNSAFE_PATTERN ) - return unsafe_character_pattern.sub("_", unsafe_path_string) + + for component in components: + if component in ("/", "\\"): + sanitised_components.append(component) + elif component: + # 1. Strip trailing dots and spaces + # 2. Apply unsafe character regex + # 3. Check for reserved names + sanitised = component.rstrip(". ") + if not sanitised: + sanitised = "_" + else: + sanitised = unsafe_character_pattern.sub("_", sanitised) + + sanitised_components.append(_sanitise_reserved(sanitised)) + else: + sanitised_components.append(component) + + return "".join(sanitised_components) def make_name_safe(unsafe_name_string: str) -> str: @@ -112,11 +163,37 @@ def make_name_safe(unsafe_name_string: str) -> str: This excludes all path separators, ensuring the result is safe to use as a standalone filename or identifier component. + This also handles Windows reserved names and trailing dots/spaces. + :param unsafe_name_string: The original name string to sanitise. :returns: A version of the input string safe to use as a file name or identifier. """ - return _NAME_UNSAFE_PATTERN.sub("_", unsafe_name_string) + # 1. Strip trailing dots and spaces + # 2. Apply unsafe character regex + # 3. Check for reserved names + name = unsafe_name_string.rstrip(". ") + if not name: + return "_" + + name = _NAME_UNSAFE_PATTERN.sub("_", name) + return _sanitise_reserved(name) + + +def _sanitise_reserved(name: str) -> str: + """Check a name against Windows reserved names. + + :param name: The component to check. + :returns: The sanitised component. + """ + # Check for Windows reserved names. + # These are reserved even with an extension (e.g. NUL.txt). + # We check the part before the first dot. + base_name = name.split(".", maxsplit=1)[0].upper() + if base_name in _WINDOWS_RESERVED_NAMES: + return f"{name}_" + + return name class VersionData(BaseModel): diff --git a/tests/unit_tests/test_stitching_validation.py b/tests/unit_tests/test_stitching_validation.py new file mode 100644 index 00000000..0223847d --- /dev/null +++ b/tests/unit_tests/test_stitching_validation.py @@ -0,0 +1,49 @@ +"""Unit tests for stitching command validation.""" + +import pytest + +from openflexure_microscope_server.stitching import ( + StitcherValidationError, + validate_command, +) + + +def test_validate_command_success(): + """Test valid commands pass validation.""" + validate_command(["openflexure-stitch", "--stitching_mode", "all", "path/to/scan"]) + validate_command(["--resize", "0.5", "8192"]) + + +def test_validate_command_forbidden(): + """Test forbidden commands raise error.""" + with pytest.raises( + StitcherValidationError, match="Forbidden element 'sudo' detected" + ): + validate_command(["sudo", "rm", "-rf", "/"]) + + with pytest.raises( + StitcherValidationError, match="Forbidden element 'python' detected" + ): + validate_command(["python", "-c", "print('hello')"]) + + with pytest.raises( + StitcherValidationError, match="Forbidden element 'sh' detected" + ): + validate_command(["sh", "-c", "whoami"]) + + +def test_validate_command_unsafe_chars(): + """Test elements with unsafe characters raise error.""" + with pytest.raises(StitcherValidationError, match="contains unsafe characters"): + validate_command(["openflexure-stitch", "path;rm -rf /"]) + + with pytest.raises(StitcherValidationError, match="contains unsafe characters"): + validate_command(["openflexure-stitch", "path&echo hello"]) + + +def test_validate_command_case_insensitive(): + """Test forbidden command check is case-insensitive.""" + with pytest.raises( + StitcherValidationError, match="Forbidden element 'SUDO' detected" + ): + validate_command(["SUDO", "ls"]) diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py new file mode 100644 index 00000000..a9972a44 --- /dev/null +++ b/tests/unit_tests/test_utilities.py @@ -0,0 +1,71 @@ +"""Unit tests for utility functions.""" + +import sys + +from openflexure_microscope_server.utilities import make_name_safe, make_path_safe + + +def test_make_name_safe_basic(): + """Test basic functionality of make_name_safe.""" + assert make_name_safe("normal_name") == "normal_name" + assert make_name_safe("name with spaces") == "name_with_spaces" + assert make_name_safe("name/with/slashes") == "name_with_slashes" + assert make_name_safe("name\\with\\backslashes") == "name_with_backslashes" + + +def test_make_name_safe_trailing_chars(): + """Test that trailing dots and spaces are removed.""" + assert make_name_safe("name.") == "name" + assert make_name_safe("name ") == "name" + assert make_name_safe("name. ") == "name" + assert make_name_safe("name .") == "name" + assert make_name_safe(".") == "_" + assert make_name_safe(" ") == "_" + assert make_name_safe(". ") == "_" + + +def test_make_name_safe_reserved_names(): + """Test Windows reserved names.""" + assert make_name_safe("CON") == "CON_" + assert make_name_safe("con") == "con_" + assert make_name_safe("NUL.txt") == "NUL.txt_" + assert make_name_safe("COM1") == "COM1_" + assert make_name_safe("LPT9.tar.gz") == "LPT9.tar.gz_" + # Check that names STARTING with reserved names but not followed by . or end are safe + assert make_name_safe("CONSTANT") == "CONSTANT" + assert make_name_safe("CON2") == "CON2" + + +def test_make_path_safe_basic(): + """Test basic functionality of make_path_safe.""" + # Note: behavior depends on platform for separators + if sys.platform.startswith("win"): + assert make_path_safe("C:\\path\\to/file") == "C:\\path\\to/file" + else: + # On POSIX, \ is not a separator and might be replaced by _ if not in allowed pattern + # The regex for POSIX is [^a-zA-Z0-9_.\-/] + # And : is also not allowed + assert make_path_safe("path/to/file") == "path/to/file" + + +def test_make_path_safe_reserved_in_components(): + """Test reserved names within path components.""" + assert make_path_safe("path/CON/file") == "path/CON_/file" + assert make_path_safe("path/NUL.txt/file") == "path/NUL.txt_/file" + + if sys.platform.startswith("win"): + assert make_path_safe("C:/AUX/test") == "C:/AUX_/test" + else: + assert make_path_safe("C:/AUX/test") == "C_/AUX_/test" + + +def test_make_path_safe_trailing_in_components(): + """Test trailing chars in path components.""" + assert make_path_safe("path /to. /file ") == "path/to/file" + assert make_path_safe("path./to /file.") == "path/to/file" + + +def test_make_path_safe_separators(): + """Test that separators are preserved and components sanitised.""" + assert make_path_safe("a/b\\c") == "a/b\\c" + assert make_path_safe("a./b /c.") == "a/b/c" From 48f6e1b369ceb75e502478fcfeb59ff8df17032f Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Thu, 5 Mar 2026 13:35:21 +0000 Subject: [PATCH 006/104] Allow python and python3 as stitching commands. They are allowed. Move stitching tests into one file --- .../stitching.py | 2 - tests/unit_tests/test_stitching.py | 23 +++++++++ tests/unit_tests/test_stitching_validation.py | 49 ------------------- 3 files changed, 23 insertions(+), 51 deletions(-) delete mode 100644 tests/unit_tests/test_stitching_validation.py diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index 0f6eb5e7..bf5da43c 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/src/openflexure_microscope_server/stitching.py @@ -55,8 +55,6 @@ _FORBIDDEN_COMMANDS = { "sudo", "sh", "bash", - "python", - "python3", "perl", "ruby", "php", diff --git a/tests/unit_tests/test_stitching.py b/tests/unit_tests/test_stitching.py index 349d975c..5ae8842e 100644 --- a/tests/unit_tests/test_stitching.py +++ b/tests/unit_tests/test_stitching.py @@ -21,6 +21,7 @@ from openflexure_microscope_server.stitching import ( PreviewStitcher, StitcherValidationError, StitchingSettings, + validate_command, ) from ..shared_utils.lt_test_utils import LabThingsTestEnv @@ -32,6 +33,28 @@ THIS_DIR: str = os.path.dirname(os.path.realpath(__file__)) MOCK_STITCHER: str = os.path.join(THIS_DIR, "mock_stitching", "mock-stitch.py") +def test_validate_command_success(): + """Test valid commands pass validation.""" + validate_command(["openflexure-stitch", "--stitching_mode", "all", "path/to/scan"]) + validate_command(["--resize", "0.5", "8192"]) + + +def test_validate_command_forbidden(): + """Test forbidden commands raise error.""" + with pytest.raises( + StitcherValidationError, match="Forbidden element 'sudo' detected" + ): + validate_command(["sudo", "rm", "-rf", "/"]) + + with pytest.raises(StitcherValidationError, match="Forbidden element 'sh' detected"): + validate_command(["sh", "-c", "whoami"]) + + with pytest.raises( + StitcherValidationError, match="Forbidden element 'SUDO' detected" + ): + validate_command(["SUDO", "ls"]) + + def test_base_stitcher(): """Test the logic in BaseStitcher. diff --git a/tests/unit_tests/test_stitching_validation.py b/tests/unit_tests/test_stitching_validation.py deleted file mode 100644 index 0223847d..00000000 --- a/tests/unit_tests/test_stitching_validation.py +++ /dev/null @@ -1,49 +0,0 @@ -"""Unit tests for stitching command validation.""" - -import pytest - -from openflexure_microscope_server.stitching import ( - StitcherValidationError, - validate_command, -) - - -def test_validate_command_success(): - """Test valid commands pass validation.""" - validate_command(["openflexure-stitch", "--stitching_mode", "all", "path/to/scan"]) - validate_command(["--resize", "0.5", "8192"]) - - -def test_validate_command_forbidden(): - """Test forbidden commands raise error.""" - with pytest.raises( - StitcherValidationError, match="Forbidden element 'sudo' detected" - ): - validate_command(["sudo", "rm", "-rf", "/"]) - - with pytest.raises( - StitcherValidationError, match="Forbidden element 'python' detected" - ): - validate_command(["python", "-c", "print('hello')"]) - - with pytest.raises( - StitcherValidationError, match="Forbidden element 'sh' detected" - ): - validate_command(["sh", "-c", "whoami"]) - - -def test_validate_command_unsafe_chars(): - """Test elements with unsafe characters raise error.""" - with pytest.raises(StitcherValidationError, match="contains unsafe characters"): - validate_command(["openflexure-stitch", "path;rm -rf /"]) - - with pytest.raises(StitcherValidationError, match="contains unsafe characters"): - validate_command(["openflexure-stitch", "path&echo hello"]) - - -def test_validate_command_case_insensitive(): - """Test forbidden command check is case-insensitive.""" - with pytest.raises( - StitcherValidationError, match="Forbidden element 'SUDO' detected" - ): - validate_command(["SUDO", "ls"]) From fa9ea778127f338a973854d75c26939a2b787708 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Thu, 5 Mar 2026 13:42:55 +0000 Subject: [PATCH 007/104] Ruff check --- tests/unit_tests/test_stitching.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/test_stitching.py b/tests/unit_tests/test_stitching.py index 5ae8842e..34d7fe7d 100644 --- a/tests/unit_tests/test_stitching.py +++ b/tests/unit_tests/test_stitching.py @@ -46,7 +46,9 @@ def test_validate_command_forbidden(): ): validate_command(["sudo", "rm", "-rf", "/"]) - with pytest.raises(StitcherValidationError, match="Forbidden element 'sh' detected"): + with pytest.raises( + StitcherValidationError, match="Forbidden element 'sh' detected" + ): validate_command(["sh", "-c", "whoami"]) with pytest.raises( From 35efba19b847ec8574ab619b6e2f6c86c662ec6d Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Thu, 5 Mar 2026 13:55:05 +0000 Subject: [PATCH 008/104] Use pytest parametrize for better coverage of forbidden commands and reserved names --- .../stitching.py | 35 +++++++++---------- tests/unit_tests/test_stitching.py | 19 +++++----- tests/unit_tests/test_utilities.py | 31 +++++++++++----- 3 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index bf5da43c..393eb32e 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/src/openflexure_microscope_server/stitching.py @@ -29,6 +29,22 @@ STITCH_TILE_SIZE = 8192 DEFAULT_OVERLAP = 0.1 DEFAULT_RESIZE = 0.5 +# A list of commands that are forbidden in any part of a generated CLI command. +# This provides defense-in-depth against trying to execute arbitrary shells +# or elevation tools. +FORBIDDEN_COMMANDS = { + "sudo", + "sh", + "bash", + "perl", + "ruby", + "php", + "nc", + "netcat", + "curl", + "wget", +} + class StitchingSettings(BaseModel): """The data needed to stitch a scan.""" @@ -48,23 +64,6 @@ class StitcherValidationError(RuntimeError): """The stitcher received values that it deems unsafe to create a command from.""" -# A list of commands that are forbidden in any part of a generated CLI command. -# This provides defense-in-depth against trying to execute arbitrary shells -# or elevation tools. -_FORBIDDEN_COMMANDS = { - "sudo", - "sh", - "bash", - "perl", - "ruby", - "php", - "nc", - "netcat", - "curl", - "wget", -} - - def validate_command(cmd: list[str]) -> None: """Validate that the command only contains characters that are allowed in a path. @@ -77,7 +76,7 @@ def validate_command(cmd: list[str]) -> None: """ for element in cmd: # Check against forbidden commands (case-insensitive) - if element.lower() in _FORBIDDEN_COMMANDS: + if element.lower() in FORBIDDEN_COMMANDS: raise StitcherValidationError( f"Invalid stiching command: Forbidden element '{element}' detected." ) diff --git a/tests/unit_tests/test_stitching.py b/tests/unit_tests/test_stitching.py index 34d7fe7d..ca5b9a08 100644 --- a/tests/unit_tests/test_stitching.py +++ b/tests/unit_tests/test_stitching.py @@ -16,6 +16,7 @@ from pydantic import BaseModel import labthings_fastapi as lt from openflexure_microscope_server.stitching import ( + FORBIDDEN_COMMANDS, BaseStitcher, FinalStitcher, PreviewStitcher, @@ -39,22 +40,20 @@ def test_validate_command_success(): validate_command(["--resize", "0.5", "8192"]) -def test_validate_command_forbidden(): +@pytest.mark.parametrize("cmd_name", FORBIDDEN_COMMANDS) +def test_validate_command_forbidden(cmd_name): """Test forbidden commands raise error.""" + # As the main command with pytest.raises( - StitcherValidationError, match="Forbidden element 'sudo' detected" + StitcherValidationError, match=f"Forbidden element '{cmd_name}' detected" ): - validate_command(["sudo", "rm", "-rf", "/"]) + validate_command([cmd_name, "some_arg"]) + # As an argument (case-insensitive) with pytest.raises( - StitcherValidationError, match="Forbidden element 'sh' detected" + StitcherValidationError, match=f"Forbidden element '{cmd_name.upper()}' detected" ): - validate_command(["sh", "-c", "whoami"]) - - with pytest.raises( - StitcherValidationError, match="Forbidden element 'SUDO' detected" - ): - validate_command(["SUDO", "ls"]) + validate_command(["safe-command", cmd_name.upper()]) def test_base_stitcher(): diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index a9972a44..dc8e3ae1 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -2,10 +2,17 @@ import sys -from openflexure_microscope_server.utilities import make_name_safe, make_path_safe +import pytest + +from openflexure_microscope_server.utilities import ( + _WINDOWS_RESERVED_NAMES, + make_name_safe, + make_path_safe, +) def test_make_name_safe_basic(): + """Test basic functionality of make_name_safe.""" assert make_name_safe("normal_name") == "normal_name" assert make_name_safe("name with spaces") == "name_with_spaces" @@ -24,16 +31,24 @@ def test_make_name_safe_trailing_chars(): assert make_name_safe(". ") == "_" -def test_make_name_safe_reserved_names(): +@pytest.mark.parametrize("name", _WINDOWS_RESERVED_NAMES) +def test_make_name_safe_reserved_names(name): """Test Windows reserved names.""" - assert make_name_safe("CON") == "CON_" - assert make_name_safe("con") == "con_" - assert make_name_safe("NUL.txt") == "NUL.txt_" - assert make_name_safe("COM1") == "COM1_" - assert make_name_safe("LPT9.tar.gz") == "LPT9.tar.gz_" - # Check that names STARTING with reserved names but not followed by . or end are safe + # Base name should be sanitized + assert make_name_safe(name) == f"{name}_" + # Case-insensitive + assert make_name_safe(name.lower()) == f"{name.lower()}_" + # With extension + assert make_name_safe(f"{name}.txt") == f"{name}.txt_" + # Multiple extensions + assert make_name_safe(f"{name}.tar.gz") == f"{name}.tar.gz_" + + +def test_make_name_safe_reserved_names_false_positives(): + """Test that names containing but not equal to reserved names are safe.""" assert make_name_safe("CONSTANT") == "CONSTANT" assert make_name_safe("CON2") == "CON2" + assert make_name_safe("ICON") == "ICON" def test_make_path_safe_basic(): From f52ce85b3e19b3638c60145de797351dfd19abdb Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Thu, 5 Mar 2026 14:04:33 +0000 Subject: [PATCH 009/104] Ruff formatting --- tests/unit_tests/test_stitching.py | 3 ++- tests/unit_tests/test_utilities.py | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/test_stitching.py b/tests/unit_tests/test_stitching.py index ca5b9a08..ead2276b 100644 --- a/tests/unit_tests/test_stitching.py +++ b/tests/unit_tests/test_stitching.py @@ -51,7 +51,8 @@ def test_validate_command_forbidden(cmd_name): # As an argument (case-insensitive) with pytest.raises( - StitcherValidationError, match=f"Forbidden element '{cmd_name.upper()}' detected" + StitcherValidationError, + match=f"Forbidden element '{cmd_name.upper()}' detected", ): validate_command(["safe-command", cmd_name.upper()]) diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index dc8e3ae1..bd7f4ce3 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -12,7 +12,6 @@ from openflexure_microscope_server.utilities import ( def test_make_name_safe_basic(): - """Test basic functionality of make_name_safe.""" assert make_name_safe("normal_name") == "normal_name" assert make_name_safe("name with spaces") == "name_with_spaces" From 7a17c92d3ee073f34c599e6d425d9e1a75ffe82f Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Thu, 5 Mar 2026 15:13:42 +0000 Subject: [PATCH 010/104] Apply suggestions from code review of branch 523-improve-validation-checks Co-authored-by: Julian Stirling --- tests/unit_tests/test_utilities.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index bd7f4ce3..73b08a1f 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -48,6 +48,7 @@ def test_make_name_safe_reserved_names_false_positives(): assert make_name_safe("CONSTANT") == "CONSTANT" assert make_name_safe("CON2") == "CON2" assert make_name_safe("ICON") == "ICON" + assert make_name_safe("CHILLI CON CARNE") == "CHILLI CON CARNE" def test_make_path_safe_basic(): From 76e80b299f9c6764b377fb6dedc34f1d6c9a9587 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 5 Mar 2026 16:29:49 +0000 Subject: [PATCH 011/104] Align buttons in confirmation modal --- webapp/src/assets/less/theme.less | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/webapp/src/assets/less/theme.less b/webapp/src/assets/less/theme.less index 237f0649..f1d98304 100644 --- a/webapp/src/assets/less/theme.less +++ b/webapp/src/assets/less/theme.less @@ -242,6 +242,10 @@ h4, .uk-h4 { border-radius: 0 0 @paper-border-radius @paper-border-radius; } +.uk-modal-footer button.uk-button { + vertical-align: baseline; +} + .uk-modal-header { border-radius: @paper-border-radius @paper-border-radius 0 0; } From 713df248ffc1eb8d2d5df0255488ff17895f3577 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 6 Mar 2026 09:53:41 +0000 Subject: [PATCH 012/104] Add underscores to fix unit test --- tests/unit_tests/test_utilities.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index 73b08a1f..52f17c3f 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -48,7 +48,7 @@ def test_make_name_safe_reserved_names_false_positives(): assert make_name_safe("CONSTANT") == "CONSTANT" assert make_name_safe("CON2") == "CON2" assert make_name_safe("ICON") == "ICON" - assert make_name_safe("CHILLI CON CARNE") == "CHILLI CON CARNE" + assert make_name_safe("CHILLI CON CARNE") == "CHILLI_CON_CARNE" def test_make_path_safe_basic(): From e5429d6cda89f1ec2bce9454d93d48d1367f906c Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 6 Mar 2026 14:07:07 +0000 Subject: [PATCH 013/104] Change make_path_safe to allow single dot relative imports, but no dots anywhere else. Added new unit tests for this. --- .../utilities.py | 16 +++++++++++++- tests/unit_tests/test_utilities.py | 22 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index c93bf6ed..c4b328d4 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -137,9 +137,23 @@ def make_path_safe(unsafe_path_string: str) -> str: else _POSIX_UNSAFE_PATTERN ) - for component in components: + for i, component in enumerate(components): if component in ("/", "\\"): sanitised_components.append(component) + elif component == ".": + # Only allow '.' if its the very first + # element and the next element is a separator. + # This allows for relative paths like "./file" + # but not "file./file" + is_first = i == 0 + next_to_sep = (i + 1 < len(components)) and ( + components[i + 1] in ("/", "\\") + ) + + if is_first and next_to_sep: + sanitised_components.append(component) + else: + sanitised_components.append("_") elif component: # 1. Strip trailing dots and spaces # 2. Apply unsafe character regex diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index 52f17c3f..84c25dc3 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -79,8 +79,30 @@ def test_make_path_safe_trailing_in_components(): assert make_path_safe("path /to. /file ") == "path/to/file" assert make_path_safe("path./to /file.") == "path/to/file" + # Allow dots at the start of the path + # As this is common for hidden files on POSIX and relative paths + # And is the configuration in the manual and simulation config + # json files. + assert make_path_safe("./path/to/file") == "./path/to/file" + assert make_path_safe("./path./to /file.") == "./path/to/file" + assert make_path_safe("./openflexure/data/") == "./openflexure/data/" + def test_make_path_safe_separators(): """Test that separators are preserved and components sanitised.""" assert make_path_safe("a/b\\c") == "a/b\\c" assert make_path_safe("a./b /c.") == "a/b/c" + + +def test_make_path_safe_relative(): + """Test that relative path components are preserved. + We only allow relative paths with one dot, and at the beginning of the path, + to avoid issues with paths like "file./file" or "file../file + """ + assert make_path_safe("./openflexure/data/") == "./openflexure/data/" + + assert make_path_safe("../openflexure/data/") == "_/openflexure/data/" + assert make_path_safe("path/./to/file") == "path/_/to/file" + assert make_path_safe("path/../to/file") == "path/_/to/file" + assert make_path_safe(".") == "_" + assert make_path_safe("..") == "_" From 4395d4001c265ff9e040da18d34fed8d9b16fd73 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 6 Mar 2026 14:13:00 +0000 Subject: [PATCH 014/104] Allow relative imports with two dots at start of path. Update unit tests. --- src/openflexure_microscope_server/utilities.py | 10 +++++----- tests/unit_tests/test_utilities.py | 9 ++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index c4b328d4..5fa1db8f 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -140,11 +140,11 @@ def make_path_safe(unsafe_path_string: str) -> str: for i, component in enumerate(components): if component in ("/", "\\"): sanitised_components.append(component) - elif component == ".": - # Only allow '.' if its the very first - # element and the next element is a separator. - # This allows for relative paths like "./file" - # but not "file./file" + elif component in (".", ".."): + # Only allow '.' and '..' if they are the very first + # elements and the next element is a separator. + # This allows for relative paths like "./file" or "../file" + # but not "file./file" or "file/../file" is_first = i == 0 next_to_sep = (i + 1 < len(components)) and ( components[i + 1] in ("/", "\\") diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index 84c25dc3..52b94fad 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -95,13 +95,12 @@ def test_make_path_safe_separators(): def test_make_path_safe_relative(): - """Test that relative path components are preserved. - We only allow relative paths with one dot, and at the beginning of the path, - to avoid issues with paths like "file./file" or "file../file - """ + """Test that relative path components are preserved.""" + # We only allow relative paths with one dot, and at the beginning of the path, + # to avoid issues with paths like "file./file" or "file../file. assert make_path_safe("./openflexure/data/") == "./openflexure/data/" + assert make_path_safe("../openflexure/data/") == "../openflexure/data/" - assert make_path_safe("../openflexure/data/") == "_/openflexure/data/" assert make_path_safe("path/./to/file") == "path/_/to/file" assert make_path_safe("path/../to/file") == "path/_/to/file" assert make_path_safe(".") == "_" From 35f7adcf07207dd0328f92c7a44763acf31922cf Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 6 Mar 2026 14:16:42 +0000 Subject: [PATCH 015/104] Codespell --- src/openflexure_microscope_server/utilities.py | 2 +- tests/unit_tests/test_config_utilities.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 5fa1db8f..2f6c90a4 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -382,7 +382,7 @@ def merge_patch( :param target: The target object :param patch: The patch to be applied - :param enforce_dict: Boolean, set True enfoces that the target and patch are both + :param enforce_dict: Boolean, set True enforces that the target and patch are both dictionaries. """ if enforce_dict and not (isinstance(target, dict) and isinstance(patch, dict)): diff --git a/tests/unit_tests/test_config_utilities.py b/tests/unit_tests/test_config_utilities.py index cd72b7d0..4fb4b91c 100644 --- a/tests/unit_tests/test_config_utilities.py +++ b/tests/unit_tests/test_config_utilities.py @@ -37,7 +37,7 @@ from openflexure_microscope_server import utilities def test_merge_patch(target, patch, outcome, err_if_enforce): """Check that merge_patch returns the expected outcome for each target and patch. - These test cases are specified in Appedix A of IETF RFC 7396. + These test cases are specified in Appendix A of IETF RFC 7396. """ assert utilities.merge_patch(target, patch) == outcome From 011fc5a79525436e4f72c519f6f1b6773d983780 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 6 Mar 2026 15:06:27 +0000 Subject: [PATCH 016/104] Fix spellings found as codespell updates --- src/openflexure_microscope_server/utilities.py | 2 +- tests/unit_tests/test_config_utilities.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index d6df4df9..d38703eb 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -291,7 +291,7 @@ def merge_patch( :param target: The target object :param patch: The patch to be applied - :param enforce_dict: Boolean, set True enfoces that the target and patch are both + :param enforce_dict: Boolean, set True enforces that the target and patch are both dictionaries. """ if enforce_dict and not (isinstance(target, dict) and isinstance(patch, dict)): diff --git a/tests/unit_tests/test_config_utilities.py b/tests/unit_tests/test_config_utilities.py index cd72b7d0..4fb4b91c 100644 --- a/tests/unit_tests/test_config_utilities.py +++ b/tests/unit_tests/test_config_utilities.py @@ -37,7 +37,7 @@ from openflexure_microscope_server import utilities def test_merge_patch(target, patch, outcome, err_if_enforce): """Check that merge_patch returns the expected outcome for each target and patch. - These test cases are specified in Appedix A of IETF RFC 7396. + These test cases are specified in Appendix A of IETF RFC 7396. """ assert utilities.merge_patch(target, patch) == outcome From 2c2f97cb5627b760a014f9340e9b6626b5b76695 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 6 Mar 2026 15:37:01 +0000 Subject: [PATCH 017/104] Force file names to be lower case to avoid Windows issues around case sensitivity when checking if a file already exists. Path names can be uppercase as these will be defined on a users system and pre-exist. Files are created by us so we can force sanitation --- .../utilities.py | 18 +++++++---- tests/unit_tests/test_scan_directories.py | 30 +++++++++++++++++++ tests/unit_tests/test_utilities.py | 19 +++++++----- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 2f6c90a4..a13cf19d 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -164,7 +164,9 @@ def make_path_safe(unsafe_path_string: str) -> str: else: sanitised = unsafe_character_pattern.sub("_", sanitised) - sanitised_components.append(_sanitise_reserved(sanitised)) + sanitised_components.append( + _sanitise_reserved(sanitised, is_filename=False) + ) else: sanitised_components.append(component) @@ -194,20 +196,26 @@ def make_name_safe(unsafe_name_string: str) -> str: return _sanitise_reserved(name) -def _sanitise_reserved(name: str) -> str: +def _sanitise_reserved(name: str, is_filename: bool = True) -> str: """Check a name against Windows reserved names. :param name: The component to check. - :returns: The sanitised component. + :param is_filename: Whether the component is a filename. + If True, names will be forced into lowercase. + :returns: The sanitised component, in lower case. """ # Check for Windows reserved names. # These are reserved even with an extension (e.g. NUL.txt). # We check the part before the first dot. base_name = name.split(".", maxsplit=1)[0].upper() if base_name in _WINDOWS_RESERVED_NAMES: - return f"{name}_" + return f"{name.lower()}_" if is_filename else f"{name}_" - return name + # We force names to be lowercase to avoid issues on + # Windows where files with the same name + # but different case can cause problems when checking if + # a file already exists. + return name.lower() if is_filename else name class VersionData(BaseModel): diff --git a/tests/unit_tests/test_scan_directories.py b/tests/unit_tests/test_scan_directories.py index 603587a3..80121a32 100644 --- a/tests/unit_tests/test_scan_directories.py +++ b/tests/unit_tests/test_scan_directories.py @@ -247,6 +247,36 @@ def test_scan_sequence_and_listing(caplog): assert len(caplog.records) == 0 +def test_scan_sequence_case_sensitive(): + """Check created scans are added in order and listed correctly, even when cases are different.""" + _clear_scan_dir() + scan_dir_manager = ScanDirectoryManager(BASE_SCAN_DIR) + + # Create some scan data and mark it as successful to get an end date. + scan_data = fake_active_scan_data() + scan_data.set_final_data(result="Success") + # Make 4 scans + scan_dir = scan_dir_manager.new_scan_dir("fake_scan") + scan_dir.save_scan_data(scan_data) + scan_dir = scan_dir_manager.new_scan_dir("FAKE_SCAN") + scan_dir.save_scan_data(scan_data) + scan_dir = scan_dir_manager.new_scan_dir("Fake_Scan") + scan_dir.save_scan_data(scan_data) + scan_dir = scan_dir_manager.new_scan_dir("FAKE_scan") + scan_dir.save_scan_data(scan_data) + scan_dir = scan_dir_manager.new_scan_dir("fAkE_sCaN") + scan_dir.save_scan_data(scan_data) + + # Check they exist and are numbered sequentially + all_scans = scan_dir_manager.all_scans + assert_unique_of_length(all_scans, 5) + assert "fake_scan_0001" in all_scans + assert "fake_scan_0002" in all_scans + assert "fake_scan_0003" in all_scans + assert "fake_scan_0004" in all_scans + assert "fake_scan_0005" in all_scans + + def test_scan_name_non_sequential(): """Check new scan has the correct name if the directories are not sequential.""" _clear_scan_dir() diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index 52b94fad..dbff78e4 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -17,6 +17,7 @@ def test_make_name_safe_basic(): assert make_name_safe("name with spaces") == "name_with_spaces" assert make_name_safe("name/with/slashes") == "name_with_slashes" assert make_name_safe("name\\with\\backslashes") == "name_with_backslashes" + assert make_name_safe("sPoNgEmoCk") == "spongemock" def test_make_name_safe_trailing_chars(): @@ -34,21 +35,25 @@ def test_make_name_safe_trailing_chars(): def test_make_name_safe_reserved_names(name): """Test Windows reserved names.""" # Base name should be sanitized - assert make_name_safe(name) == f"{name}_" + assert make_name_safe(name) == f"{name.lower()}_" # Case-insensitive assert make_name_safe(name.lower()) == f"{name.lower()}_" # With extension - assert make_name_safe(f"{name}.txt") == f"{name}.txt_" + assert make_name_safe(f"{name}.txt") == f"{name.lower()}.txt_" # Multiple extensions - assert make_name_safe(f"{name}.tar.gz") == f"{name}.tar.gz_" + assert make_name_safe(f"{name}.tar.gz") == f"{name.lower()}.tar.gz_" def test_make_name_safe_reserved_names_false_positives(): """Test that names containing but not equal to reserved names are safe.""" - assert make_name_safe("CONSTANT") == "CONSTANT" - assert make_name_safe("CON2") == "CON2" - assert make_name_safe("ICON") == "ICON" - assert make_name_safe("CHILLI CON CARNE") == "CHILLI_CON_CARNE" + assert make_name_safe("CONSTANT") == "constant" + assert make_name_safe("CON2") == "con2" + assert make_name_safe("ICON") == "icon" + assert make_name_safe("icon") == "icon" + assert make_name_safe("iCon") == "icon" + assert make_name_safe("iCOn") == "icon" + assert make_name_safe("icOn") == "icon" + assert make_name_safe("CHILLI CON CARNE") == "chilli_con_carne" def test_make_path_safe_basic(): From 349485d43bf655b685e5be3e68b89700172643f1 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 6 Mar 2026 15:52:50 +0000 Subject: [PATCH 018/104] Remove multi-underscores from allowed names --- .../scan_directories.py | 3 ++ tests/unit_tests/test_scan_directories.py | 29 ++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/openflexure_microscope_server/scan_directories.py b/src/openflexure_microscope_server/scan_directories.py index 295c36dd..4f2e5215 100644 --- a/src/openflexure_microscope_server/scan_directories.py +++ b/src/openflexure_microscope_server/scan_directories.py @@ -353,6 +353,9 @@ class ScanDirectoryManager: For more explanation on the scan naming see `new_scan_dir` """ scan_name = make_name_safe(scan_name) + + # Strip all trailing underscores from the base name + scan_name = scan_name.strip("_") # A regex with the scan name and a group for the numbers scan_regex = re.compile( "^" + scan_name + "_([0-9]{" + str(SCAN_ZERO_PAD_DIGITS) + "})$" diff --git a/tests/unit_tests/test_scan_directories.py b/tests/unit_tests/test_scan_directories.py index 80121a32..0fc157fc 100644 --- a/tests/unit_tests/test_scan_directories.py +++ b/tests/unit_tests/test_scan_directories.py @@ -167,7 +167,7 @@ def test_bad_scan_names(): scan_dir = scan_dir_manager.new_scan_dir("fake scan") assert scan_dir.name == "fake_scan_0001" scan_dir = scan_dir_manager.new_scan_dir("fake scan;rm -rf /;") - assert scan_dir.name == "fake_scan_rm_-rf____0001" + assert scan_dir.name == "fake_scan_rm_-rf_0001" def test_scan_sequence_and_listing(caplog): @@ -277,6 +277,33 @@ def test_scan_sequence_case_sensitive(): assert "fake_scan_0005" in all_scans +def test_scan_names_have_single_underscores(): + """Check created scans have only one underscore in them, not two.""" + _clear_scan_dir() + scan_dir_manager = ScanDirectoryManager(BASE_SCAN_DIR) + + # Create some scan data and mark it as successful to get an end date. + scan_data = fake_active_scan_data() + scan_data.set_final_data(result="Success") + # Make 4 scans + scan_dir = scan_dir_manager.new_scan_dir("fake_scan_") + scan_dir.save_scan_data(scan_data) + scan_dir = scan_dir_manager.new_scan_dir("fake_scan__") + scan_dir.save_scan_data(scan_data) + scan_dir = scan_dir_manager.new_scan_dir("fake_scan_______") + scan_dir.save_scan_data(scan_data) + scan_dir = scan_dir_manager.new_scan_dir("fake_scan") + scan_dir.save_scan_data(scan_data) + + # Check they exist and are numbered sequentially + all_scans = scan_dir_manager.all_scans + assert_unique_of_length(all_scans, 4) + assert "fake_scan_0001" in all_scans + assert "fake_scan_0002" in all_scans + assert "fake_scan_0003" in all_scans + assert "fake_scan_0004" in all_scans + + def test_scan_name_non_sequential(): """Check new scan has the correct name if the directories are not sequential.""" _clear_scan_dir() From 538f8ffe09cc502ab5417519e423ab7e0aeec45d Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 6 Mar 2026 16:31:40 +0000 Subject: [PATCH 019/104] Update bug template to include table or configuration, more detail about how often the bug happens, confirming that the user has checked this is a new bug, and the difference in expected and current behaviour. --- .gitlab/issue_templates/Bug.md | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index 3d928164..470a7456 100644 --- a/.gitlab/issue_templates/Bug.md +++ b/.gitlab/issue_templates/Bug.md @@ -2,20 +2,35 @@ ## Summary +- [ ] I have searched existing issues to ensure this hasn't been reported. + +**Current Behavior**: + +**Expected Behavior**: ## Configuration -* **Sever version**: v2 or v3 -* **Release/Branch**: e.g. v3.0.0-alpha1 -* **Hardware Configuration:** - * Raspberry Pi Camera v2 - * Sangaboard v0.5 +| Component | Value | +| :-------- | :------- | +| **Server Version** | v2 or v3 | +| **Release/Branch** | e.g. v3.0.0-alpha1 | +| **UI** | e.g. Chrome, Firefox, OpenFlexure Connect | +| **Operating System** | e.g. Windows 11 | +| **Python Version** | e.g. Python 3.13.12 | +| **Camera Version** | e.g. Raspberry Pi Camera v2 | +| **Board Version** | Sangaboard v0.5 | ## Steps to reproduce + + +**Reproducibility:** +- [ ] **Consistent:** Happens every time. +- [ ] **Intermittent:** Happens roughly [X]% of the time or every [X] scans/clicks. +- [ ] **One-off:** Only happened once so far. ## Relevant logs and/or screenshots From cfe49b21bf46959c7582c26fd3474d835d3dd3ac Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 6 Mar 2026 16:34:28 +0000 Subject: [PATCH 020/104] Fix typos --- .gitlab/issue_templates/Bug.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index 470a7456..e43bc1e4 100644 --- a/.gitlab/issue_templates/Bug.md +++ b/.gitlab/issue_templates/Bug.md @@ -4,9 +4,9 @@ - [ ] I have searched existing issues to ensure this hasn't been reported. -**Current Behavior**: +**Current Behaviour**: -**Expected Behavior**: +**Expected Behaviour**: ## Configuration From 06f12973f4f0791812c986c7d299209f6aaed7c4 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 2 Mar 2026 19:40:44 +0000 Subject: [PATCH 021/104] Add limits to background detect and settling time settings --- .../things/background_detect.py | 8 ++++---- .../things/camera/__init__.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/openflexure_microscope_server/things/background_detect.py b/src/openflexure_microscope_server/things/background_detect.py index 19ab6874..8039605f 100644 --- a/src/openflexure_microscope_server/things/background_detect.py +++ b/src/openflexure_microscope_server/things/background_detect.py @@ -99,14 +99,14 @@ class ColourChannelDetectLUV(BackgroundDetectAlgorithm): # provided there. min_stds = [0.5, 0.3, 0.5] - channel_tolerance: float = lt.setting(default=7.0) + channel_tolerance: float = lt.setting(default=7.0, ge=0) """Channel Tolerance The number of standard deviations a pixel value must be from the background mean to be considered sample. """ - min_sample_coverage: float = lt.setting(default=25) + min_sample_coverage: float = lt.setting(default=25, ge=0, le=100) """Sample Coverage Required (%) The minimum percentage of the image that needs to be identified as sample for the @@ -223,14 +223,14 @@ class ChannelDeviationLUV(BackgroundDetectAlgorithm): # LUV colour space not converting to the CIELUV numbers) min_stds = [0.5, 0.3, 0.5] - channel_tolerance: float = lt.setting(default=7.0) + channel_tolerance: float = lt.setting(default=7.0, ge=0) """Channel Tolerance The number of standard deviations a pixel value must be from the background mean to be considered sample. """ - min_sample_coverage: float = lt.setting(default=25) + min_sample_coverage: float = lt.setting(default=25, ge=0, le=100) """Sample Coverage Required (%) The minimum percentage of the image that needs to be identified as sample for the diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index dd4f6cdd..14088f21 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -565,7 +565,7 @@ class BaseCamera(lt.Thing): except Exception as e: raise IOError(f"An error occurred while saving {jpeg_path}") from e - settling_time: float = lt.setting(default=0.2) + settling_time: float = lt.setting(default=0.2, ge=0) """The settling time when calling the ``settle()`` method.""" @lt.action From 99b9b0f3d65834b6d0dea3dd5bea1c82713818a5 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 2 Mar 2026 19:55:21 +0000 Subject: [PATCH 022/104] Add limits to noise level, sample density and downsample --- src/openflexure_microscope_server/things/camera/__init__.py | 2 +- src/openflexure_microscope_server/things/camera/simulation.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 14088f21..6b9bc323 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -274,7 +274,7 @@ class BaseCamera(lt.Thing): "CameraThings must define their own capture_array method" ) - downsampled_array_factor: int = lt.property(default=2) + downsampled_array_factor: int = lt.property(default=2, ge=1) """The downsampling factor when calling capture_downsampled_array.""" @lt.action diff --git a/src/openflexure_microscope_server/things/camera/simulation.py b/src/openflexure_microscope_server/things/camera/simulation.py index 31e3979e..5db72b8b 100644 --- a/src/openflexure_microscope_server/things/camera/simulation.py +++ b/src/openflexure_microscope_server/things/camera/simulation.py @@ -172,6 +172,8 @@ class SimulatedCamera(BaseCamera): @blob_density.setter def _set_blob_density(self, value: int) -> None: + if value < 0: + self.logger.warning("Sample density must be >= 0") self._blob_density = value if self._capture_enabled: self.generate_canvas() @@ -457,7 +459,7 @@ class SimulatedCamera(BaseCamera): return self._capture_thread.is_alive() return False - noise_level: float = lt.property(default=2.0) + noise_level: float = lt.property(default=2.0, ge=0) def _capture_frames(self) -> None: last_frame_t = time.time() From c9cc4e84682c4d67bab1bc02d4d45e149f8ad4ae Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 2 Mar 2026 20:35:25 +0000 Subject: [PATCH 023/104] Apply suggestions from code review of branch check-setting-validation Co-authored-by: Julian Stirling --- src/openflexure_microscope_server/things/camera/simulation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/openflexure_microscope_server/things/camera/simulation.py b/src/openflexure_microscope_server/things/camera/simulation.py index 5db72b8b..c8ad6f26 100644 --- a/src/openflexure_microscope_server/things/camera/simulation.py +++ b/src/openflexure_microscope_server/things/camera/simulation.py @@ -174,6 +174,7 @@ class SimulatedCamera(BaseCamera): def _set_blob_density(self, value: int) -> None: if value < 0: self.logger.warning("Sample density must be >= 0") + return self._blob_density = value if self._capture_enabled: self.generate_canvas() @@ -459,7 +460,7 @@ class SimulatedCamera(BaseCamera): return self._capture_thread.is_alive() return False - noise_level: float = lt.property(default=2.0, ge=0) + noise_level: float = lt.property(default=2.0, ge=0, le=50) def _capture_frames(self) -> None: last_frame_t = time.time() From 25c733a0c34e15e5d864bddf949a2dd2c8f1523c Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Wed, 4 Mar 2026 17:28:20 +0000 Subject: [PATCH 024/104] Apply suggestions from code review of branch check-setting-validation Co-authored-by: Julian Stirling --- src/openflexure_microscope_server/things/camera/simulation.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/simulation.py b/src/openflexure_microscope_server/things/camera/simulation.py index c8ad6f26..d75c36e5 100644 --- a/src/openflexure_microscope_server/things/camera/simulation.py +++ b/src/openflexure_microscope_server/things/camera/simulation.py @@ -173,8 +173,7 @@ class SimulatedCamera(BaseCamera): @blob_density.setter def _set_blob_density(self, value: int) -> None: if value < 0: - self.logger.warning("Sample density must be >= 0") - return + raise ValueError("Sample density must be >= 0") self._blob_density = value if self._capture_enabled: self.generate_canvas() From 14b3e5a62708033fe05ecd27a16e39473b6fc7dd Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Sat, 7 Mar 2026 18:17:43 +0000 Subject: [PATCH 025/104] Apply suggestions from code review of branch 523-improve-validation-checks Co-authored-by: Julian Stirling --- src/openflexure_microscope_server/utilities.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index a13cf19d..db75786d 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -201,7 +201,8 @@ def _sanitise_reserved(name: str, is_filename: bool = True) -> str: :param name: The component to check. :param is_filename: Whether the component is a filename. - If True, names will be forced into lowercase. + If True, names will be forced into lowercase. + :returns: The sanitised component, in lower case. """ # Check for Windows reserved names. From 1fa9e0b7724ed81dd7868a1998ef5f4a33a05179 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sat, 7 Mar 2026 18:56:44 +0000 Subject: [PATCH 026/104] Move more development information to the dev docs to minimise repitition and getting out of sync --- CONTRIBUTING.md | 94 +------------ README.md | 133 +----------------- apidocs/dev/README.md | 108 +++++++++++++- apidocs/dev/_sidebar.md | 8 +- .../dev/assets}/ofm_microscope_ui_scan.jpeg | Bin .../dev/assets}/ofm_microscope_ui_view.jpeg | Bin .../dev/assets}/ofm_sim_ui_scan.jpg | Bin .../dev/assets}/ofm_sim_ui_view.jpg | Bin apidocs/dev/contributing.md | 36 +++++ apidocs/dev/python.md | 92 ++++++++++++ .../dev/simulation_guide.md | 2 +- webapp/README.md => apidocs/dev/webapp.md | 34 ++--- 12 files changed, 255 insertions(+), 252 deletions(-) rename {docs/images => apidocs/dev/assets}/ofm_microscope_ui_scan.jpeg (100%) rename {docs/images => apidocs/dev/assets}/ofm_microscope_ui_view.jpeg (100%) rename {docs/images => apidocs/dev/assets}/ofm_sim_ui_scan.jpg (100%) rename {docs/images => apidocs/dev/assets}/ofm_sim_ui_view.jpg (100%) create mode 100644 apidocs/dev/contributing.md create mode 100644 apidocs/dev/python.md rename simulation_guide.md => apidocs/dev/simulation_guide.md (98%) rename webapp/README.md => apidocs/dev/webapp.md (58%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a108e8c9..076fc7d7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,96 +6,4 @@ Any contributions to any of our projects should follow [our code of conduct](htt If you are interested in contributing to the OpenFlexure project, and do not know where to start, we recommend you visit [our forum](https://openflexure.discourse.group/). -**The rest of this page details contribution guidelines that are specific to this repository.** - -This page generally focusses on the back-end of the server, not the front-end webapp. For more details on the webapp development see the [webapp's README](./webapp/README.md). - -## Python Development - -For installation, see the [project README](./README.md). - -### Core packages and concepts - -The OpenFlexure Microscope server is built in the [LabThings-FastAPI framework](https://labthings-fastapi.readthedocs.io/en/latest/). This provides a convenient way to expose laboratory hardware over HTTP via a [FastAPI server](https://fastapi.tiangolo.com/). - -The core elements of the server code are `Things`. A `Thing` creates a [W3C Web of Things](https://www.w3.org/TR/wot-architecture) compliant HTTP interface for hardware interactions and other server functionality. - -The project also makes use of the following libraries: - -* [Pydantic](https://docs.pydantic.dev/latest/) - For serialising/deserialising data being saved to disk or sent over HTTP. -* [OpenFlexure Stitching](https://gitlab.com/openflexure/openflexure-stitching) - Our own library for creating composite microscope images from a scan. -* [OpenCV](https://opencv.org/) and [Picamera2](https://github.com/raspberrypi/picamera2/) - For controlling cameras, and simulating cameras for tests. -* [Pillow](https://pillow.readthedocs.io) - For image manipulation and saving. -* [NumPy](https://numpy.org/) and [SciPy](https://scipy.org/) - For mathematical calculations. - -*The list above is not exhaustive!* - -## Programming style - -Ideally, python code should be: - -* Broken up into small, clear, testable functions. -* Documented with docstrings (For conventions see below). -* Type hinted -* Covered by unit tests (using [pytest](https://docs.pytest.org)) - -The server code is going through significant flux during the transition from v2 to v3, so currently our test coverage is being rebuilt and type checking does not yet pass. - -[Ruff](https://docs.astral.sh/ruff/) is used both for linting and formatting the codebase. Our custom linter rules are in our `pyproject.toml`. - -Run `ruff format` to format the code in a way that will pass on our CI. - -Run `ruff check` to check for linter errors. Any contributions that don't pass both of these checks will be automatically rejected by our pipeline. - -To check your code before each commit we recommend putting the following script in tour `.git/hooks` directory with the filename `pre-commit`. - - -```python -#!/bin/bash - -set -e - -function angrymessage() -{ - RED='\033[1;31m' - NC='\033[0m' # No Color - printf "\n${RED}Not committing as ruff failed${NC}\n\n" -} - -trap angrymessage ERR - -ruff format --check -ruff check -``` - - -## Docstrings conventions. - -We write docstrings in [reStructuredText](https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html), and process them with [pydoctor](https://pydoctor.readthedocs.io/). - -For function arguments, return values, and exceptions we use the following style recognised by reStructuredText: - -```python -def a_function(arg1: str, arg2: int) -> tuple[float, int]: - """A one sentence summary, in imperative mood, ending with punctuation. - - More detail after a blank line. This could be a whole paragraph. - - When referring to ``code`` in reStructuredText used double backticks - - :param arg1: This is the first argument, we can say everything we need to about it. - If the text wraps onto a new line, this line is indented. - :param arg2: This is the second argument. - - :returns: Say what it returns without anything within the colons. If returning - multiple values, a bulleted list is a good idea - just make sure: - - * It is indented to 1 level in - * There is a blank line before the bullets - * The bullet points are ``*`` not ``-`` - """ -``` - -+The linter rules we use for docstrings are defined by [pydocstyle](https://docs.astral.sh/ruff/rules/#pydocstyle-d). We do not use rules D203, D213, D400 for reasons explained in our `pyproject.toml` file. - -The documentation generated by pydoctor from docstrings can be viewed in any merge request (if the CI pipeline has passed) using the "View App" button. +Detailed guidelines specific to this project can be found in our [developer documentation](https://openflexure.gitlab.io/openflexure-microscope-server/dev/index.html). diff --git a/README.md b/README.md index 278598bf..ce80a5d0 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ The Python package provides a command `ofm-microscope-server` that runs the serv It is also possible to run the server in simulation mode. See Developer guidelines below. -## Settings +## Configuration and Settings The microscope is initially configured by a LabThings config file. This specifies two important things: @@ -38,133 +38,8 @@ The microscope is initially configured by a LabThings config file. This specifie By default, this configuration file should be read from `/var/openflexure/settings/ofm_config.json` when the microscope is run as a service. If it is run at the command line you should specify the configuration using the `-c` command line flag. This configuration file does not change often, and usually only needs to be updated when you change the physical hardware. It may be that this file should be made read-only, particularly in microscopes deployed for e.g. medical applications. -The settings folder is, by default, `/var/openflexure/settings/` on the SD card, or `./settings/` if run elsewhere. It can be changed in the configuration file. This holds every other persistent setting. Camera settings, calibration data, default capture settings, stream resolution and so on. There is one folder per `Thing`, each with their own file. The settings files can change very regularly, and if you need to reset your settings, it's usually these files that you should remove or reset. If you delete one settings file this should reset the corresponding `Thing`, you don't have to delete the whole folder. +The settings folder is, by default, `/var/openflexure/settings/` if running on a Raspberry Pi, or `./settings/` if run elsewhere. It can be changed in the configuration file. This holds every other persistent setting. Camera settings, calibration data, default capture settings, stream resolution and so on. There is one folder per `Thing`, each with their own file. The settings files can change very regularly, and if you need to reset your settings, it's usually these files that you should remove or reset. If you delete one settings file this should reset the corresponding `Thing`, you don't have to delete the whole folder. -# Developer guidelines +## Development -## Installation and set up - -To set up a development environment you will need Python 3.11 and the static files for the web application. - -### Installation and set up on a Raspberry Pi - -The OpenFlexure Raspberry Pi OS image comes with Python 3.11 already installed. We do not recommend trying to develop on the standard Raspberry Pi OS image. - - -* To activate the microscope virtual environment run `ofm activate` -* Navigate to the Openflexure repository with `cd /var/openflexure/application/openflexure-microscope-server` - -The Pi should already come with the static files for the web application. - -### Installation and set up on other platforms - -You should install Python 3.11. If you have a different version of python we recommend using [UV](https://docs.astral.sh/uv/) to set up a Python 3.11 virtual environment. - -You can check your Python version by running `py --version` or `python --version` in any terminal. Your Python version should be >= 3.11.0. - -* Clone the source code: `git clone https://gitlab.com/openflexure/openflexure-microscope-server.git` -* Enter the directory `cd openflexure-microscope-server` -* Create the virtual environment, for example with `uv venv --python 3.11` (or `py -m venv .venv` if not using uv) -* Activate your virtual environment with `source .venv/bin/activate` (on Linux) or `.venv\Scripts\activate.bat` (on Windows CMD) or `.venv\Scripts\activate` (on Windows Powershell) - -_Note: If you activate the venv in a CMD terminal in VSCode on Windows, the venv will not visually show as activated until you open a new powershell window._ - -* Install the application `uv pip install -e .[dev]` (or `pip install -e .[dev]` if you are not using UV) -* Run `python pull_webapp.py` to get the latest static files for the web application. - -Full video walkthroughs of the software installation process are available below: - -* [Windows Walkthrough](https://www.youtube.com/watch?v=LhnOLlIk-b8) -* [Linux Walkthrough](https://youtu.be/81jkMKOBeHQ) - -> If you are using Windows and you cannot activate your virtual environment from powershell, you probably need to change the execution policy. The official Microsoft docs have the right command in example #1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.security/set-executionpolicy?view=powershell-7.5#example-1-set-an-execution-policy. - -## Running the server - -There are two types of server available to run: - -* A live OpenFlexure Microscope Server, that runs on a Raspberry Pi within your OpenFlexure Microscope. This server processes real images from the microscope's camera. - - | ![The View Tab in the UI for the OFM Server](/docs/images/ofm_microscope_ui_view.jpeg "The View Tab in the UI for the OFM Server") | ![The Slide Scan Tab in the UI for the OFM Server](/docs/images/ofm_microscope_ui_scan.jpeg "The Slide Scan Tab in the UI for the OFM Server")| - |:---:|:---:| - -* A Simulation Server, which replicates the software functionality of the live server, but instead runs on your computer. Instead of receiving a live feed from a camera, the server 'scans' simulated data from a simulated camera. - - | ![The View Tab in the UI for the Simulated Server](/docs/images/ofm_sim_ui_view.jpg "The View Tab in the UI for the Simulated Server") | ![The Slide Scan Tab in the UI for the Simulated Server](/docs/images/ofm_sim_ui_scan.jpg "The Slide Scan Tab in the UI for the Simulated Server")| - |:---:|:---:| - - -### Running the server on a Raspberry Pi - -You can manage the server with the `ofm` command, using `ofm start`, `ofm stop`, and `ofm restart` to do the respective actions. Often, stopping the server and running it manually will make errors easier to spot. To do this, run: -``` -ofm stop -ofm activate -cd /var/openflexure -sudo -u openflexure-ws openflexure-microscope-server --fallback -c microscope_configuration.json --host 0.0.0.0 --port 5000 -``` - -### Running the simulation server on other platforms - -To start the simulation server: - -* Ensure all steps of the installation for your platform of choice are complete. -* Activate the virtual environment that was set up during installation -* Run the following command: - -`openflexure-microscope-server --fallback -c ./ofm_config_simulation.json` - -* Open the address displayed in the terminal in any browser. -* To close the simulation server, return to the terminal and press `ctrl + c`. - -> Note: If you are using Windows, we advise running this command in PowerShell. If it doesn't work this, might be a permission issue on managed machines, in which case try executing the command in a CMD terminal. - -> Note: If you receive an error saying that port 5000 is already in use, you can specify a different port to run the simulation on using `--port`: -> -> `openflexure-microscope-server --fallback --port 8081 -c ./ofm_config_simulation.json` - -A [full video walkthrough of running the simulation server](https://youtu.be/Z8verCqfj9s) is available on our YouTube Channel. - -You can find more information on using the simulation server in the [Simulation Guide](./simulation_guide.md). - - -## General development overview - -Please first read our **[project contributions guide](./CONTRIBUTING.md)** - -* We recommend all development is done on a branch. -* If you are editing the web app, please see our see the README in the [webapp](./webapp) directory. -* If you want to try a branch with webapp changes (that are already pushed to GitLab), but don't want to compile the javascript, then run `python pull_webapp.py -b ` to get the artifacts from the GitLab CI. - -**For development on a Raspberry Pi** we recommend VSCode Remote session from another computer. This allows you to use your usual developer environment to write code, but everything runs on the Raspberry Pi with real hardware. Note that the repository is cloned by default over `https`, so you will probably need to change the "remote" URL to your fork of the repository, or to SSH, before you're able to push changes. - -### Adding functionality - -The microscope comprises a number of `Thing` instances, which provide actions and properties to implement hardware control and software features. These may be imported from any Python module, using `ofm_config.json`. See the LabThings-FastAPI documentation for how to create a `Thing`. - -Currently, the camera and stage classes are provided by external libraries, `labthings-picamera2` and `labthings-sangaboard`. Replacing these is the easiest way to use alternative hardware: interfaces are defined in `openflexure_microscope_server.things.camera` and `openflexure_microscope_server.things.stage` respectively. - - -### Python: Formatting, linting, and tests - -All of the commands below assume that you are running in the OFM virtual environment, i.e. you have run `ofm activate` on an OpenFlexure SD card, or `source .venv/bin/activate` on Linux, or `.venv/Scripts/activate`on Windows Powershell, or `.venv\Scripts\activate.bat` on Windows CMD. - -**Before committing** you should lint and auto-format your code: - -* To lint run `ruff check` -* To auto-format the Python code run `ruff format` -* To auto-format the Javascript code, run - * `cd webapp` - * `npm run lint` - -**Before merging** please auto-format your code and also run the quality checks (linting, static analysis, and unit tests) - -* All commands above -* `pytest` - - -We use several code analysis and formatting libraries in this project. Our CI will check each of these automatically, so ensuring they pass locally will save you time. Currently `ruff` is used for linting/formatting and `pytest` for unit tests. `mypy` will be enabled once the codebase is ready. - -### Python environment, build, and dependencies - -As of `v3` we specify dependencies in `pyproject.toml`. These are not currently frozen due to difficulties matching versions on different platforms. On Raspberry Pi, it's best to specify `--only-binary=:all:` to ensure libraries like `numpy` and `scipy` are not compiled from source (which takes many hours, and/or fails). Pinning dependencies with `requirements.txt` and/or `requirements.in` may happen in the future. +For development please see our [developer documentation](https://openflexure.gitlab.io/openflexure-microscope-server/dev/index.html). \ No newline at end of file diff --git a/apidocs/dev/README.md b/apidocs/dev/README.md index 3498be6a..7788c072 100644 --- a/apidocs/dev/README.md +++ b/apidocs/dev/README.md @@ -2,8 +2,112 @@ This site provides information that is useful for developing the OpenFlexure Microscope. -### Viewing and Modifying Locally +## Viewing and Modifying These Docs Locally If you want to view these documents in your machine (very useful when editing them). You will need to serve the `apidocs` directory. Navigate to the `apidocs` directory and run `python -m http.server`. -All developer guidance documentation is written in markdown and then displayed with Docsify. More information on Docsify can be found at https://docsify.js.org. \ No newline at end of file +All developer guidance documentation is written in markdown and then displayed with Docsify. More information on Docsify can be found at https://docsify.js.org. + + +## Installation and set up + +To set up a development environment you will need Python 3.11 and the static files for the web application. + +### Installation and set up on a Raspberry Pi + +The OpenFlexure Raspberry Pi OS image comes with Python 3.11 already installed. We do not recommend trying to develop on the standard Raspberry Pi OS image. + + +* To activate the microscope virtual environment run `ofm activate` +* Navigate to the Openflexure repository with `cd /var/openflexure/application/openflexure-microscope-server` + +The Pi should already come with the static files for the web application. + +### Installation and set up on other platforms + +You should install Python 3.11. If you have a different version of python we recommend using [UV](https://docs.astral.sh/uv/) to set up a Python 3.11 virtual environment. + +You can check your Python version by running `py --version` or `python --version` in any terminal. Your Python version should be >= 3.11.0. + +* Clone the source code: `git clone https://gitlab.com/openflexure/openflexure-microscope-server.git` +* Enter the directory `cd openflexure-microscope-server` +* Create the virtual environment, for example with `python -m venv .venv` (or with `uv venv --python 3.11` if using uv) +* Activate your virtual environment with `source .venv/bin/activate` (on Linux) or `.venv\Scripts\activate.bat` (on Windows CMD) or `.venv\Scripts\activate` (on Windows Powershell) + +_Note: If you activate the venv in a CMD terminal in VSCode on Windows, the venv will not visually show as activated until you open a new powershell window._ + +* Install the application `pip install -e .[dev]` (or `uv pip install -e .[dev]` if you are using UV) +* Run `python pull_webapp.py` to get the latest static files for the web application. + +Full video walkthroughs of the software installation process are available below: + +* [Windows Walkthrough](https://www.youtube.com/watch?v=LhnOLlIk-b8) +* [Linux Walkthrough](https://youtu.be/81jkMKOBeHQ) + +> If you are using Windows and you cannot activate your virtual environment from powershell, you probably need to change the execution policy. The official Microsoft docs have the right command in example #1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.security/set-executionpolicy?view=powershell-7.5#example-1-set-an-execution-policy. + +## Running the server + +There are two types of server available to run: + +* A live OpenFlexure Microscope Server, that runs on a Raspberry Pi within your OpenFlexure Microscope. This server processes real images from the microscope's camera. + + | ![The View Tab in the UI for the OFM Server](/assets/ofm_microscope_ui_view.jpeg "The View Tab in the UI for the OFM Server") | ![The Slide Scan Tab in the UI for the OFM Server](/assets/ofm_microscope_ui_scan.jpeg "The Slide Scan Tab in the UI for the OFM Server")| + |:---:|:---:| + +* A Simulation Server, which replicates the software functionality of the live server, but instead runs on your computer. Instead of receiving a live feed from a camera, the server 'scans' simulated data from a simulated camera. + + | ![The View Tab in the UI for the Simulated Server](/assets/ofm_sim_ui_view.jpg "The View Tab in the UI for the Simulated Server") | ![The Slide Scan Tab in the UI for the Simulated Server](/assets/ofm_sim_ui_scan.jpg "The Slide Scan Tab in the UI for the Simulated Server")| + |:---:|:---:| + + +### Running the server on a Raspberry Pi + +You can manage the server with the `ofm` command, using `ofm start`, `ofm stop`, and `ofm restart` to do the respective actions. Often, stopping the server and running it manually will make errors easier to spot. To do this, run: +``` +ofm stop +ofm activate +cd /var/openflexure +sudo -u openflexure-ws openflexure-microscope-server --fallback -c microscope_configuration.json --host 0.0.0.0 --port 5000 +``` + +### Running the simulation server on other platforms + +To start the simulation server: + +* Ensure all steps of the installation for your platform of choice are complete. +* Activate the virtual environment that was set up during installation +* Run the following command: + +`openflexure-microscope-server --fallback -c ./ofm_config_simulation.json` + +* Open the address displayed in the terminal in any browser. +* To close the simulation server, return to the terminal and press `ctrl + c`. + +> Note: If you are using Windows, we advise running this command in PowerShell. If it doesn't work this, might be a permission issue on managed machines, in which case try executing the command in a CMD terminal. + +> Note: If you receive an error saying that port 5000 is already in use, you can specify a different port to run the simulation on using `--port`: +> +> `openflexure-microscope-server --fallback --port 8081 -c ./ofm_config_simulation.json` + +A [full video walkthrough of running the simulation server](https://youtu.be/Z8verCqfj9s) is available on our YouTube Channel. + +You can find more information on using the simulation server in the [Simulation Guide](simulation_guide). + + +## General development overview + +Please first read our **[project contributions guide](contributing)** + +* We recommend all development is done on a branch. +* If you are editing the web app, please see our see the README in the [webapp](webapp) directory. +* If you want to try a branch with webapp changes (that are already pushed to GitLab), but don't want to compile the javascript, then run `python pull_webapp.py -b ` to get the artifacts from the GitLab CI. + +**For development on a Raspberry Pi** we recommend VSCode Remote session from another computer. This allows you to use your usual developer environment to write code, but everything runs on the Raspberry Pi with real hardware. Note that the repository is cloned by default over `https`, so you will probably need to change the "remote" URL to your fork of the repository, or to SSH, before you're able to push changes. + +### Adding functionality + +The microscope comprises a number of `Thing` instances, which provide actions and properties to implement hardware control and software features. These may be imported from any Python module, using `ofm_config.json`. See the LabThings-FastAPI documentation for how to create a `Thing`. + +Currently, the camera and stage classes are provided by external libraries, `labthings-picamera2` and `labthings-sangaboard`. Replacing these is the easiest way to use alternative hardware: interfaces are defined in `openflexure_microscope_server.things.camera` and `openflexure_microscope_server.things.stage` respectively. + diff --git a/apidocs/dev/_sidebar.md b/apidocs/dev/_sidebar.md index df1ea285..a3db62e5 100644 --- a/apidocs/dev/_sidebar.md +++ b/apidocs/dev/_sidebar.md @@ -5,5 +5,9 @@

Developer Guidance

- [Home](README) -- [Software Architecture](architecture.md) -- [PiCamera Tuning Files](picamera-tuning-files.md) \ No newline at end of file +- [Contributing](contributing) +- [Python Development](python) +- [Webapp Development](webapp) +- [Running the Simulator](simulation_guide) +- [Software Architecture](architecture) +- [PiCamera Tuning Files](picamera-tuning-files) \ No newline at end of file diff --git a/docs/images/ofm_microscope_ui_scan.jpeg b/apidocs/dev/assets/ofm_microscope_ui_scan.jpeg similarity index 100% rename from docs/images/ofm_microscope_ui_scan.jpeg rename to apidocs/dev/assets/ofm_microscope_ui_scan.jpeg diff --git a/docs/images/ofm_microscope_ui_view.jpeg b/apidocs/dev/assets/ofm_microscope_ui_view.jpeg similarity index 100% rename from docs/images/ofm_microscope_ui_view.jpeg rename to apidocs/dev/assets/ofm_microscope_ui_view.jpeg diff --git a/docs/images/ofm_sim_ui_scan.jpg b/apidocs/dev/assets/ofm_sim_ui_scan.jpg similarity index 100% rename from docs/images/ofm_sim_ui_scan.jpg rename to apidocs/dev/assets/ofm_sim_ui_scan.jpg diff --git a/docs/images/ofm_sim_ui_view.jpg b/apidocs/dev/assets/ofm_sim_ui_view.jpg similarity index 100% rename from docs/images/ofm_sim_ui_view.jpg rename to apidocs/dev/assets/ofm_sim_ui_view.jpg diff --git a/apidocs/dev/contributing.md b/apidocs/dev/contributing.md new file mode 100644 index 00000000..a71f745a --- /dev/null +++ b/apidocs/dev/contributing.md @@ -0,0 +1,36 @@ +# Contributing to the OpenFlexure Microscope Server + +All contributions to the OpenFlexure project should follow our [project contributions guidelines](https://openflexure.org/contribute). + +Any contributions to any of our projects should follow [our code of conduct](https://openflexure.org/conduct). + +If you are interested in contributing to the OpenFlexure project, and do not know where to start, we recommend you visit [our forum](https://openflexure.discourse.group/). + +**The rest of this page details contribution guidelines that are specific to this repository.** + +This page generally focusses on the back-end of the server, not the front-end webapp. For more details on the webapp development see the [webapp's README](webapp). + +## Installation + +For installation, see the [home page](README). + +### Core packages and concepts + +The OpenFlexure Microscope server is built in the [LabThings-FastAPI framework](https://labthings-fastapi.readthedocs.io/en/latest/). This provides a convenient way to expose laboratory hardware over HTTP via a [FastAPI server](https://fastapi.tiangolo.com/). + +The core elements of the server code are `Things`. A `Thing` creates a [W3C Web of Things](https://www.w3.org/TR/wot-architecture) compliant HTTP interface for hardware interactions and other server functionality. + +The project also makes use of the following libraries: + +* [Pydantic](https://docs.pydantic.dev/latest/) - For serialising/deserialising data being saved to disk or sent over HTTP. +* [OpenFlexure Stitching](https://gitlab.com/openflexure/openflexure-stitching) - Our own library for creating composite microscope images from a scan. +* [OpenCV](https://opencv.org/) and [Picamera2](https://github.com/raspberrypi/picamera2/) - For controlling cameras, and simulating cameras for tests. +* [Pillow](https://pillow.readthedocs.io) - For image manipulation and saving. +* [NumPy](https://numpy.org/) and [SciPy](https://scipy.org/) - For mathematical calculations. + +*The list above is not exhaustive!* + +## Coding guidelines + +We have separate coding guidelines for [the Python server](python) and for the [Vue webapp](webapp). + diff --git a/apidocs/dev/python.md b/apidocs/dev/python.md new file mode 100644 index 00000000..955d0b5d --- /dev/null +++ b/apidocs/dev/python.md @@ -0,0 +1,92 @@ +# Python Server Development + +## Overview + + +Ideally, Python code should be: + +* Broken up into small, clear, testable functions. +* Documented with docstrings (For conventions see below). +* Type hinted +* Covered by unit tests (using [pytest](https://docs.pytest.org)) + +The server code is going through significant flux during the transition from v2 to v3, so currently our test coverage is being rebuilt and type checking does not yet pass. + +## Python environment, build, and dependencies + +As of `v3` we specify dependencies in `pyproject.toml`. These are not currently frozen due to difficulties matching versions on different platforms. On Raspberry Pi, it's best to specify `--only-binary=:all:` to ensure libraries like `numpy` and `scipy` are not compiled from source (which takes many hours, and/or fails). Pinning dependencies with `requirements.txt` and/or `requirements.in` may happen in the future. + + + +## Python: Formatting, linting, and tests + +[Ruff](https://docs.astral.sh/ruff/) is used both for linting and formatting the codebase. Our custom linter rules are in our `pyproject.toml`. + +> All of the commands below assume that you are running in the OFM virtual environment, i.e. you have run `ofm activate` on an OpenFlexure SD card, or `source .venv/bin/activate` on Linux, or `.venv/Scripts/activate`on Windows Powershell, or `.venv\Scripts\activate.bat` on Windows CMD. + +**Before committing** you should lint and auto-format your code: + +* To lint run `ruff check` +* To auto-format the Python code run `ruff format` + +**Before merging** please auto-format your code and also run the quality checks (linting, static analysis, and unit tests) + +* All commands above +* `pytest` + + +We use several code analysis and formatting libraries in this project. Our CI will check each of these automatically, so ensuring they pass locally will save you time. Currently `ruff` is used for linting/formatting and `pytest` for unit tests. `mypy` will be enabled once the codebase is ready. + + +To check your code before each commit we recommend putting the following script in tour `.git/hooks` directory with the filename `pre-commit`. + + +```python +#!/bin/bash + +set -e + +function angrymessage() +{ + RED='\033[1;31m' + NC='\033[0m' # No Color + printf "\n${RED}Not committing as ruff failed${NC}\n\n" +} + +trap angrymessage ERR + +ruff format --check +ruff check +``` + + +## Docstrings conventions. + +We write docstrings in [reStructuredText](https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html), and process them with [pydoctor](https://pydoctor.readthedocs.io/). + +For function arguments, return values, and exceptions we use the following style recognised by reStructuredText: + +```python +def a_function(arg1: str, arg2: int) -> tuple[float, int]: + """A one sentence summary, in imperative mood, ending with punctuation. + + More detail after a blank line. This could be a whole paragraph. + + When referring to ``code`` in reStructuredText used double backticks + + :param arg1: This is the first argument, we can say everything we need to about it. + If the text wraps onto a new line, this line is indented. + :param arg2: This is the second argument. + + :returns: Say what it returns without anything within the colons. If returning + multiple values, a bulleted list is a good idea - just make sure: + + * It is indented to 1 level in + * There is a blank line before the bullets + * The bullet points are ``*`` not ``-`` + """ +``` + ++The linter rules we use for docstrings are defined by [pydocstyle](https://docs.astral.sh/ruff/rules/#pydocstyle-d). We do not use rules D203, D213, D400 for reasons explained in our `pyproject.toml` file. + +The documentation generated by pydoctor from docstrings can be viewed in any merge request (if the CI pipeline has passed) using the "View App" button. \ No newline at end of file diff --git a/simulation_guide.md b/apidocs/dev/simulation_guide.md similarity index 98% rename from simulation_guide.md rename to apidocs/dev/simulation_guide.md index 258f1e3d..cf7997ae 100644 --- a/simulation_guide.md +++ b/apidocs/dev/simulation_guide.md @@ -16,7 +16,7 @@ The simulation server GUI is a copy of the GUI you would find on the hardware wi ## Starting the Simulation Server -1. **Activate your virtual environment** (the same one created during installation, as described in the [README](./README.md)). +1. **Activate your virtual environment** (the same one created during installation, as described in the [Python documentation](python)). 2. **Run the simulation server** using the following command: ```bash diff --git a/webapp/README.md b/apidocs/dev/webapp.md similarity index 58% rename from webapp/README.md rename to apidocs/dev/webapp.md index 6b0a2f7e..37516dbd 100644 --- a/webapp/README.md +++ b/apidocs/dev/webapp.md @@ -1,18 +1,11 @@ -# OpenFlexure Microscope JS Client +# WebApp Development ## Key info -* Vue.js web application providing a graphical interface for the OFM +* Vue.js web application providing a graphical interface for the Openflexure Microscope. * Once built, will be served by the API server from the host root on port 5000 - -* See [openflexure-microscope-server/README.md](https://gitlab.com/openflexure/openflexure-microscope-server/-/blob/master/README.md) for details on local installation and building - -# Developer guidelines - -## Creating releases - * JS client is coupled to the API, and is built and distributed with the server. -* See [openflexure-microscope-server/README.md](https://gitlab.com/openflexure/openflexure-microscope-server/-/blob/master/README.md) for details on creating new releases + ## Installing @@ -23,7 +16,7 @@ _Note: If you are using a Windows OS, the following commands must be executed in * Install dependencies with `npm install` * Build the static web app with `npm run build` -# Developing +## Live Server When developing it is useful to use the development server so Vue changes happen instantly without the need to rebuild. To start the development server run: @@ -33,19 +26,10 @@ The development server is accessed on a different port from the microscope API. When the development webapp starts it cannot locate the microscope API as this is served by the microscope on port 5000. Instead it will present you with a page giving you the option to "override API origin". If you are running a development server on your computer, you should enter `http://localhost:5000/`. -## VS Code and ESLint +# Javascript: Formatting and linting -To prevent the editor from interfering with ESLint, add to your project `settings.json`: +To enforce code style we and quality we use ESLint and Prettier. Both can be run together with the same command. + +- To check the for errors and warnings run `npm run lint` +- To automatically fix errors an warnings run `npm run lint:fix` -```json -{ - "editor.tabSize": 2, - "cSpell.enabled": false, - "eslint.validate": ["vue","javascript", "javascriptreact"], - "editor.formatOnSave": false, - "vetur.validation.template": false, - "editor.codeActionsOnSave": { - "source.fixAll.eslint": true - } -} -``` From e1a8b9907eb624281f970179734427d90392adaf Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sat, 7 Mar 2026 20:15:06 +0000 Subject: [PATCH 027/104] Update picamera tests --- picamera_coverage.zip | Bin 54038 -> 54038 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/picamera_coverage.zip b/picamera_coverage.zip index 4a6c18565efa1d51a3ede16982b80d3e405a658b..09438bb3cc01239acf74a6c89d403e793896500e 100644 GIT binary patch delta 103 zcmbQXjCtBJX5IjAW)=|!5XhgDzL8h=0!RL&^q5b7c%Dplx{#)BmSmZfY;0s~Y-DDb yW|U->mTHt@k(gv?Zf0SbmXexcWNv0)nw+-z{Dp0d%=wejCwE-3MVN5*k|zM+izWF0 delta 103 zcmbQXjCtBJX5IjAW)=|!5D?ptvXNK!0*Ba!lo-EzKPo0WT}V?;Nij(?OHN5Ku}HKq yGP6uJFf&Rru}m?wG%`*ywy;PxHMB5KN=)8-{=zm!X0Z(^lRGZiB1|}Y$rAvb_#{;T From 590030683552c82aace4cc45343be2c03308afef Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sat, 7 Mar 2026 21:12:07 +0000 Subject: [PATCH 028/104] Add developer documentation for the config file. --- README.md | 4 +- apidocs/dev/config-file.md | 101 +++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 apidocs/dev/config-file.md diff --git a/README.md b/README.md index ce80a5d0..00c3262d 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,9 @@ The microscope is initially configured by a LabThings config file. This specifie By default, this configuration file should be read from `/var/openflexure/settings/ofm_config.json` when the microscope is run as a service. If it is run at the command line you should specify the configuration using the `-c` command line flag. This configuration file does not change often, and usually only needs to be updated when you change the physical hardware. It may be that this file should be made read-only, particularly in microscopes deployed for e.g. medical applications. -The settings folder is, by default, `/var/openflexure/settings/` if running on a Raspberry Pi, or `./settings/` if run elsewhere. It can be changed in the configuration file. This holds every other persistent setting. Camera settings, calibration data, default capture settings, stream resolution and so on. There is one folder per `Thing`, each with their own file. The settings files can change very regularly, and if you need to reset your settings, it's usually these files that you should remove or reset. If you delete one settings file this should reset the corresponding `Thing`, you don't have to delete the whole folder. +In the future tool will be created for crating custom microscope configurations. Currently no custom configurations are recommended, except for development. More [information on the configuration file](https://openflexure.gitlab.io/openflexure-microscope-server/dev/index.html#config-file) can be found in the developer documentation. + +The settings folder holds the state of your microscope, including the calibrations. The settings folder is, by default, `/var/openflexure/settings/` if running on a Raspberry Pi, or `./settings/` if run elsewhere. It can be changed in the configuration file. This holds every other persistent setting. Camera settings, calibration data, default capture settings, stream resolution and so on. There is one folder per `Thing`, each with their own file. The settings files can change very regularly, and if you need to reset your settings, it's usually these files that you should remove or reset. If you delete one settings file this should reset the corresponding `Thing`, you don't have to delete the whole folder. ## Development diff --git a/apidocs/dev/config-file.md b/apidocs/dev/config-file.md new file mode 100644 index 00000000..bbda452c --- /dev/null +++ b/apidocs/dev/config-file.md @@ -0,0 +1,101 @@ +# The Configuration File + +By default, this configuration file is located at read from `/var/openflexure/settings/ofm_config.json` when the microscope is run as a service. If it is run at the command line, you should specify the configuration using the `-c` command line flag. + +The default configuration file links to the configuration file in the repository: + +``` +{ + "base_config_file": "/var/openflexure/application/openflexure-microscope-server/ofm_config_full.json" +} +``` + +## Understanding and modifying the base configuration file + +This section describes how to change the microscope configuration for development. For example, when making a Merge Request to add a new Thing to the standard microscope configuration. **Do not do this for customising a single microscope.** For customising the configuration of a single microscope see the next section. + +The specification for the configuration file is set by [LabThings FastAPI](https://labthings-fastapi.readthedocs.io/). The file is a JSON file. As such, it cannot contain comments. + +The configuration file has three keys: + +* `things` - A dictionary (Object) containing the [`Thing`s](https://labthings-fastapi.readthedocs.io/en/latest/using_things.html) that should be loaded. All instrument control and server interaction is via `Thing`s. +* `settings_folder` - The directory that LabThings will save settings for each `Thing`. +* `application_config` - A space for the OpenFlexure Microscope Server to list its custom configuration. The OpenFlexure Microscope Server has the following keys: + * `log_folder` - Where the log files are stored. + * `data_folder` - Where data is stored. + +The `things` dictionary can specify a `Thing` to be loaded into the server in two different ways. If there is no custom configuration for the `Thing` then: + +* The key is a **name** for the `Thing`, this defines the URL of its endpoints. +* The value is the Python object reference for the `Thing`. + +For example: + +``` +"autofocus": "openflexure_microscope_server.things.autofocus:AutofocusThing", +``` + +If further information needs to be supplied then the value should be a dictionary (Object) with the following keys: + +* `class` - *(Required)* The Python object reference for the `Thing`. +* `args` - *(Optional)* The positional arguments to be passed to the `Thing` when initialised. +* `kwargs` - *(Optional)* The keyword arguments to be passed to the `Thing` when initialised. +* `thing_slots` - *(Optional)* Information for how the [thing slots](https://labthings-fastapi.readthedocs.io/en/latest/thing_slots.html) of the `Thing` are populated. + +For example: + +``` +"camera": { + "class": "openflexure_microscope_server.things.camera.picamera:StreamingPiCamera2", + "kwargs": { + "camera_board": "picamera_v2" + } +} +``` + +## Customising the configuration + +To have a custom configuration for a specific microscope, we recommend editing `ofm_config.json` in the settings folder. **Avoid editing the base configuration in the repository.** This will keep your customisations that should not be pushed separate from changes to the base file. + +The customisations are performed using [**JSON merge patch** (IETF RFC 7396)](https://datatracker.ietf.org/doc/html/rfc7396) syntax. The patch should specify only the keys that are to change. The RFC documentation provides a number of examples. + +> **Note** +> If a key is set to `null` that key is deleted entirely. There is a [LabThings FastAPI issue](https://github.com/labthings/labthings-fastapi/issues/190) discussing how to handle the rare cases where a value may need to be patched to have the value `None` in Python. + +For our configuration file, an extra key `patch` is added that describes how to patch the base configuration file. + +### Configuration for the Raspberry Pi HQ Camera + +> **Note** +> HQ camera support is currently experimental in v3. While it can be used, it is not as thoroughly tested or supported as the standard v2 camera module. + +You can configure a microscope for the Raspberry Pi HQ Camera by specifying the `camera_board` keyword argument for the `camera` `Thing`. The value needs to change from `picamera_v2` to `picamera_hq`. This will load the HQ camera tuning file and configure the pipeline accordingly. + +As the only key that needs changing is `things.camera.kwargs.camera_board` the patch is: + +```json +{ + "base_config_file": "/var/openflexure/application/openflexure-microscope-server/ofm_config_full.json", + "patch": { + "things": { + "camera": { + "kwargs": { + "camera_board": "picamera_hq" + } + } + } + } +} +``` + +Note that even though the `camera` patch is specified as: + +```json +"camera": { + "kwargs": { + "camera_board": "picamera_hq" + } +} +``` + +This only affects the specified keys. So the camera class is unaffected. From e1b467424b7d75d3f103a284399e38efb44e7599 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sat, 7 Mar 2026 21:17:25 +0000 Subject: [PATCH 029/104] Add configuration file docs to dev docs sidebar --- apidocs/dev/_sidebar.md | 1 + 1 file changed, 1 insertion(+) diff --git a/apidocs/dev/_sidebar.md b/apidocs/dev/_sidebar.md index a3db62e5..b40cb62c 100644 --- a/apidocs/dev/_sidebar.md +++ b/apidocs/dev/_sidebar.md @@ -8,6 +8,7 @@ - [Contributing](contributing) - [Python Development](python) - [Webapp Development](webapp) +- [Configuration File](config-file) - [Running the Simulator](simulation_guide) - [Software Architecture](architecture) - [PiCamera Tuning Files](picamera-tuning-files) \ No newline at end of file From 3fee78a213426d4a98066694915d27ca15af006e Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 8 Mar 2026 18:30:17 +0000 Subject: [PATCH 030/104] Tweak css of dev docs to remove spaces between code and adjacent text. --- apidocs/dev/ofm.css | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apidocs/dev/ofm.css b/apidocs/dev/ofm.css index f3a7abae..c60cc0ca 100644 --- a/apidocs/dev/ofm.css +++ b/apidocs/dev/ofm.css @@ -833,8 +833,11 @@ body { border-radius: 2px; /*Approx the other side of the colour wheel from OFM pink.*/ color: #2fbf73; - margin: 0 2px; - padding: 3px 5px; + /*Remove the padding and margin as it spaces in-line code to much from surrounding + text and punctuation. + */ + margin: 0; + padding: 3px 0; white-space: pre-wrap } From f67699fbc8dd6d9d7daf86351f17fb2e3ed1273f Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 9 Mar 2026 13:54:43 +0000 Subject: [PATCH 031/104] Raise warnings if there are unsafe file path elements rather than coercing file paths. Stitching still raises an error if your file name is unsafe --- .../stitching.py | 10 +- .../utilities.py | 95 ++++++------ tests/unit_tests/test_utilities.py | 141 ++++++++++++------ 3 files changed, 153 insertions(+), 93 deletions(-) diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index 393eb32e..fc764eb4 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/src/openflexure_microscope_server/stitching.py @@ -78,13 +78,14 @@ def validate_command(cmd: list[str]) -> None: # Check against forbidden commands (case-insensitive) if element.lower() in FORBIDDEN_COMMANDS: raise StitcherValidationError( - f"Invalid stiching command: Forbidden element '{element}' detected." + f"Invalid stitching command: Forbidden element '{element}' detected." ) # Ensure characters are safe for a path component - if element != make_path_safe(element): + _, error_raised = make_path_safe(element) + if error_raised: raise StitcherValidationError( - f"Invalid stiching command: Element '{element}' contains unsafe characters." + f"Invalid stitching command: Element '{element}' contains unsafe characters." ) @@ -154,7 +155,8 @@ class BaseStitcher: :raises RuntimeError: if inputs are unsafe. """ - if self.images_dir != make_path_safe(self.images_dir): + _, error_raised = make_path_safe(self.images_dir) + if error_raised: raise StitcherValidationError( "Invalid directory path: Contains unsafe characters." ) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index a13cf19d..11803368 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -77,7 +77,7 @@ def requires_lock( # Compiled regular expressions for unsafe characters # Matches anything that isn't a-z, A-Z, 0-9, _, ., -, :, /, \ -_WINDOWS_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-:/\\]") +_WINDOWS_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-:/\ \\]") # Matches anything that isn't a-z, A-Z, 0-9, _, ., -, \ _POSIX_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-/]") # Matches anything that isn't a-z, A-Z, 0-9, _, ., - @@ -110,26 +110,21 @@ _WINDOWS_RESERVED_NAMES = { } -def make_path_safe(unsafe_path_string: str) -> str: - """Replace unsafe characters in a file path with underscores, preserving separators. +def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: + """Check if a file path has any unsafe elements in it, such as + unsafe characters or reserved names for its OS. - This can be used to check that user inputs have not been concatenated into an - unsafe filepath. - - This ensures compatibility across platforms by preserving only valid characters, - including path separators (forward slash on POSIX, and forward - slash/backslash/colon on Windows). - - Additionally, it sanitises each path component for Windows reserved names and - trailing dots/spaces to ensure maximum cross-platform compatibility. + The path is not coerced into a safe form because if we ask + for a file to be written to a location we shouldn't be changing + that location as we have no "consent" to write to this other location. :param unsafe_path_string: The original path string to sanitise. - :returns: A version of the input string safe to use as a file path. + :returns: The input string unchanged and a boolean indicating if + any unsafe features were found. """ # Split by separators first to sanitise components independently components = re.split(r"([/\\])", unsafe_path_string) - sanitised_components = [] unsafe_character_pattern = ( _WINDOWS_UNSAFE_PATTERN @@ -137,40 +132,52 @@ def make_path_safe(unsafe_path_string: str) -> str: else _POSIX_UNSAFE_PATTERN ) + # Ensure each warning only gets logged once per path + unsafe_relative_path = False + trailing_dots = False + trailing_whitespace = False + unsafe_characters = False + reserved_word = False + for i, component in enumerate(components): - if component in ("/", "\\"): - sanitised_components.append(component) - elif component in (".", ".."): - # Only allow '.' and '..' if they are the very first - # elements and the next element is a separator. - # This allows for relative paths like "./file" or "../file" - # but not "file./file" or "file/../file" - is_first = i == 0 - next_to_sep = (i + 1 < len(components)) and ( - components[i + 1] in ("/", "\\") - ) + # Skip separators and empty strings + if component in ("/", "\\") or not component: + continue - if is_first and next_to_sep: - sanitised_components.append(component) - else: - sanitised_components.append("_") - elif component: - # 1. Strip trailing dots and spaces - # 2. Apply unsafe character regex - # 3. Check for reserved names - sanitised = component.rstrip(". ") - if not sanitised: - sanitised = "_" - else: - sanitised = unsafe_character_pattern.sub("_", sanitised) + # 1. Check for relative paths in the wrong place + if component in (".", "..") and not unsafe_relative_path: + is_first = (i == 0) + is_second = (i == 2 and components[i-1] in ("/", "\\") and components[i-2] == "..") + if not (is_first or is_second): + LOGGER.warning(f"File path {unsafe_path_string} may be unsafe due to unexpected relative navigation.") + unsafe_relative_path = True + continue - sanitised_components.append( - _sanitise_reserved(sanitised, is_filename=False) - ) - else: - sanitised_components.append(component) + # 2. Check for trailing dots in path + if "." in component and i != len(components) - 1 and not trailing_dots: + # Check for trailing dots in the file path before the file name + # e.g.: foo/bar./file is bad, but foo/bar.file.py is fine + LOGGER.warning(f'File path {unsafe_path_string} may be unsafe due to trailing dots.') + trailing_dots = True - return "".join(sanitised_components) + # Check for trailing spaces - Windows specific + if sys.platform.startswith("win") and component.endswith(" ") and not trailing_whitespace: + LOGGER.warning(f"{unsafe_path_string} contains unsafe trailing spaces.") + trailing_whitespace = True + + # 3. Check for unsafe characters (Regex) + if unsafe_character_pattern.search(component) and not unsafe_characters: + LOGGER.warning(f"{unsafe_path_string} contains characters that may be unsafe on this platform.") + unsafe_characters = True + + # 4. Check for Reserved Names (e.g., CON, PRN, LPT1) + # Assuming _sanitise_reserved returns a different string if it's a reserved name + if _sanitise_reserved(component, is_filename=False) != component and not reserved_word: + LOGGER.warning(f"{unsafe_path_string} contains a reserved system name and may cause issues.") + reserved_word = True + + warning_raised = unsafe_relative_path or trailing_dots or trailing_whitespace or unsafe_characters or reserved_word + return unsafe_path_string, warning_raised def make_name_safe(unsafe_name_string: str) -> str: diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index dbff78e4..c6d32431 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -1,15 +1,42 @@ """Unit tests for utility functions.""" import sys - +import logging import pytest from openflexure_microscope_server.utilities import ( _WINDOWS_RESERVED_NAMES, make_name_safe, - make_path_safe, + make_path_safe ) +PATHS_WITH_DOTS = { + "a./b/c.", + "path./to/file.", + "./path./to/file." +} + +RESERVED_NAME_PATHS = { + "path/CON/file", + "path/NUL.txt/file", + "C:/AUX/test" +} + +if sys.platform.startswith("win"): + UNSAFE_CHARACTER_PATHS = { + "C:/path/asterisk/data*.csv", + "data/files/read_me_first!.txt", + "logs/system.log", + "projects/alpha#1/notes.txt", + } +else: + UNSAFE_CHARACTER_PATHS = { + "/var/log/app:backup.log", + "/etc/configs/network-settings\\v1", + "/tmp/file_with_@_symbol.txt", + "/home/user/script(1).py" + } + def test_make_name_safe_basic(): """Test basic functionality of make_name_safe.""" @@ -56,57 +83,81 @@ def test_make_name_safe_reserved_names_false_positives(): assert make_name_safe("CHILLI CON CARNE") == "chilli_con_carne" -def test_make_path_safe_basic(): +def test_make_path_safe_basic(caplog): """Test basic functionality of make_path_safe.""" # Note: behavior depends on platform for separators - if sys.platform.startswith("win"): - assert make_path_safe("C:\\path\\to/file") == "C:\\path\\to/file" - else: - # On POSIX, \ is not a separator and might be replaced by _ if not in allowed pattern - # The regex for POSIX is [^a-zA-Z0-9_.\-/] - # And : is also not allowed - assert make_path_safe("path/to/file") == "path/to/file" + with caplog.at_level(logging.WARNING): + if sys.platform.startswith("win"): + assert make_path_safe("C:\\path\\to/file")[0] == "C:\\path\\to/file" + assert make_path_safe("C:\\a very long path\\to/file")[0] == "C:\\a very long path\\to/file" + else: + assert make_path_safe("path/to/file")[0] == "path/to/file" + assert make_path_safe("a very long path/to/file")[0] == "a very long path/to/file" + + # No errors should be raised. These paths are safe. + assert len(caplog.records) == 0 -def test_make_path_safe_reserved_in_components(): +@pytest.mark.parametrize("path", UNSAFE_CHARACTER_PATHS) +def test_make_path_safe_unsafe_characters_in_components(caplog, path): + """Test unsafe characters within path components.""" + with caplog.at_level(logging.WARNING): + assert make_path_safe(path)[0] == path + assert f"{path} contains characters that may be unsafe on this platform." + + +@pytest.mark.parametrize("path", RESERVED_NAME_PATHS) +def test_make_path_safe_reserved_in_components(caplog, path): """Test reserved names within path components.""" - assert make_path_safe("path/CON/file") == "path/CON_/file" - assert make_path_safe("path/NUL.txt/file") == "path/NUL.txt_/file" - - if sys.platform.startswith("win"): - assert make_path_safe("C:/AUX/test") == "C:/AUX_/test" - else: - assert make_path_safe("C:/AUX/test") == "C_/AUX_/test" + with caplog.at_level(logging.WARNING): + assert make_path_safe(path)[0] == path + assert f"{path} contains a reserved system name and may cause issues." -def test_make_path_safe_trailing_in_components(): - """Test trailing chars in path components.""" - assert make_path_safe("path /to. /file ") == "path/to/file" - assert make_path_safe("path./to /file.") == "path/to/file" - - # Allow dots at the start of the path - # As this is common for hidden files on POSIX and relative paths - # And is the configuration in the manual and simulation config - # json files. - assert make_path_safe("./path/to/file") == "./path/to/file" - assert make_path_safe("./path./to /file.") == "./path/to/file" - assert make_path_safe("./openflexure/data/") == "./openflexure/data/" +def test_make_path_safe_trailing_space_in_components(caplog): + """Test trailing spaces in path components.""" + with caplog.at_level(logging.WARNING): + # Test Windows - cannot have trailingspaces + if sys.platform.startswith("win"): + assert make_path_safe("path /to /file ")[0] == "path /to /file " + assert make_path_safe("path/to /file")[0] == "path/to /file" + # Check directories can have spaces in them but not at the end + # TODO - dont want underscores, want to keep interim spaces + assert make_path_safe("path/a long path name /file")[0] == "path/a long path name /file" + assert len(caplog.records) == 3 + assert f"path /to /file contains unsafe trailing spaces." in caplog.messages + assert f"path/to /file contains unsafe trailing spaces." in caplog.messages + assert f"path/a long path name /file contains unsafe trailing spaces." in caplog.messages + else: + # Trailing spaces allows in POSIX systems + assert make_path_safe("path /to /file ")[0] == "path /to /file " + assert make_path_safe("path/to /file")[0] == "path/to /file" + # Check directories can have spaces in them but not at the end + assert make_path_safe("path/a long path name /file")[0] == "path/a long path name file" + # No warnings should be raised + assert len(caplog.records) == 0 -def test_make_path_safe_separators(): - """Test that separators are preserved and components sanitised.""" - assert make_path_safe("a/b\\c") == "a/b\\c" - assert make_path_safe("a./b /c.") == "a/b/c" - - -def test_make_path_safe_relative(): +def test_make_path_safe_relative(caplog): """Test that relative path components are preserved.""" - # We only allow relative paths with one dot, and at the beginning of the path, - # to avoid issues with paths like "file./file" or "file../file. - assert make_path_safe("./openflexure/data/") == "./openflexure/data/" - assert make_path_safe("../openflexure/data/") == "../openflexure/data/" + # Relative imports ./foo, ../foo or ../../foo are allowed. + with caplog.at_level(logging.WARNING): + assert make_path_safe("./openflexure/data/")[0] == "./openflexure/data/" + assert make_path_safe("../openflexure/data/")[0] == "../openflexure/data/" + assert make_path_safe("../../openflexure/data/")[0] == "../../openflexure/data/" + assert make_path_safe(".")[0] == "." + assert make_path_safe("..")[0] == ".." - assert make_path_safe("path/./to/file") == "path/_/to/file" - assert make_path_safe("path/../to/file") == "path/_/to/file" - assert make_path_safe(".") == "_" - assert make_path_safe("..") == "_" + assert len(caplog.records) == 0 + + assert make_path_safe("a_bad/../relative_path")[0] == "a_bad/../relative_path" + assert len(caplog.records) == 1 + assert(f"File path a_bad/../relative_path may be unsafe due to unexpected relative navigation.") + + +@pytest.mark.parametrize("filepath", PATHS_WITH_DOTS) +def test_make_path_safe_dots_warning(caplog, filepath): + """Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged.""" + with caplog.at_level(logging.WARNING): + assert make_path_safe(filepath)[0] == filepath + assert f"File path {filepath} may be unsafe due to trailing dots." in caplog.messages From 968237bec09719ae2e84f0c8bf451e898a8f26db Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 9 Mar 2026 14:02:15 +0000 Subject: [PATCH 032/104] Ruff checks --- .../utilities.py | 47 +++++++++--- tests/unit_tests/test_utilities.py | 71 ++++++++++++------- 2 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index d16d494b..10215af8 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -111,8 +111,7 @@ _WINDOWS_RESERVED_NAMES = { def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: - """Check if a file path has any unsafe elements in it, such as - unsafe characters or reserved names for its OS. + """Check if a file path has any unsafe elements in it. The path is not coerced into a safe form because if we ask for a file to be written to a location we shouldn't be changing @@ -124,6 +123,7 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: any unsafe features were found. """ # Split by separators first to sanitise components independently + components = re.split(r"([/\\])", unsafe_path_string) unsafe_character_pattern = ( @@ -146,10 +146,16 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: # 1. Check for relative paths in the wrong place if component in (".", "..") and not unsafe_relative_path: - is_first = (i == 0) - is_second = (i == 2 and components[i-1] in ("/", "\\") and components[i-2] == "..") + is_first = i == 0 + is_second = ( + i == 2 + and components[i - 1] in ("/", "\\") + and components[i - 2] == ".." + ) if not (is_first or is_second): - LOGGER.warning(f"File path {unsafe_path_string} may be unsafe due to unexpected relative navigation.") + LOGGER.warning( + f"File path {unsafe_path_string} may be unsafe due to unexpected relative navigation." + ) unsafe_relative_path = True continue @@ -157,26 +163,45 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: if "." in component and i != len(components) - 1 and not trailing_dots: # Check for trailing dots in the file path before the file name # e.g.: foo/bar./file is bad, but foo/bar.file.py is fine - LOGGER.warning(f'File path {unsafe_path_string} may be unsafe due to trailing dots.') + LOGGER.warning( + f"File path {unsafe_path_string} may be unsafe due to trailing dots." + ) trailing_dots = True # Check for trailing spaces - Windows specific - if sys.platform.startswith("win") and component.endswith(" ") and not trailing_whitespace: + if ( + sys.platform.startswith("win") + and component.endswith(" ") + and not trailing_whitespace + ): LOGGER.warning(f"{unsafe_path_string} contains unsafe trailing spaces.") trailing_whitespace = True # 3. Check for unsafe characters (Regex) if unsafe_character_pattern.search(component) and not unsafe_characters: - LOGGER.warning(f"{unsafe_path_string} contains characters that may be unsafe on this platform.") + LOGGER.warning( + f"{unsafe_path_string} contains characters that may be unsafe on this platform." + ) unsafe_characters = True # 4. Check for Reserved Names (e.g., CON, PRN, LPT1) # Assuming _sanitise_reserved returns a different string if it's a reserved name - if _sanitise_reserved(component, is_filename=False) != component and not reserved_word: - LOGGER.warning(f"{unsafe_path_string} contains a reserved system name and may cause issues.") + if ( + _sanitise_reserved(component, is_filename=False) != component + and not reserved_word + ): + LOGGER.warning( + f"{unsafe_path_string} contains a reserved system name and may cause issues." + ) reserved_word = True - warning_raised = unsafe_relative_path or trailing_dots or trailing_whitespace or unsafe_characters or reserved_word + warning_raised = ( + unsafe_relative_path + or trailing_dots + or trailing_whitespace + or unsafe_characters + or reserved_word + ) return unsafe_path_string, warning_raised diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index c6d32431..6df07dec 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -1,26 +1,19 @@ """Unit tests for utility functions.""" -import sys import logging +import sys + import pytest from openflexure_microscope_server.utilities import ( _WINDOWS_RESERVED_NAMES, make_name_safe, - make_path_safe + make_path_safe, ) -PATHS_WITH_DOTS = { - "a./b/c.", - "path./to/file.", - "./path./to/file." -} +PATHS_WITH_DOTS = {"a./b/c.", "path./to/file.", "./path./to/file."} -RESERVED_NAME_PATHS = { - "path/CON/file", - "path/NUL.txt/file", - "C:/AUX/test" -} +RESERVED_NAME_PATHS = {"path/CON/file", "path/NUL.txt/file", "C:/AUX/test"} if sys.platform.startswith("win"): UNSAFE_CHARACTER_PATHS = { @@ -34,7 +27,7 @@ else: "/var/log/app:backup.log", "/etc/configs/network-settings\\v1", "/tmp/file_with_@_symbol.txt", - "/home/user/script(1).py" + "/home/user/script(1).py", } @@ -89,10 +82,16 @@ def test_make_path_safe_basic(caplog): with caplog.at_level(logging.WARNING): if sys.platform.startswith("win"): assert make_path_safe("C:\\path\\to/file")[0] == "C:\\path\\to/file" - assert make_path_safe("C:\\a very long path\\to/file")[0] == "C:\\a very long path\\to/file" + assert ( + make_path_safe("C:\\a very long path\\to/file")[0] + == "C:\\a very long path\\to/file" + ) else: assert make_path_safe("path/to/file")[0] == "path/to/file" - assert make_path_safe("a very long path/to/file")[0] == "a very long path/to/file" + assert ( + make_path_safe("a very long path/to/file")[0] + == "a very long path/to/file" + ) # No errors should be raised. These paths are safe. assert len(caplog.records) == 0 @@ -103,7 +102,10 @@ def test_make_path_safe_unsafe_characters_in_components(caplog, path): """Test unsafe characters within path components.""" with caplog.at_level(logging.WARNING): assert make_path_safe(path)[0] == path - assert f"{path} contains characters that may be unsafe on this platform." + assert ( + f"{path} contains characters that may be unsafe on this platform." + in caplog.messages + ) @pytest.mark.parametrize("path", RESERVED_NAME_PATHS) @@ -111,29 +113,40 @@ def test_make_path_safe_reserved_in_components(caplog, path): """Test reserved names within path components.""" with caplog.at_level(logging.WARNING): assert make_path_safe(path)[0] == path - assert f"{path} contains a reserved system name and may cause issues." + assert ( + f"{path} contains a reserved system name and may cause issues." + in caplog.messages + ) def test_make_path_safe_trailing_space_in_components(caplog): """Test trailing spaces in path components.""" with caplog.at_level(logging.WARNING): - # Test Windows - cannot have trailingspaces + # Test Windows - cannot have trailingspaces if sys.platform.startswith("win"): assert make_path_safe("path /to /file ")[0] == "path /to /file " assert make_path_safe("path/to /file")[0] == "path/to /file" # Check directories can have spaces in them but not at the end - # TODO - dont want underscores, want to keep interim spaces - assert make_path_safe("path/a long path name /file")[0] == "path/a long path name /file" + assert ( + make_path_safe("path/a long path name /file")[0] + == "path/a long path name /file" + ) assert len(caplog.records) == 3 - assert f"path /to /file contains unsafe trailing spaces." in caplog.messages - assert f"path/to /file contains unsafe trailing spaces." in caplog.messages - assert f"path/a long path name /file contains unsafe trailing spaces." in caplog.messages + assert "path /to /file contains unsafe trailing spaces." in caplog.messages + assert "path/to /file contains unsafe trailing spaces." in caplog.messages + assert ( + "path/a long path name /file contains unsafe trailing spaces." + in caplog.messages + ) else: # Trailing spaces allows in POSIX systems assert make_path_safe("path /to /file ")[0] == "path /to /file " assert make_path_safe("path/to /file")[0] == "path/to /file" # Check directories can have spaces in them but not at the end - assert make_path_safe("path/a long path name /file")[0] == "path/a long path name file" + assert ( + make_path_safe("path/a long path name /file")[0] + == "path/a long path name file" + ) # No warnings should be raised assert len(caplog.records) == 0 @@ -152,7 +165,10 @@ def test_make_path_safe_relative(caplog): assert make_path_safe("a_bad/../relative_path")[0] == "a_bad/../relative_path" assert len(caplog.records) == 1 - assert(f"File path a_bad/../relative_path may be unsafe due to unexpected relative navigation.") + assert ( + "File path a_bad/../relative_path may be unsafe due to unexpected relative navigation." + in caplog.messages + ) @pytest.mark.parametrize("filepath", PATHS_WITH_DOTS) @@ -160,4 +176,7 @@ def test_make_path_safe_dots_warning(caplog, filepath): """Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged.""" with caplog.at_level(logging.WARNING): assert make_path_safe(filepath)[0] == filepath - assert f"File path {filepath} may be unsafe due to trailing dots." in caplog.messages + assert ( + f"File path {filepath} may be unsafe due to trailing dots." + in caplog.messages + ) From b1d47c8aaeb69ce9132372753d1b4a33b2a36d1c Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 9 Mar 2026 14:09:00 +0000 Subject: [PATCH 033/104] Add final test case for path checking --- tests/unit_tests/test_utilities.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index 6df07dec..a1256e4b 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -119,6 +119,13 @@ def test_make_path_safe_reserved_in_components(caplog, path): ) +def test_make_path_safe_reserved_name_components(caplog): + """Test reserved names are okay as part of a larger dir name.""" + with caplog.at_level(logging.WARNING): + assert make_path_safe("chilli con carne/recipe")[0] == "chilli con carne/recipe" + assert len(caplog.records) == 0 + + def test_make_path_safe_trailing_space_in_components(caplog): """Test trailing spaces in path components.""" with caplog.at_level(logging.WARNING): From f50b3d5cfa7c5d1725652f3bde3d9994eef3e47c Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 9 Mar 2026 14:29:24 +0000 Subject: [PATCH 034/104] Allow spaces in linux paths --- src/openflexure_microscope_server/utilities.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 10215af8..6c73262f 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -76,10 +76,10 @@ def requires_lock( # Compiled regular expressions for unsafe characters -# Matches anything that isn't a-z, A-Z, 0-9, _, ., -, :, /, \ +# Matches anything that isn't a-z, A-Z, 0-9, _, ., -, :, /, \, space _WINDOWS_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-:/\ \\]") -# Matches anything that isn't a-z, A-Z, 0-9, _, ., -, \ -_POSIX_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-/]") +# Matches anything that isn't a-z, A-Z, 0-9, _, ., -, \, space +_POSIX_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-/ ]") # Matches anything that isn't a-z, A-Z, 0-9, _, ., - _NAME_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-]") From ada75857061cb880b3d627fcab70bfdba47a05fc Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 9 Mar 2026 14:40:51 +0000 Subject: [PATCH 035/104] Fix typos --- tests/unit_tests/test_utilities.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index a1256e4b..f17bfa3f 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -25,7 +25,7 @@ if sys.platform.startswith("win"): else: UNSAFE_CHARACTER_PATHS = { "/var/log/app:backup.log", - "/etc/configs/network-settings\\v1", + "/etc/configs/network-settings\v1", "/tmp/file_with_@_symbol.txt", "/home/user/script(1).py", } @@ -152,7 +152,7 @@ def test_make_path_safe_trailing_space_in_components(caplog): # Check directories can have spaces in them but not at the end assert ( make_path_safe("path/a long path name /file")[0] - == "path/a long path name file" + == "path/a long path name /file" ) # No warnings should be raised assert len(caplog.records) == 0 From 3776828699fa44f69d352b073768f00b711076a7 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 9 Mar 2026 14:52:23 +0000 Subject: [PATCH 036/104] Fix final typo --- tests/unit_tests/test_utilities.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index f17bfa3f..7cc17d7e 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -151,7 +151,7 @@ def test_make_path_safe_trailing_space_in_components(caplog): assert make_path_safe("path/to /file")[0] == "path/to /file" # Check directories can have spaces in them but not at the end assert ( - make_path_safe("path/a long path name /file")[0] + make_path_safe("path/a long path name /file")[0] == "path/a long path name /file" ) # No warnings should be raised From 6d66dd0ed8d97a8c434824d04fe6df72c1291b77 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 9 Mar 2026 15:45:32 +0000 Subject: [PATCH 037/104] Change make_path_safe to is_path_safe and update tests to reflect this --- .../stitching.py | 10 +-- .../utilities.py | 8 +- tests/unit_tests/test_utilities.py | 82 +++++++------------ 3 files changed, 38 insertions(+), 62 deletions(-) diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index fc764eb4..49916e10 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/src/openflexure_microscope_server/stitching.py @@ -18,7 +18,7 @@ from pydantic import BaseModel import labthings_fastapi as lt -from openflexure_microscope_server.utilities import make_path_safe +from openflexure_microscope_server.utilities import is_path_safe IS_WINDOWS = os.name == "nt" @@ -68,7 +68,7 @@ def validate_command(cmd: list[str]) -> None: """Validate that the command only contains characters that are allowed in a path. The values in the commands should be numbers, commandline flags, paths, and - executables. All of these should be allowed by ``make_path_safe``. + executables. All of these should be allowed by ``is_path_safe``. This also checks against a blacklist of forbidden commands for defense-in-depth. @@ -82,8 +82,7 @@ def validate_command(cmd: list[str]) -> None: ) # Ensure characters are safe for a path component - _, error_raised = make_path_safe(element) - if error_raised: + if not is_path_safe(element): raise StitcherValidationError( f"Invalid stitching command: Element '{element}' contains unsafe characters." ) @@ -155,8 +154,7 @@ class BaseStitcher: :raises RuntimeError: if inputs are unsafe. """ - _, error_raised = make_path_safe(self.images_dir) - if error_raised: + if not is_path_safe(self.images_dir): raise StitcherValidationError( "Invalid directory path: Contains unsafe characters." ) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 6c73262f..7da53cc6 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -110,7 +110,7 @@ _WINDOWS_RESERVED_NAMES = { } -def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: +def is_path_safe(unsafe_path_string: str) -> bool: """Check if a file path has any unsafe elements in it. The path is not coerced into a safe form because if we ask @@ -119,8 +119,7 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: :param unsafe_path_string: The original path string to sanitise. - :returns: The input string unchanged and a boolean indicating if - any unsafe features were found. + :returns: A boolean indicating if any unsafe features were found. """ # Split by separators first to sanitise components independently @@ -195,14 +194,13 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]: ) reserved_word = True - warning_raised = ( + return not ( unsafe_relative_path or trailing_dots or trailing_whitespace or unsafe_characters or reserved_word ) - return unsafe_path_string, warning_raised def make_name_safe(unsafe_name_string: str) -> str: diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py index 7cc17d7e..bc9076cd 100644 --- a/tests/unit_tests/test_utilities.py +++ b/tests/unit_tests/test_utilities.py @@ -7,12 +7,14 @@ import pytest from openflexure_microscope_server.utilities import ( _WINDOWS_RESERVED_NAMES, + is_path_safe, make_name_safe, - make_path_safe, ) PATHS_WITH_DOTS = {"a./b/c.", "path./to/file.", "./path./to/file."} +PATHS_WITH_SPACES = {"path /to /file ", "path/to /file", "path/a long path name /file"} + RESERVED_NAME_PATHS = {"path/CON/file", "path/NUL.txt/file", "C:/AUX/test"} if sys.platform.startswith("win"): @@ -76,32 +78,26 @@ def test_make_name_safe_reserved_names_false_positives(): assert make_name_safe("CHILLI CON CARNE") == "chilli_con_carne" -def test_make_path_safe_basic(caplog): - """Test basic functionality of make_path_safe.""" +def test_is_path_safe_basic(caplog): + """Test basic functionality of is_path_safe.""" # Note: behavior depends on platform for separators with caplog.at_level(logging.WARNING): if sys.platform.startswith("win"): - assert make_path_safe("C:\\path\\to/file")[0] == "C:\\path\\to/file" - assert ( - make_path_safe("C:\\a very long path\\to/file")[0] - == "C:\\a very long path\\to/file" - ) + assert is_path_safe("C:\\path\\to/file") + assert is_path_safe("C:\\a very long path\\to/file") else: - assert make_path_safe("path/to/file")[0] == "path/to/file" - assert ( - make_path_safe("a very long path/to/file")[0] - == "a very long path/to/file" - ) + assert is_path_safe("path/to/file") + assert is_path_safe("a very long path/to/file") # No errors should be raised. These paths are safe. assert len(caplog.records) == 0 @pytest.mark.parametrize("path", UNSAFE_CHARACTER_PATHS) -def test_make_path_safe_unsafe_characters_in_components(caplog, path): +def test_is_path_safe_unsafe_characters_in_components(caplog, path): """Test unsafe characters within path components.""" with caplog.at_level(logging.WARNING): - assert make_path_safe(path)[0] == path + assert not is_path_safe(path) assert ( f"{path} contains characters that may be unsafe on this platform." in caplog.messages @@ -109,68 +105,52 @@ def test_make_path_safe_unsafe_characters_in_components(caplog, path): @pytest.mark.parametrize("path", RESERVED_NAME_PATHS) -def test_make_path_safe_reserved_in_components(caplog, path): +def test_is_path_safe_reserved_in_components(caplog, path): """Test reserved names within path components.""" with caplog.at_level(logging.WARNING): - assert make_path_safe(path)[0] == path + assert not is_path_safe(path) assert ( f"{path} contains a reserved system name and may cause issues." in caplog.messages ) -def test_make_path_safe_reserved_name_components(caplog): +def test_is_path_safe_reserved_name_components(caplog): """Test reserved names are okay as part of a larger dir name.""" with caplog.at_level(logging.WARNING): - assert make_path_safe("chilli con carne/recipe")[0] == "chilli con carne/recipe" + assert is_path_safe("chilli con carne/recipe") assert len(caplog.records) == 0 -def test_make_path_safe_trailing_space_in_components(caplog): +@pytest.mark.parametrize("path", PATHS_WITH_SPACES) +def test_is_path_safe_trailing_space_in_components(caplog, path): """Test trailing spaces in path components.""" with caplog.at_level(logging.WARNING): - # Test Windows - cannot have trailingspaces + # Test Windows - cannot have trailing spaces if sys.platform.startswith("win"): - assert make_path_safe("path /to /file ")[0] == "path /to /file " - assert make_path_safe("path/to /file")[0] == "path/to /file" - # Check directories can have spaces in them but not at the end - assert ( - make_path_safe("path/a long path name /file")[0] - == "path/a long path name /file" - ) - assert len(caplog.records) == 3 - assert "path /to /file contains unsafe trailing spaces." in caplog.messages - assert "path/to /file contains unsafe trailing spaces." in caplog.messages - assert ( - "path/a long path name /file contains unsafe trailing spaces." - in caplog.messages - ) + assert not is_path_safe(path) + assert len(caplog.records) == 1 + assert f"{path} contains unsafe trailing spaces." in caplog.messages else: # Trailing spaces allows in POSIX systems - assert make_path_safe("path /to /file ")[0] == "path /to /file " - assert make_path_safe("path/to /file")[0] == "path/to /file" - # Check directories can have spaces in them but not at the end - assert ( - make_path_safe("path/a long path name /file")[0] - == "path/a long path name /file" - ) + assert is_path_safe(path) # No warnings should be raised assert len(caplog.records) == 0 -def test_make_path_safe_relative(caplog): +def test_is_path_safe_relative(caplog): """Test that relative path components are preserved.""" # Relative imports ./foo, ../foo or ../../foo are allowed. with caplog.at_level(logging.WARNING): - assert make_path_safe("./openflexure/data/")[0] == "./openflexure/data/" - assert make_path_safe("../openflexure/data/")[0] == "../openflexure/data/" - assert make_path_safe("../../openflexure/data/")[0] == "../../openflexure/data/" - assert make_path_safe(".")[0] == "." - assert make_path_safe("..")[0] == ".." + assert is_path_safe("./openflexure/data/") + assert is_path_safe("../openflexure/data/") + assert is_path_safe("../../openflexure/data/") + assert is_path_safe(".") + assert is_path_safe("..") assert len(caplog.records) == 0 - assert make_path_safe("a_bad/../relative_path")[0] == "a_bad/../relative_path" + assert not is_path_safe("a_bad/../relative_path") assert len(caplog.records) == 1 assert ( "File path a_bad/../relative_path may be unsafe due to unexpected relative navigation." @@ -179,10 +159,10 @@ def test_make_path_safe_relative(caplog): @pytest.mark.parametrize("filepath", PATHS_WITH_DOTS) -def test_make_path_safe_dots_warning(caplog, filepath): +def test_is_path_safe_dots_warning(caplog, filepath): """Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged.""" with caplog.at_level(logging.WARNING): - assert make_path_safe(filepath)[0] == filepath + assert not is_path_safe(filepath) assert ( f"File path {filepath} may be unsafe due to trailing dots." in caplog.messages From a9fdb376fe6acaf1eedfeeccbafaacd560b7f924 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 9 Mar 2026 15:50:22 +0000 Subject: [PATCH 038/104] Tab index -1 for spinners so we dont tab to them --- webapp/src/components/labThingsComponents/inputFromSchema.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/labThingsComponents/inputFromSchema.vue b/webapp/src/components/labThingsComponents/inputFromSchema.vue index 891075ea..fa1f4bd4 100644 --- a/webapp/src/components/labThingsComponents/inputFromSchema.vue +++ b/webapp/src/components/labThingsComponents/inputFromSchema.vue @@ -34,8 +34,8 @@ @animationend="animationEnd" /> From d5bc855114be8e1aa9e2f292764e09352dc60b9c Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 9 Mar 2026 16:18:45 +0000 Subject: [PATCH 039/104] Update prettier to use automatic line endings --- webapp/.prettierrc.json | 3 ++- .../components/labThingsComponents/inputFromSchema.vue | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/webapp/.prettierrc.json b/webapp/.prettierrc.json index 8ff73226..456d2afb 100644 --- a/webapp/.prettierrc.json +++ b/webapp/.prettierrc.json @@ -3,5 +3,6 @@ "semi": true, "singleQuote": false, "printWidth": 100, - "tabWidth": 2 + "tabWidth": 2, + "endOfLine": "auto" } \ No newline at end of file diff --git a/webapp/src/components/labThingsComponents/inputFromSchema.vue b/webapp/src/components/labThingsComponents/inputFromSchema.vue index fa1f4bd4..47ff8634 100644 --- a/webapp/src/components/labThingsComponents/inputFromSchema.vue +++ b/webapp/src/components/labThingsComponents/inputFromSchema.vue @@ -34,8 +34,12 @@ @animationend="animationEnd" /> From 16e5e091336030d7f028a78bc722fcdd1f7f332c Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 9 Mar 2026 17:03:10 +0000 Subject: [PATCH 040/104] Edit Bug.md --- .gitlab/issue_templates/Bug.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index e43bc1e4..68873825 100644 --- a/.gitlab/issue_templates/Bug.md +++ b/.gitlab/issue_templates/Bug.md @@ -4,10 +4,10 @@ - [ ] I have searched existing issues to ensure this hasn't been reported. -**Current Behaviour**: - **Expected Behaviour**: - + +**Current Behaviour**: + ## Configuration From b0c9223b043feee0c1cba071e72c1f6aca8b711b Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 9 Mar 2026 18:53:39 +0000 Subject: [PATCH 041/104] Move pre_scan and switch stream as late as possible --- src/openflexure_microscope_server/things/smart_scan.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 57ba5190..cb7488fd 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -376,11 +376,7 @@ class SmartScanThing(OFMThing): starting x,y,z position. """ try: - self._cam.start_streaming(main_resolution=(3280, 2464)) self._scan_data = self._collect_scan_data(workflow) - - workflow.pre_scan_routine(self._scan_data.workflow_settings) - self.ongoing_scan.save_scan_data(self._scan_data) images_dir = self.ongoing_scan.images_dir # Type narrowing if images_dir is None: @@ -388,6 +384,10 @@ class SmartScanThing(OFMThing): "Couldn't run scan, images directory was not created." ) + self.ongoing_scan.save_scan_data(self._scan_data) + self._cam.start_streaming(main_resolution=(3280, 2464)) + workflow.pre_scan_routine(self._scan_data.workflow_settings) + # If stitching settings are None then this type of scan can't be stitched if self.scan_data.stitching_settings is not None: # Settings exist, so create preview stitcher From 11d8c6d6042285ea52e5bb22f59e65065ddab7cb Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 9 Mar 2026 19:45:40 +0000 Subject: [PATCH 042/104] Stop closing seadragon when going full screen --- .../scanListComponents/openSeadragonViewer.vue | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/tabContentComponents/scanListComponents/openSeadragonViewer.vue b/webapp/src/components/tabContentComponents/scanListComponents/openSeadragonViewer.vue index a6712a89..b8f1c58a 100644 --- a/webapp/src/components/tabContentComponents/scanListComponents/openSeadragonViewer.vue +++ b/webapp/src/components/tabContentComponents/scanListComponents/openSeadragonViewer.vue @@ -83,7 +83,8 @@ export default { this.loadOpenSeaDragon(this.src); } } else { - if (this.osdViewer) { + // Don't destroy if viewer is in fullscreen + if (this.osdViewer && !this.osdViewer.isFullPage()) { this.osdViewer.destroy(); this.osdViewer = null; } @@ -121,7 +122,12 @@ export default { }, openFullscreen() { if (this.osdViewer) { - this.osdViewer.setFullScreen(true); + // Wait a bit for DOM to resize + setTimeout(() => { + this.osdViewer.setFullScreen(true); + this.updateFilter(); + this.osdViewer.forceRedraw(); + }, 500); } }, }, From 22999f7d523a12fa82388a65681c6458636463ba Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Tue, 10 Mar 2026 12:12:55 +0000 Subject: [PATCH 043/104] Remove uk-input from checkbox --- .../tabContentComponents/aboutComponents/devTools.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/components/tabContentComponents/aboutComponents/devTools.vue b/webapp/src/components/tabContentComponents/aboutComponents/devTools.vue index 2f3a2152..3b85ab33 100644 --- a/webapp/src/components/tabContentComponents/aboutComponents/devTools.vue +++ b/webapp/src/components/tabContentComponents/aboutComponents/devTools.vue @@ -4,7 +4,7 @@ From a11f9b7f6272674b81ab909f5b4e37e797423b05 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 8 Mar 2026 00:16:51 +0000 Subject: [PATCH 044/104] Start making a structure for general UI specification --- src/openflexure_microscope_server/ui.py | 154 +++++++++++++++++++++++- 1 file changed, 152 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index 3cb4375e..f24726d3 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/src/openflexure_microscope_server/ui.py @@ -1,11 +1,127 @@ """Functionality for communicating the required user interface for a thing.""" -from typing import Any, Optional +from html import escape +from html.parser import HTMLParser +from typing import Annotated, Any, Literal, Optional -from pydantic import BaseModel +from pydantic import BaseModel, BeforeValidator import labthings_fastapi as lt +ALLOWED_TAGS = { + "p", + "i", + "b", + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "ul", + "li", + "a", +} + + +class SafeHTMLParser(HTMLParser): + """An HTMLParser that only allows a very limited subset of HTML.""" + + def __init__(self) -> None: + """Initialise the parser.""" + super().__init__() + self.output = "" + + def handle_starttag(self, tag: str, attrs: dict[str, str]) -> None: + """Append a start tag found in the HTML if it is in the allowed list. + + This is called by the base class when a start tag is found. We only append + the tag if it is our allowed list. We re-write the tag so no extra attributes + can be added. + + No tags can have attributes except for ``a`` tags only allows ``href``. We then + append ``target="_blank" rel="noopener noreferrer"`` to ensure the link opens + in a new window. + + :param tag: the tag name + :param attrs: the attributes for the tag. + """ + if tag not in ALLOWED_TAGS: + return + + if tag == "a": + href = None + for attr, value in attrs: + if attr == "href": + href = value.strip() + + if href: + safe_href = escape(href, quote=True) + # Always in a new tab + self.output.append( + f'' + ) + else: + self.output += "" + else: + self.output += f"<{tag}>" + + def handle_endtag(self, tag: str) -> None: + """Append the end tag only if it is in the allowed list.""" + if tag in ALLOWED_TAGS: + self.output += f"" + + def handle_startendtag(self, tag: str, _attrs: dict[str, str]) -> None: + """Append a self-closing tag if it is a new line.""" + if tag == "br": + self.output += "
" + + def handle_data(self, data: str) -> None: + """Append any data inside tags after escaping it.""" + self.output += escape(data) + + def handle_entityref(self, name: str) -> None: + """Append any named character.""" + self.output += f"&{name};" + + def handle_charref(self, name: str) -> None: + """Append any character references.""" + self.output += f"&#{name};" + + +def sanitize_html(html: str) -> str: + """Santitise HTML to only have a small list of allowed tags. + + Tags allowed without attrs: ``

, , ,

,

,

,

,

,
,`` + ``
    ,
  • `` + + Tags allowed with attrs: ```` is allowed with ``href`` only. This automatically + appends ``target="_blank" rel="noopener noreferrer"`` so the link opens externally. + + Self closing tags allowed ``
    ``. + + :param html: The input HTML. + + :return: The sanitised HTML. + """ + parser = SafeHTMLParser() + parser.feed(html) + parser.close() + return parser.output + + +HtmlFragment = Annotated[str, BeforeValidator(sanitize_html)] + + +class HTMLBlock(BaseModel): + """The data required for creating a block of HTML for the UI. + + The HTML is limited to a small number of tags. + """ + + element_type: Literal["html_block"] = "html_block" + html: HtmlFragment + class ActionButton(BaseModel): """The data required for creating an actionButton in Vue. @@ -13,6 +129,8 @@ class ActionButton(BaseModel): Currently this cannot be used to submitData, or to submitOnEvent. """ + element_type: Literal["action_button"] = "action_button" + thing: str """The Thing "path" for the Thing instance.""" @@ -72,6 +190,8 @@ class PropertyControl(BaseModel): Currently this cannot be used to submitData, or to submitOnEvent. """ + element_type: Literal["property_control"] = "property_control" + thing: str """The Thing "path" for the Thing instance.""" @@ -121,3 +241,33 @@ def property_control_for( if "label" not in kwargs: kwargs["label"] = property_name return PropertyControl(thing=thing.name, property_name=property_name, **kwargs) + + +class Accordion(BaseModel): + """The data required for creating an accordion in the UI. + + The HTML is limited to a small number of tags. + """ + + element_type: Literal["accordion"] = "accordion" + title: str + children: list["UIElementModels"] + + +class Container(BaseModel): + """The data required for creating ``
    `` in the UI. + + The HTML is limited to a small number of tags. + """ + + element_type: Literal["container"] = "container" + css_class: str + children: list["UIElementModels"] + + +UIElementModels = HTMLBlock | PropertyControl | ActionButton | Accordion | Container + + +# Resolve forward references so Pydantic can build a sensible recursive schema +Accordion.model_rebuild() +Container.model_rebuild() From 9f2cde2275d0bdf19b9e3ec7abfbc35a7769627c Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 8 Mar 2026 11:06:04 +0000 Subject: [PATCH 045/104] Implement recursive Interface in vue --- .../things/scan_workflows.py | 107 ++++++++++-------- src/openflexure_microscope_server/ui.py | 24 +++- .../genericComponents/simpleAccordion.vue | 23 ++++ .../serverSpecifiedInterface.vue | 47 ++++++++ .../tabContentComponents/slideScanContent.vue | 14 +-- webapp/src/mixins/labThingsMixins.js | 9 ++ 6 files changed, 167 insertions(+), 57 deletions(-) create mode 100644 webapp/src/components/genericComponents/simpleAccordion.vue create mode 100644 webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 2380173a..df8c6e52 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -4,8 +4,6 @@ This module contains the base ``ScanWorkflow`` class that all workflows should s as well as specific workflows. """ -from __future__ import annotations - import os from typing import ( Generic, @@ -41,7 +39,12 @@ from openflexure_microscope_server.things.background_detect import ( from openflexure_microscope_server.things.camera import BaseCamera, CaptureParams from openflexure_microscope_server.things.camera_stage_mapping import CameraStageMapper from openflexure_microscope_server.things.stage import BaseStage -from openflexure_microscope_server.ui import PropertyControl, property_control_for +from openflexure_microscope_server.ui import ( + UI_ELEMENT_RESPONSE, + Accordion, + UIElementList, + property_control_for, +) SettingModelType = TypeVar("SettingModelType", bound=BaseModel) @@ -160,9 +163,9 @@ class ScanWorkflow(Generic[SettingModelType], lt.Thing): return True, focus_height - @lt.property - def settings_ui(self) -> list[PropertyControl]: - """A list of PropertyControl objects to create the settings in the scan tab.""" + @lt.endpoint("get", "settings_ui", responses=UI_ELEMENT_RESPONSE) + def settings_ui(self) -> UIElementList: + """Return the UI for the workflow's settings in the scan tab.""" raise NotImplementedError( "Each scan workflow must implement a settings_ui method." ) @@ -556,35 +559,43 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): return imaged, focus_height - @lt.property - def settings_ui(self) -> list[PropertyControl]: - """A list of PropertyControl objects to create the settings in the scan tab.""" - return [ - property_control_for( - self, "overlap", label="Image Overlap (0.1-0.7)", step=0.05 - ), - property_control_for( - self, "stack_images_to_save", label="Images in Stack to Save" - ), - property_control_for( - self, - "stack_min_images_to_test", - label="Minimum number of images to test for focus", - ), - property_control_for(self, "stack_dz", label="Stack dz (steps)", step=5), - property_control_for( - self, "autofocus_dz", label="Autofocus Range (steps)", step=200 - ), - property_control_for( - self, "max_range", label="Maximum Distance (steps)", step=1000 - ), - property_control_for( - self, "skip_background", label="Detect and Skip Empty Fields" - ), - property_control_for( - self, "equal_distances", label="Set Equal x and y Distances" - ), - ] + @lt.endpoint("get", "settings_ui", responses=UI_ELEMENT_RESPONSE) + def settings_ui(self) -> UIElementList: + """Return the UI for the workflow's settings in the scan tab.""" + return UIElementList( + [ + Accordion( + title="Background Detect", + children=self._background_detector.settings_ui, + ), + property_control_for( + self, "overlap", label="Image Overlap (0.1-0.7)", step=0.05 + ), + property_control_for( + self, "stack_images_to_save", label="Images in Stack to Save" + ), + property_control_for( + self, + "stack_min_images_to_test", + label="Minimum number of images to test for focus", + ), + property_control_for( + self, "stack_dz", label="Stack dz (steps)", step=5 + ).model_dump(), + property_control_for( + self, "autofocus_dz", label="Autofocus Range (steps)", step=200 + ), + property_control_for( + self, "max_range", label="Maximum Distance (steps)", step=1000 + ), + property_control_for( + self, "skip_background", label="Detect and Skip Empty Fields" + ), + property_control_for( + self, "equal_distances", label="Set Equal x and y Distances" + ), + ] + ) class RegularGridSettingsModel(RectGridSettingsModel): @@ -668,17 +679,21 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): save_resolution=settings.capture_params.save_resolution, ) - @lt.property - def settings_ui(self) -> list[PropertyControl]: - """A list of PropertyControl objects to create the settings in the scan tab.""" - return [ - property_control_for( - self, "overlap", label="Image Overlap (0.1-0.7)", step=0.05 - ), - property_control_for(self, "x_count", label="Number of columns"), - property_control_for(self, "y_count", label="Number of rows"), - property_control_for(self, "autofocus_dz", label="Autofocus Range (steps)"), - ] + @lt.endpoint("get", "settings_ui", responses=UI_ELEMENT_RESPONSE) + def settings_ui(self) -> UIElementList: + """Return the UI for the workflow's settings in the scan tab.""" + return UIElementList( + [ + property_control_for( + self, "overlap", label="Image Overlap (0.1-0.7)", step=0.05 + ), + property_control_for(self, "x_count", label="Number of columns"), + property_control_for(self, "y_count", label="Number of rows"), + property_control_for( + self, "autofocus_dz", label="Autofocus Range (steps)" + ), + ] + ) class SnakeWorkflow(RegularGridWorkflow): diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index f24726d3..eb5051f1 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/src/openflexure_microscope_server/ui.py @@ -4,7 +4,7 @@ from html import escape from html.parser import HTMLParser from typing import Annotated, Any, Literal, Optional -from pydantic import BaseModel, BeforeValidator +from pydantic import BaseModel, BeforeValidator, RootModel import labthings_fastapi as lt @@ -268,6 +268,28 @@ class Container(BaseModel): UIElementModels = HTMLBlock | PropertyControl | ActionButton | Accordion | Container +class UIElementList(RootModel[list[UIElementModels]]): + """A list of user interface elements. + + Note! Two important things to consider: + + * This cannot be the return of a lt.property. To fully describe the UI recursion + is used. Web of Things DataSchema doesn't support rectursion. Use a + lt.endpoint instead + * If the module using this class imoports future annotations then this will cause + havoc with pydantics forward references. + """ + + # Resolve forward references so Pydantic can build a sensible recursive schema Accordion.model_rebuild() Container.model_rebuild() +UIElementList.model_rebuild() + +# This constant can be used as the value for the `responses` argument of lt.endpoint. +UI_ELEMENT_RESPONSE = { + 200: { + "description": "A list of UI element specifications.", + "model": UIElementList, + } +} diff --git a/webapp/src/components/genericComponents/simpleAccordion.vue b/webapp/src/components/genericComponents/simpleAccordion.vue new file mode 100644 index 00000000..3fcecf8b --- /dev/null +++ b/webapp/src/components/genericComponents/simpleAccordion.vue @@ -0,0 +1,23 @@ + + + diff --git a/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue b/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue new file mode 100644 index 00000000..2afafcfe --- /dev/null +++ b/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue @@ -0,0 +1,47 @@ + + + + + diff --git a/webapp/src/components/tabContentComponents/slideScanContent.vue b/webapp/src/components/tabContentComponents/slideScanContent.vue index 281954b7..19b6c98e 100644 --- a/webapp/src/components/tabContentComponents/slideScanContent.vue +++ b/webapp/src/components/tabContentComponents/slideScanContent.vue @@ -24,13 +24,7 @@
  • Scan Settings
    -
    - -
    +
  • @@ -125,7 +119,7 @@ - diff --git a/webapp/src/components/tabContentComponents/backgroundDetectContent.vue b/webapp/src/components/tabContentComponents/backgroundDetectContent.vue deleted file mode 100644 index 7af5b0be..00000000 --- a/webapp/src/components/tabContentComponents/backgroundDetectContent.vue +++ /dev/null @@ -1,25 +0,0 @@ - - - From eccf26851e8a5f6ff036f1d550c911e3e12af0ef Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 8 Mar 2026 16:53:58 +0000 Subject: [PATCH 049/104] Reintroduce recactive button state for background detect --- .../things/scan_workflows.py | 2 ++ src/openflexure_microscope_server/ui.py | 9 +++++++++ .../components/labThingsComponents/actionButton.vue | 3 +++ .../serverSpecifiedActionButton.vue | 6 +++++- .../serverSpecifiedInterface.vue | 13 +++++++++++-- .../tabContentComponents/slideScanContent.vue | 2 +- 6 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 13b6dba3..9d80390d 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -625,12 +625,14 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): can_terminate=False, notify_on_success=True, success_message="Background image has been updated", + update_interface_on_response=True, ) check_bg_button = action_button_for( self, "check_background", poll_interval=0.1, submit_label="Check Current Image", + disabled=not self._background_detector.ready, can_terminate=False, notify_on_success=True, response_is_success_message=True, diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index 9df04f5d..771dccc7 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/src/openflexure_microscope_server/ui.py @@ -158,6 +158,9 @@ class ActionButton(BaseModel): button_primary: bool = True """Specify whether the button is styled as a primary button.""" + disabled: bool = False + """Specify whether the button is disabled.""" + modal_progress: bool = False """Specify whether to show a progress modal.""" @@ -180,6 +183,12 @@ class ActionButton(BaseModel): success_message: str = "Success!" """The message to show on successful completion.""" + update_interface_on_response: bool = False + """If True the action button emits a requestUpdate signal when completed. + + Downstream components can subscribe to this and request an update. + """ + def action_button_for(thing: lt.Thing, action_name: str, **kwargs: Any) -> ActionButton: """Create a ActionButton data for the specified Thing Action. diff --git a/webapp/src/components/labThingsComponents/actionButton.vue b/webapp/src/components/labThingsComponents/actionButton.vue index e5bfda75..eaaaf115 100644 --- a/webapp/src/components/labThingsComponents/actionButton.vue +++ b/webapp/src/components/labThingsComponents/actionButton.vue @@ -322,6 +322,9 @@ export default { } else if (response.data.status == "cancelled") { this.$emit("cancelled", response.data); this.modalNotify(`The action '${this.submitLabel}' was cancelled.`); + } else if (response.data.status == "error") { + const err_msg = this.log?.[0]?.message ?? "Unknown error"; + this.$emit("error", err_msg); } } this.taskUrl = null; diff --git a/webapp/src/components/labThingsComponents/serverSpecifiedActionButton.vue b/webapp/src/components/labThingsComponents/serverSpecifiedActionButton.vue index df1afd23..44b129fe 100644 --- a/webapp/src/components/labThingsComponents/serverSpecifiedActionButton.vue +++ b/webapp/src/components/labThingsComponents/serverSpecifiedActionButton.vue @@ -8,6 +8,7 @@ :requires-confirmation="actionData.requires_confirmation" :confirmation-message="actionData.confirmation_message" :button-primary="actionData.button_primary" + :is-disabled="actionData.disabled" :modal-progress="actionData.modal_progress" :stream-with-modal="actionData.stream_with_modal" @response="actionResponse" @@ -34,7 +35,7 @@ export default { }, }, - emits: ["response", "finished"], + emits: ["response", "finished", "requestUpdate"], methods: { /** @@ -50,6 +51,9 @@ export default { } else { this.modalNotify(this.actionData.success_message); } + if (this.actionData.update_interface_on_response) { + this.$emit("requestUpdate"); + } this.$emit("response", response); } }, diff --git a/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue b/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue index 8c47c622..10aa5636 100644 --- a/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue +++ b/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue @@ -10,12 +10,19 @@ - +
    - +
    @@ -41,6 +48,8 @@ export default { required: true, }, }, + + emits: ["requestUpdate"], }; diff --git a/webapp/src/components/tabContentComponents/slideScanContent.vue b/webapp/src/components/tabContentComponents/slideScanContent.vue index 8dd82d4c..c74a739e 100644 --- a/webapp/src/components/tabContentComponents/slideScanContent.vue +++ b/webapp/src/components/tabContentComponents/slideScanContent.vue @@ -16,7 +16,7 @@ - +
    • Stitching Settings From 88da4a18d9f05d9c6b2116644c6498a9746db808 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 8 Mar 2026 17:30:29 +0000 Subject: [PATCH 050/104] Update unit tests --- tests/unit_tests/test_background_detectors.py | 4 +- tests/unit_tests/test_scan_workflows.py | 50 ++++++++++++++++--- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/tests/unit_tests/test_background_detectors.py b/tests/unit_tests/test_background_detectors.py index f017ef7a..ea746044 100644 --- a/tests/unit_tests/test_background_detectors.py +++ b/tests/unit_tests/test_background_detectors.py @@ -73,7 +73,7 @@ def test_partial_base_classes(): bad_algo.ready with pytest.raises(NotImplementedError): - bad_algo.settings_ui + bad_algo.settings_ui() with pytest.raises(NotImplementedError): bad_algo.set_background(background_image) @@ -256,7 +256,7 @@ def test_background_detect_settings_ui(detector_cls): As both have identical settings they can be tested together """ detector = create_thing_without_server(detector_cls) - ui = detector.settings_ui + ui = detector.settings_ui().root assert len(ui) == 2 assert isinstance(ui[0], PropertyControl) diff --git a/tests/unit_tests/test_scan_workflows.py b/tests/unit_tests/test_scan_workflows.py index 483b1f99..b9f0fa07 100644 --- a/tests/unit_tests/test_scan_workflows.py +++ b/tests/unit_tests/test_scan_workflows.py @@ -18,7 +18,13 @@ from openflexure_microscope_server.things.scan_workflows import ( HistoScanWorkflow, ScanWorkflow, ) -from openflexure_microscope_server.ui import PropertyControl +from openflexure_microscope_server.ui import ( + Accordion, + ActionButton, + HTMLBlock, + PropertyControl, + UIElementList, +) def test_partial_base_classes(): @@ -56,7 +62,7 @@ def test_partial_base_classes(): bad_workflow.acquisition_routine(settings, xyz_pos=[0, 0, 0]) with pytest.raises(NotImplementedError): - bad_workflow.settings_ui + bad_workflow.settings_ui() @pytest.fixture @@ -370,15 +376,45 @@ def test_histo_acquisition_on_sample(histo_workflow, mocker, caplog): assert histo_workflow._autofocus.run_smart_stack.call_count == 2 -def test_histo_workflow_settings_ui(histo_workflow): +def test_histo_workflow_settings_ui(histo_workflow, mocker): """Check that the workflow specifies the expected controls.""" - ui = histo_workflow.settings_ui + mock_prop_control = mocker.Mock(spec=PropertyControl) + histo_workflow._background_detector.settings_ui.return_value = UIElementList( + [mock_prop_control] + ) - assert len(ui) == 8 - for element in ui: + ui = histo_workflow.settings_ui().root + + # The UI is in 3 blocks + assert len(ui) == 3 + # The top HTML block + assert isinstance(ui[0], HTMLBlock) + assert "

      Histo Scan

      " in ui[0].html + # Followed by a background detect accordion + assert isinstance(ui[1], Accordion) + assert ui[1].title == "Background Detect" + # And a Scan Settings accordion + assert isinstance(ui[2], Accordion) + assert ui[2].title == "Scan Settings" + + # Background UI is ... + background_ui = ui[1].children.root + # what the detector specifies as it settings ... + assert background_ui[0] is mock_prop_control + # plus 2 action buttons + assert len(background_ui) == 3 + assert isinstance(background_ui[1], ActionButton) + assert background_ui[1].action == "set_background" + assert isinstance(background_ui[2], ActionButton) + assert background_ui[2].action == "check_background" + + # Finally check the contents of the scan settings accordion + scan_settings = ui[2].children.root + assert len(scan_settings) == 8 + for element in scan_settings: assert isinstance(element, PropertyControl) - names = [el.property_name for el in ui] + names = [el.property_name for el in scan_settings] expected_names = [ "overlap", "stack_images_to_save", From 0bfec5d046f4d2b335566851463a5d0d4530c3c2 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 10 Mar 2026 09:35:29 +0000 Subject: [PATCH 051/104] Apply suggestions from code review of branch dynamic-ui Co-authored-by: Joe Knapper --- .../things/scan_workflows.py | 2 +- src/openflexure_microscope_server/ui.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 9d80390d..7679207a 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -577,7 +577,7 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): def set_background(self) -> None: """Set the background for this background detector. - This sets the background for this workflows background detector as opposed to + This sets the background for this workflow's background detector as opposed to the active background detector for the camera. """ image_array = self._cam.grab_as_array(stream_name="lores") diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index 771dccc7..1ed23127 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/src/openflexure_microscope_server/ui.py @@ -39,7 +39,7 @@ class SafeHTMLParser(HTMLParser): the tag if it is our allowed list. We re-write the tag so no extra attributes can be added. - No tags can have attributes except for ``a`` tags only allows ``href``. We then + No tags can have attributes except for ``a`` tags, why only allow ``href``. We then append ``target="_blank" rel="noopener noreferrer"`` to ensure the link opens in a new window. @@ -177,7 +177,7 @@ class ActionButton(BaseModel): response_is_success_message: bool = False """Set True to use the action response as the success message. - If set ``success_message`` has is never used. + If ``success_message`` is set, this is never used. """ success_message: str = "Success!" @@ -292,9 +292,9 @@ class UIElementList(RootModel[list[UIElementModels]]): Note! Two important things to consider: * This cannot be the return of a lt.property. To fully describe the UI recursion - is used. Web of Things DataSchema doesn't support rectursion. Use a + is used. Web of Things DataSchema doesn't support recursion. Use an lt.endpoint instead - * If the module using this class imoports future annotations then this will cause + * If the module using this class imports future annotations then this will cause havoc with pydantics forward references. """ From 85ebfef44a50dd6dcce25904d5a051f32e3fa48c Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 10 Mar 2026 12:41:00 +0000 Subject: [PATCH 052/104] Improve and test html sanitisation --- src/openflexure_microscope_server/ui.py | 27 ++++- tests/shared_utils/md_test_cases.py | 109 ++++++++++++++++++ .../data/html_sanitisation_test_cases.md | 84 ++++++++++++++ tests/unit_tests/test_ui.py | 24 ++++ 4 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 tests/shared_utils/md_test_cases.py create mode 100644 tests/unit_tests/data/html_sanitisation_test_cases.md create mode 100644 tests/unit_tests/test_ui.py diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index 1ed23127..b6692785 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/src/openflexure_microscope_server/ui.py @@ -3,6 +3,7 @@ from html import escape from html.parser import HTMLParser from typing import Annotated, Any, Literal, Optional +from urllib.parse import urlparse from pydantic import BaseModel, BeforeValidator, RootModel @@ -23,6 +24,25 @@ ALLOWED_TAGS = { "a", } +ALLOWED_SCHEMES = {"http", "https"} + + +def strip_control_chars(input_string: str) -> str: + """Remove unprintable characters.""" + return "".join(char for char in input_string if char.isprintable()) + + +def sanitize_http_url(url: str) -> Optional[str]: + """Santitise any url only allowing http and https without queries.""" + url = strip_control_chars(url) + parsed = urlparse(url) + if parsed.scheme not in ALLOWED_SCHEMES: + return None + if not parsed.netloc: + return None + fragment_str = "#" + parsed.fragment if parsed.fragment else "" + return f"{parsed.scheme}://{parsed.netloc}{fragment_str}" + class SafeHTMLParser(HTMLParser): """An HTMLParser that only allows a very limited subset of HTML.""" @@ -53,7 +73,8 @@ class SafeHTMLParser(HTMLParser): href = None for attr, value in attrs: if attr == "href" and value is not None: - href = value.strip() + # If it is not a httplink this will stay as None + href = sanitize_http_url(value.strip()) if href: safe_href = escape(href, quote=True) @@ -92,7 +113,7 @@ class SafeHTMLParser(HTMLParser): self.output += f"&#{name};" -def sanitize_html(html: str) -> str: +def sanitise_html(html: str) -> str: """Santitise HTML to only have a small list of allowed tags. Tags allowed without attrs: ``

      , , ,

      ,

      ,

      ,

      ,

      ,
      ,`` @@ -113,7 +134,7 @@ def sanitize_html(html: str) -> str: return parser.output -HtmlFragment = Annotated[str, BeforeValidator(sanitize_html)] +HtmlFragment = Annotated[str, BeforeValidator(sanitise_html)] class HTMLBlock(BaseModel): diff --git a/tests/shared_utils/md_test_cases.py b/tests/shared_utils/md_test_cases.py new file mode 100644 index 00000000..9a3e3b37 --- /dev/null +++ b/tests/shared_utils/md_test_cases.py @@ -0,0 +1,109 @@ +"""Read multi-line test cases from markdown files. + +When checking if text coercion is working multiline Python can be very difficult +to read and maintain. This file allows the input and expected results to be written +into markdown codeblocks. + +Each test is a second level heading. +""" + +import re + +# Level 2 markdown heading. Group 1 is the text +CASE_REGEX = re.compile(r"^##\s(.*)", re.MULTILINE) +# Level 3 markdown heading. Group 1 is the text +LEVEL3_REGEX = re.compile(r"^###\s(.*)", re.MULTILINE) +# Markdown code block. +# Group 1 is the language specification after the back ticks +# Group 2 is the code +CODE_REGEX = re.compile( + r"^```([a-zA-Z]+)?\s?\n(?:(.*?)\n)?```", re.MULTILINE | re.DOTALL +) + + +class MdTestCase: + """A class to represent a test case read from a markdown file. + + Useful attributes: + + * title - The title of the test case + * input_code - the input code for the test case + * result_code - the expected result code for the test case + """ + + def __init__(self, title: str, case_text: str, block_type: str) -> None: + """Initialise the test case. + + :param title: The title of the test case. + :param case_text: The full text of the test case. + :param block_type: The type of code block expected (the label after the triple + backticks). + """ + self.title = title + self.block_type = block_type + self.input_code, self.result_code = self._parse(case_text) + + def __repr__(self): + """Return the string representation of the test case.""" + return f"" + + def _parse(self, case_text: str) -> tuple[str, str]: + """Parse the case text and return the input and expected return code.""" + h3_matches = list(LEVEL3_REGEX.finditer(case_text)) + n_h3_matches = len(h3_matches) + if n_h3_matches != 2: + raise ValueError( + f"Case {self.title} has {n_h3_matches} level 3 headings. 2 expected." + ) + if h3_matches[0].group(1) != "Input" or h3_matches[1].group(1) != "Result": + raise ValueError( + f"Case {self.title} does not have `Input` and `Result` headings." + ) + + input_section = case_text[h3_matches[0].end() : h3_matches[1].start()] + result_section = case_text[h3_matches[1].end() : None] + + input_code_blocks = list(CODE_REGEX.finditer(input_section)) + result_code_blocks = list(CODE_REGEX.finditer(result_section)) + if len(input_code_blocks) != 1 or len(result_code_blocks) != 1: + raise ValueError( + f"Case {self.title} must have exactly 1 code block per section." + ) + + input_code_type = input_code_blocks[0].group(1) + input_code = input_code_blocks[0].group(2) + result_code_type = result_code_blocks[0].group(1) + result_code = result_code_blocks[0].group(2) + if input_code_type != self.block_type or result_code_type != self.block_type: + raise ValueError( + f"Case {self.title}: each code block must be labeled as " + f"{self.block_type}" + ) + + # If the code block is empty Regex returns None. Coerce to empty string. + input_code = "" if input_code is None else input_code.strip() + result_code = "" if result_code is None else result_code.strip() + return input_code, result_code + + +def find_test_cases(path: str, block_type: str) -> list[MdTestCase]: + """Return all test cases in the markdown file. + + :param path: Path to the markdown file containing cases. + :param block_type: The label for each triple backtick code block. + + :return: A list of MdTestCase objects for each case. + """ + with open(path, "r", encoding="utf-8") as file_obj: + text = file_obj.read() + test_case_headings = list(CASE_REGEX.finditer(text)) + test_cases = [] + n_headings = len(test_case_headings) + for i, heading_match in enumerate(test_case_headings): + title = heading_match.group(1) + start = heading_match.end() + end = test_case_headings[i + 1].start() if i + 1 < n_headings else None + + case_text = text[start:end] + test_cases.append(MdTestCase(title, case_text, block_type)) + return test_cases diff --git a/tests/unit_tests/data/html_sanitisation_test_cases.md b/tests/unit_tests/data/html_sanitisation_test_cases.md new file mode 100644 index 00000000..2e550d78 --- /dev/null +++ b/tests/unit_tests/data/html_sanitisation_test_cases.md @@ -0,0 +1,84 @@ +# HTML Sanitisation Test Cases + +This file is in markdown not python for readability. + +Each test is a level 2 heading. The input and expected values are level 3 headings. + +The HTML to test is in triple backtick code blocks. + +Anything not in code blocks is a comment. + +## Test scripts removed + +The script is removed but the contents remain. + +### Input +```html + +

      Hello

      +``` +### Result +```html +alert("x") +

      Hello

      +``` +## Test script in bold tag + +### Input +```html +click me! +``` +### Result +```html +click me! +``` + +## Test script in disallowed tag +### Input +```html + +``` +### Result +```html +``` + +## Test cookie grabber script is escaped +The script should become escaped and have no tags +### Input +```html + +``` +### Result +```html +var adr = '../evil.php?cakemonster=' + escape(document.cookie); +``` + +## Test with non-http links +Remove the javascript from the links. +### Input +```html +link +encoded +Email +Relative +``` +### Result +```html +link +encoded +Email +Relative +``` + +## Test http links become external +Remove the javascript from the links. +### Input +```html +OpenFlexure +``` +### Result +```html +OpenFlexure +``` \ No newline at end of file diff --git a/tests/unit_tests/test_ui.py b/tests/unit_tests/test_ui.py new file mode 100644 index 00000000..1233458c --- /dev/null +++ b/tests/unit_tests/test_ui.py @@ -0,0 +1,24 @@ +"""Testing the server specified user interface.""" + +import os + +import pytest + +from openflexure_microscope_server import ui + +from ..shared_utils.md_test_cases import find_test_cases + +THIS_DIR = os.path.dirname(os.path.abspath(__file__)) +HTML_SANITISATION_TEST_FILE = os.path.join( + THIS_DIR, "data", "html_sanitisation_test_cases.md" +) +HTML_SANITISATION_TEST_CASES = find_test_cases(HTML_SANITISATION_TEST_FILE, "html") + + +@pytest.mark.parametrize("test_case", HTML_SANITISATION_TEST_CASES) +def test_html_block_sanitises(test_case): + """Check each case in the test case file sanitises as expected.""" + sanitised = ui.sanitise_html(test_case.input_code).strip() + assert sanitised == test_case.result_code, ( + f"Test case: '{test_case.title}': Input code doesn't sanitise as expected." + ) From c4d0c0e9daa49a9c742a6000cfe231965b247444 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 10 Mar 2026 15:25:04 +0000 Subject: [PATCH 053/104] Explicit header, text and bullet blocks rather than one HTML block in ui spec --- .../things/scan_workflows.py | 9 ++- src/openflexure_microscope_server/ui.py | 60 +++++++++++-------- .../data/html_sanitisation_test_cases.md | 13 +++- tests/unit_tests/test_scan_workflows.py | 29 +++++---- .../serverSpecifiedInterface.vue | 12 +++- 5 files changed, 82 insertions(+), 41 deletions(-) diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 7679207a..1bf5b826 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -42,7 +42,8 @@ from openflexure_microscope_server.things.stage import BaseStage from openflexure_microscope_server.ui import ( UI_ELEMENT_RESPONSE, Accordion, - HTMLBlock, + HeaderBlock, + TextBlock, UIElementList, action_button_for, property_control_for, @@ -642,7 +643,8 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): return UIElementList( [ - HTMLBlock(html=f"

      {self.display_name}

      {self.ui_blurb}

      "), + HeaderBlock(text=self.display_name, level=4), + TextBlock(text=self.ui_blurb), Accordion(title="Background Detect", children=background_ui), Accordion( title="Scan Settings", @@ -750,7 +752,8 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): ) return UIElementList( [ - HTMLBlock(html=f"

      {self.display_name}

      {self.ui_blurb}

      "), + HeaderBlock(text=self.display_name, level=4), + TextBlock(text=self.ui_blurb), Accordion( title="Scan Settings", children=scan_settings, diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index b6692785..2524515e 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/src/openflexure_microscope_server/ui.py @@ -5,24 +5,12 @@ from html.parser import HTMLParser from typing import Annotated, Any, Literal, Optional from urllib.parse import urlparse -from pydantic import BaseModel, BeforeValidator, RootModel +from pydantic import BaseModel, BeforeValidator, Field, RootModel import labthings_fastapi as lt -ALLOWED_TAGS = { - "p", - "i", - "b", - "h1", - "h2", - "h3", - "h4", - "h5", - "h6", - "ul", - "li", - "a", -} +# Only allowed tags are italic, bold, and link. Also allows self closing br tags. +ALLOWED_TAGS = {"i", "b", "a"} ALLOWED_SCHEMES = {"http", "https"} @@ -116,8 +104,7 @@ class SafeHTMLParser(HTMLParser): def sanitise_html(html: str) -> str: """Santitise HTML to only have a small list of allowed tags. - Tags allowed without attrs: ``

      , , ,

      ,

      ,

      ,

      ,

      ,
      ,`` - ``
        ,
      • `` + Tags allowed without attrs: ``, ,`` Tags allowed with attrs: ```` is allowed with ``href`` only. This automatically appends ``target="_blank" rel="noopener noreferrer"`` so the link opens externally. @@ -137,14 +124,31 @@ def sanitise_html(html: str) -> str: HtmlFragment = Annotated[str, BeforeValidator(sanitise_html)] -class HTMLBlock(BaseModel): - """The data required for creating a block of HTML for the UI. +class HeaderBlock(BaseModel): + """The data required for header.""" - The HTML is limited to a small number of tags. - """ + element_type: Literal["header_block"] = "header_block" + text: HtmlFragment + """The header text (can include basic HTML tags).""" - element_type: Literal["html_block"] = "html_block" - html: HtmlFragment + level: int = Field(default=2, ge=1, le=6) + """The header level. A number from 1-6.""" + + +class TextBlock(BaseModel): + """The data required for creating a block text.""" + + element_type: Literal["text_block"] = "text_block" + text: HtmlFragment + """The text (can include basic HTML tags).""" + + +class BulletBlock(BaseModel): + """The data required for creating a block text.""" + + element_type: Literal["bullet_block"] = "bullet_block" + bullets: list[HtmlFragment] + """A list of text elements (can include basic HTML tags).""" class ActionButton(BaseModel): @@ -304,7 +308,15 @@ class Container(BaseModel): children: "UIElementList" -UIElementModels = HTMLBlock | PropertyControl | ActionButton | Accordion | Container +UIElementModels = ( + HeaderBlock + | TextBlock + | BulletBlock + | PropertyControl + | ActionButton + | Accordion + | Container +) class UIElementList(RootModel[list[UIElementModels]]): diff --git a/tests/unit_tests/data/html_sanitisation_test_cases.md b/tests/unit_tests/data/html_sanitisation_test_cases.md index 2e550d78..f1488883 100644 --- a/tests/unit_tests/data/html_sanitisation_test_cases.md +++ b/tests/unit_tests/data/html_sanitisation_test_cases.md @@ -20,7 +20,7 @@ The script is removed but the contents remain. ### Result ```html alert("x") -

        Hello

        +Hello ``` ## Test script in bold tag @@ -81,4 +81,15 @@ Remove the javascript from the links. ### Result ```html         OpenFlexure +``` + +## Test basic formatting preserved +Check basic elements are not changed +### Input +```html +This is bold italic and on a new
        line. +``` +### Result +```html +This is bold italic and on a new
        line. ``` \ No newline at end of file diff --git a/tests/unit_tests/test_scan_workflows.py b/tests/unit_tests/test_scan_workflows.py index b9f0fa07..0ffa4a00 100644 --- a/tests/unit_tests/test_scan_workflows.py +++ b/tests/unit_tests/test_scan_workflows.py @@ -21,8 +21,9 @@ from openflexure_microscope_server.things.scan_workflows import ( from openflexure_microscope_server.ui import ( Accordion, ActionButton, - HTMLBlock, + HeaderBlock, PropertyControl, + TextBlock, UIElementList, ) @@ -385,20 +386,24 @@ def test_histo_workflow_settings_ui(histo_workflow, mocker): ui = histo_workflow.settings_ui().root - # The UI is in 3 blocks - assert len(ui) == 3 - # The top HTML block - assert isinstance(ui[0], HTMLBlock) - assert "

        Histo Scan

        " in ui[0].html + # The UI is in 4 blocks + assert len(ui) == 4 + # The top is a header + assert isinstance(ui[0], HeaderBlock) + assert ui[0].text == "Histo Scan" + # The then some text (note the & is escaped) + assert isinstance(ui[1], TextBlock) + fragment = "This scan workflow is optimised for scanning H&E stained biopsies." + assert fragment in ui[1].text # Followed by a background detect accordion - assert isinstance(ui[1], Accordion) - assert ui[1].title == "Background Detect" - # And a Scan Settings accordion assert isinstance(ui[2], Accordion) - assert ui[2].title == "Scan Settings" + assert ui[2].title == "Background Detect" + # And a Scan Settings accordion + assert isinstance(ui[3], Accordion) + assert ui[3].title == "Scan Settings" # Background UI is ... - background_ui = ui[1].children.root + background_ui = ui[2].children.root # what the detector specifies as it settings ... assert background_ui[0] is mock_prop_control # plus 2 action buttons @@ -409,7 +414,7 @@ def test_histo_workflow_settings_ui(histo_workflow, mocker): assert background_ui[2].action == "check_background" # Finally check the contents of the scan settings accordion - scan_settings = ui[2].children.root + scan_settings = ui[3].children.root assert len(scan_settings) == 8 for element in scan_settings: assert isinstance(element, PropertyControl) diff --git a/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue b/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue index 10aa5636..f69ae102 100644 --- a/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue +++ b/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue @@ -1,7 +1,17 @@ From ffb9a4d5097e0402f7c433718045e848999e4533 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Wed, 4 Mar 2026 13:12:43 +0000 Subject: [PATCH 066/104] Add CChip workflow to scan_workflows --- ofm_config_full.json | 1 + ofm_config_simulation.json | 1 + .../things/scan_workflows.py | 152 ++++++++++++++++-- 3 files changed, 142 insertions(+), 12 deletions(-) diff --git a/ofm_config_full.json b/ofm_config_full.json index 27024a2c..47fd7e47 100644 --- a/ofm_config_full.json +++ b/ofm_config_full.json @@ -17,6 +17,7 @@ "default_workflow": "histo_scan_workflow" } }, + "CChip_workflow": "openflexure_microscope_server.things.scan_workflows:CChipWorkflow", "histo_scan_workflow": "openflexure_microscope_server.things.scan_workflows:HistoScanWorkflow", "snake_workflow": "openflexure_microscope_server.things.scan_workflows:SnakeWorkflow", "raster_workflow": "openflexure_microscope_server.things.scan_workflows:RasterWorkflow", diff --git a/ofm_config_simulation.json b/ofm_config_simulation.json index b04988d4..dbc246f7 100644 --- a/ofm_config_simulation.json +++ b/ofm_config_simulation.json @@ -12,6 +12,7 @@ "default_workflow": "histo_scan_workflow" } }, + "CChip_workflow": "openflexure_microscope_server.things.scan_workflows:CChipWorkflow", "histo_scan_workflow": "openflexure_microscope_server.things.scan_workflows:HistoScanWorkflow", "snake_workflow": "openflexure_microscope_server.things.scan_workflows:SnakeWorkflow", "raster_workflow": "openflexure_microscope_server.things.scan_workflows:RasterWorkflow", diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 1bf5b826..93a3d8bc 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -32,6 +32,7 @@ from openflexure_microscope_server.things.autofocus import ( AutofocusParams, AutofocusThing, SmartStackParams, + StackParams, ) from openflexure_microscope_server.things.background_detect import ( ChannelDeviationLUV, @@ -666,7 +667,14 @@ class RegularGridSettingsModel(RectGridSettingsModel): style: Literal["snake", "raster"] -class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): +RegGridSettingModelType = TypeVar( + "RegGridSettingModelType", bound=RegularGridSettingsModel +) + + +class RegularGridWorkflow( + RectGridWorkflow[RegGridSettingModelType], Generic[RegGridSettingModelType] +): """A base workflow for any workflow that uses a regular rectangular grid.""" x_count: int = lt.setting(default=3, ge=1) @@ -674,20 +682,20 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): y_count: int = lt.setting(default=2, ge=1) """The number of rows in the scan.""" - _settings_model = RegularGridSettingsModel + _settings_model: type[RegGridSettingModelType] _planner_cls = RegularGridPlanner _grid_style: Literal["snake", "raster"] - def _build_scan_settings(self, base_kwargs: dict) -> RegularGridSettingsModel: + def _build_scan_settings(self, base_kwargs: dict) -> RegGridSettingModelType: """Construct the SettingModel for all_settings.""" - return RegularGridSettingsModel( + return self._settings_model( **base_kwargs, x_count=self.x_count, y_count=self.y_count, style=self._grid_style, ) - def pre_scan_routine(self, settings: RegularGridSettingsModel) -> None: + def pre_scan_routine(self, settings: RegGridSettingModelType) -> None: """Perform these steps before starting the scan. In this case, only autofocus. @@ -699,11 +707,11 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): ) def new_scan_planner( - self, settings: RegularGridSettingsModel, position: Mapping[str, int] + self, settings: RegGridSettingModelType, position: Mapping[str, int] ) -> ScanPlanner: """Return a new scan planner object. - :param settings: The settings for this scan as a SnakeSettingsModel + :param settings: The settings for this scan as the relevant SettingsModel type. :param position: The starting position as a mapping of axes names to int. """ planner_settings = { @@ -719,11 +727,11 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): ) def acquisition_routine( - self, settings: RegularGridSettingsModel, xyz_pos: tuple[int, int, int] + self, settings: RegGridSettingModelType, xyz_pos: tuple[int, int, int] ) -> tuple[bool, Optional[int]]: """Autofocus and capture. - :param settings: The settings for this scan as a RegularGridSettingsModel + :param settings: The settings for this scan as the relevant SettingsModel type. :param xyz_pos: The current position as a tuple or 3 ints. :return: A tuple of whether an image was taken, and the z-position for focus. If failed to find focus, returns for the focus z-position. @@ -762,7 +770,7 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): ) -class SnakeWorkflow(RegularGridWorkflow): +class SnakeWorkflow(RegularGridWorkflow[RegularGridSettingsModel]): """A workflow optimised for snaking around samples. This workflow generates a list of coordinates in a rectangle, and snakes @@ -777,10 +785,11 @@ class SnakeWorkflow(RegularGridWorkflow): ), readonly=True, ) + _settings_model = RegularGridSettingsModel _grid_style = "snake" -class RasterWorkflow(RegularGridWorkflow): +class RasterWorkflow(RegularGridWorkflow[RegularGridSettingsModel]): """A workflow optimised for snaking around samples. This workflow generates a list of coordinates in a rectangle, and always @@ -796,5 +805,124 @@ class RasterWorkflow(RegularGridWorkflow): ), readonly=True, ) - + _settings_model = RegularGridSettingsModel _grid_style = "raster" + + +class CChipScanSettingsModel(RegularGridSettingsModel): + """The settings for a scan with the CChipWorkflow. + + This includes settings calculated when starting. This will be held by smart scan + during a scan and serialised to disk. + """ + + stack_params: StackParams + + +class CChipWorkflow(RegularGridWorkflow[CChipScanSettingsModel]): + """A workflow optimised for scanning the well of a CChip. + + This workflow generates a list of coordinates in a rectangle, and snakes + around them from the top left (assuming positive dx and dy), stacking the + grid and above. + """ + + display_name: str = lt.property(default="C-Chip Scan", readonly=True) + ui_blurb: str = lt.property( + default=( + "This scan workflow is optimised for scanning a C-Chip. It focuses on " + "a grid, then stacks images above the grid to complete a volumetric scan." + ), + readonly=True, + ) + _grid_style = "snake" + _settings_model = CChipScanSettingsModel + + overlap: float = lt.setting(default=0.1, ge=0.1, le=0.7) + """The fraction that adjacent images should overlap in x and y. + + This must be between 0.1 and 0.7. + """ + x_count: int = lt.setting(default=5, readonly=False) + """The number of columns in the scan.""" + y_count: int = lt.setting(default=7, readonly=False) + """The number of rows in the scan.""" + + stack_images_to_save: int = lt.setting(default=9, readonly=False) + """The number of images to save in a stack. + + Defaults to 1 unless you need to see either side of focus + """ + + stack_dz: int = lt.setting(default=500, readonly=False) + """Distance in steps between images in a z-stack.""" + + def create_stack_params( + self, + ) -> StackParams: + """Set up the parameters used for all stacks in a scan. + + :returns: A StackSmartParams object with the required parameters. + """ + return StackParams( + stack_dz=self.stack_dz, images_to_save=self.stack_images_to_save + ) + + def _build_scan_settings(self, base_kwargs: dict) -> CChipScanSettingsModel: + """Construct the SettingModel for all_settings.""" + stack_params = self.create_stack_params() + return self._settings_model( + **base_kwargs, + x_count=self.x_count, + y_count=self.y_count, + style=self._grid_style, + stack_params=stack_params, + ) + + def pre_scan_routine(self, settings: CChipScanSettingsModel) -> None: + """No autofocus, a looping autofocus on a CChip could corrupt the entire scan.""" + pass + + def acquisition_routine( + self, + settings: CChipScanSettingsModel, + xyz_pos: tuple[int, int, int], # noqa: ARG002 + ) -> tuple[bool, Optional[int]]: + """Autofocus and capture a z-stack starting at the focused position. + + The routine performs a fast autofocus using the provided ``dz``. + The focused z-height becomes the starting position for the stack + and is also the height of the first captured image. + + A stack of ``self.stack_images_to_save`` images is then acquired. + Each subsequent image is captured after moving the stage upward + by ``self.stack_dz`` steps in z. + + :param xyz_pos: The (x, y, z) position associated with this acquisition. + :param settings: The settings for this scan as a CChipSettingsModel + + :return: (True, focus_height) where focus_height is the autofocus + z-position and the height of the first image in the stack. + """ + # Perform autofocus + self._autofocus.fast_autofocus(dz=settings.autofocus_params.dz) + focus_height = self._stage.get_xyz_position()[2] + self._autofocus.run_basic_stack(settings.stack_params, settings.capture_params) + + return True, focus_height + + @lt.property + def settings_ui(self) -> list[PropertyControl]: + """A list of PropertyControl objects to create the settings in the scan tab.""" + return [ + property_control_for(self, "overlap", label="Image Overlap (0.1-0.7)"), + property_control_for(self, "x_count", label="Number of columns"), + property_control_for(self, "y_count", label="Number of rows"), + property_control_for(self, "autofocus_dz", label="Autofocus Range (steps)"), + property_control_for( + self, "stack_dz", label="Distance in z between images in stack (steps)" + ), + property_control_for( + self, "stack_images_to_save", label="Images to save per xy site" + ), + ] From b43150c8bbb5ecd6c690d2daaffd792da215151c Mon Sep 17 00:00:00 2001 From: jaknapper Date: Wed, 4 Mar 2026 18:43:41 +0000 Subject: [PATCH 067/104] Remove backlash correction from basic stack, as this is handled by move rel --- .../things/autofocus.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 909ff229..9bb4b801 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -67,11 +67,6 @@ class StackParams(BaseModel): settling_time: float = Field(default=0.3, ge=0) """Time (in seconds) between moving and capturing an image""" - backlash_correction: int = 250 - """ - Distance (in steps) to overshoot a move and then undo, to account for backlash - """ - origin: StackOrigin = StackOrigin.START """Where the stack is positioned relative to the current z position.""" @@ -760,7 +755,7 @@ class AutofocusThing(lt.Thing): logic, or autofocus is performed. :param stack_parameters: StackParams defining stack spacing, - image count and backlash correction. + and image count. :param capture_parameters: CaptureParams defining save resolution, images directory. @@ -781,14 +776,7 @@ class AutofocusThing(lt.Thing): else: target_offset = 0 - # Apply backlash correction: overshoot and move back - overshoot = target_offset - stack_parameters.backlash_correction - if overshoot != 0: - self._stage.move_relative(z=overshoot) - - # Move back to starting point if needed - if stack_parameters.backlash_correction != 0: - self._stage.move_relative(z=stack_parameters.backlash_correction) + self._stage.move_relative(z=target_offset) # Capture images_to_save images for move_count in range(stack_parameters.images_to_save): From 5ce77538d160b04472ec6c72d20d852b9b412960 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Tue, 10 Mar 2026 15:04:42 +0000 Subject: [PATCH 068/104] Fix tests now we dont do extra backlash moves --- .../things/autofocus.py | 4 +- tests/unit_tests/test_stack.py | 96 ------------------- 2 files changed, 2 insertions(+), 98 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 9bb4b801..5cbc050c 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -775,8 +775,8 @@ class AutofocusThing(lt.Thing): target_offset = -total_range else: target_offset = 0 - - self._stage.move_relative(z=target_offset) + if target_offset != 0: + self._stage.move_relative(z=target_offset) # Capture images_to_save images for move_count in range(stack_parameters.images_to_save): diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 6adf2df7..6e022143 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -694,7 +694,6 @@ def test_run_basic_stack_simple( stack_dz=10, images_to_save=3, settling_time=0, - backlash_correction=0, origin=StackOrigin.START, ) @@ -757,7 +756,6 @@ def test_run_basic_stack_center_origin( stack_dz=10, images_to_save=5, settling_time=0, - backlash_correction=0, origin=StackOrigin.CENTER, ) @@ -804,97 +802,6 @@ def test_run_basic_stack_center_origin( assert final_z == expected_final_z, "Final Z position is incorrect" -def test_run_basic_stack_backlash_applied( - autofocus_thing, mocker, fake_capture, fake_move_relative -): - """Backlash correction should overshoot first, then move back before starting stack.""" - stack_params = StackParams( - stack_dz=10, - images_to_save=3, - settling_time=0, - backlash_correction=50, - origin=StackOrigin.START, - ) - - capture_params = mocker.Mock() - capture_params.images_dir = "dummy" - capture_params.save_resolution = (100, 100) - - start_z = 0 - autofocus_thing._stage.position = {"x": 0, "y": 0, "z": start_z} - - autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) - autofocus_thing._cam.save_from_memory = mocker.Mock() - autofocus_thing._cam.clear_buffers = mocker.Mock() - - autofocus_thing.run_basic_stack(stack_params, capture_params) - - calls = autofocus_thing._stage.move_relative.call_args_list - - # First move is overshoot for backlash - assert calls[0].kwargs["z"] == -stack_params.backlash_correction, ( - "Backlash overshoot not applied correctly" - ) - - # Second move corrects back to starting point - assert calls[1].kwargs["z"] == stack_params.backlash_correction, ( - "Backlash correction move not applied correctly" - ) - - -def test_run_basic_stack_backlash_and_offset( - autofocus_thing, mocker, fake_capture, fake_move_relative -): - """Backlash correction and stack offset both applied. - - Stack should begin by moving down by half the height of the stack, - plus backlash correction, then move up by backlash correction, then run the basic stack. - """ - stack_params = StackParams( - stack_dz=100, - images_to_save=3, - settling_time=0, - backlash_correction=50, - origin=StackOrigin.CENTER, - ) - - capture_params = mocker.Mock() - capture_params.images_dir = "dummy" - capture_params.save_resolution = (100, 100) - - start_z = 0 - autofocus_thing._stage.position = {"x": 0, "y": 0, "z": start_z} - - autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) - autofocus_thing._cam.save_from_memory = mocker.Mock() - autofocus_thing._cam.clear_buffers = mocker.Mock() - - autofocus_thing.run_basic_stack(stack_params, capture_params) - - calls = autofocus_thing._stage.move_relative.call_args_list - - # Combined initial offset + backlash overshoot - total_stack_range = stack_params.stack_dz * (stack_params.images_to_save - 1) - expected_first_move = -(total_stack_range // 2 + stack_params.backlash_correction) - assert calls[0].kwargs["z"] == expected_first_move, ( - "Combined overshoot not applied correctly" - ) - - # Backlash correction back to base of stack - assert calls[1].kwargs["z"] == stack_params.backlash_correction, ( - "Backlash correction move not applied correctly" - ) - - # Subsequent moves in stack - expected_stack_moves = [stack_params.stack_dz] * (stack_params.images_to_save - 1) - actual_stack_moves = [c.kwargs["z"] for c in calls[2:]] - assert actual_stack_moves == expected_stack_moves, ( - "Stack moves after backlash/offset not correct" - ) - - def test_run_basic_stack_end_origin( autofocus_thing, mocker, fake_capture, fake_move_relative ): @@ -903,7 +810,6 @@ def test_run_basic_stack_end_origin( stack_dz=10, images_to_save=4, settling_time=0, - backlash_correction=0, origin=StackOrigin.END, ) @@ -948,7 +854,6 @@ def test_invalid_stack_images_raises(): stack_dz=10, images_to_save=capture_count, settling_time=0, - backlash_correction=0, origin=StackOrigin.START, ) @@ -960,7 +865,6 @@ def test_invalid_stack_settling_raises(): stack_dz=10, images_to_save=1, settling_time=-1, - backlash_correction=0, origin=StackOrigin.START, ) From 68c9f76fc08fbbd6d08000c4de85abaea85d89b3 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 12 Mar 2026 16:11:41 +0000 Subject: [PATCH 069/104] Fix missing import --- src/openflexure_microscope_server/things/scan_workflows.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 93a3d8bc..6ce8e42b 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -44,6 +44,7 @@ from openflexure_microscope_server.ui import ( UI_ELEMENT_RESPONSE, Accordion, HeaderBlock, + PropertyControl, TextBlock, UIElementList, action_button_for, From 2675dff8dcec16e7680269691aa4fe7010f547ef Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 12 Mar 2026 17:04:48 +0000 Subject: [PATCH 070/104] Edit ofm_config_full.json --- ofm_config_full.json | 1 - 1 file changed, 1 deletion(-) diff --git a/ofm_config_full.json b/ofm_config_full.json index 47fd7e47..27024a2c 100644 --- a/ofm_config_full.json +++ b/ofm_config_full.json @@ -17,7 +17,6 @@ "default_workflow": "histo_scan_workflow" } }, - "CChip_workflow": "openflexure_microscope_server.things.scan_workflows:CChipWorkflow", "histo_scan_workflow": "openflexure_microscope_server.things.scan_workflows:HistoScanWorkflow", "snake_workflow": "openflexure_microscope_server.things.scan_workflows:SnakeWorkflow", "raster_workflow": "openflexure_microscope_server.things.scan_workflows:RasterWorkflow", From e5b042874021211f57ea36b2181b95a45fb97775 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 12 Mar 2026 17:05:15 +0000 Subject: [PATCH 071/104] Edit ofm_config_simulation.json --- ofm_config_simulation.json | 1 - 1 file changed, 1 deletion(-) diff --git a/ofm_config_simulation.json b/ofm_config_simulation.json index dbc246f7..b04988d4 100644 --- a/ofm_config_simulation.json +++ b/ofm_config_simulation.json @@ -12,7 +12,6 @@ "default_workflow": "histo_scan_workflow" } }, - "CChip_workflow": "openflexure_microscope_server.things.scan_workflows:CChipWorkflow", "histo_scan_workflow": "openflexure_microscope_server.things.scan_workflows:HistoScanWorkflow", "snake_workflow": "openflexure_microscope_server.things.scan_workflows:SnakeWorkflow", "raster_workflow": "openflexure_microscope_server.things.scan_workflows:RasterWorkflow", From a4aac53072c0f826171152e98c9918bb805dd08c Mon Sep 17 00:00:00 2001 From: jaknapper Date: Fri, 13 Mar 2026 10:48:22 +0000 Subject: [PATCH 072/104] Flush more frames and use a request when doing exposure time --- .../things/camera/picamera_recalibrate_utils.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 0a5637ab..6e694301 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -253,9 +253,18 @@ def _test_exposure_settings(camera: Picamera2, percentile: float) -> _ExposureTe percentile (which will be compared to the target), as well as the camera's shutter and gain values. """ - camera.capture_array("raw") # controls might not be updated for the first frame? + # Flush stale frames + for _ in range(4): + r = camera.capture_request() + r.release() + + # A single request, to ensure metadata matches frame + request = camera.capture_request() + metadata = request.get_metadata() + image = request.make_array("raw") + request.release() max_brightness = np.percentile( - _channels_from_bayer_array(camera.capture_array("raw")), + _channels_from_bayer_array(image), percentile, ) # The reported brightness can, theoretically, be negative or zero @@ -268,7 +277,6 @@ def _test_exposure_settings(camera: Picamera2, percentile: float) -> _ExposureTe "camera's black level compensation has gone wrong." ) max_brightness = 1 - metadata = camera.capture_metadata() result = _ExposureTest( level=max_brightness, exposure_time=int(metadata["ExposureTime"]), From ae04f3b13322d4b245add041ee3428caf5d535b0 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Fri, 13 Mar 2026 13:22:19 +0000 Subject: [PATCH 073/104] Drop frame drop to 2 --- .../things/camera/picamera_recalibrate_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 6e694301..b5bc6c06 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -254,10 +254,10 @@ def _test_exposure_settings(camera: Picamera2, percentile: float) -> _ExposureTe the camera's shutter and gain values. """ # Flush stale frames - for _ in range(4): + for _ in range(2): r = camera.capture_request() r.release() - + # A single request, to ensure metadata matches frame request = camera.capture_request() metadata = request.get_metadata() From cf6bf8243c5d1830a3045e14dde3f007d40ba556 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Fri, 13 Mar 2026 16:28:33 +0000 Subject: [PATCH 074/104] Close calibration with confirmation Centre CSM button in calibration Add the word "Data" to "No CSM available" --- .../modalComponents/calibrationWizard.vue | 22 +++++++++++++++++++ .../csmSteps/runCsmStep.vue | 2 ++ .../CSMCalibrationSettings.vue | 2 +- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/webapp/src/components/modalComponents/calibrationWizard.vue b/webapp/src/components/modalComponents/calibrationWizard.vue index f4729936..eb7de58e 100644 --- a/webapp/src/components/modalComponents/calibrationWizard.vue +++ b/webapp/src/components/modalComponents/calibrationWizard.vue @@ -1,6 +1,13 @@ From 7fa04e5dbca9fdc9b95b7ff0f4f001cad472c0f6 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 16 Mar 2026 12:55:41 +0000 Subject: [PATCH 075/104] codespell rc to ignore ./git file, not extension --- .codespellrc | 2 +- .codespellrc2 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.codespellrc b/.codespellrc index 73d461c1..5832630e 100644 --- a/.codespellrc +++ b/.codespellrc @@ -2,7 +2,7 @@ skip = node_modules, *-lock.json, - *.git, + ./.git, *.pyc, __pycache__, ./build, diff --git a/.codespellrc2 b/.codespellrc2 index c6226e69..b3f96563 100644 --- a/.codespellrc2 +++ b/.codespellrc2 @@ -7,7 +7,7 @@ # by the normal RC file, and we get false positives from git hashes. skip = node_modules, *-lock.json, - *.git, + ./.git, *.pyc, __pycache__, ./build, From 3b84c266b4635d450c1fdcdc141925895c636d49 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 16 Mar 2026 14:31:21 +0000 Subject: [PATCH 076/104] Apply suggestions from code review of branch exposure-test Co-authored-by: Richard Bowman --- .../things/camera/picamera_recalibrate_utils.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index b5bc6c06..90d6785f 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -260,9 +260,11 @@ def _test_exposure_settings(camera: Picamera2, percentile: float) -> _ExposureTe # A single request, to ensure metadata matches frame request = camera.capture_request() - metadata = request.get_metadata() - image = request.make_array("raw") - request.release() + try: + metadata = request.get_metadata() + image = request.make_array("raw") + finally: + request.release() max_brightness = np.percentile( _channels_from_bayer_array(image), percentile, From a11d46777fb19916b94a34cd9c3b572047225990 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Mon, 16 Mar 2026 15:01:10 +0000 Subject: [PATCH 077/104] Only drop frames when setting min exposure time --- .../things/camera/picamera_recalibrate_utils.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 90d6785f..1ce49f86 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -241,6 +241,9 @@ def _set_minimum_exposure(camera: Picamera2, sensor_info: SensorInfo) -> None: # to the minimum possible, which is ~8us for PiCamera v2 camera.set_controls({"AeEnable": False, "AnalogueGain": 1, "ExposureTime": 1}) time.sleep(sensor_info.long_pause) + # Flush stale frames + for _ in range(2): + r = camera.capture_metadata() def _test_exposure_settings(camera: Picamera2, percentile: float) -> _ExposureTest: @@ -253,11 +256,6 @@ def _test_exposure_settings(camera: Picamera2, percentile: float) -> _ExposureTe percentile (which will be compared to the target), as well as the camera's shutter and gain values. """ - # Flush stale frames - for _ in range(2): - r = camera.capture_request() - r.release() - # A single request, to ensure metadata matches frame request = camera.capture_request() try: From 8fa0217ec9876d29ba3b72cce5c37ec3f17258b7 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Mon, 16 Mar 2026 15:49:51 +0000 Subject: [PATCH 078/104] Update camera cal instructions to look through empty area if possible --- .../cameraCalibrationSteps/camCalibrationExplanation.vue | 4 +++- .../cameraCalibrationSteps/cameraMainCalibrationStep.vue | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/modalComponents/calibrationWizardComponents/cameraCalibrationSteps/camCalibrationExplanation.vue b/webapp/src/components/modalComponents/calibrationWizardComponents/cameraCalibrationSteps/camCalibrationExplanation.vue index 28243aa3..5b0918c1 100644 --- a/webapp/src/components/modalComponents/calibrationWizardComponents/cameraCalibrationSteps/camCalibrationExplanation.vue +++ b/webapp/src/components/modalComponents/calibrationWizardComponents/cameraCalibrationSteps/camCalibrationExplanation.vue @@ -4,7 +4,9 @@ Before starting camera calibration:

          -
        • Remove any samples from your microscope
          • +
        • Insert a slide.
          • +
        • Move your slide to an empty area, ideally looking through the coverslip.
          • +
        • If your slide has no empty areas, remove it and instead calibrate on no sample.
        • Ensure your illumination is on, well focused, and centred.
        diff --git a/webapp/src/components/modalComponents/calibrationWizardComponents/cameraCalibrationSteps/cameraMainCalibrationStep.vue b/webapp/src/components/modalComponents/calibrationWizardComponents/cameraCalibrationSteps/cameraMainCalibrationStep.vue index ed0709e4..400ada49 100644 --- a/webapp/src/components/modalComponents/calibrationWizardComponents/cameraCalibrationSteps/cameraMainCalibrationStep.vue +++ b/webapp/src/components/modalComponents/calibrationWizardComponents/cameraCalibrationSteps/cameraMainCalibrationStep.vue @@ -1,7 +1,7 @@