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

Auto recover #80

Merged
merged 9 commits into from
Sep 2, 2021
Merged

Auto recover #80

merged 9 commits into from
Sep 2, 2021

Conversation

ghalym
Copy link
Collaborator

@ghalym ghalym commented Aug 26, 2021

Adding the FIX to GCC State change

@ghalym ghalym requested a review from jyin999 August 26, 2021 15:52
Copy link
Contributor

@klauer klauer left a comment

Choose a reason for hiding this comment

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

Looks like LineIDs.dbg snuck into the repository in this PR.

(Feel free to ignore my sideline comments - was just looking this over during the eng gen.)

IG.eState := ValidLo; //LO
ELSIF rV > 9.6 AND rV<= 9.9 THEN
ELSIF rV > 9.6 AND rV<= 9.9 AND (IG.xHV_SW) THEN
IG.eState := ValidHi; //HIGH
ELSIF rV < 0.18 THEN //
Copy link
Contributor

@klauer klauer Aug 30, 2021

Choose a reason for hiding this comment

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

Probably a dumb question as I'm drive-by reviewing for a project I have nothing to do with, but should this include: AND Ig.xHV_SW like the others?
(If so, it might be a bit more readable to do a top-level IF xHV_SW THEN (check voltages) ELSE set_off_state)

Copy link
Contributor

Choose a reason for hiding this comment

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

is it missing the range rv< 0.6 and rv>0.22? which should be valid or valid low?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should these values be local constants? Avoiding hardcoded values?

ghalym added 5 commits August 31, 2021 14:42
…and the GCC. The timer is default but can be set from the user program. AutoOn pv is available through epics to switch this function off. The Autorecover is set to always run unless a value has been written to that persistent variable. Still to be tested.
@jyin999 jyin999 merged commit 26e2b9a into pcdshub:master Sep 2, 2021
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.

4 participants