Utilties-Hardware Autofocus #800

nicost · 2025-12-16T01:20:42Z

Combines a camera, shutter, and Zstage into a hardware autofocus. Works with the Cephla microscope on my desk.

that combines a shutter and a camera into an autofocus device.

autofocus device that uses an existing camera, shutter and focus stage to image a reflection spot, can calibrate the movement of this spot as a function of the postion of the z stage, then move the stage to place that spot back in the desired position whenever a full focus is desired. There is code to discern 2 spots (top and bottom of coverslip_ and to pick the correct one (at least when both spots are visible).

Saves all values in calibration file, inclusing camera ROI and binning. Needs extensive documentation.

on current position. Now always subtracts a dark iameg from each image with IR. More reliable recognition of top and bottom reflection.

…ly grouped in the UI.

…ramData/Micro-Manager

Copilot

Pull request overview

This pull request adds a hardware-based autofocus device adapter for Micro-Manager that combines a camera, shutter, and Z-stage to achieve focus by detecting and tracking IR reflection spots. The implementation includes calibration, continuous focus monitoring, and offset measurement capabilities.

Key Changes:

New AutoFocus device class with image analysis and calibration algorithms
Integration with existing AutoFocusStage device for automatic registration
OpenCV 2.4.13.6 dependency added for image processing capabilities

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 25 comments.

Show a summary per file

File	Description
DeviceAdapters/Utilities/Utilities.vcxproj	Added OpenCV include paths, library directories, and dependencies for both Debug and Release configurations
DeviceAdapters/Utilities/Utilities.vcxproj.filters	Added AutoFocus.cpp to project source files filter
DeviceAdapters/Utilities/Utilities.h	Added error codes, threading/OpenCV includes, and AutoFocus class declaration with calibration data structures
DeviceAdapters/Utilities/Utilities.cpp	Registered new AutoFocus device in module initialization
DeviceAdapters/Utilities/AutoFocusStage.cpp	Added registration/unregistration logic to connect with AutoFocus device
DeviceAdapters/Utilities/AutoFocus.cpp	Complete implementation (2189 lines) including initialization, calibration, image analysis, continuous focus thread, and JSON-based calibration persistence

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-16T01:30:15Z

DeviceAdapters/Utilities/AutoFocus.cpp

+      if (ret != DEVICE_OK)
+      {
+         // Try to restore original position
+         pStage->SetPositionUm(startPos);


Incorrect error handling on SetPositionUm failure. When pStage->SetPositionUm(targetPos) fails, the code attempts to call pStage->SetPositionUm(startPos) but doesn't check if this second call succeeds. If the stage is in a bad state, this could fail too. Additionally, it should restore to originalPos, not startPos.

Suggested change

pStage->SetPositionUm(startPos);

int restoreRet = pStage->SetPositionUm(originalPos);

if (restoreRet != DEVICE_OK) {

// Optionally, log both errors or return a combined error code

// For now, return the restoration error code to signal a more severe problem

return restoreRet;

}

Copilot · 2025-12-16T01:30:16Z

DeviceAdapters/Utilities/AutoFocus.cpp

+   ImgBuffer resultImage(lightImage.Width(), lightImage.Height(), lightImage.Depth());
+   const unsigned char* lightPixels = lightImage.GetPixels();
+   const unsigned char* darkPixels = darkImage.GetPixels();
+   unsigned int numPixels = lightImage.Width() * lightImage.Height();


Potential integer overflow in image pixel calculations. When calculating numPixels as width * height, there's no check that this multiplication doesn't overflow an unsigned int. For very large images (e.g., 65536 x 65536 pixels), this could overflow. Consider using size_t or adding overflow checks.

Copilot · 2025-12-16T01:30:16Z

DeviceAdapters/Utilities/AutoFocus.cpp

+
+   shutter->SetOpen(true);
+   CDeviceUtils::SleepMs(10); // wait for shutter to open
+   // Snap image with shutter open
+   camera->SnapImage();
+   ImgBuffer lightImage;
+   ret = GetImageFromBuffer(lightImage);
+   shutter->SetOpen(false);


Shutter left open after SnapAndAnalyze completes. The function opens the shutter at line 968, and while it closes it at line 974, if any error occurs between these lines (like at line 966 or 973 where GetImageFromBuffer could fail but isn't checked), the shutter will remain open. The shutter should be closed in all code paths, preferably using RAII or ensuring cleanup even on error paths.

Suggested change

shutter->SetOpen(true);

CDeviceUtils::SleepMs(10); // wait for shutter to open

// Snap image with shutter open

camera->SnapImage();

ImgBuffer lightImage;

ret = GetImageFromBuffer(lightImage);

shutter->SetOpen(false);

if (ret != DEVICE_OK)

return ret;

// RAII guard to ensure shutter is closed

struct ShutterGuard {

MM::Shutter* shutter_;

bool closed_;

ShutterGuard(MM::Shutter* s) : shutter_(s), closed_(false) {}

void Close() { if (!closed_ && shutter_) { shutter_->SetOpen(false); closed_ = true; } }

~ShutterGuard() { Close(); }

} shutterGuard(shutter);

shutter->SetOpen(true);

CDeviceUtils::SleepMs(10); // wait for shutter to open

// Snap image with shutter open

camera->SnapImage();

ImgBuffer lightImage;

ret = GetImageFromBuffer(lightImage);

if (ret != DEVICE_OK)

return ret;

// shutter will be closed by shutterGuard destructor

Copilot · 2025-12-16T01:30:16Z

DeviceAdapters/Utilities/Utilities.h

 #define ERR_AUTOFOCUS_NOT_SUPPORTED        10012
 #define ERR_NO_PHYSICAL_STAGE              10013
 #define ERR_NO_SHUTTER_DEVICE_FOUND        10014
+#define ERR_TAGET_TOO_HIGH                 10015


Typo in error code constant name. "TAGET" should be "TARGET".

Suggested change

#define ERR_TAGET_TOO_HIGH 10015

#define ERR_TARGET_TOO_HIGH 10015

Copilot · 2025-12-16T01:30:16Z

DeviceAdapters/Utilities/AutoFocus.cpp

+   const double MIN_SLOPE = 1e-6;  // Threshold for effectively zero slope
+   const double INVALID_Z = -1.0e10;  // Sentinel value
+
+   // bool xValid = fabs(cal.slopeX) > MIN_SLOPE;


Commented out code should be removed. Line 1814 contains a duplicate variable assignment that's commented out. If this was intentional for debugging, it should be removed for production code.

Suggested change

// bool xValid = fabs(cal.slopeX) > MIN_SLOPE;

Copilot · 2025-12-16T01:30:21Z

DeviceAdapters/Utilities/AutoFocus.cpp

+   SetErrorText(ERR_NO_SHUTTER_DEVICE_FOUND, "No Shutter device found.  Please select a valid shutter in the Shutter property.");
+   SetErrorText(ERR_NO_AUTOFOCUS_DEVICE, "No AutoFocus Device selected");
+   SetErrorText(ERR_NO_AUTOFOCUS_DEVICE_FOUND, "No AutoFocus Device loaded");
+   SetErrorText(ERR_TAGET_TOO_HIGH, "Target position exceeds allowed maximum.  Likely error in autofocus");


The error message refers to the misspelled constant ERR_TAGET_TOO_HIGH. This should be ERR_TARGET_TOO_HIGH to match the corrected constant name.

Suggested change

SetErrorText(ERR_TAGET_TOO_HIGH, "Target position exceeds allowed maximum. Likely error in autofocus");

SetErrorText(ERR_TARGET_TOO_HIGH, "Target position exceeds allowed maximum. Likely error in autofocus");

Copilot · 2025-12-16T01:30:21Z

DeviceAdapters/Utilities/AutoFocus.cpp

+         double track1X = 0.0;
+         double track1Y = 0.0;
+         double track2X = 0.0;
+         double track2Y = 0.0;;


Unnecessary semicolon after variable declaration. This creates an empty statement and should be removed.

Suggested change

double track2Y = 0.0;;

double track2Y = 0.0;

Copilot · 2025-12-16T01:30:22Z

DeviceAdapters/Utilities/AutoFocus.cpp

+      binning_ = pCam->GetBinning();
+      if (binning_ <= 0)
+         binning_ = 1;
+      GetCoreCallback()->OnPropertyChanged(this, "Binning", CDeviceUtils::ConvertToString(binning_));


Inconsistent property name reference. The code calls OnPropertyChanged with "Binning" but the property was created as "Camera_Binning" on line 210. This will cause the property change notification to fail silently.

Suggested change

GetCoreCallback()->OnPropertyChanged(this, "Binning", CDeviceUtils::ConvertToString(binning_));

GetCoreCallback()->OnPropertyChanged(this, "Camera_Binning", CDeviceUtils::ConvertToString(binning_));

Copilot · 2025-12-16T01:30:22Z

DeviceAdapters/Utilities/AutoFocus.cpp

+         GetCoreCallback()->OnPropertyChanged(this, "ROI-X", CDeviceUtils::ConvertToString((long)roiX_));
+         GetCoreCallback()->OnPropertyChanged(this, "ROI-Y", CDeviceUtils::ConvertToString((long)roiY_));
+         GetCoreCallback()->OnPropertyChanged(this, "ROI-Width", CDeviceUtils::ConvertToString((long)roiWidth_));
+         GetCoreCallback()->OnPropertyChanged(this, "ROI-Height", CDeviceUtils::ConvertToString((long)roiHeight_));


Inconsistent property name references. The code calls OnPropertyChanged with "ROI-X", "ROI-Y", "ROI-Width", and "ROI-Height", but the properties were created with "Camera_ROI-X", "Camera_ROI-Y", "Camera_ROI-Width", and "Camera_ROI-Height" on lines 194, 198, 202, and 206. This will cause the property change notifications to fail silently.

Suggested change

GetCoreCallback()->OnPropertyChanged(this, "ROI-X", CDeviceUtils::ConvertToString((long)roiX_));

GetCoreCallback()->OnPropertyChanged(this, "ROI-Y", CDeviceUtils::ConvertToString((long)roiY_));

GetCoreCallback()->OnPropertyChanged(this, "ROI-Width", CDeviceUtils::ConvertToString((long)roiWidth_));

GetCoreCallback()->OnPropertyChanged(this, "ROI-Height", CDeviceUtils::ConvertToString((long)roiHeight_));

GetCoreCallback()->OnPropertyChanged(this, "Camera_ROI-X", CDeviceUtils::ConvertToString((long)roiX_));

GetCoreCallback()->OnPropertyChanged(this, "Camera_ROI-Y", CDeviceUtils::ConvertToString((long)roiY_));

GetCoreCallback()->OnPropertyChanged(this, "Camera_ROI-Width", CDeviceUtils::ConvertToString((long)roiWidth_));

GetCoreCallback()->OnPropertyChanged(this, "Camera_ROI-Height", CDeviceUtils::ConvertToString((long)roiHeight_));

Copilot · 2025-12-16T01:30:22Z

DeviceAdapters/Utilities/AutoFocus.cpp

+   // Snap image with shutter open
+   camera->SnapImage();
+   ImgBuffer lightImage;
+   ret = GetImageFromBuffer(lightImage);


Missing check for return value. The function GetImageFromBuffer is called but its return value is assigned to ret but never checked. If the image retrieval fails, the code continues to use potentially invalid lightImage data.

Suggested change

ret = GetImageFromBuffer(lightImage);

ret = GetImageFromBuffer(lightImage);

if (ret != DEVICE_OK)

return ret;

marktsuchida · 2025-12-16T18:53:09Z

As you already know from our previous chats, very excited to see reflection-based focus maintenance with software in the loop!

Some questions and discussion points:

What is your sense on how specific (to Cephla) vs generic this is, given that the algorithm is hard-coded (despite the apparent property)? Also, will naming the current algorithm "Standard" age well when others are added in the future?
Given that this contains non-trivial logic (the image processing algorithm, and potentially the Z search algorithm), should it be in Utilities (all the others of which are very straightforward adapters), or should it be a separate device adapter? I feel that the Utilities devices should maintain very good backward compatibility to keep people's config files stable; maybe it would be better not to constrain this one quite as much?
Am I right that this assumes the camera and shutter are dedicated to focusing and not shared with regular acquisition (and therefore there is no mechanism to deal with what might happen if that is not the case)? This seems what the documentation indicates but it wouldn't hurt to be very clear about this, because otherwise people will think "I have a camera and a Z stage, maybe I can use this to add autofocus". Especially if it's in Utilities.

I agree that it's good to make this generally usable (as opposed to making it only work with Cephla devices), but at the same time I feel that is's specialized and complex enough that it would be best placed somewhere other than in Utilities whose existing devices have very simple input-output relationships (it's nice to be able to use that simple explanation for Utilities devices).

Another reason to prefer a separate device adapter is that the code is large enough that it could already benefit from being split into multiple files (would be good to separate the data processing from the device adapter so that it can be unit tested, and even more so if additional algorithms are added in the future). This would crowd and complicate Utilities.

I would maybe also suggest giving the name some thought, because this is actually focus maintenance, which is superior to autofocus (which usually means focusing based on the viewfinder image). (Yes, I'm aware many people don't make the distinction, but....) I bit of history is that we used to have a SimpleAutofocus device adapter that tried to do something that a device adapter cannot possibly do correctly (with no ability to coordinate with the acquisition engine).

nicost · 2025-12-18T21:51:39Z

Superseded by #803

nicost added 13 commits October 17, 2025 18:35

Utilities: started work on autofocu device adapter

2b6b135

that combines a shutter and a camera into an autofocus device.

Utilities: Autofocus device.

caf4088

Utilities-Autofocus: start toying with openCV-based image analysis.

926fbb6

Utilities-Autofocus: adde properties for camera settings.

def6f79

Utilities-Autofocus: do not talk to camera until after initialization.

9b79730

Utilities-Autofocus: Ready for image anlaysis code

5e20a6b

Utilities autofocus adapter woprking quite well.

aba1678

Saves all values in calibration file, inclusing camera ROI and binning. Needs extensive documentation.

utilities - Autofocus: Adds function to measure offset based

0966ed5

on current position. Now always subtracts a dark iameg from each image with IR. More reliable recognition of top and bottom reflection.

Utilities-Autofocus: Renamed properties so that they are more logical…

62a01c9

…ly grouped in the UI.

Utilities-Autofocus: deal with compiler warnings.

b529abc

Utilities-Autofocus: name properties correctly everywhere.

3a5f490

Utilities-Autofocus: On Windows will save Util-Autofocus.json in Prog…

5a7b9cf

…ramData/Micro-Manager

nicost requested a review from Copilot December 16, 2025 01:20

Copilot started reviewing on behalf of nicost December 16, 2025 01:21 View session

Copilot AI reviewed Dec 16, 2025

View reviewed changes

nicost closed this Dec 18, 2025

-         pStage->SetPositionUm(startPos);
+         int restoreRet = pStage->SetPositionUm(originalPos);
+         if (restoreRet != DEVICE_OK) {
+            // Optionally, log both errors or return a combined error code
+            // For now, return the restoration error code to signal a more severe problem
+            return restoreRet;
+         }

	#define ERR_TAGET_TOO_HIGH 10015
	#define ERR_TARGET_TOO_HIGH 10015

	SetErrorText(ERR_TAGET_TOO_HIGH, "Target position exceeds allowed maximum. Likely error in autofocus");
	SetErrorText(ERR_TARGET_TOO_HIGH, "Target position exceeds allowed maximum. Likely error in autofocus");

	GetCoreCallback()->OnPropertyChanged(this, "Binning", CDeviceUtils::ConvertToString(binning_));
	GetCoreCallback()->OnPropertyChanged(this, "Camera_Binning", CDeviceUtils::ConvertToString(binning_));

Utilties-Hardware Autofocus #800

Utilties-Hardware Autofocus #800

Uh oh!

Conversation

nicost commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

marktsuchida commented Dec 16, 2025

Uh oh!

nicost commented Dec 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants