search

labscript-suite / labscript-devices

A modular and extensible plugin architecture to control experiment hardware using the 𝘭𝘢𝘣𝘴𝘤𝘳𝘪𝘱𝘵 𝘴𝘶𝘪𝘵𝘦.
http://labscriptsuite.org
Other
5 stars 58 forks source link

Port IMAQdxCamera to other backends #34

Closed philipstarkey closed 5 years ago

philipstarkey commented 5 years ago

Original report (archived issue) by David Meyer (Bitbucket: dihm, GitHub: dihm).

So have used a wide variety of cameras with labscript in the past, but have never wanted to spring for the NI Vision Development software necessary to use the universal IMAQdx driver. This has led me to make old-style camera_servers using different, freely available, SDK backends provided by the manufacturers (Pylon, FlyCapture2, AndorSDK3, and PICAM so far). As such, I could fairly easily port the backend of the new IMAQdxCamera class to these other SDKs to open up some other options for camera integration. General thoughts?

I have two specific questions to start with:

I’ve already made a first pass at a port to Basler’s PyPylon. Manual mode operation is tested and functioning well. Still need to test buffered shots but I expect that to be fine since most of the heavy lifting has already been done by @chrisjbillington in IMAQdxCamera. In keeping with the discussion at Issue #22, should this be mainlined or do I send it to our lab’s devices repo instead?

Generally speaking, the IMAQdxCamera class is very easy to subclass when porting over to a new SDK with one glaring exception: the camera properties dictionaries all contain ‘imaqdx’ in their variable names. While I could just use those names and live with the inevitable confusion about why you set imaqdx properties for a PylonCamera, I’d rather not. As such, there is a bunch of unnecessary subclassing, particularly in the blacs_tab and labscript_device, just to rename these variables. Would it be too much to ask to make IMAQdxCamera a little more agnostic? I suppose the ideal would be to make a common parent class that IMAQdx also inherits from…

philipstarkey commented 5 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

Sounds like a good idea! I'll rename them camera_attributes and manual_mode_camera_attributes. Since the IMAQdx camera class should not have any behaviour that isn't general, I think it makes sense to subclass it rather than make a common parent class, or at least, I wouldn't want to go ahead and make the parent class unless it was obviously needed.

And mainlining more camera device support is definitely what I would prefer to it being in lab-specific repositories! There are many people who would appreciate not having to buy the NI Vision SDK.

philipstarkey commented 5 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

Ok, I had a go at making things more subclass friendly in PR #68.

philipstarkey commented 5 years ago

Original comment by David Meyer (Bitbucket: dihm, GitHub: dihm).

Thanks Chris! PR #69 has the Pylon backend port, which depends on PR #68. We mostly use Basler cameras in our lab at the moment so this code will certainly be the best tested with those devices. Once the kinks are worked out I can get the other backends ported over, hopefully without too much issue.

philipstarkey commented 5 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

make IMAQdxCamera friendier to subclassing. Fixes issue #34

Renamed imaqdx_attributes to just camera_attributes. No backward compatibility provided for this, since this class is fairly new.

Allow specifying worker class and camera interface class as class attributes, in the case that the subclass simply changes these and nothing else about worker class creation, or about the instantiation of the interface class.

Remove IMAQdx specific code from worker class, into interface class.

→ \<\<cset 09bc3b6d48596c484ec79d2ac95e001d33701ad0>>