micro-manager / mmCoreAndDevices

Micro-Manager's device control layer, written in C++
40 stars 105 forks source link

Improve interface for DA/AD devices #35

Open nicost opened 3 years ago

nicost commented 3 years ago

From a small group email thread May 2020. Suggestions from Jon Daniels that make a lot of sense to me:

I like the idea of breaking the interface/device into 2 separate device types, one for analog input and the other for analog output. In the ASITiger device adapter we have both input and output devices.

Instead of simply converting the existing SignalIO interface to output-only, maybe it would be better to create two new device types and keep the existing one as-is for backward compatibility? Then device adapters which are actively maintained could be converted to use the new device types and the old device type marked as deprecated.

I also concur with Nico's suggestion of renaming things but I suggest slightly different names. How about AOSignal for analog output and then AISignal for analog input? That matches with the NI convention of AO and AI, and also specifies that it's an analog signal. I think it would be best to avoid "DA" in the method names; people with electronics background immediately recognize "DA" as "digital to analog" meaning it's an analog output, but that is far from obvious to everyone.

So I am suggesting API methods could be

void setAOSignal(const char signalILabel, double volt) throw (CMMError); double getAOSignal(const char signalILabel); // returns the voltage that was actually set on the device, which could differ from the previously set voltage double getAOLowerLimit(const char signalILabel) throw (CMMError); double getAOUpperLimit(const char signalILabel) throw (CMMError);

The sequence functions could look like:

long getAOSequenceMaxLength(const char signalILabel) throw (CMMError); void startAOSequence(const char signalILabel) throw (CMMError); void stopAOSequence(const char signalILabel) throw (CMMError); void loadAOSequence(const char signalILabel, std::vector voltSequence) throw (CMMError);

Then with the analog input there would be at minimum the API method (and maybe others aren't needed)

double getAISignal(const char* signalILabel); // returns the voltage read from the hardware

marktsuchida commented 3 years ago

I agree that SignalIO needs to be deprecated and replaced with better solutions. It is named as if it does input and output, but actually only ever did output (except where people misinterpreted GetSignal, which presumably was supposed to return the currently set output voltage, not any sort of input voltage -- or at least that's how I look at it, but one could also look at it the other way around). The word "signal" is also a poor choice.

Up till now, the "correct" (but poorly advertised) way to implement an analog input was as a read-only property, but it has too many limitations, because all sorts of code (like the Device Property Browser) read properties whenever they wish, whereas often we need deliberate "measurements" for analog inputs at well defined timings. Also we will likely want to add triggered and sequenced input, at least in the future, which won't work with properties. (These could be considered limitations of our property interface, but that is harder to change.)

So adding an AnalogInput type seems like the right thing to do. I am not 100% sure about the use of signalLabel (with all the management that would be needed around it). Is there a reason why we can't use a device per input channel? That way, we get label management for free (because the device label would be used), and I feel that adding another layer of labels just adds unnecessary complexity.

I would prefer the method to be named read or measure rather than get, because it implies an actual hardware action (AD conversion), rather than merely retrieving a (potentially cached) value.

A lot of times, people are interested in the voltage (in volts) seen by an AD converter. But applications inevitably come up where the raw AD units (8-bit, 16-bit, or whatever) are needed, such as anything that involves histogramming the input values. In addition, we need to support devices that don't know their own voltage calibration, and some AI devices support switching to current input. So I think the readAnalogInput method should return an unsigned (32-bit) integer, with a separate method to retrieve offset and scale factor to convert that to a voltage (or current, although that's lower priority to support). The conversion to voltage can be implemented in MMCore so that user code can read a voltage directly (using user-provided offset and scaling when the device doesn't). We also need a third method to query the number of bits.

(We may encounter AI devices that have built-in voltage calibration that is nonlinear. Supporting those will need a device method to directly read the voltage. But I don't think these are common (good ADCs and DACs are pretty darn linear compared to the rest of the world), and in any case it should be treated as the exceptional case, not the default. Otherwise we'll start seeing a lot of duplicated code in device adapters.)

Regarding analog output, I don't think we even need a dedicated device type unless we are adding new functionality: plain properties can perform all the methods listed above, including sequencing.

However, it probably makes sense to have an AnalogOutput device for symmetry and for better support by the Utilities virtual devices, among other things. Here, too, I don't see the need for a second layer of labels (the hub-peripheral mechanism already accomplishes this; see how NIMultiAnalog does this with SignalIO), and we need the primary method to use DAC units, not voltage.

jondaniels commented 3 years ago

@marktsuchida makes some very good points. @AdvancedResearchConsulting may have some good ideas based on his work with the Triggerscope. There is a thread at https://forum.image.sc/t/new-kind-of-device-in-micro-manager/45471/7 regarding adding analog input device that accepts a stream of data.

Based on others' thoughts, here's an updated API suggestion for analog output devices:

void setAOSteps(long) throw (CMMError);  // would normally be implemented the device adapter, like stage's setPositionSteps()
long readAOSteps(); // returns the digital value that was actually set on the device
void setVoltsPerStep(double voltsPerStep);
void setVoltageOffset(double voltageOffset);
double getVoltsPerStep();
double getVoltageOffset();

void setAOVoltage(double volt) throw (CMMError);  // Device.h would provide base implementation using setAOSteps()
double readAOVoltage(); // Device.h would provide base implementation using readAOSteps

<maybe methods to get and set limits or get the bitdepth of the hardware>

long getAOSequenceMaxLength() throw (CMMError);
void startAOSequence() throw (CMMError);
void stopAOSequence() throw (CMMError);
void loadAOSequence(std::vector<double> voltSequence) throw (CMMError);

My thoughts for the analog input device aren't as well formed. We need to accommodate different bit widths (those with experience with cameras will have better input than I do here). It's worth accommodating streams of data as well as single values, whether in the same device type or a different one. Here is an initial stab at an API.

long readAIStepsLong(); // reads a single long integer value from the hardware
std::vector<long> readAIStepLongVector(); // reads an array of long integer values (should we specify a length?)
unsigned short readAISteps16bit); // reads a single unsigned short (16 bit) integer value from the hardware
std::vector<unsigned short> readAIStes16bit(); // reads an array of unsigned short (16 bit) integer values (should we specify a length?)
unsigned char readAISteps8bit); // reads a single byte value from the hardware
std::vector<unsigned char> readAISteps8bit(); // reads an array of byte values (should we specify a length?)
AdvancedResearchConsulting commented 3 years ago

