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

gas needle with different motors and encoders installed #4

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

tongju12
Copy link
Contributor

@tongju12 tongju12 commented Jun 5, 2024

TMO swap the motors for gas needle by smaller size and less current.
Meanwhile, adding encoder into every axe.

Tune the parameter, link to encoder, rewrite code in main.

Motivation and Context

Request from scientists in new MRCO chamber

How Has This Been Tested?

Yes, I tested with scientists to setup soft limits and move to the whole range for every axe.

Where Has This Been Documented?

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

Screenshots (if appropriate):

Pre-merge checklist

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

@tongju12 tongju12 requested review from ZLLentz and NSLentz June 5, 2024 22:58
@tongju12
Copy link
Contributor Author

tongju12 commented Jun 5, 2024

Sorry, forgot the pre-commit run. should do that.

@ZLLentz
Copy link
Member

ZLLentz commented Jun 6, 2024

Tong, were you going to run pre-commit and push back here? I was going to review but was going to wait if you had more commits to push.

@tongju12
Copy link
Contributor Author

tongju12 commented Jun 6, 2024

Yes, Zach. Sorry. I am busy with Sample paddle X, Y, Z test today and it is almost done except Y motor has strange noise. I will push again with pre-commit run next week. Thanks!

@tongju12
Copy link
Contributor Author

tongju12 commented Jun 7, 2024

Zach, I ran pre-commit before second PR and hope it is ok for you?

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.

Looks good to me overall! I'm hitting approve!

Small nitpick: you have a few commented out sections that should probably be deleted
It looks like they completely changed the hardware here
I didn't notice anything strange in the diff

Comment on lines -1331 to +1335
<Encoder Name="Enc" EncType="4">
<EncPara ScaleFactorNumerator="9.921875e-05" MaxCount="#x0000ffff">
<SoftEndMinControl Enable="true" Range="-100"/>
<SoftEndMaxControl Enable="true" Range="100"/>
<Inc RefMode="2" RefSoftSyncMask="#x0000ffff"/>
<ParameterChanged>8</ParameterChanged>
<Encoder Name="Enc" EncType="19">
<EncPara ScaleFactorNumerator="5e-05" MaxCount="#xffffffff">
<SoftEndMinControl Enable="true" Range="31"/>
<SoftEndMaxControl Enable="true" Range="52"/>
<Inc RefSoftSyncMask="#x0000ffff"/>
Copy link
Member

Choose a reason for hiding this comment

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

Wow, the stages changed a lot!

@tongju12 tongju12 merged commit 0df951c into pcdshub:master Jun 7, 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