Skip to content
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

Added detection and status messages for bailout to OC / switch back to CC for CCR dives. #41

Conversation

mikeller
Copy link
Member

@mikeller mikeller commented Dec 8, 2022

This might need a bit of discussion.
As a CCR diver I would like to see the ability to properly track the 'on loop' / 'bailed out to open circuit' status directly in Subsurface, because this is an important (or even the most important) bit of status information during a CCR dive. This should extend to the use of the correct ppO2 / gas mix in the deco ceiling / tissue model calculation. My idea for how to do this would be to track the 'type' ('diluent' / 'OC bailout') for every gasmix / tank that is reported by the dive computer. Most CCR capable dive computers that I am familiar with require the user to enter two different gas lists for diluent and bailout, so this should work with the existing libdivecomputer API for these.
Unfortunately I think making this change in Subsurface will require a bit of work, as the libdivecomputer field capable of tracking the 'type' of a gas or tank (cache->tankinfo[]) does not seem to be consumed at all in Subsurface. So this pull request is just providing a prerequisite for the change in Subsurface by populating tankinfo[]. In addition to this it also triggers a message on every switch from CC to OC and back, at least giving a visual indication of these diver triggered events. The messages can probably be removed from libdivecomputer again once 'loop status' tracking has been added to Subsurface.

image

Also included is a fix of the tab expansion mess that I created in #40. Apologies for this, I've switched to using a custom .vimrc for this project now.

Signed-off-by: Michael Keller github@ike.ch

@mikeller mikeller force-pushed the added_garmin_ccr_bailout_messages branch 4 times, most recently from bd1dca7 to 39a9109 Compare January 17, 2023 06:04
mikeller added a commit to mikeller/libdc that referenced this pull request Jan 17, 2023
Pushed access to cached fields down into `field-cache.c` from parsers
that use the field cache.
As a result, all cached fields will be available to Subsurface in computer models that use the field cache.
In particular this means that, together with subsurface#41, the proper use type (diluent / OC bailout) of tanks will be shown for the Garmin Descent dive computers (others to follow).

Signed-off-by: Michael Keller <github@ike.ch>
mikeller added a commit to mikeller/libdc that referenced this pull request Jan 17, 2023
Pushed access to cached fields down into `field-cache.c` from parsers
that use the field cache.
As a result, all cached fields will be available to Subsurface in computer models that use the field cache.
In particular this means that, together with subsurface#41, the proper use type (diluent / OC bailout) of tanks will be shown for the Garmin Descent dive computers (others to follow).

Signed-off-by: Michael Keller <github@ike.ch>
@mikeller mikeller force-pushed the added_garmin_ccr_bailout_messages branch from 8742f55 to 65bab71 Compare January 17, 2023 22:15
@mikeller
Copy link
Member Author

mikeller commented Feb 8, 2023

I hate to ask, but is it possible to get some traction on this one?
I am trying to improve CCR related gas logging for other dive computer models as well, and getting this (and #43) reviewed will help to clarify that the direction this is going is right.

@dirkhh
Copy link
Collaborator

dirkhh commented Feb 9, 2023

I poked Linus - he doesn't always see GitHub notifications.

@torvalds
Copy link
Collaborator

torvalds commented Feb 9, 2023

Hmm. The commit message is a bit of a mess on that first commit, with overlong lines (that's a 702-character line!)

We had that earlier with the "Added parsing of the CCR setpoint information for Garmin Descent computers." that I merged as part of "added_garmin_ccr_setpoint_info", and I dread getting more of these.

But the changes look sane per se.

@mikeller mikeller force-pushed the added_garmin_ccr_bailout_messages branch from 65bab71 to 3f5b542 Compare February 9, 2023 01:16
This might need a bit of discussion.
As a CCR diver I would like to see the ability to properly track the 'on loop' /
'bailed out to open circuit' status directly in Subsurface, because this is an
important (or even the most important) bit of status information during a CCR
dive. This should extend to the use of the correct ppO2 / gas mix in the deco
ceiling / tissue model calculation. My idea for how to do this would be to track
the 'type' ('diluent' / 'OC bailout') for every gasmix / tank that is reported
by the dive computer. Most CCR capable dive computers that I am familiar with
require the user to enter two different gas lists for diluent and bailout, so
this should work with the existing libdivecomputer API for these. Unfortunately
I think making this change in Subsurface will require a bit of work, as the
libdivecomputer field capable of tracking the 'type' of a gas or tank
(`cache->tankinfo[]`) does not seem to be consumed at all in Subsurface. So this
pull request is just providing a prerequisite for the change in Subsurface by
populating `tankinfo[]`. In addition to this it also triggers a message on every
switch from CC to OC and back, at least giving a visual indication of these
diver triggered events. The messages can probably be removed from
libdivecomputer again once 'loop status' tracking has been added to Subsurface.
Also included is a fix of the tab expansion mess that I created in
subsurface#40. Apologies for this, I've switched
to using a custom `.vimrc` for this project now.

Signed-off-by: Michael Keller <github@ike.ch>
@mikeller mikeller force-pushed the added_garmin_ccr_bailout_messages branch from 3f5b542 to 2b5d618 Compare February 9, 2023 01:26
@mikeller
Copy link
Member Author

mikeller commented Feb 9, 2023

Apologies, I am new to the approach to add a detailed description of the changes to the commit message, instead of adding it to the pull request, and did not take care to rewrap my commit messages after editing. Fixed now.

@torvalds
Copy link
Collaborator

torvalds commented Feb 9, 2023

Cherry-picked the one remainming commit in the updated pull.

@torvalds torvalds closed this Feb 9, 2023
@mikeller mikeller deleted the added_garmin_ccr_bailout_messages branch February 9, 2023 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants