Skip to content

Small change to avoid Vimba redefinition of the acquire PV - #37

Open
Thomas Hopkins (thopkins32) wants to merge 1 commit into
mainfrom
small-vimba-acquire-change
Open

Thomas Hopkins (thopkins32) wants to merge 1 commit into
mainfrom
small-vimba-acquire-change

Conversation

@thopkins32

Copy link
Copy Markdown
Contributor

Dan Henriksen (@dihenriksen) I think this solution is a bit cleaner than what we settled on yesterday in #36

This is how it is done here: https://github.com/bluesky/ophyd-async/blob/1ce94eb9854a105d4890a50ad33f8c4e5e6c5c9b/src/ophyd_async/epics/adcore/_acquire_logic.py#L26-L33

I would test at the beamline before merging just to be sure.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 20:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The removed Annotated alias is still used by existing annotations and must be restored to avoid a startup failure.

1 open issue
What changed in this PR

This PR simplifies Vimba acquisition handling by using the standard driver instead of redefining the Acquire process variable.

Changes:

  • Removes CustomVimbaDriverIO and uses advimba.VimbaDriverIO directly.
  • Restores the cached acquisition state by waiting for readback without requiring set completion.
  • Updates imports and removes the explicit TmpAdvimba.stats1 annotation.
File Description
startup/​30-area-detectors.py Simplifies the Vimba driver and adjusts acquisition-state restoration.

Review effort: Balanced

@@ -1,17 +1,9 @@
from typing import Annotated as A
from typing import Sequence
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.

2 participants