Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eiger exts logic #54

Merged
merged 33 commits into from
Jul 24, 2023
Merged

Eiger exts logic #54

merged 33 commits into from
Jul 24, 2023

Conversation

abbiemery
Copy link
Collaborator

@abbiemery abbiemery commented Jun 16, 2023

Fixes #53

Changes:

  • Refactor Eiger detector logic for improved readability
  • Convert some de/serialization from apischema to pydantic, more work to be done here in Move Eiger internals from apischema to pydantic #66
  • Improve eiger tests, including system tests
  • Add and test EXTS mode support

src/tickit_devices/eiger/eiger.py Show resolved Hide resolved
src/tickit_devices/eiger/eiger.py Show resolved Hide resolved
src/tickit_devices/eiger/eiger.py Show resolved Hide resolved
src/tickit_devices/eiger/eiger.py Show resolved Hide resolved
src/tickit_devices/eiger/eiger.py Show resolved Hide resolved
src/tickit_devices/eiger/eiger.py Show resolved Hide resolved

def __getitem__(self, key: str) -> Any: # noqa: D105
f = {}
for field_ in fields(self):
f[field_.name] = vars(self)[field_.name]
f[field_.name] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we cache this value?

Or else calculate it without the intermediate object?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#67

src/tickit_devices/eiger/stream/eiger_stream.py Outdated Show resolved Hide resolved
device: EigerStream

@HTTPEndpoint.get(f"/{STREAM_API}" + "/status/{param}")
async def get_stream_status(self, request: web.Request) -> web.Response:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we no longer exposing this endpoint?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was moved into eiger_adapters.py. I replaced multiple inheritance with one big class.

pyproject.toml Outdated Show resolved Hide resolved
Comment on lines +7 to +10
from apischema import serialized
from apischema.fields import with_fields_set
from apischema.metadata import skip
from apischema.serialization import serialize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apischema? I'll look into whether this can be pulled out easily

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have pulled it out

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, pulling this out proved to be more complicated than expected and caused upstream issues with the IOC. It will require a proper look to debug and test those differences. For now I have reset and left this internal use of apischema in. WIP on pulling out here: #66

src/tickit_devices/eiger/eiger_schema.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Eiger: review "eiger-exts-logic" changes
3 participants