Skip to content

Shearwater: Display Dive Mode. #89

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 23, 2025
Merged

Shearwater: Display Dive Mode. #89

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions src/shearwater_predator_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
#define M_OC_REC 6
#define M_FREEDIVE 7

#define SM_3_GAS_NX 0
#define SM_AIR 1
#define SM_NX 2
#define SM_OC_REC 3

#define AI_OFF 0
#define AI_HPCCR 4
#define AI_ON 5
Expand Down Expand Up @@ -428,12 +433,12 @@ static void print_calibration(shearwater_predator_parser_t *parser)
{
for (size_t i = 0; i < 3; ++i) {
if (parser->calibrated & (1 << i)) {
static const char *name[] = {
static const char *names[] = {
"Sensor 1 calibration [bar / V]",
"Sensor 2 calibration [bar / V]",
"Sensor 3 calibration [bar / V]",
};
dc_field_add_string_fmt(&parser->cache, name[i], "%.2f", parser->calibration[i] * 1000);
dc_field_add_string_fmt(&parser->cache, names[i], "%.2f", parser->calibration[i] * 1000);
}
}
}
Expand Down Expand Up @@ -522,6 +527,7 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)

unsigned int offset = headersize;
unsigned int length = size - footersize;
unsigned int sub_mode = SM_OC_REC;
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The variable 'sub_mode' is initialized with a default value but only used conditionally much later in the code. Consider initializing it closer to where it's used or documenting why this specific default value is chosen.

Suggested change
unsigned int sub_mode = SM_OC_REC;

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The sub_mode variable is initialized to SM_OC_REC but only gets updated conditionally. Consider initializing it to a more neutral value or adding a comment explaining why SM_OC_REC is the default.

Suggested change
unsigned int sub_mode = SM_OC_REC;
unsigned int sub_mode = UNDEFINED; // Initialize to UNDEFINED to indicate it has not been set yet.

Copilot uses AI. Check for mistakes.
unsigned int stack_time_total_s = 0;
unsigned int stack_time_remaining_s;
while (offset + parser->samplesize <= length) {
Expand Down Expand Up @@ -718,6 +724,11 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)
dc_field_add_string_fmt(&parser->cache, "Remaining stack time at start [hh:mm:ss]", "%d:%02d:%02d", stack_time_remaining_s / 3600, (stack_time_remaining_s / 60) % 60, stack_time_remaining_s % 60);
}
}
if (logversion >= 12) {
if (divemode == M_OC_REC) {
sub_mode = data[offset + 5];
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number '5' for the offset should be defined as a named constant to improve code readability and maintainability.

Suggested change
sub_mode = data[offset + 5];
sub_mode = data[offset + SUB_MODE_OFFSET];

Copilot uses AI. Check for mistakes.
}
}
if (logversion >= 13) {
tank[0].enabled = data[offset + 19];
memcpy (tank[0].name, data + offset + 20, sizeof (tank[0].name));
Expand Down Expand Up @@ -904,14 +915,13 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)
break;
}

dc_status_t rc = DC_STATUS_SUCCESS;
if (parser->needs_divecan_calibration_estimate) {
struct dc_parser_sensor_calibration_t data = { 0 };

rc = shearwater_predator_parser_samples_foreach(abstract, NULL, (void *)&data);
dc_status_t rc = shearwater_predator_parser_samples_foreach(abstract, NULL, (void *)&data);

bool calibrated = false;
if (data.sum_ppo2 != 0) {
if (rc == DC_STATUS_SUCCESS && data.sum_ppo2 != 0) {
double calibration = data.sum_calculated_ppo2 / data.sum_ppo2;
if (calibration < 0.98 || calibration > 1.02) {
// The calibration scaling is significant, use it.
Expand All @@ -933,6 +943,29 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)
parser->needs_divecan_calibration_estimate = false;
}

static const char *name = "Divemode";
if (divemode == M_OC_REC) {
static const char *oc_rec_modes[] = {
"3 Gas Nitrox",
"Air",
"Nitrox",
"OC Recreational",
};
dc_field_add_string_fmt(&parser->cache, name, "%s", sub_mode <= SM_OC_REC ? oc_rec_modes[sub_mode] : "Unknown divemode");
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array bounds check uses <= SM_OC_REC but the oc_rec_modes array has 4 elements (indices 0-3). If sub_mode equals SM_OC_REC (which is 3), this will access the last valid index, but the logic suggests this should be a strict bounds check.

Suggested change
dc_field_add_string_fmt(&parser->cache, name, "%s", sub_mode <= SM_OC_REC ? oc_rec_modes[sub_mode] : "Unknown divemode");
dc_field_add_string_fmt(&parser->cache, name, "%s", sub_mode < 4 ? oc_rec_modes[sub_mode] : "Unknown divemode");

Copilot uses AI. Check for mistakes.
} else {
static const char *modes[] = {
"CC / BO",
"OC Technical",
"Gauge",
"PPO2 Display",
"SC / BO",
"CC / BO 2",
"OC Recreational",
"Freedive",
};
dc_field_add_string_fmt(&parser->cache, name, "%s", divemode <= M_FREEDIVE ? modes[divemode] : "Unknown divemode");
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar array bounds issue: the modes array has 8 elements (indices 0-7) and M_FREEDIVE is 7. The <= comparison is correct here, but this inconsistency with the sub_mode check above suggests a potential logic error in one of them.

Suggested change
dc_field_add_string_fmt(&parser->cache, name, "%s", divemode <= M_FREEDIVE ? modes[divemode] : "Unknown divemode");
dc_field_add_string_fmt(&parser->cache, name, "%s", divemode < sizeof(modes) / sizeof(modes[0]) ? modes[divemode] : "Unknown divemode");

Copilot uses AI. Check for mistakes.
}

return DC_STATUS_SUCCESS;
}

Expand Down
Loading