From 936dfaa740c13264bd1129656b1cb3a4274c07eb Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 25 Sep 2024 10:55:59 +0100 Subject: [PATCH] Warn rather than fail if the webcam fails (#802) --- src/dodal/devices/webcam.py | 40 +++++++++++++++---- tests/devices/unit_tests/test_webcam.py | 52 +++++++++++++++++-------- 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/dodal/devices/webcam.py b/src/dodal/devices/webcam.py index 3805b08e7f..0d7b54ea97 100644 --- a/src/dodal/devices/webcam.py +++ b/src/dodal/devices/webcam.py @@ -1,12 +1,24 @@ +from collections.abc import ByteString +from io import BytesIO from pathlib import Path import aiofiles from aiohttp import ClientSession from bluesky.protocols import Triggerable from ophyd_async.core import AsyncStatus, HintedSignal, StandardReadable, soft_signal_rw +from PIL import Image from dodal.log import LOGGER +PLACEHOLDER_IMAGE_SIZE = (1024, 768) +IMAGE_FORMAT = "png" + + +def create_placeholder_image() -> ByteString: + image = Image.new("RGB", PLACEHOLDER_IMAGE_SIZE) + image.save(buffer := BytesIO(), format=IMAGE_FORMAT) + return buffer.getbuffer() + class Webcam(StandardReadable, Triggerable): def __init__(self, name, prefix, url): @@ -18,19 +30,33 @@ def __init__(self, name, prefix, url): self.add_readables([self.last_saved_path], wrapper=HintedSignal) super().__init__(name=name) - async def _write_image(self, file_path: str): + async def _write_image(self, file_path: str, image: ByteString): + async with aiofiles.open(file_path, "wb") as file: + await file.write(image) + + async def _get_and_write_image(self, file_path: str): async with ClientSession() as session: async with session.get(self.url) as response: - response.raise_for_status() - LOGGER.info(f"Saving webcam image from {self.url} to {file_path}") - async with aiofiles.open(file_path, "wb") as file: - await file.write(await response.read()) + if not response.ok: + LOGGER.warning( + f"Webcam responded with {response.status}: {response.reason}. Attempting to read anyway." + ) + try: + data = await response.read() + LOGGER.info(f"Saving webcam image from {self.url} to {file_path}") + except Exception as e: + LOGGER.warning( + f"Failed to read data from {self.url} ({e}). Using placeholder image." + ) + data = create_placeholder_image() + + await self._write_image(file_path, data) @AsyncStatus.wrap async def trigger(self) -> None: filename = await self.filename.get_value() directory = await self.directory.get_value() - file_path = Path(f"{directory}/{filename}.png").as_posix() - await self._write_image(file_path) + file_path = Path(f"{directory}/{filename}.{IMAGE_FORMAT}").as_posix() + await self._get_and_write_image(file_path) await self.last_saved_path.set(file_path) diff --git a/tests/devices/unit_tests/test_webcam.py b/tests/devices/unit_tests/test_webcam.py index bf8f28c5bf..797fe10a61 100644 --- a/tests/devices/unit_tests/test_webcam.py +++ b/tests/devices/unit_tests/test_webcam.py @@ -1,10 +1,12 @@ +from io import BytesIO from unittest.mock import AsyncMock, MagicMock, patch import pytest from bluesky.run_engine import RunEngine +from PIL import Image from dodal.beamlines import i03 -from dodal.devices.webcam import Webcam +from dodal.devices.webcam import Webcam, create_placeholder_image @pytest.fixture @@ -37,9 +39,6 @@ async def test_given_filename_and_directory_when_trigger_and_read_then_returns_e webcam: Webcam, ): mock_get.return_value.__aenter__.return_value = AsyncMock() - mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) - # raise_for_status should be MagicMock() not AsyncMock() - mock_response.raise_for_status = MagicMock() await webcam.filename.set(filename) await webcam.directory.set(directory) await webcam.trigger() @@ -53,8 +52,6 @@ async def test_given_data_returned_from_url_when_trigger_then_data_written( mock_get: MagicMock, mock_aiofiles, webcam: Webcam ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) - # raise_for_status should be MagicMock() not AsyncMock() - mock_response.raise_for_status = MagicMock() mock_response.read.return_value = (test_web_data := "TEST") mock_open = mock_aiofiles.open mock_open.return_value.__aenter__.return_value = (mock_file := AsyncMock()) @@ -65,20 +62,43 @@ async def test_given_data_returned_from_url_when_trigger_then_data_written( mock_file.write.assert_called_once_with(test_web_data) -@patch("dodal.devices.webcam.aiofiles", autospec=True) @patch("dodal.devices.webcam.ClientSession.get", autospec=True) -async def test_given_response_throws_exception_when_trigger_then_exception_rasied( - mock_get: MagicMock, mock_aiofiles, webcam: Webcam +async def test_given_response_has_bad_status_but_response_read_still_returns_then_still_write_data( + mock_get: MagicMock, webcam: Webcam ): - class MyException(Exception): - pass + mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + mock_response.ok = MagicMock(return_value=False) + mock_response.read.return_value = (test_web_data := b"TEST") + + webcam._write_image = (mock_write := AsyncMock()) + + await webcam.filename.set("file") + await webcam.directory.set("/tmp") + await webcam.trigger() - def _raise(): - raise MyException() + mock_write.assert_called_once_with("/tmp/file.png", test_web_data) + +@patch("dodal.devices.webcam.create_placeholder_image", autospec=True) +@patch("dodal.devices.webcam.ClientSession.get", autospec=True) +async def test_given_response_read_fails_then_placeholder_image_written( + mock_get: MagicMock, mock_placeholder_image: MagicMock, webcam: Webcam +): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) - mock_response.raise_for_status = _raise + mock_response.read = AsyncMock(side_effect=Exception()) + mock_placeholder_image.return_value = (test_placeholder_data := b"TEST") + + webcam._write_image = (mock_write := AsyncMock()) + await webcam.filename.set("file") await webcam.directory.set("/tmp") - with pytest.raises(MyException): - await webcam.trigger() + await webcam.trigger() + + mock_write.assert_called_once_with("/tmp/file.png", test_placeholder_data) + + +def test_create_place_holder_image_gives_expected_bytes(): + image_bytes = create_placeholder_image() + placeholder_image = Image.open(BytesIO(image_bytes)) + assert placeholder_image.width == 1024 + assert placeholder_image.height == 768