Skip to content

Separate the Reporting of 'bearing' vs. 'heading. #99

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
mikeller merged 1 commit into from
Jan 18, 2026
Merged

Separate the Reporting of 'bearing' vs. 'heading. #99

Show file tree
Hide file tree
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
8 changes: 3 additions & 5 deletions src/hw_ostc_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,6 @@ hw_ostc_parser_internal_foreach (hw_ostc_parser_t *parser, dc_sample_callback_t

if (callback) {
unsigned int value = array_uint16_le(data + offset);
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

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

Inconsistent function call style. The array_uint16_le function call should have a space before the opening parenthesis to match the coding style used throughout the rest of the file (e.g., lines 369, 495, 498, 573, 576, etc.). It should be 'array_uint16_le (data + offset)' instead of 'array_uint16_le(data + offset)'.

Suggested change
unsigned int value = array_uint16_le(data + offset);
unsigned int value = array_uint16_le (data + offset);

Copilot uses AI. Check for mistakes.
dc_sample_type_t eventType = DC_SAMPLE_EVENT;
dc_sample_value_t sample = {
.event.type = SAMPLE_EVENT_STRING,
.event.flags = SAMPLE_FLAGS_SEVERITY_INFO,
Expand All @@ -1194,21 +1193,20 @@ hw_ostc_parser_internal_foreach (hw_ostc_parser_t *parser, dc_sample_callback_t
if (value & OSTC4_COMPASS_HEADING_CLEARED_FLAG) {
snprintf(buf, BUFLEN, "Cleared compass heading");
} else {
sample.event.value = heading;
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

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

The placement of "sample.event.value = heading" is incorrect. When the OSTC4_COMPASS_HEADING_CLEARED_FLAG is set (line 1193), the heading was cleared, so setting sample.event.value to the heading value doesn't make sense. In the original code, sample.event.value was only set in the else branch (when the heading was not cleared). This line should be moved inside the else block, after line 1195.

Copilot uses AI. Check for mistakes.

if (value & OSTC4_COMPASS_HEADING_SET_FLAG) {
eventType = DC_SAMPLE_BEARING;
sample.bearing = heading;
sample.event.type = SAMPLE_EVENT_HEADING;
snprintf(buf, BUFLEN, "Set compass heading [degrees]%s", sample.event.value ? "" : ": 0");
} else {
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

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

The "Logged compass heading" case (else branch) maintains the event type as SAMPLE_EVENT_STRING, which means it will be displayed as a string event rather than as a heading event or bearing sample. Based on the PR description, if "Logged compass heading" represents user-set headings (like "Set compass heading"), it should also use SAMPLE_EVENT_HEADING. However, if it represents automatic periodic logging of the current bearing, it might need different handling. Please verify that SAMPLE_EVENT_STRING is the correct type for this case, or if it should be SAMPLE_EVENT_HEADING to match the "Set compass heading" behavior.

Suggested change
} else {
} else {
sample.event.type = SAMPLE_EVENT_HEADING;

Copilot uses AI. Check for mistakes.
sample.event.value = heading;
snprintf(buf, BUFLEN, "Logged compass heading [degrees]%s", sample.event.value ? "" : ": 0");
}
Comment on lines +1196 to 1203
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

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

The "Logged compass heading" case (when OSTC4_COMPASS_HEADING_SET_FLAG is not set) is not being handled correctly. According to the PR description, there are two approaches: 1) user-set headings (should use SAMPLE_EVENT_HEADING), and 2) automatic periodic bearing reports (should use DC_SAMPLE_BEARING). The code currently only converts the "Set compass heading" case to SAMPLE_EVENT_HEADING, but leaves the "Logged compass heading" case as SAMPLE_EVENT_STRING with a numeric value, which is inconsistent. Based on the PR description, "Logged compass heading" represents periodic automatic bearing reports and should be reported using DC_SAMPLE_BEARING instead of as a STRING event.

Copilot uses AI. Check for mistakes.

}

sample.event.name = buf;

callback(eventType, &sample, userdata);
callback(DC_SAMPLE_EVENT, &sample, userdata);
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Inconsistent spacing: this callback invocation is missing a space before the opening parenthesis, unlike most other callback invocations in this function (e.g., lines 984, 991, 997, 1003, 1009, 1068, 1102, etc.).

Suggested change
callback(DC_SAMPLE_EVENT, &sample, userdata);
callback (DC_SAMPLE_EVENT, &sample, userdata);

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

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

Inconsistent function call style. The callback invocation should have a space before the opening parenthesis to match the coding style used throughout the rest of the file (e.g., lines 1102, 1126, 1141, 1172). It should be 'callback (DC_SAMPLE_EVENT, &sample, userdata);' instead of 'callback(DC_SAMPLE_EVENT, &sample, userdata);'.

Suggested change
callback(DC_SAMPLE_EVENT, &sample, userdata);
callback (DC_SAMPLE_EVENT, &sample, userdata);

Copilot uses AI. Check for mistakes.
}

offset += 2;
Expand Down
9 changes: 7 additions & 2 deletions src/uwatec_smart_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1192,8 +1192,13 @@ uwatec_smart_parse (uwatec_smart_parser_t *parser, dc_sample_callback_t callback
}

if (have_bearing) {
sample.bearing = bearing;
if (callback) callback (DC_SAMPLE_BEARING, &sample, userdata);
sample.event.type = SAMPLE_EVENT_HEADING;
sample.event.time = 0;
sample.event.flags = SAMPLE_FLAGS_SEVERITY_INFO;
sample.event.value = bearing;
sample.event.name = NULL;

if (callback) callback(DC_SAMPLE_EVENT, &sample, userdata);
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Inconsistent spacing: this callback invocation is missing a space before the opening parenthesis, unlike other callback invocations in this function (e.g., lines 1149, 1158, 1164, 1172, 1177, 1185, 1191, 1207).

Suggested change
if (callback) callback(DC_SAMPLE_EVENT, &sample, userdata);
if (callback) callback (DC_SAMPLE_EVENT, &sample, userdata);

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

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

Inconsistent function call style. The callback invocation should have a space before the opening parenthesis to match the coding style used throughout the rest of the file (e.g., lines 1149, 1158, 1164, 1172, 1177, 1185, 1191, 1207). It should be 'callback (DC_SAMPLE_EVENT, &sample, userdata);' instead of 'callback(DC_SAMPLE_EVENT, &sample, userdata);'.

Suggested change
if (callback) callback(DC_SAMPLE_EVENT, &sample, userdata);
if (callback) callback (DC_SAMPLE_EVENT, &sample, userdata);

Copilot uses AI. Check for mistakes.
have_bearing = 0;
}

Expand Down
Loading