Generic events from Devices

[henrypinkard]

As I've moved forward on the prototype for the new camera API (#243), one thing that has occurred to me is that it would be really useful to make a more generic version of callbacks from devices to the core (a.k.a. "events").

This would lead to several improvements:

1. It would allow for devices to post device-specific events about updates of their status. I guess the way this is done now is through properties, but this really doesn't cover a lot of cases. For example, if a camera wants to send an ExposureEndEvent, I guess you could do this by changing a property from true and then back to false, but this is ugly. (Not the perfect example because ExposureEndEvent is something we'd ideally like every camera that can to report, but you get the idea)
2. We could add a mechanism whereby devices declare what events/callbacks they support. As I understand, with the current setup, higher level code just has to wait and see if callbacks come or if they don't, making harder to differentiate bugs where a callback is missed vs an unsupported feature for a particular device. This might also allow the user to turn on and off the posting of different events
3. It would make easier to add more callbacks/events in the future (whether they are customized to a particular device or standardized in some way)

How to do this

• Following in the way properties are set up in the core, have devices call CreateEvent when they are initialized.
• Have a standardized list of events in micro-manager that can be created, or allow custom events
• Create a way to serialize/deserialize events as they pass across the Device-Core boundary. This is a little more complicated than properties, because events can have other data associated with them. For example, GenICam ExposureEndEvents have timestamps + image numbers. But I think adding key/value pairs and copying the code from the serialization/deserialization Metadata class should more or less accomplish this.
• Deprecate all the existing notifications in CoreCallback (OnStagePositionChanged, OnExposureChanged, etc). In the Core, route all existing calls to these through a new generic int OnDeviceEvent(const MM::Device* caller, const char* eventType, const char* serializedEvent), and instruct future device adapter writers got through the latter mechanism.

I still think there's something to figure out in terms of how to implement the standardized events. That is, a better mechanism that just having a list of standardized strings that name these events and hoping people use them correctly. Maybe making very lightweight classes for each standardized event in the device layer. This also relates to discussions about standardized properties that we've been having lately (I plan to open an issue on this shortly).

@marktsuchida @nicost what do you think? If you don't see any major reasons to not pursue this, I think I could fairly quickly mock up a prototype in the new camera api branch and see what unexpected issues arise.

[marktsuchida]

I'm not necessarily opposed to the high-level idea of having generic events (although I would like to see some actual use cases first). However:

If you don't see any major reasons to not pursue this

In the form you have it above, at least, I'm really not convinced that it is an improvement over the existing events. It's better to have the event data be expressed as function parameters than an opaque serializedEvent string. Any perceived flexibility from using a serialized string is not actually useful without data schemas on the producing and consuming side, possibly with versioning and whatnot -- but then expressing the fields in C++ is probably easier and definitely simpler. (If both sides need to know the data schema anyway, it is much better to express it in the interface itself.)

There are likely other ways to implement generic events (maybe there could be a struct or class associated with each event), but I'm also not sure that collapsing to a single function, by itself, will improve the current situation (other than, perhaps, the amount of typing needed to add a new event, but that is not something we should be optimizing for). It would seem like you would need something almost as complex as properties, just without the stateful value.

I think I could fairly quickly mock up a prototype in the new camera api branch

It really should be an independent branch that focuses on just this. It's best to keep things small and separated, or it will be hard/impossible to review. Git branches are cheap and pull requests should strive to be single-topic.

