mmc.getNumberOfStates(const char& stateDeviceLabel) with an unknown device throws an uncaught exception

[nicost]

which in turn makes the application disappear.

Tested with the demo configuration. In the script panel type:

mmc.getNumberOfStates("Dichroic");
<10>
mmc.getNumberOfStates("D");

resulting in uncaught exception.

[nicost]

Code looks as follows:

long CMMCore::getNumberOfStates(const char* deviceLabel)
{
 boost::shared_ptr<StateInstance> pStateDev =
      deviceManager_->GetDeviceOfType<StateInstance>(deviceLabel);
   mm::DeviceModuleLockGuard guard(pStateDev);
   return pStateDev->GetNumberOfPositions();
}

It is easy to test whether or not the device exists, but unclear what to return if it doesn't (since the function does not throw). It also looks like many (all?) other functions taking a device label have the same issue. It makes for a bit of unpleasant scripting if a single typo results in the application disappearing;).

[marktsuchida]

Ugh. Of course we could add throw and bump the major version of MMCore to signal the backward incompatibility. But since that would be disruptive, and since this fix only benefits people using relatively rarely called functions, I think it is better to do a short-term fix that preserves the API while preventing crashes (see below), and have a plan to do a proper fix at some future time when we are breaking a bunch of APIs anyway.

(In principle we could throw an unchecked exception (subclass of RuntimeException). It's hard to say if that is better or worse, though. This option is my least favorite.)

For the short-term fix of getNumberOfStates() specifically, we could just return -1 (or 0) when the device does not exist or is not a state device. It is bad behavior, but a lot less bad than crashing. Since the behavior when the argument is not a state device is not currently specified, this fix can be applied without bumping the major version.

I did a quick pass over MMCore.h and the following functions with no throw clause look suspect:

PropertyBlock getStateLabelData(const char* stateDeviceLabel, const char* stateLabel);
PropertyBlock getData(const char* stateDeviceLabel);
void setFocusDirection(const char* stageLabel, int sign);
bool supportsDeviceDetection(char* deviceLabel);
MM::DeviceDetectionStatus detectDevice(char* deviceLabel);

PropertyBlock is an old feature that is no longer used (by our code at least), so everything related to it should probably be deprecated. In any case, on error an empty PropertyBlock can be returned.
setFocusDirection() ought to throw when the given label is not a stage, but the short-term fix can be to do nothing.
supportsDeviceDetection() can return false if the device doesn't exist.
detectDevice() can return MM::Unimplemented if the device doesn't exist.

Would be great if you could also look over the functions to see if I missed any. I'm happy to make these changes if you like.

For the proper fix, there is also the possibility of deprecating the current functions and adding new ones. For example,

MMCORE_DEPRECATED(long getNumberOfStates(const char* stateDeviceLabel));  // Returns -1 on error
long getStateDeviceNumberOfStates(const char *stateDeviceLabel) throw (CMMError);  // New function

[nicost]

Not sure if this is related, but::
vector<string> CMMCore::getAvailableConfigs(const char* group) const
calls CheckConfigGroupName(group); which is defined as:
void CMMCore::CheckConfigGroupName(const char* groupName) throw (CMMError)

Your list looks quite complete. Most everything else seems to throw (CMMError). Will be wonderful if you can make the changes you propose. Thanks!

[marktsuchida]

Will do.

Looking at the code, not only getAvailableConfigs() but also getAvailableDevices() looks like it needs a fix.
So my task is to review every function without throw (CMMError) and make sure that exceptions are never thrown.

[marktsuchida]

Crashes should no longer occur (#173), but I'm leaving this open because we should consider adding functions with better error reporting (see the getStateDeviceNumberOfStates() example above). Probably less urgent than other MMCore issues.