Hi all, Jon thanks for mentioning this to me! I agree we need/want an **In device type. I'm actually getting more and more requests for this. Not to pile on but we may want to consider some form of reading in a buffer. One limitation on AI is that the measuring device may collect some frequency of samples at a higher rate than the desired ability to pull into MM. So for instance if that happened, and if the collecting system had local storage, how could we pull in the data and store it in the image metadata? Or maybe that's too big of an ask? Anyway - overall I like the idea of a dedicated outbound and inbound specification!

artmeln commented 3 years ago

Following up on some posts on the MM forum, I think it would be nice to have an input MM device that can accommodate a general photon counting card.

A typical photon counting card streams arrays of data through a high speed port such as USB2. One record in the data array corresponds to one photon. The record contains information about the arrival time of the photon and which channel the photon was registered in. Exactly how this information is recorded depends on the manufacturer of the device but invariably it is not useful without further processing. One example of such processing is calculation of cross-correlation functions between two channels. Another is calculation of decay histograms in lifetime measurements.

Based on this, I think there is a need for a MM device that has the following minimal set of functions:

Start - tell the card to start streaming data; Stop - tell the card to stop streaming data; GetBuffer - get the latest data buffer from the card; GetDataLength - get the number of events in the buffer (since it is not guaranteed to be the same every time); GetDroppedDataFlag - read the flag that will notify the user that some data was dropped before it could be read in.

Other functions (such as setting the mode of operation or possibly time resolution) can be handled through the standard user-defined property mechanism. And when it comes to processing the streamed data and calculating something useful from it for user display, I think these can be handled through a mechanism similar to ProcessorPlugin.

artmeln commented 2 years ago

I'd like to revive this discussion and propose an interim solution. It seems to me that modifying the existing DA/AD device prototype (or adding another data acquisition device type) would entail a lot of work, and the details of input/output devices still need to be ironed out. In the meantime, I think it would be useful to add functionality to GenericDevice that would allow it to receive arbitrary data. It can be similar to retrieving a camera image but without specifying what kind of data it is or what to do with it (this would be left to the user to implement in the spirit of keeping the device generic). Specifically, the following calls would be necessary:

CollectData - tell the device to start collecting; after receiving this call the device should start filling its internal buffer with data. GetDataBufferSize - bytes of data available for reading, should be 0 unless the device is ready to send data. GetDataBuffer - get n bytes of data from the device where n has been populated by a previous call to GetDataBufferSize StopDataCollection - stop collecting data GetDataOverflow - a flag that indicates the internal device buffer overflowed before data could be retrieved ResetDataOverflow - reset overflow flag

I tried to name these methods so that they are self-explanatory but also can't be confused with calls to other devices that already exist. I think these additions would make GenericDevice more versatile by letting it receive any kind of data but also allowing it to serve as analog input device if need be.

If this seems like a good idea, I'd be happy to implement this change. Since we are talking about modifying an existing device, I think the disturbance to MM code would be minimal. Implementation of these calls by a device adapter can be optional, and the change will be entirely back compatible.

marktsuchida commented 2 years ago

Thanks for the suggestions!

I'm not sure I get why you think it is a good idea to add these to GenericDevice. We would then need another way to express devices that don't have any specific feature set, meaning we would need to invent another generic device type. Or else we would end up with "generic devices that are also analog input devices" and "generic devices that are not analog input devices", and all user code would need to have if-else statements all over the place.

