Menu

#5 Add sbapi_spectrometer_get_maximum_intensity

3.0
closed
None
2015-11-18
2015-10-20
No

This adds sbapi_spectrometer_get_maximum_intensity.
It returns the hardcoded maxIntensity value for the spectrometer feature.

1 Attachments

Discussion

  • Aaron Gage

    Aaron Gage - 2015-11-14

    Thank you for contributing this patch.

    I have merged this into the amg-merge-5 branch and made some modifications. I'd like to explain the motivation for the modifications briefly.

    I changed the return type for the getMaximumIntensity() methods to double so that this would better match the spectrum intensity type for getFormattedSpectrum(). It seems to me that the most likely use for this function is to determine if the spectrometer is saturated, and the vast majority of the time, this is going to be done to a fully formatted spectrum.

    I could perhaps see a reason to be able to pull the raw intensity maximum, but that is less likely to be needed at the API layer. I am open to adding methods that get this value as well (though I'd like to see if there are any cases where it makes a difference), but in that case, I'd recommend that the return type be unsigned long for the new functions and that they have a "Counts" suffix to make it clear they refer to the raw output of an ADC.

    The only real danger I see to making these doubles is that doing equality comparisons on doubles is always a little dodgy. However, I think that for the sake of consistency and future-proofing, double is probably still the better type.

    I'll leave this in the branch for a little while to allow you or others time to comment. If anyone needs it urgently, the branch seems to be usable: I have tested with an STS and USB2000+ and verified there are no memory leaks under valgrind. If I don't see any traffic on this ticket for a while, I'll go ahead and merge it back as it now stands.

     

    Last edit: Aaron Gage 2015-11-14
  • Andreas Poehlmann

    Nice, thanks for creating the branch. Changing the return type to double makes sense.

     
  • Aaron Gage

    Aaron Gage - 2015-11-18

    The branch was merged (225:228) back to trunk at revision 230. Closing this issue. Thanks again.

     
  • Aaron Gage

    Aaron Gage - 2015-11-18
    • status: open --> closed
    • assigned_to: Aaron Gage
     

Log in to post a comment.