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

Updating upper limit of M1K2 pitch #145

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Conversation

jyotiphy
Copy link
Contributor

Description

Updating soft limits for M1K2 pitch
From Georgi: "The upper limit currently is at -25urad, please change this to 0."

Motivation and Context

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

How Has This Been Tested?

yes, looks good

Where Has This Been Documented?

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

Screenshots (if appropriate):

Pre-merge checklist

  • [x ] Code works interactively
  • Code contains descriptive comments
  • Test suite passes locally
  • [x ] Libraries are set to fixed versions and not Always Newest
  • [x ] Code committed with pre-commit (alternatively pre-commit run --all-files)

@jyotiphy jyotiphy requested a review from nrwslac January 30, 2025 18:05
@@ -1328,7 +1328,7 @@ External Setpoint Generation:
<EncPara ScaleFactorNumerator="0.00256" Offset="-25648.08" MaxCount="#xffffffff">
<FilterTime TPos="0.05"/>
<SoftEndMinControl Enable="true" Range="-350"/>
<SoftEndMaxControl Enable="true" Range="-25"/>
<SoftEndMaxControl Enable="true"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to check on this, I think 0 might actually mean no max. To alleviate any ambiguity, I'd use -0.001. Or test this, have we tried moving to 0.02 with max set to 0?

Copy link
Contributor

@nrwslac nrwslac Jan 30, 2025

Choose a reason for hiding this comment

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

@NSLentz Happen to know the answer to this? I didn't see this called out in the documentation, but I believe setting the Limit Switch to 0 disables it. As seen above, the range parameter was removed.

Copy link

Choose a reason for hiding this comment

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

@nrwslac
No, setting the limit switch to zero just applies a limit at position 0. The Enable="true" would be missing if it was disabled. See below example.

FALSE Looks like:
image
image

TRUE Looks like:
image
image

Back to FALSE, but with setting value to 5:
image

Back to TRUE, keeping value at 5:
image

Back to value of 0, but keeping it TRUE:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for taking a look. Also, Epics limits behavior seems improved. It wont let me exceed max PLC limit pos which is nice.

Copy link
Contributor

@nrwslac nrwslac left a comment

Choose a reason for hiding this comment

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

Tested the motion to 0.0 urad. lgtm!

@jyotiphy jyotiphy merged commit 56a65fe into pcdshub:master Feb 4, 2025
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.

3 participants