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

mikeller · 2026-01-13T03:16:59Z

There seems to be two different ways that dive computers report
directional information:

report headings that are set by the user by pointing the device in
the desired direction, then pressing a button;
report the direction that the device is pointing in at set intervals
(like every 10 seconds) during the dive.

For a user-friendly visualisation, these two approaches need to be shown
in different ways in Subsurface:

show an event in the profile every time the user executes the 'set
heading' operation (and potentially at the start of the dive for the
initial heading):

show the current bearing in the information box in the profile when
the mouse pointer traverses the profile:

Subsurface will be using different ways for libdivecomputer backends to
import this data:

using DC_SAMPLE_EVENT samples with SAMPLE_EVENT_HEADING type;
using DC_SAMPLE_BEARING samples.

This changeset updates the following dive computers to use 1. to match
their type of reporting:

Heinrichs Weikamp OSTC3 / 4 / 5
ScubaPro / Uwatec Smart

The following dive computers are already using 1. correctly:

Suunto EON Steel
Suunto D9

And the following dive computer seems to be using 2. correctly:

Divesoft Freedom / Liberty (@jakubhecko are you able to confirm this?)

The following dive computers are currently using 2., but we don't have
any information if this is correct:

DiveSystem iDive
Shearwater Predator
Halcyon Symbios

Signed-off-by: Michael Keller [email protected]

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

dirkhh

looks reasonable

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-14T01:59:24Z

src/uwatec_smart_parser.c

+				dc_sample_value_t sample = {
+					.event.type = SAMPLE_EVENT_HEADING,
+					.event.flags = SAMPLE_FLAGS_SEVERITY_INFO,
+					.event.value = bearing,
+				};


Variable shadowing issue: declaring a new local variable 'sample' here shadows the outer 'sample' variable declared at line 928. This causes the 'event.time' and 'event.name' fields to not be initialized (defaulting to 0 and NULL respectively). Consider using the outer 'sample' variable instead, similar to how the bookmark event is handled at lines 1167-1172. The event.time field should be set to 0 to match the bookmark handling pattern.

Suggested change

dc_sample_value_t sample = {

.event.type = SAMPLE_EVENT_HEADING,

.event.flags = SAMPLE_FLAGS_SEVERITY_INFO,

.event.value = bearing,

};

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;

Copilot · 2026-01-14T01:59:24Z

src/uwatec_smart_parser.c

+					.event.value = bearing,
+				};
+
+				if (callback) callback(DC_SAMPLE_EVENT, &sample, userdata);


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 · 2026-01-14T01:59:25Z

src/hw_ostc_parser.c

 					sample.event.name = buf;

-					callback(eventType, &sample, userdata);
+					callback(DC_SAMPLE_EVENT, &sample, userdata);


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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-18T12:06:44Z

src/uwatec_smart_parser.c

+                sample.event.time = 0;
+				sample.event.flags = SAMPLE_FLAGS_SEVERITY_INFO;
+				sample.event.value = bearing;
+                sample.event.name = NULL;


Inconsistent indentation detected. Lines 1196 and 1199 use spaces for indentation, while the surrounding code and the rest of the file use tabs. This should be changed to use tabs to match the coding style of the rest of the file.

Suggested change

sample.event.time = 0;

sample.event.flags = SAMPLE_FLAGS_SEVERITY_INFO;

sample.event.value = bearing;

sample.event.name = NULL;

sample.event.time = 0;

sample.event.flags = SAMPLE_FLAGS_SEVERITY_INFO;

sample.event.value = bearing;

sample.event.name = NULL;

Copilot · 2026-01-18T12:06:44Z

src/hw_ostc_parser.c

-							sample.bearing = heading;
+							sample.event.type = SAMPLE_EVENT_HEADING;
 							snprintf(buf, BUFLEN, "Set compass heading [degrees]%s", sample.event.value ? "" : ": 0");
 						} else {


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 · 2026-01-18T12:06:44Z

src/uwatec_smart_parser.c

+				sample.event.value = bearing;
+                sample.event.name = NULL;
+
+				if (callback) callback(DC_SAMPLE_EVENT, &sample, userdata);


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 · 2026-01-18T12:06:45Z

src/hw_ostc_parser.c

 					sample.event.name = buf;

-					callback(eventType, &sample, userdata);
+					callback(DC_SAMPLE_EVENT, &sample, userdata);


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 · 2026-01-18T12:06:45Z

src/hw_ostc_parser.c

 				}

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


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);

There seems to be two different ways that dive computers report directional information: 1. report headings that are set by the user by pointing the device in the desired direction, then pressing a button; 2. report the direction that the device is pointing in at set intervals (like every 10 seconds) during the dive. For a user-friendly visualisation, these two approaches need to be shown in different ways in Subsurface: 1. show an event in the profile every time the user executes the 'set heading' operation (and potentially at the start of the dive for the initial heading); 2. show the current bearing in the information box in the profile when the mouse pointer traverses the profile. Subsurface will be using different ways for libdivecomputer backends to import this data: 1. using `DC_SAMPLE_EVENT` samples with `SAMPLE_EVENT_HEADING` type; 2. using `DC_SAMPLE_BEARING` samples. This changeset updates the following dive computers to use 1. to match their type of reporting: - Heinrichs Weikamp OSTC3 / 4 / 5 - ScubaPro / Uwatec Smart The following dive computers are already using 1. correctly: - Suunto EON Steel - Suunto D9 And the following dive computer seems to be using 2. correctly: - Divesoft Freedom / Liberty The following dive computers are currently using 2., but we don't have any information if this is correct: - DiveSystem iDive - Shearwater Predator - Halcyon Symbios Signed-off-by: Michael Keller <[email protected]>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-18T12:17:15Z

src/hw_ostc_parser.c

 					if (value & OSTC4_COMPASS_HEADING_CLEARED_FLAG) {
 						snprintf(buf, BUFLEN, "Cleared compass heading");
 					} else {
+						sample.event.value = heading;


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 · 2026-01-18T12:17:15Z

src/hw_ostc_parser.c

+						sample.event.value = heading;

 						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 {
-							sample.event.value = heading;
 							snprintf(buf, BUFLEN, "Logged compass heading [degrees]%s", sample.event.value ? "" : ": 0");
 						}


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 AI review requested due to automatic review settings January 13, 2026 03:17

Copilot AI reviewed Jan 13, 2026

View reviewed changes

dirkhh approved these changes Jan 13, 2026

View reviewed changes

mikeller requested a review from Copilot January 14, 2026 01:55

Copilot started reviewing on behalf of mikeller January 14, 2026 01:56 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

mikeller force-pushed the separate_bearing_heading branch from 534e7d5 to ad17680 Compare January 18, 2026 11:32

mikeller requested a review from Copilot January 18, 2026 11:51

Copilot started reviewing on behalf of mikeller January 18, 2026 12:03 View session

Copilot AI reviewed Jan 18, 2026

View reviewed changes

mikeller force-pushed the separate_bearing_heading branch from ad17680 to f93a3fa Compare January 18, 2026 12:12

mikeller requested a review from Copilot January 18, 2026 12:12

Copilot started reviewing on behalf of mikeller January 18, 2026 12:13 View session

Copilot AI reviewed Jan 18, 2026

View reviewed changes

mikeller merged commit dde0960 into subsurface:Subsurface-DS9 Jan 18, 2026
14 checks passed

mikeller deleted the separate_bearing_heading branch January 18, 2026 12:23

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

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

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

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

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

Conversation

mikeller commented Jan 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

dirkhh left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants