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

show how to connect energy from local controls #41

Merged
merged 14 commits into from
Oct 3, 2020
Merged

Conversation

prjemian
Copy link
Contributor

@prjemian prjemian commented Oct 1, 2020

fixes #25, fixes #26, fixes #42

@prjemian prjemian added this to the 0.3.15 milestone Oct 1, 2020
@prjemian prjemian self-assigned this Oct 1, 2020
@prjemian
Copy link
Contributor Author

prjemian commented Oct 1, 2020

@ambarb, @gfabbris: If you have time, can you look over the documentation added here (in the changes) and see if it answers the requests in #25 & #26. Seems a bit wordy to me.

@ambarb
Copy link

ambarb commented Oct 2, 2020

@prjemian I can this afternoon. Hope that is ok

Copy link
Member

@mrakitin mrakitin 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. I left a few sphinx-related and style comments.

doc/source/diffract.rst Outdated Show resolved Hide resolved
doc/source/diffract.rst Outdated Show resolved Hide resolved
doc/source/diffract.rst Show resolved Hide resolved
doc/source/diffract.rst Outdated Show resolved Hide resolved
@prjemian prjemian marked this pull request as ready for review October 2, 2020 15:23
Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Thanks, Pete!

@gfabbris
Copy link

gfabbris commented Oct 2, 2020

otherwise looks good to me.

@ambarb
Copy link

ambarb commented Oct 2, 2020

If optics:energy_locked is 0 (do not update calc engine), and someone wants to update the calc engine's energy manually, is there a way to do this with the energy_offset applied? I don't think this is part of the commit so I will continue to look, but maybe this is something we missed the first time around.

Otherwise the wording of things is clear. I particularly appreciate you explaining the sign convention you employed for energy_offset. I looked for the math first, so maybe move that explanation before the code block???

It might be, especially for the simulation example, nice to have URL link(s) on how to setup orientation matrix for convenience.

@prjemian
Copy link
Contributor Author

prjemian commented Oct 2, 2020

@ambarb: It is possible to update the calc engine's energy manually. Need to do the various steps in the _energy_changed() method:

If you know the numbers:

diffractometer.calc.energy = (energy_eV + offset_eV) * 0.001

If you use the signals (and know the signals are in eV:

diffractometer.calc.energy = (diffractometer.energy.get()+ diffractometer.energy_offset.get()) * 0.001

...

I'll add this and your other suggestions.

@prjemian
Copy link
Contributor Author

prjemian commented Oct 2, 2020

slightly easier way is:

  1. %mov diffractometer.energy_update_calc 1
  2. diffractometer._energy_changed(diffractometer.energy.get())
  3. %mov diffractometer.energy_update_calc 0

@prjemian
Copy link
Contributor Author

prjemian commented Oct 2, 2020

BUT, setting diffractometer.calc.energy should appear before diffractometer.energy since the former is the standard, the latter is a convenience. That can be stated in the first example.

@ambarb
Copy link

ambarb commented Oct 2, 2020

@prjemian regarding #41 (comment)

This is basically what we do now, but nicer in that we reduce chance of a mistake. I ask if we can make the method for both locked and unlocked the same so as to have more of a consistent user experience. Maybe this is an enhancement?

@prjemian
Copy link
Contributor Author

prjemian commented Oct 2, 2020

Can do.

@prjemian prjemian merged commit 38dc924 into main Oct 3, 2020
@prjemian prjemian deleted the 25-26-local-energy branch October 3, 2020 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants