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

Stats2 #146

Merged
merged 4 commits into from
Feb 12, 2025
Merged

Stats2 #146

merged 4 commits into from
Feb 12, 2025

Conversation

KaushikMalapati
Copy link
Contributor

Description

Removing MR1K1:BEND:ENC:PITCH: pvs from FB_RMSWatch

Changed the units for MR1K1:BEND:MMS:PITCH:ENCDIFF:STATS:RMS_RBV, SP1K1:MONO:MMS:M_PI:ENCDIFF:STATS:RMS_RBV and SP1K1:MONO:MMS:G_PI:ENCDIFF:STATS:RMS_RBV to be in nrad instead of urad (I mistakenly said urad in my commit message)

Motivation and Context

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

How Has This Been Tested?

Not tested yet

Where Has This Been Documented?

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)

@jyotiphy
Copy link
Contributor

jyotiphy commented Feb 5, 2025

Looks okay to me! @KaushikMalapati are we removing mean and stdev PVs for M1K1 pitch as well ?
@nrwslac do you agree with removing these PVs ?

@KaushikMalapati
Copy link
Contributor Author

Looks okay to me! @KaushikMalapati are we removing mean and stdev PVs for M1K1 pitch as well?

They are still being calculated by fbMR1K1PiPosDiffStats in FB_Stats

@KaushikMalapati
Copy link
Contributor Author

This is running and seems to be working

@@ -68,7 +68,7 @@ tonNewGpiMove(
PT:=T#15s,
);

fMpiEncoderPosDiff := M6.nEncoderCount - (M6.Axis.NcToPlc.SetPos - M6.stAxisParameters.fEncOffset) / 0.004505;
fMpiEncoderPosDiff := ((UDINT_TO_LREAL(M6.nEncoderCount) * 0.004505) - (M6.Axis.NcToPlc.SetPos - M6.stAxisParameters.fEncOffset)) * 1000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we scaling the diff value by 1000? @KaushikMalapati

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Georgi asked for units of nanoradians for convenience

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, that checks out, I see scaled to user from encoder counts minus set position = urad * 1000 = nrad.

nrwslac
nrwslac previously approved these changes Feb 11, 2025
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.

Encoder scaling lgtm!

@KaushikMalapati
Copy link
Contributor Author

@nrwslac could you approve again? I just released a new version of ads-ioc that archives another motor record field I want to use in a grafana alert

@KaushikMalapati KaushikMalapati merged commit e54ab3e into pcdshub:master Feb 12, 2025
9 checks passed
@KaushikMalapati KaushikMalapati deleted the stats2 branch February 12, 2025 01:12
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