Add async property callback to DemoCamera

[ianhi]

Allows for more comprehensive testing of downstream libraries.
It seems that it was necessary to make a follower and leader property
as with only a single property there is a callback fired as soon
the action returns, so added a second "Follower" property that
is set to the leaders value after a (configurable) delay.

[ianhi]

This is just fixing the indenting from tab to spaces in order to match the rest of the file.

[ianhi]

Closes: #151

Thanks for the guidance @marktsuchida, especially the hint about saving the future really saved me some time.

As discussed in the commit message I didn't see an obvious way to do this without introducing a second property, but if there's a better way that I'm missing happy to adjust.

[ianhi]

Also cc @tlambert03 as this has implications for testing threading in pymmcore+

[marktsuchida]

I like the direction!

One thing to consider: most properties are implemented in one of two styles:

• The property value is stored in the property object itself (as you have done with the follower and delay properties), and there is no action handler. I.e., it is a software-only property that is never changed except by the application, and is only read by the device as needed.
• The property value is stored elsewhere (either in a hardware setting, or in a data member of the device class), and an action handler handles the get/set operations. The value stored in the property object itself is only used to transmit the value to/from the action handler.

The majority of useful properties use the second style (because it is the only way to do anything interesting). Sometimes it seems redundant (and definitely confusing to someone new to the interface), but that is what we've got.

If you use this style, I think it is possible to have a single test property: on AfterSet it would launch a delayed action and immediately return. The delayed action would update the data member storing the value and fire the notification. On BeforeGet, you would always return the current value of the data member.

One reason I bring this up is that, by storing the value in a data member that you control, it becomes possible to protect access to it using a mutex. (You would acquire the mutex whenever reading or writing it, whether in the BeforeGet handler or in the async setter.) Otherwise there is a data race, even though it might be rare to encounter it in the context of this test property (but I think it is good for test facilities to be robust).

(Whether you have a single property or a leader and follower is not a huge issue per se in my opinion. Either way works for testing.)

Also, just a few minor style comments: Our convention for C++ code is to start member (and non-member) function names with a capital letter (so SlowPropertyUpdate) and to add _ as a suffix (not prefix) to data members (so fut_).

[ianhi]

The majority of useful properties use the second style (because it is the only way to do anything interesting). Sometimes it seems redundant (and definitely confusing to someone new to the interface), but that is what we've got.

This is very helpful thanks! A nights sleep and that explanation has it all making a lot more sense. I've updated it to use this as well as locks (I think) as you suggested.

If you use this style, I think it is possible to have a single test property: on AfterSet it would launch a delayed action and immediately return. The delayed action would update the data member storing the value and fire the notification. On BeforeGet, you would always return the current value of the data member.

Oh true! Though, I still went with two because I have a hunch that that may be handy in downstream testing.

Three follow up questions:

1. Locks

I agree that this should have locks, and by the look around and copy approach I think I got it right but it'd be nice to confirm my understanding with you. In particular whats the difference between: MMThreadLock and MMThreadGuard one is an object and one is a function that uses the lock?

What is the g doing here?

mmCoreAndDevices/DeviceAdapters/DemoCamera/DemoCamera.cpp

Line 1994 in 6a48a3a

MMThreadGuard g(imgPixelsLock_);

and finally re locks: do they just get released at the end of the function when a destructor comes in because they are locally scoped? (that's my guess as to what's happening here)

2. How and where should I document these test properties?
3. Tests?

[ianhi]

an extra thought regarding documentation. I think you should definitely add your description in the above comment to the properties section of: https://micro-manager.org/Building_Micro-Manager_Device_Adapters it's a very helpful description, it could even point to this code as an example for extra completeness.

[marktsuchida]

The relationship between MMThreadLock and MMThreadGuard is analogous to the relationship between std::mutex and std::lock_guard<std::mutex> in modern C++. The former defines the mutex itself, while the latter is an object whose only role is to acquire the mutex for its lifetime (an example of RAII). That is, creating an object g of type MMThreadGuard acquires the mutex, and when g is destroyed (typically by going out of scope by any code path), the mutex is automatically released.

By creating the guard within a block { }, you can get something similar to a Python with block. And you should generally avoid calling external functions that may do complex things from within such a block, to minimize the possibility of deadlocking. In this example, I would really only hold the mutex while accessing the data members asyncLeader_ and asyncFollower_, and definitely avoid holding the mutex while calling, e.g., OnPropertyChanged().

Documenting properties: the device page would be the place. Clearly lots of undone work here....

