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

Multi beamline #129

Merged
merged 2 commits into from
Jun 11, 2024
Merged

Multi beamline #129

merged 2 commits into from
Jun 11, 2024

Conversation

nrwslac
Copy link
Contributor

@nrwslac nrwslac commented Jun 11, 2024

In launcher.py change the beamline argument to a list.

e.g. load beamline K3 for TXI: lucid K3
e.g. load beamline K3 and TXI: lucid K3 TXI

We should actually not use TXI as a beamline?

Tested by launching a screen using lucid K3 and lucid K3 TXI

@nrwslac nrwslac requested review from ZLLentz and tangkong June 11, 2024 16:45
@@ -47,6 +47,30 @@
"invalid": true,
"functional_group": "Vacuum",
"location_group": "S 2"
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know where this came from.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is normal json, the diff just looks a little funny

Copy link
Member

Choose a reason for hiding this comment

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

git decided that this was the new } instead of the trailing one on line 74

Copy link
Contributor

@tangkong tangkong left a comment

Choose a reason for hiding this comment

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

This seems fine in theory, but my naive attempts to test this have produced some truly zany screens
image

How do we intend to support multiple toolbars?

@@ -47,6 +47,30 @@
"invalid": true,
"functional_group": "Vacuum",
"location_group": "S 2"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This is normal json, the diff just looks a little funny

@ZLLentz
Copy link
Member

ZLLentz commented Jun 11, 2024

How do we intend to support multiple toolbars?

In a future version of lucid, the command-line args will go away entirely and everything will be in a config file that will implement both the toolbars and the hutch setup, among other things.

For now, I think it's sufficient to support just one toolbar.

@ZLLentz
Copy link
Member

ZLLentz commented Jun 11, 2024

@tangkong what input did you use to get that output? I usually see that when there are happi entries that don't have location groups defined.

@tangkong
Copy link
Contributor

I guess I naively thought that lucid XCS CXI would give something sensible. That'll teach me to assume

@ZLLentz
Copy link
Member

ZLLentz commented Jun 11, 2024

I don't think it was naïve- lucid should do something smarter when the group designations are missing
added #130

@@ -47,6 +47,30 @@
"invalid": true,
"functional_group": "Vacuum",
"location_group": "S 2"
},
"TV5L0-VGC-1": {
Copy link
Member

Choose a reason for hiding this comment

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

What's up with this extra device in general? What is it used for?

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 is what I meant. I don't know where this device came from, but I guess its part of the demo database so I didn't look too much into it.

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.

I'm 👍 on this change, a little confused on the demo update

@ZLLentz
Copy link
Member

ZLLentz commented Jun 11, 2024

We should actually not use TXI as a beamline?

I agree with this statement, it's too ambiguous
Unless we want to use TXI for everything on both and separate the lines via K3/L1 number in the other field that lightpath uses

@nrwslac nrwslac merged commit 6162e0b into pcdshub:master Jun 11, 2024
11 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.

3 participants