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.

Sign up for GitHub

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

Jump to bottom

Optics #4

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Optics #4

wants to merge 19 commits into from

Conversation

MDecarabas
Copy link
Collaborator

@MDecarabas MDecarabas self-assigned this Dec 19, 2024
@MDecarabas MDecarabas added the enhancement New feature or request label Dec 19, 2024
prjemian
prjemian reviewed Dec 19, 2024
View reviewed changes
src/instrument/devices/mct_optics.py

cross_select = Cpt(EpicsSignal, "CrossSelect")
sync = Cpt(EpicsSignal, "Sync", string="true")
server_running = Cpt(EpicsSignal, "ServerRunning", string="true")
Copy link

Choose a reason for hiding this comment

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

perhaps kind="config"?

@prjemian
Copy link

This code covers the connection of Python objects with EPICS PVs. Code reduction is substantial. Good job!

What's the intention for the additional code in the original file (the code that operates the optics)? syn_status(), crop_detector(), sync_binning_select(), ... To me, these look like they should be implemented as plan stubs.

prjemian
prjemian approved these changes Dec 19, 2024
View reviewed changes
Copy link

@prjemian prjemian left a comment

Choose a reason for hiding this comment

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

Ok by me.

@prjemian
Copy link

Attend to the merge conflicts

@MDecarabas
Copy link
Collaborator Author

Hey @prjemian, I will let @decarlof play with the branch as is for now. Then will resolve conflicts and merge into main once he has some time experiencing it.

@MDecarabas
Copy link
Collaborator Author

This code covers the connection of Python objects with EPICS PVs. Code reduction is substantial. Good job!

What's the intention for the additional code in the original file (the code that operates the optics)? syn_status(), crop_detector(), sync_binning_select(), ... To me, these look like they should be implemented as plan stubs.

We have decided that the logic lives with the python server for now. This repo just provides a bluesky interface

@MDecarabas MDecarabas requested a review from decarlof December 20, 2024 15:30
@MDecarabas MDecarabas marked this pull request as ready for review December 20, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prjemian prjemian prjemian approved these changes

@ravescovi ravescovi Awaiting requested review from ravescovi

@decarlof decarlof Awaiting requested review from decarlof

Assignees

@MDecarabas MDecarabas

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

create mct Optics device for 32ID
2 participants
@MDecarabas @prjemian