-
Notifications
You must be signed in to change notification settings - Fork 105
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.
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
base: Subsurface-DS9
Are you sure you want to change the base?
Separate the Reporting of 'bearing' vs. 'heading. #99
Conversation
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 <github@ike.ch>
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
dirkhh
left a comment
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.
looks reasonable
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
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.
| dc_sample_value_t sample = { | ||
| .event.type = SAMPLE_EVENT_HEADING, | ||
| .event.flags = SAMPLE_FLAGS_SEVERITY_INFO, | ||
| .event.value = bearing, | ||
| }; |
Copilot
AI
Jan 14, 2026
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.
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.
| 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; |
| .event.value = bearing, | ||
| }; | ||
|
|
||
| if (callback) callback(DC_SAMPLE_EVENT, &sample, userdata); |
Copilot
AI
Jan 14, 2026
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.
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).
| if (callback) callback(DC_SAMPLE_EVENT, &sample, userdata); | |
| if (callback) callback (DC_SAMPLE_EVENT, &sample, userdata); |
| sample.event.name = buf; | ||
|
|
||
| callback(eventType, &sample, userdata); | ||
| callback(DC_SAMPLE_EVENT, &sample, userdata); |
Copilot
AI
Jan 14, 2026
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.
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.).
| callback(DC_SAMPLE_EVENT, &sample, userdata); | |
| callback (DC_SAMPLE_EVENT, &sample, userdata); |
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:
heading' operation (and potentially at the start of the dive for the
initial heading):
the mouse pointer traverses the profile:
Subsurface will be using different ways for libdivecomputer backends to
import this data:
using
DC_SAMPLE_EVENTsamples withSAMPLE_EVENT_HEADINGtype;using
DC_SAMPLE_BEARINGsamples.This changeset updates the following dive computers to use 1. to match
their type of reporting:
The following dive computers are already using 1. correctly:
And the following dive computer seems to be using 2. correctly:
The following dive computers are currently using 2., but we don't have
any information if this is correct:
Signed-off-by: Michael Keller github@ike.ch