-
Notifications
You must be signed in to change notification settings - Fork 128
MMDevice: adds functions to XYStage and Stage to let them signal whether they use callbacks #808
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
base: main
Are you sure you want to change the base?
Conversation
let them signal whether they use callbacks to update the UI about their position.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds functions to Stage and XYStage device interfaces to allow them to signal whether they use callbacks to update the UI about their position. This is the second part of issue #806 and will enable UI components (Stage control window, Snap-on-Move, HCS plugin) to determine whether polling is necessary.
Key Changes:
- Increments device interface version from 74 to 75
- Adds pure virtual functions
UsesOnStagePositionChanged()andUsesOnXYStagePositionChanged()to respective interfaces - Provides default implementations (returning
false) inCStageBaseandCXYStageBaseclasses
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| MMDevice/MMDevice.h | Adds pure virtual functions to Stage and XYStage interfaces with documentation; increments interface version |
| MMDevice/DeviceBase.h | Provides default implementations returning false for both base classes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
MMDevice/MMDevice.h
Outdated
| * in which case the UI code should use polling. This function signals whether | ||
| * the device adapters uses callbacks, so that the UI knows it does not need | ||
| * to poll this device | ||
| */ | ||
| virtual bool UsesOnStagePositionChanged() = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious: how do you intend to determine from the UI whether a specific device returns True on this... does it imply that you intend to add another public method to MMCore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good point! Completely forgot about that part. Added now. Please feel free to suggest better function names.
isXYStageUsingCallbacks functions. Bump MMCore version to 11.12.0.
marktsuchida
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definitely makes sense, and I think the only question is how do we migrate.
If all existing stages fall into one of the 2 categories (never issue the callback, always issue the callback), then we should update all the stages at the same time as merging this (or close).
If there are any examples of stages that do something in between (for example, issue callbacks on joystick-initiated move but not software-initiated move), we probably need to have a plan for how to handle that -- which might be as simple as specifying what to return in that case, but it'd be good to be sure that this will actually be usable by apps.
Also: I think it should be documented that the return value must be constant (i.e., cannot change between Initialize() and Shutdown()).
let them signal whether they use callbacks to update the UI about their position.
Second part of issue #806. I believe this will be useful once implemented (for instance, the Stage control window, Snap-on-Move, and the HCS plugin could all make use of it), but can easily be convinced otherwise.