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

More user functions and improvements #94

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

ambarb
Copy link
Contributor

@ambarb ambarb commented Oct 23, 2024

  • Added docstrings to common functions
  • Added smarter inout "in" and "out" function because the device actuates even if it is already in requested position. the ophyd device should be fixed instead so it has this default behavior when using mv(inout, "In") or mv(inout, "Out"). Unlikely the EPS programming will be fixed anytime soon.
  • removed unused channesl from fccd.mcs.wfm. tested in the field and it works.

Copy link
Contributor

@maffettone maffettone 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, minor follow up on sleep, and might as well add the docs to the new functions. Won't let that block approval.

@@ -59,7 +69,7 @@ def pol_L(pol, epu_cal_offset=None, epu_table_number=None):


yield from bps.mv(diag6_pid.enable,1) # finepitch feedback ON
yield from bps.sleep(1)
yield from bps.sleep(0.1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sleeps frighten me, although less so to see an existing one reduced, is there something this is waiting on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can reduce the time because we are able to run the diag6 prosilica much faster. diag6_pid is the pid loop that keeps the m1 finepitch under constant control to keep the beam on the exit slit. There could be some instability in the beginning of the feedback loop when we turn it back on. The sleep is to allow that instability to happen without letting the user collect data. the daig6_pid knows nothing about the output error that is running at dt = 0.005 seconds or faster.

@@ -166,3 +184,19 @@ def wait_for_peaks(pool_interval, timeout, peaks, peaks_fields=None):
break
else:
yield from bps.sleep(pool_interval)

def _block_beam(block_beam_bit): ##TODO move this to a child of inout
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def _block_beam(block_beam_bit): ##TODO move this to a child of inout
def _block_beam(block_beam_bit): ##TODO move this to a child of inout
"""Protective inout function that only moves if necessary. The EPS will always actuate on bps.mv.
Parameters:
------------
block_beam_bit : int, bool
"""

if inout.status.get() =="Inserted":
yield from mv(inout, "Out")

def block_beam():
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def block_beam():
def block_beam():
"""Helper plan to move inout to block beam. Aliased to beam_block()."""

Choose a reason for hiding this comment

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

The alias info might be difficult to keep consistent with future changes. Consider moving that info to a comment where the aliases are defined.

Suggested change
def block_beam():
def block_beam():
"""Helper plan to move inout to block beam."""


def block_beam():
yield from _block_beam(1)
def show_beam():
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def show_beam():
def show_beam():
"""Helper plan to move inout to show beam. Aliased to beam_show()."""

Choose a reason for hiding this comment

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

Suggested change
def show_beam():
def show_beam():
"""Helper plan to move inout to show beam."""

Copy link

@padraic-shafer padraic-shafer left a comment

Choose a reason for hiding this comment

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

A few typographical recommendations.

@@ -34,7 +34,17 @@ def ct_dark_all_patch(frames=None):


def pol_L(pol, epu_cal_offset=None, epu_table_number=None):

"""A helper bluesky plan to change vertical and horizontal poloarization

Choose a reason for hiding this comment

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

Suggested change
"""A helper bluesky plan to change vertical and horizontal poloarization
"""A helper bluesky plan to change vertical and horizontal polarization

pol : string
'H' or 'V' for horizontal (epu phase = 0) or vertical (epu phase = 24.6) polarization for CSX's undulator
epu_cal_offset : float
offset to tune the precice epu gap for a paricular energy using a particular epu calibration table

Choose a reason for hiding this comment

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

Suggested change
offset to tune the precice epu gap for a paricular energy using a particular epu calibration table
offset to tune the precise epu gap for a particular energy using a particular epu calibration table

Comment on lines +189 to +194
if block_beam_bit:
if inout.status.get() =="Not Inserted":
yield from mv(inout, "In")
else:
if inout.status.get() =="Inserted":
yield from mv(inout, "Out")

Choose a reason for hiding this comment

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

Suggested change
if block_beam_bit:
if inout.status.get() =="Not Inserted":
yield from mv(inout, "In")
else:
if inout.status.get() =="Inserted":
yield from mv(inout, "Out")
if block_beam_bit:
if inout.status.get() == "Not Inserted":
yield from mv(inout, "In")
else:
if inout.status.get() == "Inserted":
yield from mv(inout, "Out")

if inout.status.get() =="Inserted":
yield from mv(inout, "Out")

def block_beam():

Choose a reason for hiding this comment

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

The alias info might be difficult to keep consistent with future changes. Consider moving that info to a comment where the aliases are defined.

Suggested change
def block_beam():
def block_beam():
"""Helper plan to move inout to block beam."""


def block_beam():
yield from _block_beam(1)
def show_beam():

Choose a reason for hiding this comment

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

Suggested change
def show_beam():
def show_beam():
"""Helper plan to move inout to show beam."""

Comment on lines +201 to +202
beam_block = block_beam
beam_show = show_beam

Choose a reason for hiding this comment

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

Suggested change
beam_block = block_beam
beam_show = show_beam
# Convenience aliases: Don't need to memorize order of noun/verb
beam_block = block_beam
beam_show = show_beam

def relabel_fig(fig, new_label):
fig.set_label(new_label)
fig.canvas.manager.set_window_title(fig.get_label())

# fccd.hints = {'fields': ['fccd_stats1_total']}
for i in [1, 2, 3, 4, 5]:
getattr(fccd, f'stats{i}').total.kind = 'hinted'
fccd.mcs.read_attrs = fccd.mcs.read_attrs[0:7] #silences the channels we do not use (7-32)

Choose a reason for hiding this comment

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

Pre-line comments are generally easier to read when they are more than 1-2 words.

Suggested change
fccd.mcs.read_attrs = fccd.mcs.read_attrs[0:7] #silences the channels we do not use (7-32)
# Silence the channels we do not use (7-32)
fccd.mcs.read_attrs = fccd.mcs.read_attrs[0:7]

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