Skip to content

Commit

Permalink
Fix eiger put responses to reflect real detector behaviour (Fixes #114)…
Browse files Browse the repository at this point in the history
… (#115)

Return correct changed parameters list for eiger detector put requests
Return empty list for puts to non detector eiger API paths to reflect real API
Test response of get_changed_parameters serialises properly
  • Loading branch information
jsouter authored Oct 8, 2024
1 parent 05bc859 commit ce7a7f8
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 17 deletions.
70 changes: 70 additions & 0 deletions src/tickit_devices/eiger/eiger.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,3 +242,73 @@ def _set_state(self, state: State) -> None:

def _is_in_state(self, state: State) -> bool:
return self.get_state() is state


def get_changed_parameters(key: str) -> list[str]:
"""Get the list of parameters that may have changed as a result of putting
to the parameter provided.
Args:
key: string key of the changed parameter within the detector subsystem
Returns:
list[str]: a list of keys which may have been changed after a PUT request
"""
match key:
case "auto_summation":
return ["auto_summation", "frame_count_time"]
case "count_time" | "frame_time":
return [
"bit_depth_image",
"bit_depth_readout",
"count_time",
"countrate_correction_count_cutoff",
"frame_count_time",
"frame_time",
]
case "flatfield":
return ["flatfield", "threshold/1/flatfield"]
case "incident_energy" | "photon_energy":
return [
"element",
"flatfield",
"incident_energy",
"photon_energy",
"threshold/1/energy",
"threshold/1/flatfield",
"threshold/2/energy",
"threshold/2/flatfield",
"threshold_energy",
"wavelength",
]
case "pixel_mask":
return ["pixel_mask", "threshold/1/pixel_mask"]
case "threshold/1/flatfield":
return ["flatfield", "threshold/1/flatfield"]
case "roi_mode":
return ["count_time", "frame_time", "roi_mode"]
case "threshold_energy" | "threshold/1/energy":
return [
"flatfield",
"threshold/1/energy",
"threshold/1/flatfield",
"threshold/2/flatfield",
"threshold_energy",
]
case "threshold/2/energy":
return [
"flatfield",
"threshold/1/flatfield",
"threshold/2/energy",
"threshold/2/flatfield",
]
case "threshold/1/mode":
return ["threshold/1/mode", "threshold/difference/mode"]
case "threshold/2/mode":
return ["threshold/2/mode", "threshold/difference/mode"]
case "threshold/1/pixel_mask":
return ["pixel_mask", "threshold/1/pixel_mask"]
case "threshold/difference/mode":
return ["difference_mode"] # replicating API inconsistency
case _:
return [key]
24 changes: 13 additions & 11 deletions src/tickit_devices/eiger/eiger_adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from tickit.adapters.specifications import HttpEndpoint
from tickit.adapters.zmq import ZeroMqPushAdapter

from tickit_devices.eiger.eiger import EigerDevice
from tickit_devices.eiger.eiger import EigerDevice, get_changed_parameters
from tickit_devices.eiger.eiger_schema import SequenceComplete, construct_value
from tickit_devices.eiger.stream.eiger_stream import EigerStream
from tickit_devices.eiger.stream.eiger_stream_2 import EigerStream2
Expand Down Expand Up @@ -75,7 +75,10 @@ async def put_config(self, request: web.Request) -> web.Response:
self.device.settings[param] = attr

LOGGER.debug("Set " + str(param) + " to " + str(attr))
return web.json_response(serialize([param]))

changed_parameters = get_changed_parameters(param)

return web.json_response(serialize(changed_parameters))
else:
LOGGER.debug("Eiger has no config variable: " + str(param))
return web.json_response(status=404)
Expand Down Expand Up @@ -123,15 +126,14 @@ async def put_threshold_config(self, request: web.Request) -> web.Response:
config = self.device.settings.threshold_config
if threshold in config and hasattr(config[threshold], param):
attr = response["value"]

LOGGER.debug(
f"Changing to {str(attr)} for threshold/{threshold}{str(param)}"
)

config[threshold][param] = attr

LOGGER.debug(f"Set threshold/{threshold}{str(param)} to {str(attr)}")
return web.json_response(serialize([param]))

full_param = f"threshold/{threshold}/{param}"
changed_parameters = get_changed_parameters(full_param)

return web.json_response(serialize(changed_parameters))
else:
LOGGER.debug("Eiger has no config variable: " + str(param))
return web.json_response(status=404)
Expand Down Expand Up @@ -368,7 +370,7 @@ async def put_stream_config(self, request: web.Request) -> web.Response:
self.device.stream_config[param] = attr

LOGGER.debug("Set " + str(param) + " to " + str(attr))
return web.json_response(serialize([param]))
return web.json_response([])
else:
LOGGER.debug("Eiger has no config variable: " + str(param))
return web.json_response(status=404)
Expand Down Expand Up @@ -415,7 +417,7 @@ async def put_monitor_config(self, request: web.Request) -> web.Response:
self.device.monitor_config[param] = attr

LOGGER.debug("Set " + str(param) + " to " + str(attr))
return web.json_response(serialize([param]))
return web.json_response([])
else:
LOGGER.debug("Eiger has no config variable: " + str(param))
return web.json_response(status=404)
Expand Down Expand Up @@ -482,7 +484,7 @@ async def put_filewriter_config(self, request: web.Request) -> web.Response:
self.device.filewriter_config[param] = attr

LOGGER.debug("Set " + str(param) + " to " + str(attr))
return web.json_response(serialize([param]))
return web.json_response([])
else:
LOGGER.debug("Eiger has no config variable: " + str(param))
return web.json_response(status=404)
Expand Down
59 changes: 58 additions & 1 deletion tests/eiger/test_eiger_adapters.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest
from pytest_mock import MockerFixture

from tickit_devices.eiger.eiger import EigerDevice
from tickit_devices.eiger.eiger import EigerDevice, get_changed_parameters
from tickit_devices.eiger.eiger_adapters import EigerRESTAdapter, EigerZMQAdapter


Expand Down Expand Up @@ -72,3 +72,60 @@ async def test_rest_adapter_command_404(mocker: MockerFixture):
assert (await eiger_adapter.trigger_eiger(request)).status == 404
assert (await eiger_adapter.cancel_eiger(request)).status == 404
assert (await eiger_adapter.abort_eiger(request)).status == 404


@pytest.mark.asyncio
async def test_detector_put_responses(mocker: MockerFixture):
# test special keys have non-trivial response
custom_response_keys = [
"auto_summation",
"count_time",
"frame_time",
"flatfield",
"incident_energy",
"photon_energy",
"pixel_mask",
"threshold/1/flatfield",
"roi_mode",
"threshold_energy",
"threshold/1/energy",
"threshold/2/energy",
"threshold/1/mode",
"threshold/2/mode",
"threshold/1/pixel_mask",
"threshold/difference/mode",
]

for key in custom_response_keys:
assert get_changed_parameters(key) != [key]

assert get_changed_parameters("phi_start") == ["phi_start"]
assert get_changed_parameters("threshold/1/pixel_mask") == [
"pixel_mask",
"threshold/1/pixel_mask",
]
assert get_changed_parameters("threshold/difference/mode") == ["difference_mode"]

eiger_adapter = EigerRESTAdapter(EigerDevice())

request = mocker.MagicMock()
request.json = mocker.AsyncMock()

request.match_info = {"parameter_name": "count_time", "value": 1.0}
response = await eiger_adapter.put_config(request)
assert response.body == (
b'["bit_depth_image", "bit_depth_readout", "count_time",'
b' "countrate_correction_count_cutoff",'
b' "frame_count_time", "frame_time"]'
)
# trivial case just returns the single parameter
request.match_info = {"parameter_name": "phi_start", "value": 1.0}
response = await eiger_adapter.put_config(request)
assert response.body == b'["phi_start"]'

# test threshold responses work

request.match_info = {"parameter_name": "mode", "threshold": "1", "value": 1}
request.json = mocker.AsyncMock(return_value={"value": 1})
response = await eiger_adapter.put_threshold_config(request)
assert response.body == b'["threshold/1/mode", "threshold/difference/mode"]'
16 changes: 11 additions & 5 deletions tests/eiger/test_eiger_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ async def get_status(status, expected):
json={"value": "enabled"},
timeout=REQUEST_TIMEOUT,
) as response:
assert ["mode"] == (await response.json())
assert [] == (await response.json())

# Test filewriter, monitor and stream endpoints
async with session.get(
Expand All @@ -108,7 +108,7 @@ async def get_status(status, expected):
json={"value": "enabled"},
timeout=REQUEST_TIMEOUT,
) as response:
assert ["mode"] == (await response.json())
assert [] == (await response.json())

async with session.get(
MONITOR_URL + "status/error",
Expand All @@ -128,15 +128,21 @@ async def get_status(status, expected):
json={"value": "enabled"},
timeout=REQUEST_TIMEOUT,
) as response:
assert ["mode"] == (await response.json())
assert [] == (await response.json())

async with session.put(
DETECTOR_URL + "config/threshold/1/energy",
headers=headers,
json={"value": 6829},
timeout=REQUEST_TIMEOUT,
) as response:
assert ["energy"] == (await response.json())
assert [
"flatfield",
"threshold/1/energy",
"threshold/1/flatfield",
"threshold/2/flatfield",
"threshold_energy",
] == (await response.json())

async with session.get(
DETECTOR_URL + "config/threshold/1/energy",
Expand All @@ -151,7 +157,7 @@ async def get_status(status, expected):
json={"value": "enabled"},
timeout=REQUEST_TIMEOUT,
) as response:
assert ["mode"] == (await response.json())
assert ["difference_mode"] == (await response.json())

async with session.get(
DETECTOR_URL + "config/threshold/difference/mode",
Expand Down

0 comments on commit ce7a7f8

Please sign in to comment.