diff --git a/.copier-answers.yml b/.copier-answers.yml index f73ec6a..7f8da05 100644 --- a/.copier-answers.yml +++ b/.copier-answers.yml @@ -1,5 +1,5 @@ # Changes here will be overwritten by Copier -_commit: 2.1.0 +_commit: 2.1.0-40-g9e70b8b _src_path: gh:DiamondLightSource/python-copier-template author_email: gary.yendell@diamond.ac.uk author_name: Gary Yendell diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000..aa65892 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,21 @@ +--- +name: Bug Report +about: The template to use for reporting bugs and usability issues +title: " " +labels: 'bug' +assignees: '' + +--- + +Describe the bug, including a clear and concise description of the expected behavior, the actual behavior and the context in which you encountered it (ideally include details of your environment). + +## Steps To Reproduce +Steps to reproduce the behavior: +1. Go to '...' +2. Click on '....' +3. Scroll down to '....' +4. See error + + +## Acceptance Criteria +- Specific criteria that will be used to judge if the issue is fixed diff --git a/.github/ISSUE_TEMPLATE/issue.md b/.github/ISSUE_TEMPLATE/issue.md new file mode 100644 index 0000000..52c84dd --- /dev/null +++ b/.github/ISSUE_TEMPLATE/issue.md @@ -0,0 +1,13 @@ +--- +name: Issue +about: The standard template to use for feature requests, design discussions and tasks +title: " " +labels: '' +assignees: '' + +--- + +A brief description of the issue, including specific stakeholders and the business case where appropriate + +## Acceptance Criteria +- Specific criteria that will be used to judge if the issue is fixed diff --git a/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md b/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md new file mode 100644 index 0000000..8200afe --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md @@ -0,0 +1,8 @@ +Fixes #ISSUE + +### Instructions to reviewer on how to test: +1. Do thing x +2. Confirm thing y happens + +### Checks for reviewer +- [ ] Would the PR title make sense to a user on a set of release notes diff --git a/.github/workflows/_container.yml b/.github/workflows/_container.yml index 4857ee9..3ee61f1 100644 --- a/.github/workflows/_container.yml +++ b/.github/workflows/_container.yml @@ -25,7 +25,7 @@ jobs: password: ${{ secrets.GITHUB_TOKEN }} - name: Build and export to Docker local cache - uses: docker/build-push-action@v5 + uses: docker/build-push-action@v6 with: context: . # Need load and tags so we can test it below @@ -46,7 +46,7 @@ jobs: - name: Push cached image to container registry if: github.ref_type == 'tag' - uses: docker/build-push-action@v5 + uses: docker/build-push-action@v6 # This does not build the image again, it will find the image in the # Docker cache and publish it with: diff --git a/.github/workflows/_docs.yml b/.github/workflows/_docs.yml index 9d2f0c6..a1cafca 100644 --- a/.github/workflows/_docs.yml +++ b/.github/workflows/_docs.yml @@ -47,7 +47,7 @@ jobs: if: github.ref_type == 'tag' || github.ref_name == 'main' # We pin to the SHA, not the tag, for security reasons. # https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions - uses: peaceiris/actions-gh-pages@373f7f263a76c20808c831209c920827a82a2847 # v3.9.3 + uses: peaceiris/actions-gh-pages@4f9cc6602d3f66b9c108549d475ec49e8ef4d45e # v4.0.0 with: github_token: ${{ secrets.GITHUB_TOKEN }} publish_dir: .github/pages diff --git a/.github/workflows/_release.yml b/.github/workflows/_release.yml index e55efdb..10d8ed8 100644 --- a/.github/workflows/_release.yml +++ b/.github/workflows/_release.yml @@ -23,7 +23,7 @@ jobs: - name: Create GitHub Release # We pin to the SHA, not the tag, for security reasons. # https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions - uses: softprops/action-gh-release@9d7c94cfd0a1f3ed45544c887983e9fa900f0564 # v2.0.4 + uses: softprops/action-gh-release@c062e08bd532815e2082a85e87e3ef29c3e6d191 # v2.0.8 with: prerelease: ${{ contains(github.ref_name, 'a') || contains(github.ref_name, 'b') || contains(github.ref_name, 'rc') }} files: "*" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e6dfa29..cc8cafb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,7 @@ jobs: if: needs.check.outputs.branch-pr == '' uses: ./.github/workflows/_container.yml permissions: + contents: read packages: write docs: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5a4cbf7..60fc23f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,6 +5,7 @@ repos: - id: check-added-large-files - id: check-yaml - id: check-merge-conflict + - id: end-of-file-fixer - repo: local hooks: diff --git a/.vscode/settings.json b/.vscode/settings.json index 394a941..101c75f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -5,6 +5,7 @@ "editor.codeActionsOnSave": { "source.organizeImports": "explicit" }, + "files.insertFinalNewline": true, "[python]": { "editor.defaultFormatter": "charliermarsh.ruff", }, diff --git a/Dockerfile b/Dockerfile index 5edb6ee..0915542 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,7 +16,7 @@ ENV PATH=/venv/bin:$PATH FROM developer AS build COPY . /context WORKDIR /context -RUN pip install --upgrade pip && pip install . +RUN touch dev-requirements.txt && pip install -c dev-requirements.txt . # The runtime stage copies the built venv into a slim runtime container FROM python:${PYTHON_VERSION}-slim AS runtime diff --git a/catalog-info.yaml b/catalog-info.yaml index 93ef467..183e12e 100644 --- a/catalog-info.yaml +++ b/catalog-info.yaml @@ -7,4 +7,4 @@ metadata: spec: type: documentation lifecycle: experimental - owner: user:mef65357 \ No newline at end of file + owner: user:mef65357 diff --git a/docs/how-to/contribute.md b/docs/how-to/contribute.md index f9c4ca1..6e41979 100644 --- a/docs/how-to/contribute.md +++ b/docs/how-to/contribute.md @@ -1,2 +1,2 @@ ```{include} ../../.github/CONTRIBUTING.md -``` \ No newline at end of file +``` diff --git a/docs/images/dls-logo.svg b/docs/images/dls-logo.svg index 0af1a17..4fcaa86 100644 --- a/docs/images/dls-logo.svg +++ b/docs/images/dls-logo.svg @@ -1,11 +1,11 @@ - - - - - - - - - - \ No newline at end of file + + + + + + + + + + diff --git a/pyproject.toml b/pyproject.toml index 148b9ac..d2539f0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ["setuptools>=64", "setuptools_scm[toml]>=6.2"] +requires = ["setuptools>=64", "setuptools_scm[toml]>=8"] build-backend = "setuptools.build_meta" [project] @@ -12,7 +12,7 @@ classifiers = [ description = "Eiger control system integration with FastCS" dependencies = [ "aiohttp", - "fastcs~=0.4.0", + "fastcs~=0.5.0", "numpy", "pillow", "typer", @@ -54,7 +54,7 @@ email = "gary.yendell@diamond.ac.uk" name = "Gary Yendell" [tool.setuptools_scm] -write_to = "src/eiger_fastcs/_version.py" +version_file = "src/eiger_fastcs/_version.py" [tool.mypy] ignore_missing_imports = true # Ignore missing stubs in imported modules @@ -104,11 +104,18 @@ commands = src = ["src", "tests"] line-length = 88 lint.select = [ - "B", # flake8-bugbear - https://docs.astral.sh/ruff/rules/#flake8-bugbear-b - "C4", # flake8-comprehensions - https://docs.astral.sh/ruff/rules/#flake8-comprehensions-c4 - "E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e - "F", # pyflakes rules - https://docs.astral.sh/ruff/rules/#pyflakes-f - "W", # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#warning-w - "I", # isort - https://docs.astral.sh/ruff/rules/#isort-i - "UP", # pyupgrade - https://docs.astral.sh/ruff/rules/#pyupgrade-up + "B", # flake8-bugbear - https://docs.astral.sh/ruff/rules/#flake8-bugbear-b + "C4", # flake8-comprehensions - https://docs.astral.sh/ruff/rules/#flake8-comprehensions-c4 + "E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e + "F", # pyflakes rules - https://docs.astral.sh/ruff/rules/#pyflakes-f + "W", # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#warning-w + "I", # isort - https://docs.astral.sh/ruff/rules/#isort-i + "UP", # pyupgrade - https://docs.astral.sh/ruff/rules/#pyupgrade-up + "SLF", # self - https://docs.astral.sh/ruff/settings/#lintflake8-self ] + +[tool.ruff.lint.per-file-ignores] +# By default, private member access is allowed in tests +# See https://github.com/DiamondLightSource/python-copier-template/issues/154 +# Remove this line to forbid private member access in tests +"tests/**/*" = ["SLF001"] diff --git a/src/eiger_fastcs/eiger_controller.py b/src/eiger_fastcs/eiger_controller.py index 2f3683c..46c5ecb 100644 --- a/src/eiger_fastcs/eiger_controller.py +++ b/src/eiger_fastcs/eiger_controller.py @@ -46,6 +46,10 @@ } +def command_uri(key: str) -> str: + return f"detector/api/1.8.0/command/{key}" + + def detector_command(fn) -> Any: return command(group="DetectorCommand")(fn) @@ -63,7 +67,7 @@ class EigerHandler: update_period: float = 0.2 async def put(self, controller: "EigerController", _: AttrW, value: Any) -> None: - parameters_to_update = await controller._connection.put(self.name, value) + parameters_to_update = await controller.connection.put(self.name, value) if not parameters_to_update: parameters_to_update = [self.name.split("/")[-1]] print(f"Manually fetching parameter {parameters_to_update}") @@ -76,7 +80,7 @@ async def put(self, controller: "EigerController", _: AttrW, value: Any) -> None async def update(self, controller: "EigerController", attr: AttrR) -> None: try: - response = await controller._connection.get(self.name) + response = await controller.connection.get(self.name) await attr.set(response["value"]) except Exception as e: print(f"Failed to get {self.name}:\n{e.__class__.__name__} {e}") @@ -93,7 +97,7 @@ async def update(self, controller: "EigerController", attr: AttrR) -> None: await super().update(controller, attr) if isinstance(attr, AttrRW): # Sync readback value to demand - await attr._write_display_callback(attr.get()) + await attr.update_display_without_process(attr.get()) self.first_poll_complete = True @@ -156,18 +160,18 @@ class EigerController(Controller): Sets up all connections with the Simplon API to send and receive information """ - # Detector Parameters - ntrigger = AttrRW(Int()) # TODO: Include URI and validate type from API + # Detector parameters to use in internal logic + trigger_mode = AttrRW(String()) # TODO: Include URI and validate type from API - # Logic Parameters - manual_trigger = AttrRW(Bool(), handler=LogicHandler()) + # Internal Attributes stale_parameters = AttrR(Bool()) + trigger_exposure = AttrRW(Float(), handler=LogicHandler()) def __init__(self, ip: str, port: int) -> None: super().__init__() self._ip = ip self._port = port - self._connection = HTTPConnection(self._ip, self._port) + self.connection = HTTPConnection(self._ip, self._port) # Parameter update logic self._parameter_updates: set[str] = set() @@ -179,7 +183,7 @@ def __init__(self, ip: str, port: int) -> None: async def connect(self) -> None: """Reopen connection on backend asyncio loop""" - self._connection.open() + self.connection.open() async def initialise(self) -> None: """Create attributes by introspecting detector. @@ -187,13 +191,13 @@ async def initialise(self) -> None: The detector will be initialized if it is not already. """ - self._connection.open() + self.connection.open() # Check current state of detector_state to see if initializing is required. - state_val = await self._connection.get("detector/api/1.8.0/status/state") + state_val = await self.connection.get("detector/api/1.8.0/status/state") if state_val["value"] == "na": print("Initializing Detector") - await self._connection.put("detector/api/1.8.0/command/initialize", "") + await self.connection.put("detector/api/1.8.0/command/initialize") try: parameters = await self._introspect_detector() @@ -206,7 +210,7 @@ async def initialise(self) -> None: for name, attribute in attributes.items(): setattr(self, name, attribute) - await self._connection.close() + await self.connection.close() async def _introspect_detector(self) -> list[EigerParameter]: parameters = [] @@ -215,13 +219,13 @@ async def _introspect_detector(self) -> list[EigerParameter]: ): subsystem_keys = [ parameter - for parameter in await self._connection.get( + for parameter in await self.connection.get( f"{subsystem}/api/1.8.0/{mode}/keys" ) if parameter not in IGNORED_KEYS ] + MISSING_KEYS[subsystem][mode] requests = [ - self._connection.get(f"{subsystem}/api/1.8.0/{mode}/{key}") + self.connection.get(f"{subsystem}/api/1.8.0/{mode}/{key}") for key in subsystem_keys ] responses = await asyncio.gather(*requests) @@ -276,6 +280,7 @@ def _create_attributes(self, parameters: list[EigerParameter]): datatype, handler=EIGER_HANDLERS[parameter.mode](parameter.uri), group=group, + allowed_values=parameter.response.get("allowed_values", None), ) return attributes @@ -299,54 +304,37 @@ def _tag_key_clashes(parameters: list[EigerParameter]): async def close(self) -> None: """Closing HTTP connection with device""" - await self._connection.close() - - async def arm(self): - """Arming Detector called by the start acquisition button""" - await self._connection.put("detector/api/1.8.0/command/arm", "") + await self.connection.close() @detector_command async def initialize(self): - """Command to initialize Detector - will create a PVI button""" - await self._connection.put("detector/api/1.8.0/command/initialize", "") + await self.connection.put(command_uri("initialize")) @detector_command - async def disarm(self): - """Command to disarm Detector - will create a PVI button""" - await self._connection.put("detector/api/1.8.0/command/disarm", "") + async def arm(self): + await self.connection.put(command_uri("arm")) @detector_command - async def abort(self): - """Command to abort any tasks Detector - will create a PVI button""" - await self._connection.put("detector/api/1.8.0/command/abort", "") + async def trigger(self): + match self.trigger_mode.get(), self.trigger_exposure.get(): + case ("inte", exposure) if exposure > 0.0: + await self.connection.put(command_uri("trigger"), exposure) + case ("ints" | "inte", _): + await self.connection.put(command_uri("trigger")) + case _: + raise RuntimeError("Can only do soft trigger in 'ints' or 'inte' mode") @detector_command - async def cancel(self): - """Command to cancel readings from Detector - will create a PVI button""" - await self._connection.put("detector/api/1.8.0/command/cancel", "") + async def disarm(self): + await self.connection.put(command_uri("disarm")) @detector_command - async def trigger(self): - """ - Command to trigger Detector when manual triggering is switched on. - will create a PVI button - """ - await self._connection.put("detector/api/1.8.0/command/trigger", "") + async def abort(self): + await self.connection.put(command_uri("abort")) @detector_command - async def start_acquisition(self): - """ - Command to start acquiring detector image. - - Iterates through arming and triggering the detector if manual trigger off. - If manual triggers are on, it arms the detector, ready for triggering. - """ - await self.arm() - # Current functionality is for ints triggering and multiple ntriggers for - # automatic triggering - if not self.manual_trigger.get(): - for _ in range(self.ntrigger.get()): - await self.trigger() + async def cancel(self): + await self.connection.put(command_uri("cancel")) async def queue_update(self, parameters: list[str]): """Add the given parameters to the list of parameters to update. @@ -392,7 +380,7 @@ async def update(self): @scan(1) async def handle_monitor(self): """Poll monitor images to display.""" - response, image_bytes = await self._connection.get_bytes( + response, image_bytes = await self.connection.get_bytes( "monitor/api/1.8.0/images/next" ) if response.status != 200: diff --git a/src/eiger_fastcs/http_connection.py b/src/eiger_fastcs/http_connection.py index 1505c36..3029f76 100644 --- a/src/eiger_fastcs/http_connection.py +++ b/src/eiger_fastcs/http_connection.py @@ -1,4 +1,4 @@ -from aiohttp import ClientResponse, ClientSession +from aiohttp import ClientResponse, ClientSession, ClientTimeout class HTTPRequestError(ConnectionError): @@ -56,7 +56,9 @@ async def get(self, uri) -> dict[str, str]: """ session = self.get_session() - async with session.get(self.full_url(uri), timeout=3) as response: + async with session.get( + self.full_url(uri), timeout=ClientTimeout(total=3) + ) as response: if response.status != 200: raise HTTPRequestError(f"Failed to get {uri}", response) else: @@ -75,7 +77,7 @@ async def get_bytes(self, uri) -> tuple[ClientResponse, bytes]: async with session.get(self.full_url(uri)) as response: return response, await response.read() - async def put(self, uri, value) -> list[str]: + async def put(self, uri, value=None) -> list[str]: """Perform HTTP PUT request and return response content as json. If successful, the response is a list of parameters whose values may have @@ -90,7 +92,7 @@ async def put(self, uri, value) -> list[str]: session = self.get_session() async with session.put( self.full_url(uri), - json={"value": value}, + json={"value": value} if value is not None else None, headers={"Content-Type": "application/json"}, ) as response: if response.status != 200: diff --git a/tests/system/parameters.json b/tests/system/parameters.json index 8b21b22..a799bdf 100644 --- a/tests/system/parameters.json +++ b/tests/system/parameters.json @@ -930,4 +930,4 @@ "value_type": "string" } } -} \ No newline at end of file +} diff --git a/tests/system/test_introspection.py b/tests/system/test_introspection.py index e0cc3b7..dbd2300 100644 --- a/tests/system/test_introspection.py +++ b/tests/system/test_introspection.py @@ -30,21 +30,37 @@ def eiger_controller(): yield EigerController("i04-1-eiger01", 80) +# Stolen from tickit-devices +# https://docs.pytest.org/en/latest/example/parametrize.html#indirect-parametrization @pytest.fixture -def sim_eiger_controller(): - cmd = ["tickit", "all", str(HERE / "eiger.yaml")] - sim = subprocess.Popen(cmd) +def sim_eiger_controller(request): + """Subprocess that runs ``tickit all ``.""" + config_path: str = request.param + proc = subprocess.Popen( + ["tickit", "all", config_path], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + ) + + # Wait until ready + while True: + line = proc.stdout.readline() + if "Starting HTTP server..." in line: + break sleep(3) yield EigerController("127.0.0.1", 8081) - sim.send_signal(signal.SIGTERM) - sleep(0.1) - sim.kill() + proc.send_signal(signal.SIGINT) + print(proc.communicate()[0]) @pytest.mark.asyncio +@pytest.mark.parametrize( + "sim_eiger_controller", [str(HERE / "eiger.yaml")], indirect=True +) async def test_introspection(sim_eiger_controller: EigerController): controller = sim_eiger_controller # controller = eiger_controller