(If you want my honest recommendation, I think it is best to avoid having #243 depend on this. I know it's tempting to fix everything, but event callbacks touch a lot more things than trigger settings.)

[henrypinkard]

It's better to have the event data be expressed as function parameters than an opaque serializedEvent string. Any perceived flexibility from using a serialized string is not actually useful without data schemas on the producing and consuming side, possibly with versioning and whatnot -- but then expressing the fields in C++ is probably easier and definitely simpler. (If both sides need to know the data schema anyway, it is much better to express it in the interface itself.)

Good point, I see what you mean and agree.

There are likely other ways to implement generic events (maybe there could be a struct or class associated with each event),

Can this be done in a POD way compatible with the Device-Core interface? If its a generic event, you wouldn't know how many fields it would have, so I would think you'd have to have some serialization mechanism to make it general purpose.

It really should be an independent branch that focuses on just this. It's best to keep things small and separated, or it will be hard/impossible to review. Git branches are cheap and pull requests should strive to be single-topic.

(If you want my honest recommendation, I think it is best to avoid having #243 depend on this. I know it's tempting to fix everything, but event callbacks touch a lot more things than trigger settings.)

I could update the event system in an independent branch as a first step, but I don't see how it could be avoided to have the camera API depend on this. Improvement number 2 mentioned above would seem to be a necessity in order to build a better acquisition engine on top of the new camera API, since not every camera is likely to support every callback/event of the new API

[marktsuchida]

The POD way would be to have an enum constant (event type) together with a pointer to the event-specific struct. Or a function-per-event together with an event-specific struct -- which ends up being similar to what we have.

What I meant was that if I were you, I would try to solve #243 without expanding to a generic event mechanism. But I think I see what you mean, too, and maybe these camera events do indeed need a generic-ish mechanism. I wouldn't be opposed to a single function for "camera timing events" that takes a constant indicating the timing type.

Also it would appear critical that there is an interface to turn on and off the individual events (at the hardware level), given the scary warning about bandwidth and acknowledgment in the Basler docs -- although that could just be boolean properties.

Maybe what I'll say for now is that it might be best to concentrate on the right interface for the camera events, and not try to subsume the existing notification mechanism just yet. We can look at the latter once we know what the camera API will look like.

[henrypinkard]

The POD way would be to have an enum constant (event type) together with a pointer to the event-specific struct. Or a function-per-event together with an event-specific struct -- which ends up being similar to what we have.

I'm still confused about how this could be used in a general way. For example: camera X has some special weird event with its own data fields. We don't have a hard coded struct for it already in MMDevice. One could make a struct for this event type within the device adapter, but then there's no dedicated function for passing that type of struct to the core. So how does the core and other higher level code know fields it contains? Is there some kind of reflection needed? I would think that one would have to pack it into a dictionary-like structure, and (like cameras adding metadata to images) and serialize/deserialize that structure in some a-priori known way.

What I meant was that if I were you, I would try to solve #243 without expanding to a generic event mechanism. But I think I see what you mean, too, and maybe these camera events do indeed need a generic-ish mechanism. I wouldn't be opposed to a single function for "camera timing events" that takes a constant indicating the timing type.

Okay that works. I think it will likely already be generic enough such that if we decide other callbacks should be subsumed into it, it will be straightforward

Also it would appear critical that there is an interface to turn on and off the individual events (at the hardware level), given the scary warning about bandwidth and acknowledgment in the Basler docs -- although that could just be boolean properties.

Yes. Could be property corresponding to each event, or events could be their own class with a required on/off field

[marktsuchida]

I'm still confused about how this could be used in a general way. For example: camera X has some special weird event with its own data fields. We don't have a hard coded struct for it already in MMDevice. One could make a struct for this event type within the device adapter, but then there's no dedicated function for passing that type of struct to the core. So how does the core and other higher level code know fields it contains? Is there some kind of reflection needed? I would think that one would have to pack it into a dictionary-like structure, and (like cameras adding metadata to images) and serialize/deserialize that structure in some a-priori known way.

If you want to support arbitrary events with arbitrary payloads, then indeed, you need a JSON-like (perhaps MsgPack or similar) encoding, or something equally elaborate. Or at least key-value pairs.

• I don't think we have seen evidence that we need support for device-specific arbitrary events, so maybe we can wait until we have actual use cases and know what the requirements are
  • Device-specific events can only be used by apps that are aware of that specific device, so they are not something that is generally desirable unless we have a clear need for them
• It might still be better to have a fixed schema for standard events, so that the required fields and their meaning are clear and can be relied upon, and the event producer code is cleaner
• We can always add arbitrary key-value pairs (akin to image metadata) later to every event; it will require the MMDevice version to be bumped but should be doable without requiring source code change in the devices

[henrypinkard]

Yeah that makes sense

[henrypinkard]

Coming back around to this. Summary of the plan going forward:

• Not going to do anything related to generic events because of the complexity this adds without especially obvious crucial use-cases
• I will need to add some new callback functions for event types specific which inherently have an event specific struct (e.g. image number and timestamp for exposure end events). This can be implemented in the same style as existing events
• Need a way to turn off specific events to prevent an overwhelming number of events being generated (and it might be nice for an acquisition engine to be able to turn off things it knows it won't care about to optimize performance)
• Need a way to declare if events of a particular type are supported (number 2 in my original post)

One possibility for these latter two is to use standardized properties (#258), so a reasonable path forward would seem to be to focus on that PR first and then move to this