It is not clear to me what you are referring to by "buffer". Do you mean a buffer implemented in the device adapter, or the implicit buffer hidden in data acquisition SDKs (such as the buffering done by NI DAQmx)? If the former, we should avoid reinventing in each device and let the Core handle it cleanly. If the latter, the Core only needs to pass through the calls, and the ones you list might be roughly right, but they will be limiting - user code can only acquire by polling, and the necessary frequency of polling will be highly dependent on the device SDK. I also would feel more comfortable if there is some evidence that the API (whose parameters and types also need discussion) cleanly fits with several actual devices.

artmeln commented 2 years ago

Here are a couple of actual devices that illustrate the case:

  1. A board for collecting fluorescence lifetime data. The board collects photons for a specified amount of time and produces as an output a set of data points (time on the X axis, number of events on the Y axis).
  2. A hardware correlator used in dynamic light scattering and fluorescence correlation spectroscopy. Again, the board collects photons for a predetermined amount of time and produces several sets of data with time on the X axis and correlation values on the Y axis.
  3. A generic photon counting card. Each detected photon has a record (arrival time, optical channel, lifetime information, etc), and the card streams hundreds of thousands such records per second. The exact data format depends on the manufacturer of the card.

I think these examples illustrate the need to retrieve data from hardware but I doubt that any one of them warrants a new device category in MM. My understanding was that GenericDevice was reserved for any hardware that did not fit in any other MM device category, and it seemed natural to make GenereicDevice more versatile by giving it capability to receive data. By buffer I meant any data stored in the hardware waiting to be retrieved by the device adapter which would then pass it on to upper layers of software, just the way a camera image is handled.

I believe an AD converter can take advantage of this capability as well since it’s not practical to get data one point at a time from hardware, and right now there is no way in MM to pass chunks of data.

I hope my motivation makes more sense now, I just want a device adapter that will let me get arbitrary data from hardware.

marktsuchida commented 2 years ago

Thanks for the examples; indeed we were thinking about very different things.

I'm familiar with TCSPC cards and these are all great things to be able to support, but they are not AD devices in the usual sense. If the interface only produces "raw data" without a way for user programs to determine the type or meaning of the data, it will not be easy to write, say, code that requires an analog input device but works with any such device.

I think there is a more urgent need to support hardware-clocked acquisition/generation with "regular" AD/DA devices (voltage, maybe current), and that is what this issue was intended for. I'm not at all against supporting devices that acquire fixed-size 1D arrays (your use cases 1 &2; also possibly spectrometers and oscilloscopes) and devices that produce vendor-specific byte streams (or word streams) (your use case 3), but I think these should be addressed separately -- or at least should not prevent us from having a straightforward interface dedicated to AD/DA devices.

It does make sense to think of the AD device as a special case of a byte (or word) stream acquisition device, but having the latter alone does not satisfy the need for a dedicated AD device type that user code can use knowing that the samples are (convertible to) voltages.

I think I now see why you propose to expand GenericDevice -- it may seem like a nicely simplifying idea from the perspective of implementing device adapters. The problem is that it will then offer no hardware abstraction from the perspective of user (application) code: If I need a voltage input device, am I supposed to check every loaded device and see if it has that capability?

Of course there are ways to make that easier (maybe all devices should be "generic" but implement one or more functional "profiles"), but the current way we have is to have specific device types. I would almost certainly avoid this design if starting over from scratch, but we're stuck with it for now, and I'm opposed to having a mix of very different design principles in the interface in the absence of a convincing plan to unify (we already have plenty of inconsistency problems).

To be clear, I'm not opposed to supporting arbitrary byte streams as long as the interface accomplishes the goal of making it easy to implement and use AD devices. Provided that it doesn't delay us in coming up with a good solution for AD/DA.

nicost commented 2 years ago

I totally see the need for transferring byte streams from devices to user land code, and I am thankful @artmeln for bringing this up and your proposals!

One idea (probably stupid;) would be to use properties. There could be a property type "byteStream" or something similar. It should probably have a mechanism to set and get the size. Data collection could be started and stopped with other properties. That would make it available to all device adapters of all types.

marktsuchida commented 2 years ago

Actually I don't think that's stupid. The only problem is that all existing user code expects properties to be one of the known types, so I'm not sure how we would deal with adding a new one. Next time anybody designs property-like things, the API should be such that it forces user code to handle "unknown" types....

artmeln commented 2 years ago

Thank you for your responses!

Based on this discussion, I think I’ll concentrate my efforts on the problem of byte streaming for now and not on the separate problem of implementing AD/DA devices (I have a more immediate need for the former and unfortunately I’m less familiar with the latter).

I need to educate myself on MM architecture first, so that I can better contribute to this discussion. If this sounds like a reasonable plan, in a few days I’ll open a new issue to continue this discussion with specific proposals for how byte streaming functionality could be implemented.

marktsuchida commented 2 years ago

@artmeln That sounds great! Depending on what that looks like, we might be able to support AD devices as a special case, but feel free to not worry about that for now.