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

Made panda use pva signal backend instead of pvi_get #43

Merged
merged 11 commits into from
Mar 7, 2024

Conversation

rosesyrett
Copy link
Collaborator

@rosesyrett rosesyrett commented Oct 31, 2023

Make use of PVASignalBackend standard Backend

@rosesyrett rosesyrett marked this pull request as draft October 31, 2023 12:12
@DiamondJoseph
Copy link
Contributor

Still need, but do not need to get the initial state, just get the pvi.

@DiamondJoseph DiamondJoseph force-pushed the make-panda-use-backend branch 3 times, most recently from eb19e25 to 1aea92c Compare February 14, 2024 16:01
@DiamondJoseph DiamondJoseph marked this pull request as ready for review February 14, 2024 16:22
@DiamondJoseph
Copy link
Contributor

@coretl: just checking that I am going in the right direction with this change? I've gotten to the point that the new code looks correct as far as I understand, but the pva fixture isn't playing nicely (timing out tests, not letting the test suite complete). It looks like INCOMPLETE_BLOCK_RECORD, INCOMPLETE_RECORD ,EXTRA_BLOCKS_RECORD don't actually exist in tests/panda/db?

@DiamondJoseph
Copy link
Contributor

DiamondJoseph commented Feb 14, 2024

Also: is there a standard for "FOO" + ":BAR" vs. "FOO:" + "BAR:" ? The latter seems to be more common in dodal, so I've been using it, assuming the trailing colon won't cause issue.

I see in Rose's documentation change she also enforced that prefix should end with : if it isn't already
https://github.com/bluesky/ophyd-async/pull/107/files#diff-fbf444aec05ace8f7baf32ccb7fe37cbb7bb988607c665bb235f89d06dfa1c49

@coretl
Copy link
Collaborator

coretl commented Feb 16, 2024

Also: is there a standard for "FOO" + ":BAR" vs. "FOO:" + "BAR:" ? The latter seems to be more common in dodal, so I've been using it, assuming the trailing colon won't cause issue.

I see in Rose's documentation change she also enforced that prefix should end with : if it isn't already https://github.com/bluesky/ophyd-async/pull/107/files#diff-fbf444aec05ace8f7baf32ccb7fe37cbb7bb988607c665bb235f89d06dfa1c49

DLS convention used to be no colon in the prefix, then colon added in the database. This only work because DLS PVs look like BLxxI-MY-DEVICE-01:RECORD, it doesn't work for all facilities that might use a different separator. Much better is to include the colon in the prefix which is what Rose's change did

@DiamondJoseph DiamondJoseph force-pushed the make-panda-use-backend branch 5 times, most recently from b5f7a10 to 76c4cd5 Compare February 16, 2024 15:14
@DiamondJoseph DiamondJoseph force-pushed the make-panda-use-backend branch 3 times, most recently from 5003241 to 348438f Compare February 16, 2024 16:22
@DiamondJoseph DiamondJoseph force-pushed the make-panda-use-backend branch 3 times, most recently from 48c1f8b to 05a4d85 Compare February 28, 2024 13:05
tests/epics/test_signals.py Outdated Show resolved Hide resolved
@DiamondJoseph
Copy link
Contributor

Test for getting the PVI info as a PVADict currently times out. I don't know enough about Epics DB Records to diagnose

DiamondJoseph and others added 10 commits March 5, 2024 10:20
Copy link
Contributor

@DiamondJoseph DiamondJoseph left a comment

Choose a reason for hiding this comment

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

Tom should Approve

src/ophyd_async/epics/_backend/_p4p.py Show resolved Hide resolved
tests/epics/test_signals.py Outdated Show resolved Hide resolved
tests/epics/test_signals.py Outdated Show resolved Hide resolved
@evalott100 evalott100 force-pushed the make-panda-use-backend branch 2 times, most recently from c56f4da to 289b3fe Compare March 5, 2024 14:57
@evalott100 evalott100 requested a review from coretl March 6, 2024 09:00
@evalott100 evalott100 self-assigned this Mar 6, 2024
@evalott100 evalott100 merged commit c42afcb into main Mar 7, 2024
14 checks passed
@evalott100 evalott100 deleted the make-panda-use-backend branch March 7, 2024 10:48
@evalott100 evalott100 restored the make-panda-use-backend branch March 7, 2024 11:07
@evalott100 evalott100 deleted the make-panda-use-backend branch March 7, 2024 11:13
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.

None yet

5 participants