Skip to content

Conversation

SutterBruce
Copy link

Propose passing exception code to CMMError constructor, overriding default code.
The created CMMError object better reflects the exception that occurred.

 Propose passing exception code to CMMError constructor, overriding default code.
@marktsuchida
Copy link
Member

Hi, thanks for the proposal. I think the code in MMError was intended for Core-specific error codes (defined in MMCore/ErrorCodes.h, though probably not used consistently), not the error codes produced by device adapters. The latter is not standardized and subject to change (in a few cases even dynamically generated), so there is no way for an application to interpret them directly, at least in general. Also, there is currently no way to prevent devices from generating codes that overlap with the ones defined by MMCore -- so this would be a breaking change if any user code is checking for Core error codes.

So I'm a little hesitant to provide these as a numeric value, which might raise the wrong expectations. (We do include it in the error message text, because there it is clear that it comes from a device and it's helpful when troubleshooting at the code level.)

Would you mind explaining the motivation for wanting this change? For example, is it motivated by a particular device's behavior? Maybe there are other ways we can think about addressing your use case.

@SutterBruce
Copy link
Author

Hi Mark, Thanks for the feedback. Here's the scenario behind my proposal.

In working on a programatic interface to Micro-Manager, I ran into circumstances where CMMCore::setProperty( ) could be called with a property-value that is out of bounds. For example, imagine setting the property "Exposure" to 10,000,000 s. on the OpenCVgrabber device-adapter, whose valid range is 0->10,000. This particular call fails in MM::FloatProperty::Set( ), returning 'false' to PropertyCollection::Set( ), which in turn returns the error code DEVICE_INVALID_PROPERTY_VALUE. The name of the property ("Exposure") is recorded, and DeviceInstance::SetProperty( ) then calls ThrowIfError( ) passing in the error code. ThrowIfError( ) creates and throws a standard CMMError exception object.

My proposed change was to the protected member-function, DeviceInstance::MakeExceptionForCode( ), which formats the error-message and creates the CMMError object that is thrown. My goal was to have the foundational error-code (in this example, DEVICE_INVALID_PROPERTY_VALUE) available in the exception object, so higher-level code can include the error code when reporting the error.

For comparison with my code, I also checked Micro-Manager's normal handling of such errors by using the Java-based UI. As best I can tell, the UI intercepts and suppresses attempts to set properties to values outside their proper bounds. Consequently, I can see that my scenario won't arise under normal circumstances.

Any thoughts?
Thx.

@marktsuchida
Copy link
Member

It would indeed be better if application code could distinguish between different types of errors. Ideally this would be done by exception type, although using error codes might be an easier solution in the short term. However, there are two problems with this:

  • The error code contained in CMMError is not propagated to Java, because MMCoreJ currently replaces CMMError with plain java.lang.Exception (for reasons -- I hope to improve this situation eventually).

  • Because of the above, MMCore has not been consistent about attaching error codes, so even if your code is in C++ (is it?), you will likely get MMERR_GENERIC in many cases (of which the example you hit here is one).

So if you just want to have a specific error code in this particular case, it would probably make sense to add the range check to MMCore (even if redundant), introduce a new code MMERR_InvalidPropertyValue, and throw the exception when the check fails (before the device's SetProperty() is even called).

Setting the CMMError's code to DEVICE_INVALID_PROPERTY_VALUE would be mixing error code types, so would not make sense (MMCore error codes (MMERR_*) are separate from MMDevice error codes (DEVICE_*)). I would also hesitate to blindly translate from device error codes (even the standardized ones) to something that the application will directly see, because some devices are not careful about ensuring that their own error codes don't clash with the standard ones (another situation I'd like to improve, but probably a major undertaking). In other words, the codes returned by devices to the Core, while useful for human troubleshooting, are not safe for applications to directly rely on for mechanical interpretation -- especially if we want it to be future-proof (which we do, of course).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants