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

Shorten scan setpoints #126

Merged
merged 5 commits into from
Sep 6, 2021
Merged

Shorten scan setpoints #126

merged 5 commits into from
Sep 6, 2021

Conversation

wnichols1
Copy link
Contributor

this alters what scan-setpoints does, and also setting num stages to 2, and dwell time to 500.

@wnichols1 wnichols1 requested a review from ajgdls June 8, 2021 21:25
@wnichols1
Copy link
Contributor Author

@coveralls
Copy link

coveralls commented Jun 8, 2021

Coverage Status

Coverage remained the same at 55.753% when pulling e02884d on shorten-scan-setpoints into 880ae1a on master.

@wnichols1
Copy link
Contributor Author

There are some tidying commits at the start of this, before the two commits which alter things.

@prcvlusr
Copy link
Collaborator

A) regarding rename odin config files: it is OK
B) regarding set num scan-stages to 2, and dwell to 500ms. Add apply-setpoints.:

  1. I do not have problem with changing the ramp from 4 steps, 2s to 2 steps, 0.5sec
  2. but please do not change:
  • DESY/W3C3/user_scripts/TEST_BEFORE_POWERUP.sh
  • DESY/W3C3/user_scripts/TEST_slow_POWERUP_000_onlyVDDs.sh
    these are debugging scripts.
  1. regarding minimize the action of scan_set_points
    it is important that we retain a version of the scan_set_points command that acts as it used to do (i.e. it applies all the setting from the start).
    It does not need to have the same command name, but we need to be able, in any point, to apply a scan_set_points-as it was-before.
    this is required for debugging or emergency actions (e.g. a ramp fails and you end in the middle of two stages)
    I do not understand from the code how can we achieve this.

@wnichols1
Copy link
Contributor Author

I think the command you are looking for is percival-hl-apply-setpoint.
and you will see things like this in the new scripts:
percival-hl-apply-setpoint -s 00_0_0V0A

I will revert (2)

@wnichols1 wnichols1 requested a review from prcvlusr July 19, 2021 12:14
@wnichols1
Copy link
Contributor Author

I am going to suggest that we delete these scripts in (2) from github because I think
i) they are easily recreated, being mostly copies of other things
ii) they are not generally useful and I doubt anyone will use them except Alessandro
iii) It's unclear what they are meant to do

is that ok?

@prcvlusr prcvlusr closed this Jul 26, 2021
@wnichols1
Copy link
Contributor Author

closed accidentally; reopening!

@wnichols1 wnichols1 reopened this Jul 26, 2021
@wnichols1 wnichols1 requested a review from lstebel July 27, 2021 10:42
@lstebel lstebel merged commit 695e0cf into master Sep 6, 2021
@wnichols1
Copy link
Contributor Author

I think there is a difference between MERGING a branch and APPROVING a branch.
I wanted approval from agreer before merging, but he can do this retrospectively.

@ajgdls
Copy link
Contributor

ajgdls commented Sep 6, 2021

I don't think I can approve it now by clicking on any buttons, so you can consider this comment as my approval @wnichols1

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.

None yet

5 participants