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

ADTimePix3 detector ophyd device classes #1069

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

Conversation

kgofron
Copy link

@kgofron kgofron commented Sep 7, 2022

No description provided.

Comment on lines +1325 to +1328
chip0 = TimePix3Chip(0)
chip1 = TimePix3Chip(1)
chip2 = TimePix3Chip(2)
chip3 = TimePix3Chip(3)
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
chip0 = TimePix3Chip(0)
chip1 = TimePix3Chip(1)
chip2 = TimePix3Chip(2)
chip3 = TimePix3Chip(3)
chip0 = Cpt(TimePix3Chip, 0)
chip1 = Cpt(TimePix3Chip, 1)
chip2 = Cpt(TimePix3Chip, 2)
chip3 = Cpt(TimePix3Chip, 3)

Copy link
Contributor

Choose a reason for hiding this comment

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

These need to be components or the TimePix3Chip instances will be instantiated on the class instance and shared across all instances of the device (and also miss getting the PV information).

@@ -1289,6 +1290,171 @@ class RoperDetectorCam(CamBase):
roper_shutter_mode = ADCpt(SignalWithRBV, "RoperShutterMode")


class TimePix3Chip(Device):

def __init__(self, chip_number):
Copy link
Contributor

Choose a reason for hiding this comment

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

The components need to be at the class level. I think you want to use FormattedComponents

ophyd/ophyd/device.py

Lines 369 to 396 in 75a35fd

class FormattedComponent(Component[K]):
"""A Component which takes a dynamic format string
This differs from Component in that the parent prefix is not automatically
added onto the Component suffix. Additionally, `str.format()` style strings
are accepted, allowing access to Device instance attributes:
>>> from ophyd import (Component as Cpt, FormattedComponent as FCpt)
>>> class MyDevice(Device):
... # A normal component, where 'suffix' is added to prefix verbatim
... cpt = Cpt(EpicsSignal, 'suffix')
... # A formatted component, where 'self' refers to the Device instance
... ch = FCpt(EpicsSignal, '{self.prefix}{self._ch_name}')
... # A formatted component, where 'self' is assumed
... ch = FCpt(EpicsSignal, '{prefix}{_ch_name}')
...
... def __init__(self, prefix, ch_name=None, **kwargs):
... self._ch_name = ch_name
... super().__init__(prefix, **kwargs)
>>> dev = MyDevice('prefix:', ch_name='some_channel', name='dev')
>>> print(dev.cpt.pvname)
prefix:suffix
>>> print(dev.ch.pvname)
prefix:some_channel
For additional documentation, refer to Component.
"""
or DynamicDeviceComponent

ophyd/ophyd/device.py

Lines 407 to 437 in 75a35fd

class DynamicDeviceComponent(Component["Device"]):
"""An Device component that dynamically creates an ophyd Device
Parameters
----------
defn : OrderedDict
The definition of all attributes to be created, in the form of::
defn['attribute_name'] = (SignalClass, pv_suffix, keyword_arg_dict)
This will create an attribute on the sub-device of type `SignalClass`,
with a suffix of pv_suffix, which looks something like this::
parent.sub.attribute_name = Cpt(SignalClass, pv_suffix, **keyword_arg_dict)
Keep in mind that this is actually done in the metaclass creation, and
not exactly as written above.
clsname : str, optional
The name of the class to be generated
This defaults to {parent_name}{this_attribute_name.capitalize()}
doc : str, optional
The docstring to put on the dynamically generated class
default_read_attrs : list, optional
A class attribute to put on the dynamically generated class
default_configuration_attrs : list, optional
A class attribute to put on the dynamically generated class
component_class : class, optional
Defaults to Component
base_class : class, optional
Defaults to Device
"""
here.

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.

2 participants