search

bluesky / ophyd

hardware abstraction in Python with an emphasis on EPICS
https://blueskyproject.io/ophyd
BSD 3-Clause "New" or "Revised" License
51 stars 79 forks source link

Remove obsolete PV PoolMaxBuffers #1173

Open xiaoqiangwang opened 11 months ago

xiaoqiangwang commented 11 months ago

This PV was removed from ADCore since 3.3.1, see https://github.com/areaDetector/ADCore/commit/47c70bf10644c63955c00646eaffd881f8a1a9bc

tacaswell commented 10 months ago

This is not the only place we we have this PV set (it is also on the plugins).

We have support for AD versions back to 1.9 iirc. If we are going to start pushing up the minimum version of AD we support then there is a whole bunch more that can be pulled out.

xiaoqiangwang commented 10 months ago

This PV is an information to the configured buffer limit. Removing it is unlikely to break application code. But keeping it does break when connecting to areaDetector 3.4+ IOCs.

1159 addresses a similar issue with areaDetector file plugin.

tacaswell commented 10 months ago

I think we either need to do something like we do with the plugins and have generations of these classes to match the AD versions or put a floor on supported AD versions and simplify a lot of the code.

We are using newer IOCs at NSLS-II. I see a few local patches like this and I suspect in other cases we are setting it to not be in the configuration so we just happen to never connect.

xiaoqiangwang commented 10 months ago

I suspect in other cases we are setting it to not be in the configuration so we just happen to never connect.

Exactly, we find out such broken links only with,

wait_for_connection(all_signals=True)
prjemian commented 10 months ago

Something like these in apstools: https://github.com/BCDA-APS/apstools/blob/93694e7e42ab8e6a9bab96a78cd4a3edbd9b5d7a/apstools/devices/area_detector_support.py#L768-L825

prjemian commented 10 months ago

We've been using the updates in apstools for AreaDetector versions as high as 3.12 (current).

prjemian commented 10 months ago

This is not the only place we we have this PV set (it is also on the plugins).

AFAIK, the V34 area detector plugins in ophyd are working up to the current AD tag {3.12.1).

Instead of this PR (or as branch to this one), I propose hoisting these classes from apstools to ophyd:

xiaoqiangwang commented 8 months ago
  • apstools.devices.area_detector_support.CamMixin_V3_1_1
  • apstools.devices.area_detector_support.CamMixin_V34
  • apstools.devices.area_detector_support.SingleTrigger_V34

After a period of idling I come back to implement the versioned CamBase, but realised one problem. i.e. Unlike NDPlugins, a special camera class e.g. SimDetectorCam has its own version and at the same time derives from a range of CamBase_Vxx. The number of classes (SimDetectorCam_BaseVxx_Vxx) would be the multiplication of both. This becomes quickly intractable.