-
Notifications
You must be signed in to change notification settings - Fork 104
Shearwater: Add guessing of the calibration for DiveCAN controllers. #81
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: Add guessing of the calibration for DiveCAN controllers. #81
Conversation
1f08de8 to
84d3fd1
Compare
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.
Pull Request Overview
This PR introduces logic to detect when a DiveCAN controller returns the default calibration value and then estimate a more accurate calibration scaling factor.
- Added a flag and related logic to determine when sensor calibration values are default.
- Calculated an estimated calibration scaling factor to adjust sensor readings when necessary.
src/shearwater_predator_parser.c
Outdated
| double calibration_scaling_factor = calculated_ppo2 / (ppo2_sum / ppo2_count); | ||
| if (calibration_scaling_factor < 0.95 || calibration_scaling_factor > 1.05) { | ||
| // The calibration scaling is significant, use it. | ||
| calibration *= calibration_scaling_factor; |
Copilot
AI
May 22, 2025
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.
There's a potential division by zero when ppo2_count is zero. Please add a condition to ensure that ppo2_count is greater than zero before performing the division.
| double calibration_scaling_factor = calculated_ppo2 / (ppo2_sum / ppo2_count); | |
| if (calibration_scaling_factor < 0.95 || calibration_scaling_factor > 1.05) { | |
| // The calibration scaling is significant, use it. | |
| calibration *= calibration_scaling_factor; | |
| if (ppo2_count > 0) { | |
| double calibration_scaling_factor = calculated_ppo2 / (ppo2_sum / ppo2_count); | |
| if (calibration_scaling_factor < 0.95 || calibration_scaling_factor > 1.05) { | |
| // The calibration scaling is significant, use it. | |
| calibration *= calibration_scaling_factor; | |
| } |
If the Shearwater default calibration factor (2100) is returned for the ppO2 calibration factor, we know that the dive computer might be a DiveCAN controller, where the calibration is not available on the dive computer. In this case, calculate the scaling factor that is required to make the averaged sensor reading match the dive computer reported ppO2, and use this instead of the returned calibration factor. This will give us: - individual sensor graphs that show the sensor behaviour, and have the approximately correct ppO2 reading; - a calculated ppO2 used by Subsurface that is close to what the dive computer would have used, without the smoothing seen in the calculated ppO2 returned by the dive computer. Signed-off-by: Michael Keller <[email protected]>
84d3fd1 to
482b546
Compare
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.
Pull Request Overview
Adds logic to detect when a Shearwater dive computer returns its default sensor calibration (2100) and treats it as a DiveCAN controller. In that case, the parser walks the sample data to estimate a more accurate per-sensor calibration factor and applies it to each PP02 reading.
- Introduces
SENSOR_CALIBRATION_DEFAULTand aneeds_divecan_calibration_estimateflag - Captures and sums raw vs. calculated ppO₂ in a temporary struct via
samples_foreach - Adjusts both the cached calibration prints and the per-sample PP02 callback to use the estimated factor
Comments suppressed due to low confidence (2)
src/shearwater_predator_parser.c:899
- [nitpick] The local variable
datashadows the parser's raw-data pointer and is confusing; consider renaming this tocalib_dataor similar for clarity.
struct dc_parser_sensor_calibration_t data = { 0 };
src/shearwater_predator_parser.c:898
- The new DiveCAN calibration-estimation branch isn’t covered by existing tests; add unit or integration tests that simulate default calibration values and verify both estimation and fallback paths.
if (parser->needs_divecan_calibration_estimate) {
src/shearwater_predator_parser.c
Outdated
| double calibration = data.sum_calculated_ppo2 / data.sum_ppo2; | ||
| if (calibration < 0.95 || calibration > 1.05) { | ||
| // The calibration scaling is significant, use it. | ||
| calibration *= SENSOR_CALIBRATION_DEFAULT / 100000.0; | ||
| parser->calibration[0] = calibration; | ||
| parser->calibration[1] = calibration; | ||
| parser->calibration[2] = calibration; | ||
|
|
||
| dc_field_add_string_fmt(&parser->cache, "Estimated (DiveCAN?) sensor calibration [bar / V]", "%.2f", calibration * 1000); | ||
| } else { | ||
| print_calibration(parser); |
Copilot
AI
Jun 1, 2025
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.
Guard against division by zero on data.sum_ppo2 before computing the estimated calibration factor, otherwise you may produce NaN or infinities when no samples are accumulated.
| double calibration = data.sum_calculated_ppo2 / data.sum_ppo2; | |
| if (calibration < 0.95 || calibration > 1.05) { | |
| // The calibration scaling is significant, use it. | |
| calibration *= SENSOR_CALIBRATION_DEFAULT / 100000.0; | |
| parser->calibration[0] = calibration; | |
| parser->calibration[1] = calibration; | |
| parser->calibration[2] = calibration; | |
| dc_field_add_string_fmt(&parser->cache, "Estimated (DiveCAN?) sensor calibration [bar / V]", "%.2f", calibration * 1000); | |
| } else { | |
| print_calibration(parser); | |
| double calibration = 0.0; | |
| if (data.sum_ppo2 != 0) { | |
| calibration = data.sum_calculated_ppo2 / data.sum_ppo2; | |
| if (calibration < 0.95 || calibration > 1.05) { | |
| // The calibration scaling is significant, use it. | |
| calibration *= SENSOR_CALIBRATION_DEFAULT / 100000.0; | |
| parser->calibration[0] = calibration; | |
| parser->calibration[1] = calibration; | |
| parser->calibration[2] = calibration; | |
| dc_field_add_string_fmt(&parser->cache, "Estimated (DiveCAN?) sensor calibration [bar / V]", "%.2f", calibration * 1000); | |
| } else { | |
| print_calibration(parser); | |
| } | |
| } else { | |
| // Handle the case where no samples are accumulated. | |
| // Optionally log a warning or assign a default calibration value. | |
| calibration = SENSOR_CALIBRATION_DEFAULT / 1000.0; // Default calibration value | |
| parser->calibration[0] = calibration; | |
| parser->calibration[1] = calibration; | |
| parser->calibration[2] = calibration; |
| struct dc_parser_sensor_calibration_t { | ||
| double sum_ppo2; | ||
| double sum_calculated_ppo2; | ||
| unsigned int ppo2_sample_count; |
Copilot
AI
Jun 1, 2025
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.
[nitpick] The ppo2_sample_count field in dc_parser_sensor_calibration_t is incremented but never used after collection; consider removing it or adding logic to use it (e.g., logging or validation).
src/shearwater_predator_parser.c
Outdated
| rc = shearwater_predator_parser_samples_foreach(abstract, NULL, (void *)&data); | ||
|
|
||
| double calibration = data.sum_calculated_ppo2 / data.sum_ppo2; | ||
| if (calibration < 0.95 || calibration > 1.05) { |
Copilot
AI
Jun 1, 2025
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.
[nitpick] The thresholds 0.95 and 1.05 are magic numbers—consider defining named constants (e.g., DIVECAN_CALIBRATION_MIN_RATIO) to clarify their significance.
| if (calibration < 0.95 || calibration > 1.05) { | |
| if (calibration < DIVECAN_CALIBRATION_MIN_RATIO || calibration > DIVECAN_CALIBRATION_MAX_RATIO) { |
d1be4bb to
f278a9d
Compare
Signed-off-by: Michael Keller <[email protected]>
f278a9d to
8c52ffd
Compare
If the Shearwater default calibration factor (2100) is returned for the
ppO2 calibration factor, we know that the dive computer might be a DiveCAN
controller, where the calibration is not available on the dive computer.
In this case, calculate the scaling factor that is required to make the
averaged sensor reading match the dive computer reported ppO2, and use
this instead of the returned calibration factor.
This will give us:
approximately correct ppO2 reading;
computer would have used, without the smoothing seen in the calculated
ppO2 returned by the dive computer.
Signed-off-by: Subsurface CI [email protected]