DemoCamera: Fixed DemoXYStage position reporting bugs and added a polling thread#946
DemoCamera: Fixed DemoXYStage position reporting bugs and added a polling thread#946nicost wants to merge 5 commits into
Conversation
…ling thread that fires OnXYStagePositionChanged callbacks during moves. The thread now wakes immediately when a move starts via a condition variable.
There was a problem hiding this comment.
Pull request overview
This PR updates the DemoCamera XY stage simulation to improve position reporting during moves by introducing a background polling thread that emits OnXYStagePositionChanged callbacks mid-move, and it fixes related position/relative-move behavior.
Changes:
- Added a
PollingThreadto CDemoXYStage that wakes on move start and periodically reports interpolated XY positions during motion. - Added locking around move state (
timeOutTimer_, positions) and simplifiedBusy()/Stop()behavior. - Minor cleanup: silence unused
exposureparameter in bead image generation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| DeviceAdapters/DemoCamera/DemoStages.cpp | Adds XY-stage polling thread, locking, updated move interpolation/reporting, and relative-move/stop logic changes. |
| DeviceAdapters/DemoCamera/DemoCamera.h | Declares new relative-move override and polling thread members for CDemoXYStage. |
| DeviceAdapters/DemoCamera/DemoImageGeneration.cpp | Marks exposure parameter unused in GenerateBeadsImage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@marktsuchida and @tlambert03, let me know if this is OK to merge. This adds a polling thread that will update the core with the current position of the stage even while it is moving. This polling thread can be switched off through a property which will also causes the UsesOnXYStagePositionChanged function to no longer signal true. Should be useful for testing position updates in the UI with and without polling. |
|
I'm curious why you felt like it was the responsibility of the device adapter itself (rather than the core, or the application) to create the polling thread? |
|
Some device adapters will report stage positions themselves, others do not. They can declare that through the UsesOnXYStagePositionChanged function. We are planning on creating a single polling loop in the UI that will query all the stages that are not themselves calling OnXYStagePositionChanged. It seems handy to have a demo device with which we can test both approaches. So, if a device has a built-in mechanism to report changed positions, we should use it, otherwise we should leave it to application (or possibly the core, I am not quite sure where the best place for that polling loop is). Curious if you prefer this to be in the core or in the application. |
marktsuchida
left a comment
marktsuchida
left a comment
There was a problem hiding this comment.
Looks good to me — the setting is pre-init so it conforms to the UsesOnXYStagePositionChanged() rules (no dynamic switching).
A few comments below.
Note that NotificationTester already provides the most robust (and uncluttered) way to test all the different stage notification behaviors — although we haven't updated it for the new UsesOnXYStagePositionChanged() function yet. There we probably need to make the NotificationsEnabled property pre-init.
| #ifdef _WIN32 | ||
| HANDLE moveEvent_; | ||
| #else | ||
| pthread_mutex_t eventMutex_; | ||
| pthread_cond_t eventCond_; | ||
| bool eventSignaled_ = false; | ||
| #endif |
There was a problem hiding this comment.
Can standard C++ std::mutex and std::condition_variable be used?
| void StartPollingThread(); | ||
| void StopPollingThread(); |
There was a problem hiding this comment.
The thread doesn't do any polling, so maybe better to name something else. (NotificationThread?)
Polling thread fires OnXYStagePositionChanged callbacks during moves. The thread now wakes immediately when a move starts via a condition variable.