-
Notifications
You must be signed in to change notification settings - Fork 87
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
Shearwater: Fix Sensor Calibration for DiveCAN Controllers. #59
Shearwater: Fix Sensor Calibration for DiveCAN Controllers. #59
Conversation
// calibration values (2100), they are probably not calibrated | ||
// properly. To avoid returning incorrect ppO2 values to the | ||
// application, they are manually disabled (e.g. marked as | ||
// uncalibrated). |
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 comment seems to explain well why this makes sense.
Am I understanding you correctly that 2100 could, in fact, be a valid calibration value?
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.
If I remember correctly, there are certain models (those DiveCAN models?) where the calibration is done externally. Thus the Shearwater doesn't even know the calibration values and in that case all values are set to their default value (2100). Using those defaults values for the calibration anyway, will result in ppO2 values that are wrong and that's why they are manually disabled here. Theoretically it's possible that a real calibration results in a value of 2100, but I think that in practice the chance of ALL values being exactly 2100 is extremely low.
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.
Am I understanding you correctly that 2100 could, in fact, be a valid calibration value?
Yes, of course 2100 can be a valid calibration value - it just happens to be the default value, and a value that is within the expected output range for a lot of the sensors used in CCRs (i.e. 10 mV corresponds to a ppO2 of 0.21 bar).
To make matters worse, on Shearwater dive computers that were built with a 'DiveCAN' interface to act as controllers on a number of CCR models, the calibration and conversion to ppO2 is done in the CCR and not in the controller, and these models always assume a 'calibration' of 2100 for all sensors that were detected, and return the cell readings in the assumed mV range that this yields. So before this fix Subsurface is not really usable with these divecomputers, as all CCR ppO2 information for them is discarded - and they are not likely to ever be used in open circuit mode.
the comment seems to explain well why this makes sense.
I don't know - we already read and apply the information that we are getting from the dive computer about which sensors have a valid calibration (stored in calibrated
), and ignore the ones that don't.
After all, the dive computer uses the information from the sensors that it thinks are calibrated correctly as input for the voting logic which then yields the assumed ppO2 value that is used to drive the decompression calculation - so discarding these values will result in the decompression information in Subsurface being based on data that is different from what was shown to the diver.
To me it seems to be wrong to wilfully throw away information that was assumed by the dive computer to be accurate, and used in its calculation.
We could maybe add a warning if we get a valid calibration, but all values are 2100 - but since this will apply to all dives logged by a DiveCAN controller, and we currently don't have a way to detect these models (except for this), I have not done this.
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.
So you say the DiveCAN models have already adjusted the mV values, such that after the "calibration" with the default value of 2100, the resulting ppO2 value is correct, right? That sounds reasonable.
The problem is that I added this feature after receiving reports from users that the ppO2 values were incorrect. If we remove this again, then those users will get incorrect ppO2 values again. Unfortunately I don't remember whether this were DiveCAN models or not. I'll need to look at the discussions around that time again.
I quickly checked a few of my datasets, and I'm certainly seeing some inconsistent results. For example a voted value of 0.70, while the 3 sensors report 0.78, 0.80 and 0.78. That makes no sense to me. How can the voted value be completely outside the range of the 3 sensors? The voted value is stored directly as ppO2 and doesn't need any calibration, so my only explanation is that these default calibration values for the sensors are wrong here.
Do you happen to have data from a DiveCAN model?
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.
So you say the DiveCAN models have already adjusted the mV values, such that after the "calibration" with the default value of 2100, the resulting ppO2 value is correct, right? That sounds reasonable.
Yes, that is what I am seeing when downloading from a DiveCAN controller - which makes sense as this way they can supply ppO2 values in a way that doesn't break the format specification, even without knowing the calibration values.
The problem is that I added this feature after receiving reports from users that the ppO2 values were incorrect. If we remove this again, then those users will get incorrect ppO2 values again. Unfortunately I don't remember whether this were DiveCAN models or not. I'll need to look at the discussions around that time again.
Do you have any logs from dive computers that report all cells as calibrated correctly while supplying incorrect calibration values?
And even in this case, the graph of the recorded ppO2 sensor readings is not completely incorrect - it just has an incorrect scaling in ppO2 attached. And as a rebreather diver I would argue that having this graph with an incorrect scaling is preferable (by a lot) over just throwing away the data because we cannot establish the scaling. A lot of post-dive diagnosis, like identifying a misbehaving sensor, can be done without the scaling.
I quickly checked a few of my datasets, and I'm certainly seeing some inconsistent results. For example a voted value of 0.70, while the 3 sensors report 0.78, 0.80 and 0.78. That makes no sense to me. How can the voted value be completely outside the range of the 3 sensors? The voted value is stored directly as ppO2 and doesn't need any calibration, so my only explanation is that these default calibration values for the sensors are wrong here.
It will probably be worthwhile to look at the sequence of values as graphs instead of point values - from what I can tell a lot of dive computers apply a low pass filter to the result of the sensor reading voting logic before using it for decompression calculations, in order to avoid the TTS / next stop values jumping around wildly when the ppO2 fluctuates. So this point value could be a capture of a situation where the sensor readings are increasing, but the ppO2 used for calculations is lagging behind because of the lowpass filter.
Do you happen to have data from a DiveCAN model?
It's not my computer, but I can ask the owner if he's ok to share.
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.
I was guilty for this way back in time. My plan was to be able to do analysis on individual sensor values but I never came around to this.
As I remember it, we don't have real ppo2 values from the sensors, just mV and thus we need the calibration value to convert those to ppo2 to be able to report them.
Also, as I remember it, we will always report the "voted" ppo2 value if we have it, which as far as I remember we should always have for dives with sensors, and that value is stored as ppo2.
The current sensor interface doesn't expose the raw mV values. We could adjust the interface to expose those, so one can graph those and look at those even if we don't have valid calibration values.
Maybe they save the DiveCAN calibration values somewhere else? Or maybe they just get the ppo2 and mV values over DiveCAN and just saves the mV and the voted ppo2, in which case we can't without (some) guessing reproduce the ppo2 values used for each sensor.
I feel like it's a quite good feature to be able to analyze the individual sensor values, to gauge response time, and potentially find any early signs of current limitations in the sensors. Now when I've started diving rebreather myself I might get around to implement such a feature.
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.
As I remember it, we don't have real ppo2 values from the sensors, just mV and thus we need the calibration value to convert those to ppo2 to be able to report them.
Correct.
Also, as I remember it, we will always report the "voted" ppo2 value if we have it, which as far as I remember we should always have for dives with sensors, and that value is stored as ppo2.
As far as I know yes.
The current sensor interface doesn't expose the raw mV values. We could adjust the interface to expose those, so one can graph those and look at those even if we don't have valid calibration values.
The problem there is the storage format - we currently persist sensor1
(which is the 'voted' value) ... sensorX
, and the value is always in bar. This is not great, but any change away from this will be breaking backwards compatibility, which sucks, in particular for the cloud storage.
Maybe they save the DiveCAN calibration values somewhere else? Or maybe they just get the ppo2 and mV values over DiveCAN and just saves the mV and the voted ppo2, in which case we can't without (some) guessing reproduce the ppo2 values used for each sensor.
Yes - with DiveCAN the calibration is saved on the O2 controller inside the unit. This way the unit can continue to function normally even when the handset is cable is severed or unplugged, or the handset battery dies.
I suspect that with DiveCAN the O2 controller uses the handset as a 'dumb display' and directly sends it the ppO2 values to be shown - so, in order to avoid breaking the specification of their log format, Shearwater then uses an assumed default calibration value to convert ppO2 values into the mV values that are logged.
I feel like it's a quite good feature to be able to analyze the individual sensor values, to gauge response time, and potentially find any early signs of current limitations in the sensors. Now when I've started diving rebreather myself I might get around to implement such a feature.
I definitely think the individual sensor graphs are important.
Also, welcome to the dark side. 😄
Remove the code that forces hiding of O2 sensor readings despite the sensors being reported as properly calibrated by the divecomputer. This fixes the problem of no sensor readings showing on DiveCAN controllers due to the fact that the sensor calibration values are returned as defaults on these devices. Also increase the timeout for data reads - this seems to improve the reliability when reading logs of long dives. Signed-off-by: Michael Keller <[email protected]>
52cdaa3
to
ebf2d9b
Compare
Add the calibration values for ppO2 sonsors in CCR mode as extra info fields. Signed-off-by: Michael Keller <[email protected]>
ebf2d9b
to
a7fb341
Compare
Remove the code that forces hiding of O2 sensor readings despite the sensors being reported as properly calibrated by the divecomputer. This fixes the problem of no sensor readings showing on DiveCAN controllers due to the fact that the sensor calibration values are returned as defaults on these devices.
Also increase the timeout for data reads - this seems to improve the reliability when reading logs of long dives.