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

motor-expert-screen checks for smaract-specific pvs so it works… #174

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

KaushikMalapati
Copy link
Contributor

… with aliases without MCS2 in their prefix

Description

Logic to select which edm screen to use now checks if LSCO_RBV pv is present, which is a smaract specific pv.

Motivation and Context

Some smaract axes have aliases without MCS2 in their prefix, so before they would use the default aerotech edm screen when launched from motor-expert-screen instead of a smaract screen.
https://jira.slac.stanford.edu/browse/ECS-4401

How Has This Been Tested?

Launching motor-expert-screen with a smaract alias pv without mcs2 in its prefix and a smaract pv with mcs2 in its prefix and ensuring it uses a smaract screen instead of an aerotech screen.

Where Has This Been Documented?

A comment in the code has been updated

… with aliases without MCS2 in their prefix
Copy link
Member

Choose a reason for hiding this comment

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

In general this approach is good if it is needed
This file is starting to get awkward though- we're getting upwards of 4 PVs in series, subjecting ourselves to potentially multiple CA timeouts. I have to wonder if a refactor is on the horizon.

edm -x -eolc -m "MOTOR=$PREFIX" mcs2_openloop.edl > /dev/null 2>&1 &
fi
else
cd /reg/neh/home/klg/epics/ioc/common/aerotech/current/motorScreens
Copy link
Contributor

@slactjohnson slactjohnson Feb 23, 2024

Choose a reason for hiding this comment

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

Woah, just noticed this. Why are we running screens out of Karl's (or Mike's) home directory? And why is that the fallback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This showed up in the diff because I nested an existing conditional block. I could add /reg/g/pcds/epics/ioc/common/aerotech/R1.2.0/motorScreens to EDMDATAFILES and use ens_main.edl without changing directories. Is that reasonable?

Copy link
Contributor

Choose a reason for hiding this comment

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

Moving to a released version is best, but that's a issue un-related to your PR. I'm still a little puzzled about the way this script decides to open the aerotech screen. I'll think about this more, and perhaps open a separate issue on this repo.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, this is a problem but it's not a new problem

Copy link
Contributor

Choose a reason for hiding this comment

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

I made a new issue for this here: #175

@ZLLentz
Copy link
Member

ZLLentz commented Feb 27, 2024

I'm going to run this myself a few times and then approve

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.

Code looks good and it works for the random sample of motors I've checked 👍

@KaushikMalapati KaushikMalapati changed the title motor-expert-screen checks for smaract-specific nvram pvs so it works… motor-expert-screen checks for smaract-specific pvs so it works… Feb 27, 2024
@KaushikMalapati KaushikMalapati merged commit f92d08a into pcdshub:master Feb 27, 2024
@KaushikMalapati KaushikMalapati deleted the smaract branch March 15, 2024 21:27
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.

3 participants