Skip to content
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

Demo devices should issue asynchronous callbacks #151

Closed
ianhi opened this issue Feb 18, 2022 · 9 comments · Fixed by #160
Closed

Demo devices should issue asynchronous callbacks #151

ianhi opened this issue Feb 18, 2022 · 9 comments · Fixed by #160

Comments

@ianhi
Copy link
Contributor

ianhi commented Feb 18, 2022

The demo devices are very useful for developing software based on this. However, they have an important omission from modern device adapter behavior. That is that the demo adapters don't use threads anywhere (in particular they never trigger a callback from an internal thread).

Having them do this would have allowed for much earlier catching of issues like: micro-manager/pymmcore#49 and also is a roadblock to being fully confident in something like https://github.com/tlambert03/pymmcore-plus/pull/73.

If given a bit of advice on how to apporach this (and maybe an example from an existing adapter) then I'd be happy to have a go at implementing this.

@ianhi
Copy link
Contributor Author

ianhi commented Feb 18, 2022

Also if someone can help point me to resources for doing this I'd love to add threads to the TE2000 adapter as I'm taken to believe that this will allow more functions to return immediately instead of blocking

@marktsuchida marktsuchida changed the title Add threads to demo device Demo devices should issue asynchronous callbacks Feb 18, 2022
@marktsuchida
Copy link
Member

To add to this, the fundamental problem we have is that the threading model of callbacks was never specified, so that different device adapters (and sometimes the same adapter) do different things. It is also easy to write client code that assumes callbacks are always on device-internal threads, and deadlock when some device issues a synchronous callback (e.g. from within SetProperty).

It is theoretically possible for MMCore to convert all synchronous callbacks into asynchronous ones, but I don't think it is worth the added complexity given that GUI applications typically need to reissue these events anyway.

The approach taken by MMStudio is to collect all callbacks in a single location, and schedule them on the UI (event dispatch) thread. I think this approach works well for GUI applications, and I don't think any other approach is really viable in most GUI frameworks.

I would be open to adding helpers to pymmcore if it can simplify callback handling in Python.

But I agree that in any case it would still be nice if the demo devices had at least some examples of asynchronous callbacks.

As for how to modify DemoCamera: my suggestion would be to wait until next week when we expect to enable C++14 (#152), then use std::async(std::launch::async, ...) to issue the callback from a property handler (but make sure to read the note regarding the std::future destructor blocking -- you'll need to keep the future around as a data member of the device class). You could add a new property AsynchronousCallbackTest (or something), following the example of the existing test properties. If you want a delay between the property being set and the callback firing, it's just a matter of sleeping in the asynchronous procedure.

There are no good examples from real device adapters that I can remember (because most of those rely on a thread in the vendor SDK if they issue asynchronous callbacks at all). Except maybe the Leica and Zeiss stand adapters, but those have a different overall architecture from the simple DemoCamera (they have a thread reading incoming serial messages).

@marktsuchida
Copy link
Member

About adding threads to the TE2000 adapter -- I'm not so sure this is a good idea. It is trickier than it initially appears, because you will essentially need to implement a command queue (many serial devices can only process one command at a time, so you can only send the next command after the reply for the previous one was received). Doing this for each and every device is not a scalable approach.

If the desire is for devices not to block on setProperty (and other similar calls), I think it is better to do something above the MMCore layer that applies to all devices. This has the additional advantage that the mechanism can be coded in Python.

Either that or the mechanism should be added to MMCore, but that's also pretty tricky to do without disrupting a lot of things.

@ianhi
Copy link
Contributor Author

ianhi commented Feb 18, 2022

I think it is better to do something above the MMCore layer that applies to all devices.

That would be ideal as it would also improve the micromanager experience. As an anecdote one of the things that started me down the road of trying to use python was frustration with micromanager freezing up when using the TE2000.

@marktsuchida
Copy link
Member

I guess maybe there is a desire for a not-so-thick layer that could sit just above pymmcore (possibly as part of pymmcore-plus), whose main role would be to sanitize MMCore behavior? (Also looking at #149 and discussion in #150.)

@ianhi
Copy link
Contributor Author

ianhi commented Feb 21, 2022

@marktsuchida do you have any advice on how to set up the submodules so that I can compile this. My understanding is that on linux I need to use the micromanager repo to actually do the build, and Ideally I should be able to link to a local git submodule but some brief googling hasn't been super helpful

@marktsuchida
Copy link
Member

@ianhi Do you mean to compile mmCoreAndDevices (including DemoCamera)?

git clone --recurse-submodules https://github.com/micro-manager/micro-manager.git
cd micro-manager
./autogen.sh
./configure   # with any needed args
make

Let me know if I misunderstood your question; I wasn't sure what you meant by "link to a local git submodule". (If you are talking about avoiding re-downloading Git objects for the submodule, I think you can do a non-recursive clone and then use git submodule update --init --reference path/to/existing-mmCoreAndDevices-checkout.)

@ianhi
Copy link
Contributor Author

ianhi commented Feb 21, 2022

Let me know if I misunderstood your question; I wasn't sure what you meant by "link to a local git submodule". (If you are talking about avoiding re-downloading Git objects for the submodule, I think you can do a non-recursive clone and then use git submodule update --init --reference path/to/existing-mmCoreAndDevices-checkout.)

I mean that I want to use my local copy of the mmCoreAndDevices repo (where I will be making changes). Currently I have that in a different folder, but maybe thats wrong?

@ianhi
Copy link
Contributor Author

ianhi commented Feb 21, 2022

I seem to have figured it out. I used submodule set-url to change the url to my fork, then i used your command to update it and now I just work in that repo. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants