Conversation
| unsigned int error = GetNumberVSSpeeds(&mNumVSPeriods); | ||
| if (error == DRV_SUCCESS && mNumVSPeriods > 0) { | ||
| for (i=0; i<numVSPeriods; i++) { |
There was a problem hiding this comment.
You don't need the mNumVSPeriods > 0 check in your if, since the for with i<numVSPeriods already accounts for that. That said, why are you moving the code to use mNumVSPeriods?
There was a problem hiding this comment.
Thanks for the catch, I adjusted the if condition.
Not sure I understand the question. Referring to lines 624ff? The idea is that if mNumVSPeriods is zero, we want to avoid calling checkStatus(GetFastestRecommendedVSSpeed(&vsIndex, &vsPeriod)); as this will again throw an exception. The altneratives would be to either adjust checkStatus() to not fail for "feature not available" or use another try-catch block
There was a problem hiding this comment.
What I mean is that the previous code was doing
checkStatus(GetNumberVSSpeeds(&numVSPeriods));
for (i=0; i<numVSPeriods; i++) {
But your version is now doing mNumVSPeriods, which seems to be wrong, due to the mNumVSPeriods++; statement in the loop, and numVSPeriods being uninitialized but still used in the loop.
This was introduced in commit 81138d5 and realesed with version R2-7.
d8cf8b5 to
5da054d
Compare
Issue
Some Andor camera models, such as the iDus, might not have a vertical dimension, reading out only a line array. For these cameras trying to read and write the Vertical Shift Speed (
VSSpeed) would not work.This parameter is initialized during start up, using the
check_status()mechanism (throwing an exception if the SDK does not report success) which prevents the further code from being properly executed, leaving the camera in a not working condition:Background
This behavior was introduced in version R2-7, with commit: 81138d5.
A very similar change, but for the vertical shift amplitude (VSAmplitude) created a similar issue as described in #49 and which was fixed with #50
Code Adjustments
Interaction with the
VSSpeedhappens on multiple locations. Unfortunately, different to theVSAmplitudecase, the availability ofVSSpeedcannot be determined by looking at themCapabilitiesreadout from the camera, which makes the fix slightly more complex. The idea is basically to do the same as thecheck_status()function, but avoid throwing an exception and continuing gracefully.Unrelated change
The first commit of this PR is an unrelated change, fixing an section of the code which was using Windows-style line endings, which is nowadays with modern editors and Git settings quite difficult to properly identify and/or is fixed "automatically". I only noticed that while trying to set up the PR, as it was not even shown to me locally as a change.
Tests
Additional Comments unrelated to this PR
2.102.30034.0and have not seen major issues, other than andorCCD:dataTask:, Data thread is running but main thread thinks we are not acquiring. #51 and one small other issueAI Disclaimer
I am not an experienced c++ developer and have used AI tools helping me to understand certain sections of the code and let the AI suggest possible solutions. While I have not used any of the suggestions in the end, my changes might be influenced by the suggestions.