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

Custom EPS screens #1195

Merged
merged 7 commits into from
Jan 30, 2024
Merged

Custom EPS screens #1195

merged 7 commits into from
Jan 30, 2024

Conversation

jozamudi
Copy link
Contributor

@jozamudi jozamudi commented Jan 25, 2024

Description

Added BeckhofAxisEPSCustom class in epics_motor.py and BeckhofAxisEpicsCusom.ui to allow the use of a custom screen to show EPS information.

Motivation and Context

We want to be able to protect motion stages from running into each other.
pcdshub/lcls-plc-crixs-motion#71

we want to be able to display this EPS information on they typhos screen to make accessible for the users.

https://jira.slac.stanford.edu/browse/ECS-3966

How Has This Been Tested?

Used typhos 'pcdsdevices.epics_motor.BeckhoffAxisEPSCustom' to open typhos screen

Where Has This Been Documented?

EPS Screens can be placed here.
reg/g/pcds/epicsdev/screens/pydm/eps_screens/${beamline}/${name}

image

Pre-merge checklist

  • Code works interactively
  • Code contains descriptive docstrings, including context and API
  • New/changed functions and methods are covered in the test suite where possible
  • Test suite passes locally
  • Test suite passes on GitHub Actions
  • Ran docs/pre-release-notes.sh and created a pre-release documentation page
  • Pre-release docs include context, functional descriptions, and contributors as appropriate

@jozamudi jozamudi requested a review from ZLLentz January 25, 2024 19:26
@ZLLentz
Copy link
Member

ZLLentz commented Jan 26, 2024

I think this is a good idea, but before we merge I think we need to:

  • finish the pre-merge checklist items
  • add a screenshot of a screen in-use rather than with a "path not found" error for documentation purposes

More thoughts/questions:

@jozamudi
Copy link
Contributor Author

@ZLLentz The standard path was documented in the BeckhofAxisEPSCustom class declaration. I added the path in the PR. Is there a better path we can use?

I don't think there is a major reason why we could not use the eps_widget more directly. The only reason would be that some information might not be relevant to a motor that does not have those eps conditions implemented in the PLC.

Copy link
Member

@ZLLentz ZLLentz left a comment

Choose a reason for hiding this comment

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

Sorry for my late reply- just small things to adjust here

@ZLLentz
Copy link
Member

ZLLentz commented Jan 30, 2024

This fell off my radar for two reasons: the github notification interface dropped the mention of "review requested", and I got distracted with Monday things yesterday. I'm not sure why github demoted my involvement here from "review requested" to "mentioned", but it made my brain skip over this in the notifications list in favor of the other tickets... my bad.

jozamudi and others added 3 commits January 30, 2024 09:43
…stom_class_in_epics_motor.py_and_BeckhofAxisEpicsCusom.ui_to_allow_the_use_of_a_custom_screen_to_show_EPS_information..rst

Co-authored-by: Zachary Lentz <[email protected]>
@jozamudi jozamudi merged commit 6fa68ba into pcdshub:master Jan 30, 2024
9 checks passed
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