-
Notifications
You must be signed in to change notification settings - Fork 30
Tango improved descriptor #865
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
base: main
Are you sure you want to change the base?
Conversation
…could probably be done more efficiently
…n_type now returns 'boolean' as its json type for bools instead of 'integer'
…hen run locally and when run on CI. This change is what CI wants but it violates my local pre-commit.
… they have values which evaluate as True
if config.cmd_name: | ||
descriptor["object_name"] = config.cmd_name | ||
|
||
return DataKey(**descriptor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering if you could make any use of
ophyd-async/src/ophyd_async/core/_signal_backend.py
Lines 180 to 195 in 90209e1
def make_datakey( | |
datatype: type[SignalDatatypeT], | |
value: SignalDatatypeT, | |
source: str, | |
metadata: SignalMetadata, | |
) -> DataKey: | |
"""Make a DataKey for a given datatype.""" | |
dtn = _datakey_dtype_numpy(datatype, value) | |
return DataKey( | |
dtype=_datakey_dtype(datatype), | |
shape=_datakey_shape(value), | |
# Ignore until https://github.com/bluesky/event-model/issues/308 | |
dtype_numpy=dtn.descr if len(dtn.descr) > 1 else dtn.str, # type: ignore | |
source=source, | |
**metadata, | |
) |
Like we do for EPICS:
ophyd-async/src/ophyd_async/epics/core/_p4p.py
Lines 391 to 396 in 90209e1
async def get_datakey(self, source: str) -> DataKey: | |
value = await context().get(self.read_pv) | |
metadata = _metadata_from_value(self.converter.datatype, value) | |
return make_datakey( | |
self.converter.datatype, self.converter.value(value), source, metadata | |
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this make the datakey from the signal itself? Is this because an EPICS PV does not easily provide this information? It doesn't look like this enforces that the signal datatype match the PV. With Tango, the descriptor can simply return information from the device server and check that the type matches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It uses the signal value for 2 reasons:
- To get the dtype, both the loose json form ('integer`) and the numpy datatype ('<i8')
- To get the shape, needed especially for waveforms
The first could be got from the EPICS metadata, but the second requires the current value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coretl I am trying to use this now. Is make_datakey only intended to be used for attributes? Commands of course have the None
datatype and value which make_datakey can't handle.
…g required event_model version to 1.22.3 which includes keys for Tango RDS
It is better to provide no dims rather than dummy place holder dims. The current primary consumer of this is generating xarrays from the collected data which can leverage dims if they exist (which in turn helps make sure that array operations are correctly aligned rather than blindly zipping by dimension index). If dims aren't provided xarray will auto-generate them at the end. Providing dummy dims reduces the value of intentionally provided ones. |
Thanks @tacaswell, I removed the dummy dims. Tango devices won't have dims in the descriptor by default. |
This improves the descriptor returned by Tango devices. If specified in the device server, the DataKey will now include limits including the Tango RDS range, choices, dims, precision, and units.
An important note here is that we are still treating Tango commands with input/output types as SignalRW instead of SignalX. Tango commands have limited configuration information so their DataKeys can only carry the keys object_name, source, dtype, and shape. Applications using the document stream must be aware of this and not try to, for example, read the units of a SignalRW backed by a Tango command.