Testing: sadly, there is none. But we can talk about it. While I am all for adding little test utilities like this to DemoCamera (as I have done myself with, e.g., the RGB color test pattern), I feel that DemoCamera is not really structured well for rigorous automated testing in general, so if we get serious about this, we might want to have a dedicated device adapter for testing.

One thing that I explored in the past (more for testing MMCore and acqEngine, but potentially relevant here) was DeviceAdapters/SequenceTester -- which performs relatively sophisticated device simulation (with hardware sequencing) and captures a journal of everything that happens. It then encodes (MsgPack) the journal into the next-acquired image, so that a test driver can analyze it. It also has a mode to show the data as text in the image, so that you can play with it manually. Unlike DemoCamera, though, it does not simulate real time delays, so a snap will finish instantly even for a long exposure. It also does not (IIRC) have any async callbacks other than sequence acquisition, which is why I didn't mention it earlier. A few real, but unmaintained, tests written to use this are in systemtest/SequenceTests in the micro-manager repo. At the time it was met with a near complete lack of interest (perhaps I did not promote it enough), so I never got back to it after using it for making a few difficult changes to acqEngine, but I can try and at least add it to the build if there is interest. Extending this for asynchronous notifications will require some thought, because the original goal was to be perfectly deterministic, but should be possible.

The other related topic is about testing device adapters as modules, which (to do right) requires bypassing MMCore. I started some work on this last year: https://github.com/micro-manager/mmdevicetests. So far it is little more than the beginnings of a mechanism to load a device adapter in Python without using MMCore. This is intended to become a generic test suite for any device adapter, but I suppose there is no reason why it couldn't also be used for tests that are specific to a particular device or one of its properties.

[ianhi]

Documenting properties: the device page would be the place. Clearly lots of undone work here....

Is there any way to couple those docs with the contents of this repo? The disconnect across repos is a bit of bummer for keeping things in sync. Like could this repo have a docs folder that generates the doxygen docs and then has pages for each device adapter. And then it could be served under micro-manager.org/MMCore and micro-manager.org/device-adapters/

The other related topic is about testing device adapters as modules, which (to do right) requires bypassing MMCore. I started some work on this last year: https://github.com/micro-manager/mmdevicetests. So far it is little more than the beginnings of a mechanism to load a device adapter in Python without using MMCore.

very cool. One thing I was wondering about when poking around is if it was possible to directly interact with a device and I suppose that that would be the way. Perhaps that could be released as pymmdevice?

Testing: sadly, there is none. But we can talk about it. While I am all for adding little test utilities like this to DemoCamera (as I have done myself with, e.g., the RGB color test pattern), I feel that DemoCamera is not really structured well for rigorous automated testing in general, so if we get serious about this, we might want to have a dedicated device adapter for testing.

I have a personal bias for python because I know how to use it. But maybe a quick and dirty intermediary step would be to add automated tests to the pymmcore repo? For example that would make it pretty easy to transform this test script I was using into something that leverages pytest:

from pymmcore_plus import CMMCorePlus

mmc: CMMCorePlus = CMMCorePlus.instance()
@mmc.events.propertyChanged.connect
def cb(*args):
    print('here!')
    print(args)
mmc.setDeviceAdapterSearchPaths(["../micro-manager/mmCoreAndDevices/DeviceAdapters/DemoCamera/.libs/"])
mmc.loadSystemConfiguration()
print(mmc.getLoadedDevices())
# mmc.setProperty("")
print(mmc.getDevicePropertyNames("Camera"))
# mmc.setProperty("Camera", "TestProperty4", 1)
# mmc.setProperty("Camera", "Binning", 2)
# mmc.setProperty("Camera", "Binning", 4)
# mmc.setProperty("Camera", "AsyncCallbackTestProperty", 1)
mmc.setProperty("Camera", "AsyncPropertyLeader", "some new value")
mmc.setProperty("Camera", "AsyncPropertyDelayMS", 3000)
print(mmc.getProperty("Camera", "AsyncPropertyFollower"))
mmc.setProperty("Camera", "AsyncPropertyFollower", "shouldn't be allowed")
import time
time.sleep(6)
print(mmc.getProperty("Camera", "AsyncPropertyFollower"))
# from IPython import embed
# embed(colors='neutral')

[ianhi]

@marktsuchida is 3ba9d48 what you had in mind with the scoping of the thread guards?

[ianhi]

One thing that I explored in the past (more for testing MMCore and acqEngine, but potentially relevant here) was DeviceAdapters/SequenceTester

I wonder how much of C++ code the pymmcore-plus tests and napari-micromanager tests cover. Obviously they only hit the demo camera but I suspect that they're actually pretty extensive. Probably no easy way to get the actual coverage of c++ unfortunately though.

