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, diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index 3d928164..68873825 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. + +**Expected Behaviour**: + +**Current Behaviour**: + ## 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 7941205b..ad9c32b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,113 @@ +# [v3.0.0-alpha5](https://gitlab.com/openflexure/openflexure-microscope-server/compare/v3.0.0-alpha4...v3.0.0-alpha5) (2026-04-08) + +The fifth alpha release of the server. This release has been a huge amount of work and brings us significantly closer to a semi-stable beta release. Some highlights are: + + +* Move from LabThings Dependencies to ThingSlots. This has allowed for: + * Much easier communication between Things + * Static type checking + * Simpler action creation. +* Created ScanWorkflows, a backend change that allows switching scan methods. Enabling new types of scanning: + * Raster Scan + * Snake Scan + * CChip Scan (experimental, requires a configuration change to use) +* Transition front end from the end of life Vue2 to Vue3 which is in active development! +* Improvements to the simulation microscope including more simulation samples and camera options. Speeding up development. +* Improved testing, including better defined testing environments, separation of integration and unit tests, and tests passing on Windows +* OpenCV camera switching. Enabling changing between cameras in settings when in manual configuration. +* "Jogging" movement. The microscope can now move continuously when an arrow key is held. +* Improved server side UI specification. This brings us closer to "Things" being able to define their own tab, which will soon enable plugins in v3. +* A user interface for recentring and range of motion. +* Improved fallback server interface. +* Backlash correction in scanning. +* A huge number of other bug fixes and improvements. + + +The following merge requests have been merged into v3: + +* !450 Update to be compatible with labthings-fastapi 0.0.12/0.0.13 +* !451 Start to remove deps +* !452 Removing dependencies from CSM, autofocus, and stage_measure +* !453 Final dependency removals +* !454 Use new LabThingsTestEnv thoughout tests +* !455 Type fixes +* !456 Proper integration tests +* !457 Fix (most) unit tests on Windows +* !458 Simulation Camera Improvements +* !460 Bugfix, stop saving scans into log folder +* !461 Background detector things +* !462 Implement ScanWorkflows allowing multiple scanning methods +* !464 SnakeScan as a scan planner +* !469 More workflow layers +* !465 Add raster scan, with generic typing to allow subclassed settings +* !468 Illumination Thing +* !473 HTTP API docs +* !479 Vue2-3 migration. +* !480 Tidy optional deps +* !478 Increase dz range to 400 to 3000 +* !472 Manually handle out of storage and JPEG too big errors +* !485 Restore CSM stream pos to right of buttons +* !486 Add URL to wiki if Sangaboard firmware out of date +* !475 Grey out read only properties in webapp +* !484 Reduce the blur for a given z movement in simulation, to improve autofocus. +* !489 Bump ESLint from unsported version to version 9 +* !483 CSM modal shows stream +* !476 Generalised Jogging +* !477 Optional options in UI property control become dropdown +* !482 Add recentre and ROM test buttons to stage settings +* !495 Add --max-warnings=0 back to eslint check. This was accidentally dropped when moving from vue-cli-service +* !490 Magnification selection in simulator by cropping canvas based on objective property +* !497 Re-enable prettier +* !496 Update release checklist from last alpha's experience +* !494 Use lt.property/setting validatiors to set numerical input range and step +* !498 Update logging page formatting. +* !499 Use Invocation errors for expected errors. +* !500 Add a fallback page template. +* !505 Remove references to old GPU preview feature. +* !506 Webapp cleanup +* !504 Use own pagination consistently rather than vue-pagination-next +* !502 Add spellchecking for camelCase, PascalCase, and kebab-case +* !501 Scan logs +* !508 Allow 0 as a setting in the webapp +* !493 Split SmartStackParams into focus, capture, stack params +* !487 Add functionality to switch OpenCV camera without restarting the server. +* !514 Move to headless opencv +* !513 More developer documnetation +* !510 Change pointer event names for clarity +* !509 Backlash correction in base stage class +* !517 Allow histo scans with equal dx and dy +* !516 Do less coersion for stacks now labthings validates settings. +* !519 Metadata testing +* !503 Create and mount application level data directory rather than just a scan directory +* !491 Decrease target brightness for both cameras as its pre-gamma +* !525 Align buttons in confirmation modal +* !526 Fix spellings found as codespell updates +* !518 Add limits to background detect and settling time settings +* !527 Update bug template to include table or configuration, more detail about how... +* !532 Reorder Bug.md expected and actual behaviour with correct comments +* !531 Don't tab to spinners in webapp +* !524 Improve Validation Checks +* !535 Remove uk-input from checkbox +* !530 Increase number of UI elements that can be specified in the server +* !442 Handle scan settings, then change stream, then pre-scan procedure when scanning +* !533 Stop closing seadragon when going full screen +* !529 Move more development information to the dev docs to minimise repitition and getting out of sync +* !474 CChip scan workflow +* !544 Update codespellrc to ignore ./git folder, not extension +* !545 Update camera cal instructions to look through empty area if possible +* !540 Flush more frames and use a request when doing exposure time +* !542 Close calibration with confirmation +* !546 Update path to ofm server and config file in docs +* !547 Explicit windows check before sigkill for mypy +* !536 Split up slide scan tab and create a generic action tab (attempt 2) +* !548 Word break long lines in notifications +* !552 Move from posix path to os path +* !554 Move smart stacking into a mixin so it can be re-used in other workflows +* !551 Sort scans by created time +* !538 Update dependency to labthings 0.1.0 +* !557 Prep for v3.0.0 alpha5 +* !560 Bump stitching version + # [v3.0.0-alpha4](https://gitlab.com/openflexure/openflexure-microscope-server/compare/v3.0.0-alpha3...v3.0.0-alpha4) (2025-12-15) The fourth alpha release of the server. This release has concentrated on colour reproduction, scanning, and the structure of the web-app. @@ -58,7 +168,7 @@ The following merge requests have been merged into v3: * !434 Improve the stability of scanning * !438 Formalise import sorting rules and enforce * !448 Bump required stitching version to 0.2.1 - +* !449 Prep for v3.0.0-alpha4 # [v3.0.0-alpha3](https://gitlab.com/openflexure/openflexure-microscope-server/compare/v3.0.0-alpha2...v3.0.0-alpha3) (2025-10-10) 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..4af9d1bf 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,10 @@ 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. +In the future, a 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. -# Developer guidelines +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. -## Installation and set up +## Development -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..bd68d26a 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 +cd /var/openflexure +sudo -u openflexure-ws application/openflexure-microscope-server/.venv/bin/openflexure-microscope-server --fallback -c settings/ofm_config.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 with a passing pipeline), 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`. + +All core functionality hardware control functionality is within `Thing` classes. Even the standard camera and stage classes are each a `Thing`. This means it is possible to use a different camera or stage by sub-classing the `BaseCamera` or `BaseStage` and editing the configuration file. + diff --git a/apidocs/dev/_sidebar.md b/apidocs/dev/_sidebar.md index df1ea285..b40cb62c 100644 --- a/apidocs/dev/_sidebar.md +++ b/apidocs/dev/_sidebar.md @@ -5,5 +5,10 @@

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) +- [Configuration File](config-file) +- [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/config-file.md b/apidocs/dev/config-file.md new file mode 100644 index 00000000..5c1e160a --- /dev/null +++ b/apidocs/dev/config-file.md @@ -0,0 +1,103 @@ +# The Configuration File + +By default, this configuration file is 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 file path using the `-c` command line flag. + +The default configuration file links to the configuration file in the repository: + +```json +{ + "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 customiston please refeer to the section on [customising the configuration](#section-customise-config). + +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`, which defines the URL of its endpoints. +* The value is the Python object reference for the `Thing`. + +For example: + +```json +"autofocus": "openflexure_microscope_server.things.autofocus:AutofocusThing", +``` + +will add an autofocus Thing to the server. The Thing is located at `openflexure_microscope_server.things.autofocus:AutofocusThing`, (allowing multiple Things per Python file), and any endpoints (such as actions and properties) in that Thing will have the URL `base_url:port/autofocus/endpoint_name` + +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: + +```json +"camera": { + "class": "openflexure_microscope_server.things.camera.picamera:StreamingPiCamera2", + "kwargs": { + "camera_board": "picamera_v2" + } +} +``` + +## Customising the configuration :id=section-customise-config + + +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" + } + } + } + } +} +``` + +This will change the camera section to: + +```json +"camera": { + "class": "openflexure_microscope_server.things.camera.picamera:StreamingPiCamera2", + "kwargs": { + "camera_board": "picamera_hq" + } +} +``` 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/ofm.css b/apidocs/dev/ofm.css index f3a7abae..8fb64a72 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 too much from surrounding + text and punctuation. + */ + margin: 0; + padding: 3px 0; white-space: pre-wrap } diff --git a/apidocs/dev/python.md b/apidocs/dev/python.md new file mode 100644 index 00000000..df9cc6a9 --- /dev/null +++ b/apidocs/dev/python.md @@ -0,0 +1,95 @@ +# Python Server Development + +## Overview + + +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)) + + +## 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:` when installing packages, 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. + +Please run: + +* `ruff check` - to lint your Python code (please manually fix any flagged issues) +* `ruff format` - to automatically format your Python code +* `codespell .` - to check for spelling errors + +**Before merging** please auto-format your code and also run the quality checks (linting, static analysis, and unit tests) + +Please run: + +* All commands above +* `mypy src` to type check your Python code +* `pytest` to run all the unit tests in `tests/unit_tests` + +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. + +To check your code before each commit we recommend putting the following script in your `.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 or spellcheck failed${NC}\n\n" +} + +trap angrymessage ERR + +ruff format --check +ruff check +codespell . +``` + + +## 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..1d7d0fc2 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 and 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 - } -} -``` diff --git a/picamera_coverage.zip b/picamera_coverage.zip index bd21c466..f6005301 100644 Binary files a/picamera_coverage.zip and b/picamera_coverage.zip differ diff --git a/pyproject.toml b/pyproject.toml index f53171b2..1242790a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ name = "openflexure-microscope-server" # Note that version needs to match a string stored in a unit test, and # the package.json file in the webapp or the CI will fail. -version = "3.0.0-alpha4" +version = "3.0.0-alpha5" authors = [ { name="Richard Bowman", email="richard.bowman@cantab.net" }, { name="Joel Collins" }, @@ -18,17 +18,17 @@ classifiers = [ "Operating System :: OS Independent", ] dependencies = [ - "labthings-fastapi==0.0.17", - "sangaboard", + "labthings-fastapi==0.1.0", + "sangaboard~=0.4.0", "camera-stage-mapping ~= 0.1.10", "opencv-python-headless ~= 4.13.0", - "openflexure-stitching[libvips]==0.2.3", + "openflexure-stitching[libvips]==0.2.5", "pillow ~= 10.4", "anyio ~= 4.0", "numpy ~= 2.2", "scipy ~=1.14", "piexif", - "pydantic ~= 2.10.6", + "pydantic", # Pydantic is not pinned to avoid conflicts with how LabThings pins pydantic "simplejpeg >= 1.8.2", "semver ~= 3.0", ] 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/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index 1f077ec6..311a00a4 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" @@ -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.""" @@ -49,17 +65,26 @@ class StitcherValidationError(RuntimeError): 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``. + executables. All of these should be allowed by ``is_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: - if element != make_path_safe(element): + # Check against forbidden commands (case-insensitive) + if element.lower() in FORBIDDEN_COMMANDS: raise StitcherValidationError( - "Invalid stiching command: Contains unsafe characters." + f"Invalid stitching command: Forbidden element '{element}' detected." + ) + + # Ensure characters are safe for a path component + if not is_path_safe(element): + raise StitcherValidationError( + f"Invalid stitching command: Element '{element}' contains unsafe characters." ) @@ -129,7 +154,7 @@ class BaseStitcher: :raises RuntimeError: if inputs are unsafe. """ - if self.images_dir != make_path_safe(self.images_dir): + if not is_path_safe(self.images_dir): raise StitcherValidationError( "Invalid directory path: Contains unsafe characters." ) @@ -200,7 +225,8 @@ class PreviewStitcher(BaseStitcher): # Windows has no SIGKILL self._popen_obj.kill() else: - self._popen_obj.send_signal(signal.SIGKILL) + # ignore this line in mypy as mypy doesn't understand using the bool as a check + self._popen_obj.send_signal(signal.SIGKILL) # type: ignore[attr-defined] raise (e) diff --git a/src/openflexure_microscope_server/things/__init__.py b/src/openflexure_microscope_server/things/__init__.py index 65314462..75ba3d24 100644 --- a/src/openflexure_microscope_server/things/__init__.py +++ b/src/openflexure_microscope_server/things/__init__.py @@ -4,7 +4,7 @@ The microscope can be extended to be used with other hardware by creating a pack with other Things and including them in the LabThings-FastAPI config file. """ -import posixpath +import os from typing import Optional, Self import labthings_fastapi as lt @@ -23,7 +23,9 @@ class OFMThing(lt.Thing): if application_config is None: raise ValueError("No application configuration was supplied.") app_data_dir = application_config["data_folder"] - self._data_dir = posixpath.join(str(app_data_dir), self.path.strip("/")) + self._data_dir = os.path.join( + os.path.normpath(str(app_data_dir)), os.path.normpath(self.path.strip("/")) + ) return self @property diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 909ff229..5cbc050c 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. @@ -780,15 +775,8 @@ class AutofocusThing(lt.Thing): target_offset = -total_range 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) + 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/src/openflexure_microscope_server/things/background_detect.py b/src/openflexure_microscope_server/things/background_detect.py index 19ab6874..c2753a75 100644 --- a/src/openflexure_microscope_server/things/background_detect.py +++ b/src/openflexure_microscope_server/things/background_detect.py @@ -13,7 +13,11 @@ from pydantic import BaseModel import labthings_fastapi as lt -from openflexure_microscope_server.ui import PropertyControl, property_control_for +from openflexure_microscope_server.ui import ( + UI_ELEMENT_RESPONSE, + UIElementList, + property_control_for, +) SettingsType = TypeVar("SettingsType", bound=BaseModel) BackgroundType = TypeVar("BackgroundType", bound=BaseModel) @@ -72,9 +76,9 @@ class BackgroundDetectAlgorithm(lt.Thing): "Each background detect algorithm must implement an set_background method." ) - @lt.property - def settings_ui(self) -> list[PropertyControl]: - """A list of PropertyControl objects to create the settings in the UI.""" + @lt.endpoint("get", "settings_ui", responses=UI_ELEMENT_RESPONSE) + def settings_ui(self) -> UIElementList: + """Return the UI for the background detect settings.""" raise NotImplementedError( "Each background detect algorithm must implement an settings_ui method." ) @@ -99,29 +103,33 @@ 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 image to be labeled as containing sample. """ - @lt.property - def settings_ui(self) -> list[PropertyControl]: - """A list of PropertyControl objects to create the settings in the UI.""" - return [ - property_control_for(self, "channel_tolerance", label="Channel Tolerance"), - property_control_for( - self, "min_sample_coverage", label="Sample Coverage Required (%)" - ), - ] + @lt.endpoint("get", "settings_ui", responses=UI_ELEMENT_RESPONSE) + def settings_ui(self) -> UIElementList: + """Return the UI for the background detect settings.""" + return UIElementList( + [ + property_control_for( + self, "channel_tolerance", label="Channel Tolerance" + ), + property_control_for( + self, "min_sample_coverage", label="Sample Coverage Required (%)" + ), + ] + ) @lt.property def ready(self) -> bool: @@ -223,29 +231,33 @@ 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 image to be labeled as containing sample. """ - @lt.property - def settings_ui(self) -> list[PropertyControl]: - """A list of PropertyControl objects to create the settings in the UI.""" - return [ - property_control_for(self, "channel_tolerance", label="Channel Tolerance"), - property_control_for( - self, "min_sample_coverage", label="Sample Coverage Required (%)" - ), - ] + @lt.endpoint("get", "settings_ui", responses=UI_ELEMENT_RESPONSE) + def settings_ui(self) -> UIElementList: + """Return the UI for the background detect settings.""" + return UIElementList( + [ + property_control_for( + self, "channel_tolerance", label="Channel Tolerance" + ), + property_control_for( + self, "min_sample_coverage", label="Sample Coverage Required (%)" + ), + ] + ) @lt.property def ready(self) -> bool: diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index dd4f6cdd..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 @@ -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 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 a11b6c91..1c00f63f 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=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=2800, + default_target_white_level=1600, short_pause=0.2, long_pause=1.0, ) @@ -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 @@ -240,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: @@ -252,9 +256,15 @@ 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? + # A single request, to ensure metadata matches frame + request = camera.capture_request() + try: + metadata = request.get_metadata() + image = request.make_array("raw") + finally: + 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 @@ -267,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"]), diff --git a/src/openflexure_microscope_server/things/camera/simulation.py b/src/openflexure_microscope_server/things/camera/simulation.py index 31e3979e..d75c36e5 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: + raise ValueError("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, le=50) def _capture_frames(self) -> None: last_frame_t = time.time() diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 2380173a..29d64841 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -4,14 +4,13 @@ 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, Literal, Mapping, Optional, + Protocol, TypeVar, ) @@ -34,6 +33,7 @@ from openflexure_microscope_server.things.autofocus import ( AutofocusParams, AutofocusThing, SmartStackParams, + StackParams, ) from openflexure_microscope_server.things.background_detect import ( ChannelDeviationLUV, @@ -41,7 +41,16 @@ 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, + HeaderBlock, + PropertyControl, + TextBlock, + UIElementList, + action_button_for, + property_control_for, +) SettingModelType = TypeVar("SettingModelType", bound=BaseModel) @@ -160,9 +169,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." ) @@ -297,6 +306,145 @@ class RectGridWorkflow( return not self._csm.calibration_required +class SmartStackCompatibleSettings(Protocol): + """A protocol for the minimum settings needed for smart stack to work.""" + + capture_params: CaptureParams + autofocus_params: AutofocusParams + smart_stack_params: SmartStackParams + + +class SmartStackMixin: + """A mixin for scan workflows that use smart stacking.""" + + stack_images_to_save: int = lt.setting(default=1, ge=1, le=9) + """The number of images to save in a stack. + + Defaults to 1 unless you need to see either side of focus + """ + + stack_min_images_to_test: int = lt.setting( + default=9, ge=MIN_TEST_IMAGE_COUNT, le=MAX_TEST_IMAGE_COUNT + ) + """The minimum number of images to capture in a stack. + + This many images are captured and tested for focus, if the focus is not central + enough more images may be captured. After new images are captured, this value sets + the number of images used for checking if focus is achieved. + + Defaults to 9 which balances reliability and speed. + """ + + stack_dz: int = lt.setting(default=50, ge=10, le=400) + """Distance in steps between images in a z-stack. + + Suggested values: + + * 50 for 60-100x + * 100 for 40x + * 200 for 20x + """ + + @property + def as_workflow(self) -> ScanWorkflow: + """Return self as a ScanWorkflow. + + Ensures this mixin is only used with ScanWorkflow instances, + raising TypeError otherwise. + """ + if not isinstance(self, ScanWorkflow): + raise TypeError("SmartStackMixin must be mixed into a ScanWorkflow") + return self + + def create_smart_stack_params(self, save_on_failure: bool) -> SmartStackParams: + """Set up the parameters used for all smart stacks in a scan. + + :returns: A StackSmartParams object with the required parameters. + """ + # Coerce min_images_to_test parameter + min_images_to_test = self.stack_min_images_to_test + if min_images_to_test % 2 == 0: + min_images_to_test += 1 + self.as_workflow.logger.warning( + "Minimum number of images to test should be odd, setting to " + f"{min_images_to_test}." + ) + # Set the Thing property to the coerced value + self.stack_min_images_to_test = min_images_to_test + + # Coerce the images to save parameter to be odd, and less than + # min_images_to_save + images_to_save = self.stack_images_to_save + if images_to_save > min_images_to_test: + self.as_workflow.logger.warning( + f"Cannot save {images_to_save} images as this above the minimum " + f"number to test. Setting images to save to {MAX_TEST_IMAGE_COUNT}." + ) + images_to_save = min_images_to_test + elif images_to_save % 2 == 0: + images_to_save += 1 + self.as_workflow.logger.warning( + f"Images to save should be odd, setting to {images_to_save}." + ) + # Set the Thing property to the coerced value + self.stack_images_to_save = images_to_save + + return SmartStackParams( + stack_dz=self.stack_dz, + images_to_save=self.stack_images_to_save, + min_images_to_test=self.stack_min_images_to_test, + save_on_failure=save_on_failure, + ) + + def _perform_smart_stack( + self, settings: SmartStackCompatibleSettings, xyz_pos: tuple[int, int, int] + ) -> tuple[bool, Optional[int]]: + """Perform acquisition a smart stack. + + :param settings: The settings for this scan as a HistoScanSettingsModel + :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. + """ + focus_height: Optional[int] + focused, focus_height = self.as_workflow._autofocus.run_smart_stack( + stack_parameters=settings.smart_stack_params, + capture_parameters=settings.capture_params, + autofocus_parameters=settings.autofocus_params, + ) + # An image was captured if we are focussed or we are not skipping background. + imaged = focused or settings.smart_stack_params.save_on_failure + + if not imaged: + msg = f"Stack failed at {xyz_pos}. Treating as background." + self.as_workflow.logger.info(msg) + + # run_smart_stage always returns a focus height for the sharpest image even + # if it failed to find a good focus. Set to None if not focussed. + if not focused: + focus_height = None + + return imaged, focus_height + + def smart_stack_property_controls(self) -> list[PropertyControl]: + """Return smart stack property controls for the UI.""" + return [ + property_control_for( + self.as_workflow, + "stack_images_to_save", + label="Images in Stack to Save", + ), + property_control_for( + self.as_workflow, + "stack_min_images_to_test", + label="Minimum number of images to test for focus", + ), + property_control_for( + self.as_workflow, "stack_dz", label="Stack dz (steps)", step=5 + ), + ] + + class HistoScanSettingsModel(RectGridSettingsModel): """The settings for a scan with the HistoScanWorkflow. @@ -309,7 +457,7 @@ class HistoScanSettingsModel(RectGridSettingsModel): smart_stack_params: SmartStackParams -class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): +class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel], SmartStackMixin): """A workflow optimised for scanning Histopathology samples. This workflow automatically plans its own path around a sample spiralling out from @@ -342,36 +490,6 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): max_range: int = lt.setting(default=45000, ge=0) """The maximum distance in steps from the centre of the scan.""" - # Stacking settings - - stack_images_to_save: int = lt.setting(default=1, ge=1, le=9) - """The number of images to save in a stack. - - Defaults to 1 unless you need to see either side of focus - """ - - stack_min_images_to_test: int = lt.setting( - default=9, ge=MIN_TEST_IMAGE_COUNT, le=MAX_TEST_IMAGE_COUNT - ) - """The minimum number of images to capture in a stack. - - This many images are captures and tested for focus, if the focus is not central - enough more images may be captured. After new images are captured, this value sets - the number of images used for checking if focus is achieved. - - Defaults to 9 which balances reliability and speed. - """ - - stack_dz: int = lt.setting(default=50, ge=10, le=400) - """Distance in steps between images in a z-stack. - - Suggested values: - - * 50 for 60-100x - * 100 for 40x - * 200 for 20x - """ - equal_distances: bool = lt.setting(default=False) """Make the distances in x and y equal in motor steps, rather than in overlap. @@ -435,49 +553,9 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): **base_kwargs, max_dist=self.max_range, skip_background=self.skip_background, - smart_stack_params=self.create_smart_stack_params(), - ) - - def create_smart_stack_params( - self, - ) -> SmartStackParams: - """Set up the parameters used for all smart stacks in a scan. - - :returns: A StackSmartParams object with the required parameters. - """ - # Coerce min_images_to_test parameter - min_images_to_test = self.stack_min_images_to_test - if min_images_to_test % 2 == 0: - min_images_to_test += 1 - self.logger.warning( - "Minimum number of images to test should be odd, setting to " - f"{min_images_to_test}." - ) - # Set the Thing property to the coerced value - self.stack_min_images_to_test = min_images_to_test - - # Coerce the images to save parameter to be odd, and less than - # min_images_to_save - images_to_save = self.stack_images_to_save - if images_to_save > min_images_to_test: - self.logger.warning( - f"Cannot save {images_to_save} images as this above the minimum " - f"number to test. Setting images to save to {MAX_TEST_IMAGE_COUNT}." - ) - images_to_save = min_images_to_test - elif images_to_save % 2 == 0: - images_to_save += 1 - self.logger.warning( - f"Images to save should be odd, setting to {images_to_save}." - ) - # Set the Thing property to the coerced value - self.stack_images_to_save = images_to_save - - return SmartStackParams( - stack_dz=self.stack_dz, - images_to_save=self.stack_images_to_save, - min_images_to_test=self.stack_min_images_to_test, - save_on_failure=not self.skip_background, + smart_stack_params=self.create_smart_stack_params( + save_on_failure=not self.skip_background + ), ) def pre_scan_routine(self, settings: HistoScanSettingsModel) -> None: @@ -536,55 +614,88 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): self.logger.info(msg) return False, None - focus_height: Optional[int] - focused, focus_height = self._autofocus.run_smart_stack( - stack_parameters=settings.smart_stack_params, - capture_parameters=settings.capture_params, - autofocus_parameters=settings.autofocus_params, + return self._perform_smart_stack(settings, xyz_pos) + + @lt.action + def check_background(self) -> str: + """Check if sample is background. + + This action is a pre-run check for feeding back to the user. + """ + image_array = self._cam.grab_as_array(stream_name="lores") + is_sample, bg_message = self._background_detector.image_is_sample(image_array) + label = "sample" if is_sample else "background" + + return f"Current image is {label} ({bg_message})" + + @lt.action + def set_background(self) -> None: + """Set the background for this background detector. + + 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") + self._background_detector.set_background(image_array) + + @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.""" + scan_settings = UIElementList( + [ + property_control_for( + self, "overlap", label="Image Overlap (0.1-0.7)", step=0.05 + ), + *self.smart_stack_property_controls(), + 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" + ), + ] + ) + background_ui = self._background_detector.settings_ui() + set_bg_button = action_button_for( + self, + "set_background", + poll_interval=0.1, + submit_label="Set Background", + 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, ) - # An image was captured if we are focussed or we are not skipping background. - imaged = focused or settings.smart_stack_params.save_on_failure - if not imaged: - msg = f"Stack failed at {xyz_pos}. Treating as background." - self.logger.info(msg) + background_ui.root += [set_bg_button, check_bg_button] - # run_smart_stage always returns a focus height for the sharpest image even - # if it failed to find a good focus. Set to None if not focussed. - if not focused: - focus_height = None - - 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" - ), - ] + return UIElementList( + [ + HeaderBlock(text=self.display_name, level=4), + TextBlock(text=self.ui_blurb), + Accordion(title="Background Detect", children=background_ui), + Accordion( + title="Scan Settings", + children=scan_settings, + ), + ] + ) class RegularGridSettingsModel(RectGridSettingsModel): @@ -596,10 +707,20 @@ class RegularGridSettingsModel(RectGridSettingsModel): x_count: int y_count: int + smart_stack_params: SmartStackParams style: Literal["snake", "raster"] -class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): +RegGridSettingModelType = TypeVar( + "RegGridSettingModelType", bound=RegularGridSettingsModel +) + + +class RegularGridWorkflow( + RectGridWorkflow[RegGridSettingModelType], + SmartStackMixin, + Generic[RegGridSettingModelType], +): """A base workflow for any workflow that uses a regular rectangular grid.""" x_count: int = lt.setting(default=3, ge=1) @@ -607,20 +728,21 @@ 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, + smart_stack_params=self.create_smart_stack_params(save_on_failure=True), ) - 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. @@ -632,11 +754,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 = { @@ -652,36 +774,46 @@ 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. """ - return self._autofocus_and_capture( - xyz_pos=xyz_pos, - dz=settings.autofocus_params.dz, - images_dir=settings.capture_params.images_dir, - save_resolution=settings.capture_params.save_resolution, + return self._perform_smart_stack(settings, xyz_pos) + + @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.""" + scan_settings = 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"), + *self.smart_stack_property_controls(), + property_control_for( + self, "autofocus_dz", label="Autofocus Range (steps)" + ), + ] + ) + return UIElementList( + [ + HeaderBlock(text=self.display_name, level=4), + TextBlock(text=self.ui_blurb), + Accordion( + title="Scan Settings", + children=scan_settings, + ), + ] ) - @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)"), - ] - -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 @@ -696,10 +828,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 @@ -715,5 +848,148 @@ class RasterWorkflow(RegularGridWorkflow): ), readonly=True, ) - + _settings_model = RegularGridSettingsModel _grid_style = "raster" + + +class CChipScanSettingsModel(RectGridSettingsModel): + """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. + """ + + x_count: int + y_count: int + stack_params: StackParams + style: Literal["snake", "raster"] + + +class CChipWorkflow(RectGridWorkflow[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: Literal["snake"] = "snake" + _planner_cls = RegularGridPlanner + _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 new_scan_planner( + self, settings: CChipScanSettingsModel, position: Mapping[str, int] + ) -> ScanPlanner: + """Return a new scan planner object. + + :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 = { + "dx": settings.dx, + "dy": settings.dy, + "x_count": settings.x_count, + "y_count": settings.y_count, + "style": settings.style, + } + return self._planner_cls( + initial_position=(position["x"], position["y"]), + planner_settings=planner_settings, + ) + + 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" + ), + ] 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 diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index 3cb4375e..4620cab2 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/src/openflexure_microscope_server/ui.py @@ -1,11 +1,155 @@ """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 urllib.parse import urlparse -from pydantic import BaseModel +from pydantic import BaseModel, BeforeValidator, Field, RootModel import labthings_fastapi as lt +# Only allowed tags are italic, bold, and link. Also allows self closing br tags. +ALLOWED_TAGS = {"i", "b", "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}{parsed.path}{fragment_str}" + + +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: list[tuple[str, Optional[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, why only allow ``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" and value is not None: + # 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) + # Always in a new tab + self.output += ( + 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: list[tuple[str, Optional[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 sanitise_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(sanitise_html)] + + +class HeaderBlock(BaseModel): + """The data required for header.""" + + element_type: Literal["header_block"] = "header_block" + text: HtmlFragment + """The header text (can include basic HTML tags).""" + + 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): """The data required for creating an actionButton in Vue. @@ -13,13 +157,15 @@ 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.""" action: str """The name of the action to be triggered.""" - poll_interval: int = 1 + poll_interval: float = 1 """The interval for polling in seconds.""" submit_label: str = "Submit" @@ -37,6 +183,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.""" @@ -50,9 +199,21 @@ class ActionButton(BaseModel): notify_on_success: bool = False """Specify whether to a notification on successful completion.""" + response_is_success_message: bool = False + """Set True to use the action response as the success message. + + If ``success_message`` is set, this is never used. + """ + 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. @@ -72,6 +233,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 +284,63 @@ 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: "UIElementList" + + +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: "UIElementList" + + +UIElementModels = ( + HeaderBlock + | TextBlock + | BulletBlock + | 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 recursion. Use an + lt.endpoint instead + * If the module using this class imports 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/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index d6df4df9..7da53cc6 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -76,34 +76,131 @@ 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_.\-:/\\]") -# 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 +_WINDOWS_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_.\-]") +# 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. - This can be used to check that user inputs have not been concatenated into an - unsafe filepath. +def is_path_safe(unsafe_path_string: str) -> bool: + """Check if a file path has any unsafe elements in it. - This ensures compatibility across platforms by preserving only valid characters, - including path separators (forward slash on POSIX, and forward - slash/backslash/colon on Windows). + 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: A boolean indicating if any unsafe features were found. """ + # Split by separators first to sanitise components independently + + components = re.split(r"([/\\])", unsafe_path_string) + unsafe_character_pattern = ( _WINDOWS_UNSAFE_PATTERN if sys.platform.startswith("win") else _POSIX_UNSAFE_PATTERN ) - return unsafe_character_pattern.sub("_", unsafe_path_string) + + # 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): + # Skip separators and empty strings + if component in ("/", "\\") or not component: + continue + + # 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 + + # 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 + + # 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 + + return not ( + unsafe_relative_path + or trailing_dots + or trailing_whitespace + or unsafe_characters + or reserved_word + ) def make_name_safe(unsafe_name_string: str) -> str: @@ -112,11 +209,44 @@ 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, is_filename: bool = True) -> str: + """Check a name against Windows reserved names. + + :param name: The component to check. + :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.lower()}_" if is_filename else f"{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): @@ -291,7 +421,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/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..ce0a8027 --- /dev/null +++ b/tests/unit_tests/data/html_sanitisation_test_cases.md @@ -0,0 +1,125 @@ +# 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. Multiline inputs and results are treated as single, multiline strings. + +The HTML to test is in triple backtick code blocks. + +Anything not in code blocks is a comment. + +This file is run by /tests/unit_tests/test_ui.py + +## 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 +``` + + +## 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. +``` + + +## Test http link with path preserved +Valid HTTP links should preserve their path. +### Input +```html +Docs +``` +### Result +```html +Docs +``` + + +## Test http link with fragment preserved +Valid HTTP links should preserve their path and fragment. +### Input +```html +Docs Section +``` +### Result +```html +Docs Section +``` \ No newline at end of file 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_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 diff --git a/tests/unit_tests/test_scan_directories.py b/tests/unit_tests/test_scan_directories.py index 603587a3..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): @@ -247,6 +247,63 @@ 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_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() diff --git a/tests/unit_tests/test_scan_workflows.py b/tests/unit_tests/test_scan_workflows.py index 483b1f99..0ffa4a00 100644 --- a/tests/unit_tests/test_scan_workflows.py +++ b/tests/unit_tests/test_scan_workflows.py @@ -18,7 +18,14 @@ 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, + HeaderBlock, + PropertyControl, + TextBlock, + UIElementList, +) def test_partial_base_classes(): @@ -56,7 +63,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 +377,49 @@ 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 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[2], Accordion) + 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[2].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[3].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", diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 6adf2df7..ced295e6 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -270,7 +270,9 @@ def test_create_stack(histo_scan_workflow, caplog): initial_min_images_to_test = histo_scan_workflow.stack_min_images_to_test initial_images_to_save = histo_scan_workflow.stack_images_to_save with caplog.at_level(logging.INFO): - stack_params = histo_scan_workflow.create_smart_stack_params() + stack_params = histo_scan_workflow.create_smart_stack_params( + save_on_failure=not histo_scan_workflow.skip_background + ) assert len(caplog.records) == 0 assert histo_scan_workflow.stack_min_images_to_test == initial_min_images_to_test @@ -294,7 +296,9 @@ def test_coercing_stack_test_ims( histo_scan_workflow.stack_min_images_to_test = initial_test_ims with caplog.at_level(logging.WARNING): - stack_params = histo_scan_workflow.create_smart_stack_params() + stack_params = histo_scan_workflow.create_smart_stack_params( + save_on_failure=not histo_scan_workflow.skip_background + ) assert len(caplog.records) == 1 assert str(caplog.records[0].msg).startswith(expected_log_start) @@ -322,7 +326,9 @@ def test_coercing_stack_save_ims( histo_scan_workflow.stack_images_to_save = initial_save_ims with caplog.at_level(logging.WARNING): - stack_params = histo_scan_workflow.create_smart_stack_params() + stack_params = histo_scan_workflow.create_smart_stack_params( + save_on_failure=not histo_scan_workflow.skip_background + ) assert len(caplog.records) == 1 assert str(caplog.records[0].msg).startswith(expected_log_start) @@ -397,7 +403,9 @@ def setup_and_run_smart_z_stack( is a list, it will be set as a side effect (and should be a list of tuples of results). If it a tuple (or anything else), it is set as a return value. """ - stack_params = histo_scan_workflow.create_smart_stack_params() + stack_params = histo_scan_workflow.create_smart_stack_params( + save_on_failure=not histo_scan_workflow.skip_background + ) stack_params.settling_time = 0 # Don't settle or tests take forever. autofocus_thing.capture_stack_image = mocker.Mock() @@ -694,7 +702,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 +764,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 +810,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 +818,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 +862,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 +873,6 @@ def test_invalid_stack_settling_raises(): stack_dz=10, images_to_save=1, settling_time=-1, - backlash_correction=0, origin=StackOrigin.START, ) diff --git a/tests/unit_tests/test_stitching.py b/tests/unit_tests/test_stitching.py index 349d975c..ead2276b 100644 --- a/tests/unit_tests/test_stitching.py +++ b/tests/unit_tests/test_stitching.py @@ -16,11 +16,13 @@ from pydantic import BaseModel import labthings_fastapi as lt from openflexure_microscope_server.stitching import ( + FORBIDDEN_COMMANDS, BaseStitcher, FinalStitcher, PreviewStitcher, StitcherValidationError, StitchingSettings, + validate_command, ) from ..shared_utils.lt_test_utils import LabThingsTestEnv @@ -32,6 +34,29 @@ 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"]) + + +@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=f"Forbidden element '{cmd_name}' detected" + ): + validate_command([cmd_name, "some_arg"]) + + # As an argument (case-insensitive) + with pytest.raises( + StitcherValidationError, + match=f"Forbidden element '{cmd_name.upper()}' detected", + ): + validate_command(["safe-command", cmd_name.upper()]) + + def test_base_stitcher(): """Test the logic in BaseStitcher. 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." + ) diff --git a/tests/unit_tests/test_utilities.py b/tests/unit_tests/test_utilities.py new file mode 100644 index 00000000..bc9076cd --- /dev/null +++ b/tests/unit_tests/test_utilities.py @@ -0,0 +1,169 @@ +"""Unit tests for utility functions.""" + +import logging +import sys + +import pytest + +from openflexure_microscope_server.utilities import ( + _WINDOWS_RESERVED_NAMES, + is_path_safe, + make_name_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"): + 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.""" + 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" + assert make_name_safe("sPoNgEmoCk") == "spongemock" + + +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(". ") == "_" + + +@pytest.mark.parametrize("name", _WINDOWS_RESERVED_NAMES) +def test_make_name_safe_reserved_names(name): + """Test Windows reserved names.""" + # Base name should be sanitized + 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.lower()}.txt_" + # Multiple extensions + 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("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_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 is_path_safe("C:\\path\\to/file") + assert is_path_safe("C:\\a very long path\\to/file") + else: + 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_is_path_safe_unsafe_characters_in_components(caplog, path): + """Test unsafe characters within path components.""" + with caplog.at_level(logging.WARNING): + assert not is_path_safe(path) + assert ( + f"{path} contains characters that may be unsafe on this platform." + in caplog.messages + ) + + +@pytest.mark.parametrize("path", RESERVED_NAME_PATHS) +def test_is_path_safe_reserved_in_components(caplog, path): + """Test reserved names within path components.""" + with caplog.at_level(logging.WARNING): + assert not is_path_safe(path) + assert ( + f"{path} contains a reserved system name and may cause issues." + in caplog.messages + ) + + +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 is_path_safe("chilli con carne/recipe") + assert len(caplog.records) == 0 + + +@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 trailing spaces + if sys.platform.startswith("win"): + 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 is_path_safe(path) + # No warnings should be raised + assert len(caplog.records) == 0 + + +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 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 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." + in caplog.messages + ) + + +@pytest.mark.parametrize("filepath", PATHS_WITH_DOTS) +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 not is_path_safe(filepath) + assert ( + f"File path {filepath} may be unsafe due to trailing dots." + in caplog.messages + ) diff --git a/tests/unit_tests/test_version_strings.py b/tests/unit_tests/test_version_strings.py index f71e59b8..d14196b6 100644 --- a/tests/unit_tests/test_version_strings.py +++ b/tests/unit_tests/test_version_strings.py @@ -19,7 +19,7 @@ THIS_DIR = os.path.dirname(__file__) TRUE_REPO_DIR = os.path.dirname(os.path.dirname(THIS_DIR)) # For explicit version checking. -VER_STRING = "3.0.0-alpha4" +VER_STRING = "3.0.0-alpha5" # This is the regex provided by https://semver.org/ SEMVER_REGEX = re.compile( 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/package-lock.json b/webapp/package-lock.json index 9952f89f..c7f688b2 100644 --- a/webapp/package-lock.json +++ b/webapp/package-lock.json @@ -1,12 +1,12 @@ { "name": "openflexure-microscope-server-ui", - "version": "3.0.0-alpha4", + "version": "3.0.0-alpha5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openflexure-microscope-server-ui", - "version": "3.0.0-alpha4", + "version": "3.0.0-alpha5", "license": "GNU General Public License v3.0 ", "dependencies": { "@vueuse/core": "^14.1.0", diff --git a/webapp/package.json b/webapp/package.json index b470125e..b9dea2e1 100644 --- a/webapp/package.json +++ b/webapp/package.json @@ -1,6 +1,6 @@ { "name": "openflexure-microscope-server-ui", - "version": "3.0.0-alpha4", + "version": "3.0.0-alpha5", "private": true, "description": "User interface for the OpenFlexure Microscope Server", "author": "OpenFlexure (https://www.openflexure.org)", diff --git a/webapp/src/assets/less/theme.less b/webapp/src/assets/less/theme.less index 237f0649..14c41ae8 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; } @@ -252,6 +256,8 @@ h4, .uk-h4 { .uk-notification-message { border-radius: @paper-border-radius; box-shadow: @big-shadow; + word-break: break-word; + overflow-wrap: anywhere; } .uk-notification-message-danger { diff --git a/webapp/src/components/appContent.vue b/webapp/src/components/appContent.vue index 51428b5f..502462d7 100644 --- a/webapp/src/components/appContent.vue +++ b/webapp/src/components/appContent.vue @@ -82,7 +82,6 @@ import tabContent from "./genericComponents/tabContent.vue"; // Import new content components import aboutContent from "./tabContentComponents/aboutContent.vue"; -import backgroundDetectContent from "./tabContentComponents/backgroundDetectContent.vue"; import controlContent from "./tabContentComponents/controlContent.vue"; import loggingContent from "./tabContentComponents/loggingContent.vue"; import powerContent from "./tabContentComponents/powerContent.vue"; @@ -150,15 +149,6 @@ export default { component: markRaw(controlContent), requiredThings: [], }, - { - id: "background-detect", - title: "Background Detect", - icon: "background_replace", - component: markRaw(backgroundDetectContent), - // While stage isn't needed; automatic background detect has little function - // for a manual microscope. - requiredThings: ["stage"], - }, { id: "slide-scan", title: "Slide Scan", 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/actionButton.vue b/webapp/src/components/labThingsComponents/actionButton.vue index e5bfda75..fe06c106 100644 --- a/webapp/src/components/labThingsComponents/actionButton.vue +++ b/webapp/src/components/labThingsComponents/actionButton.vue @@ -50,6 +50,18 @@ export default { type: String, required: true, }, + // forceId and forceUrl can be used if it is known that when this button is mounted + // the action should be running. + forceId: { + type: [String, null], + required: false, + default: null, + }, + forceUrl: { + type: [String, null], + required: false, + default: null, + }, submitData: { type: [Object, Array], required: false, @@ -197,17 +209,22 @@ export default { }, ); - // Check for already running tasks - if (this.taskStarted != true) { - this.checkExistingTasks(); - } - // A global signal listener to perform the action - if (this.submitOnEvent) { - eventBus.on(this.submitOnEvent, () => { - if (this.isDisabled) return; - // Bootstrap task if button is not disabled. - this.bootstrapTask(); - }); + if (this.forceId && this.forceUrl) { + this.taskStarted = true; + this.startPollingTask(this.forceId, this.forceUrl); + } else { + // Check for already running tasks + if (this.taskStarted != true) { + this.checkExistingTasks(); + } + // A global signal listener to perform the action + if (this.submitOnEvent) { + eventBus.on(this.submitOnEvent, () => { + if (this.isDisabled) return; + // Bootstrap task if button is not disabled. + this.bootstrapTask(); + }); + } } }, @@ -240,18 +257,13 @@ export default { * */ async checkExistingTasks() { - let response = await this.findOngoingActions(this.thing, this.action); - // Exit if response is null, due to an error. - if (response == null) return; - // Check for a task that is ongoing. - // We can't handle multiple tasks ongoing, so this picks the first. - const ongoingTask = response.data.find((t) => ["pending", "running"].includes(t.status)); + const ongoingTask = await this.getOngingAction(this.thing, this.action); if (ongoingTask) { // There is a started task this.taskStarted = true; - this.$emit("taskStarted"); // Find its URL const taskUrl = ongoingTask.links.find((t) => t.rel == "self").href; + this.$emit("taskStarted", ongoingTask.id, taskUrl); this.startPollingTask(ongoingTask.id, taskUrl); } }, @@ -275,7 +287,6 @@ export default { this.$emit("submit", this.submitData); // Send a request to start a task this.taskStarted = true; - this.$emit("taskStarted"); let response; try { response = await this.invokeAction( @@ -289,6 +300,9 @@ export default { this.onTaskEnd(); return; } + // Wait until after the task is acknowledged before confirming to parent + // component that the task is started. + this.$emit("taskStarted", response.data.id, response.data.href); if (this.modalProgress) { this.$refs.statusModal.show(); } @@ -322,6 +336,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/inputFromSchema.vue b/webapp/src/components/labThingsComponents/inputFromSchema.vue index 891075ea..47ff8634 100644 --- a/webapp/src/components/labThingsComponents/inputFromSchema.vue +++ b/webapp/src/components/labThingsComponents/inputFromSchema.vue @@ -34,8 +34,12 @@ @animationend="animationEnd" />
diff --git a/webapp/src/components/labThingsComponents/serverSpecifiedActionButton.vue b/webapp/src/components/labThingsComponents/serverSpecifiedActionButton.vue index 78f81dd0..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: { /** @@ -45,7 +46,14 @@ export default { */ actionResponse: function (response) { if (this.actionData.notify_on_success) { - this.modalNotify(this.actionData.success_message); + if (this.actionData.response_is_success_message) { + this.modalNotify(response.output); + } 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 new file mode 100644 index 00000000..f69ae102 --- /dev/null +++ b/webapp/src/components/labThingsComponents/serverSpecifiedInterface.vue @@ -0,0 +1,66 @@ + + + + + diff --git a/webapp/src/components/modalComponents/calibrationWizard.vue b/webapp/src/components/modalComponents/calibrationWizard.vue index f4729936..0dd1dcc2 100644 --- a/webapp/src/components/modalComponents/calibrationWizard.vue +++ b/webapp/src/components/modalComponents/calibrationWizard.vue @@ -1,6 +1,13 @@