-
Notifications
You must be signed in to change notification settings - Fork 133
Add getBinning/setBinning methods to MMCore #811
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
Changes from 3 commits
6f30d60
58d3e9c
7a5748b
e37129f
64ee449
94ea852
c7c220e
ffcebdb
3606c2e
49b06ad
41f6d87
66d6d3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -106,7 +106,7 @@ namespace mmi = mmcore::internal; | |||||||||||||||||||||||||||||||||||||||
| * (Keep the 3 numbers on one line to make it easier to look at diffs when | ||||||||||||||||||||||||||||||||||||||||
| * merging/rebasing.) | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| const int MMCore_versionMajor = 11, MMCore_versionMinor = 11, MMCore_versionPatch = 0; | ||||||||||||||||||||||||||||||||||||||||
| const int MMCore_versionMajor = 11, MMCore_versionMinor = 12, MMCore_versionPatch = 0; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /////////////////////////////////////////////////////////////////////////////// | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -4407,6 +4407,187 @@ double CMMCore::getExposure(const char* label) MMCORE_LEGACY_THROW(CMMError) | |||||||||||||||||||||||||||||||||||||||
| return 0.0; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * Returns the current binning setting of the camera. | ||||||||||||||||||||||||||||||||||||||||
| * Binning values in "NxN" format (e.g., "2x2") are parsed to return the integer factor (e.g., 2). | ||||||||||||||||||||||||||||||||||||||||
| * @return the binning factor (1-100) | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| int CMMCore::getBinning() MMCORE_LEGACY_THROW(CMMError) | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| std::shared_ptr<mmi::CameraInstance> camera = currentCameraDevice_.lock(); | ||||||||||||||||||||||||||||||||||||||||
| if (!camera) | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| throw CMMError(getCoreErrorText(MMERR_CameraNotAvailable).c_str(), MMERR_CameraNotAvailable); | ||||||||||||||||||||||||||||||||||||||||
nicost marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| std::string cameraName; | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| mmi::DeviceModuleLockGuard guard(camera); | ||||||||||||||||||||||||||||||||||||||||
| cameraName = camera->GetLabel(); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| return getBinning(cameraName.c_str()); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * Returns the current binning setting of the specified camera. | ||||||||||||||||||||||||||||||||||||||||
| * Binning values in "NxN" format (e.g., "2x2") are parsed to return the integer factor (e.g., 2). | ||||||||||||||||||||||||||||||||||||||||
| * @param label the camera device label | ||||||||||||||||||||||||||||||||||||||||
| * @return the binning factor (1-100) | ||||||||||||||||||||||||||||||||||||||||
nicost marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| int CMMCore::getBinning(const char* label) MMCORE_LEGACY_THROW(CMMError) | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| std::shared_ptr<mmi::CameraInstance> pCamera = | ||||||||||||||||||||||||||||||||||||||||
| deviceManager_->GetDeviceOfType<mmi::CameraInstance>(label); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| mmi::DeviceModuleLockGuard guard(pCamera); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (!pCamera->HasProperty(MM::g_Keyword_Binning)) | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| throw CMMError("Camera does not support binning property"); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| std::string binningValue = pCamera->GetProperty(MM::g_Keyword_Binning); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Parse the binning value - handle both integer ("2") and NxN ("2x2") formats | ||||||||||||||||||||||||||||||||||||||||
| if (binningValue.empty()) | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| throw CMMError("Binning property returned empty value"); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // atoi() naturally handles both formats - it parses until it hits non-digit | ||||||||||||||||||||||||||||||||||||||||
| // "2" -> 2, "2x2" -> 2, "4x4" -> 4 | ||||||||||||||||||||||||||||||||||||||||
| int binning = atoi(binningValue.c_str()); | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
| // atoi() naturally handles both formats - it parses until it hits non-digit | |
| // "2" -> 2, "2x2" -> 2, "4x4" -> 4 | |
| int binning = atoi(binningValue.c_str()); | |
| // Use std::stoi to parse leading digits and detect invalid formats | |
| // "2" -> 2, "2x2" -> 2, "4x4" -> 4 | |
| int binning = 0; | |
| try | |
| { | |
| std::size_t parsedChars = 0; | |
| binning = std::stoi(binningValue, &parsedChars, 10); | |
| if (parsedChars == 0) | |
| { | |
| throw CMMError("Invalid binning value (no leading digits): " + binningValue); | |
| } | |
| } | |
| catch (const std::exception&) | |
| { | |
| throw CMMError("Invalid binning value: " + binningValue); | |
| } |
nicost marked this conversation as resolved.
Show resolved
Hide resolved
nicost marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Jan 8, 2026
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.
The format detection logic only checks for the presence of 'x' in the current value. This could incorrectly detect formats if the camera returns values like "max", "mixed", or other strings containing 'x'. Consider using a more robust check, such as a regex pattern to match "NxN" format (e.g., checking for digits before and after 'x'), or checking for a pattern like std::isdigit() before 'x' and after 'x'.
Outdated
Copilot
AI
Jan 8, 2026
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.
The catch block catches all exceptions (catch (...)) but provides no logging or indication of what went wrong. This makes debugging difficult if GetProperty fails. Consider either catching specific exceptions and logging them at DEBUG level, or at minimum adding a debug log message indicating that the current value couldn't be retrieved and the default format will be tried.
| // If we can't get current value, we'll try integer format first | |
| // If we can't get current value, we'll try integer format first | |
| LOG_DEBUG(coreLogger_) << "Could not retrieve current binning value for camera " | |
| << label << "; will try integer format first."; |
nicost marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Jan 8, 2026
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.
The new getBinning and setBinning methods lack test coverage. Given that the codebase includes unit tests (e.g., PixelSize-Tests.cpp tests binning-related functionality), these new API methods should have corresponding tests to verify:
- Parsing of integer format ("2")
- Parsing of NxN format ("2x2")
- Setting binning in both formats
- Error handling for cameras without binning support
- Error handling for invalid binning values
- The fallback mechanism when one format fails
Uh oh!
There was an error while loading. Please reload this page.