[marktsuchida]

Exactly.

[marktsuchida]

Documenting properties: the device page would be the place. Clearly lots of undone work here....

Is there any way to couple those docs with the contents of this repo? The disconnect across repos is a bit of bummer for keeping things in sync. Like could this repo have a docs folder that generates the doxygen docs and then has pages for each device adapter. And then it could be served under micro-manager.org/MMCore and micro-manager.org/device-adapters/

The idea of having the docs (or at least certain parts of the docs) in the same repo used to be unpopular, but might be viable now. I've always wanted that.

But it doesn't make sense to use Doxygen for device adapters, which merely implement the uniform MMDevice interface; properties are not C++ constructs. I do like the idea of having a defined format (YAML? TOML? standardized Markdown?) for device property documentation (and other relevant device metadata) that lives with the device adapter code. It has to be noted that some devices dynamically generate properties, so it's not always simple. Maybe it is better for device adapters to call a SetPropertyHelpString() function.

Anyway, a huge topic.

[ianhi]

The idea of having the docs (or at least certain parts of the docs) in the same repo used to be unpopular, but might be viable now. I've always wanted that.

:O I never lived in that world, so I wasn't even aware. What was the argument against having them coupled?

[ianhi]

Maybe it is better for device adapters to call a SetPropertyHelpString() function.

Independtly I think this would be a good thing! a docstring of sorts for properties. The names are often fairly descriptive, but sometimes it would be good to have more detailed descriptions, that ideally are accessible when scripting.

So mmc.getPropretyDetails(device: str, property: str) would give something like

name: Property Name
valid values: min=0, max=10
Long Description:
Some longer description about what this property does and blah blah blah

[marktsuchida]

The other related topic is about testing device adapters as modules, which (to do right) requires bypassing MMCore. I started some work on this last year: https://github.com/micro-manager/mmdevicetests. So far it is little more than the beginnings of a mechanism to load a device adapter in Python without using MMCore.

very cool. One thing I was wondering about when poking around is if it was possible to directly interact with a device and I suppose that that would be the way. Perhaps that could be released as pymmdevice?

No, that doesn't make sense to me (I'm curious why people keep asking for this :). The tiny bit of sanity that is preserved due to MMCore being the only way to access the device adapters is extremely precious for keeping the MM device adapter collection usable. We have enough problems as such; if we end up in a situation where some devices are tested better with MMCore and others tested with something else, all hope is lost. If something other than MMCore is going to talk to device adapters (other than for testing), it should take the full responsibility of replacing and eliminating MMCore, once and for all. (But it's hard (not impossible) for Python code to play this role.)

There is a lot of duplicated logic between device adapters, which a better-designed interface would have placed in MMCore. There are also useful things in device adapters that, on the surface, it looks as if MMCore obscures. But trying to access such functionality without doing the work of updating MMCore to cleanly expose it is only going to add to the chaos, imho.

The story would be different if the MMDevice interface were more watertight and precisely defined.

[marktsuchida]

I have a personal bias for python because I know how to use it. But maybe a quick and dirty intermediary step would be to add automated tests to the pymmcore repo? For example that would make it pretty easy to transform this test script I was using into something that leverages pytest:

I'm all for using Python for tests. I wouldn't be opposed to something like this, as long as it doesn't live in the pymmcore repo. Once we make progress with micro-manager/micro-manager#1392, tests like this could live in the prospective mmdev-DemoCamera repo without introducing a circular dependency (because pymmcore would by then depend on the prospective MMCore repo and not on mmCoreAndDevices). Until then, I would prefer if it were in a separate repo, or, since the immediate purpose is to verify assumptions for your use, in one of your repos. (Nobody has ever promised that DemoCamera properties will be stable over time -- if we do make such a promise, we should probably review the existing properties (some of which are suspect) and list the set that will be maintained like an API. Having tests would certainly help with this.)

[marktsuchida]

Maybe it is better for device adapters to call a SetPropertyHelpString() function.

Independtly I think this would be a good thing! a docstring of sorts for properties. The names are often fairly descriptive, but sometimes it would be good to have more detailed descriptions, that ideally are accessible when scripting.

Yes, and tooltips in the Device Property Browser.

[marktsuchida]

@ianhi Thanks for the update.

[ianhi]

Thanks for the help!

My understanding is that this will be included in the next nightly release? But that I don't need to update pymmcore version to get this?

[marktsuchida]

That's correct. You only need to update pymmcore to get changes to MMCore code.