Subsurface minimum gas calculation - First version for review

[Stefan Fuchs]

Hi Robert, hi Rick, hi All,


Am 15.02.2017 um 02:23 schrieb Rick Walsh:
> On 15 February 2017 at 06:50, Stefan Fuchs <sfuchs at gmx.de
> <mailto:sfuchs at gmx.de>> wrote:
>
>     Hi All,
>
>     Joachim (R.) and I finished our first basic version of the minimum
>     gas calculation for the planner.
>     We are now looking for testing and feedback :-)
>
> Great work on tackling this.  It's really good to see new ideas and
> new developers contributing to the code.  I will test when I get a
> chance, which unfortunately won't be for a few days.  In the meantime,
> I have a few comments/suggestions.
Thanks! Despite the fact that I still have to learn a lot this is really
fun :-)

>     More details below...
>
>     Link to initial idea and discussion:
>     https://github.com/Subsurface-divelog/subsurface/issues/188
>     <https://github.com/Subsurface-divelog/subsurface/issues/188>
>
>     Link to feature branch on Github:
>     https://github.com/sfuchs79/subsurface/tree/feature_minimum_gas
>     <https://github.com/sfuchs79/subsurface/tree/feature_minimum_gas>
>
>
>     My feedback to comments from Robert and Joachim (B.):
>     [...]
>     - Support rec dives: Yes, clearly, calculation is active for all
>     OC dives. I put some limitations for min depth (>18m) and min tank
>     size (12l). Calculating minimum gas for smaller cylinders is from
>     my point of view almost useless.
>
> I don't see the reason for limiting tank size and depth.  10.5l steel
> cylinders are pretty common where I dive, and AL80 (11.1l) cylinders
> are the most common in warm water.  Also there are smaller 300bar
> cylinders popular in some areas.  Minimum gas is also relevant for
> bailout cylinders, and some people use AL40 (5.6l) for them (although
> we don't do any automatic bailout calculations).  Surely having
> sufficient gas is just as important a consideration for small cylinders.
Ok, agreed. Let's remove this limit completely.
I at the beginning was unhappy that my implementation was also showing
such results like "your minimum gas is 537bar" if you select a small
cylinder ;-)
But I meanwhile avoid this with the simple check of minimum gas result
against the starting pressure of the cylinder and print a warning
instead of any result then. One may say this is also still not perfect
but for me at the moment it's good enough.

> I agree minimum gas is usually less of a concern for shallow dives,
> but spend an hour or more at 18m on air without a deco cylinder and
> you will require a significant amount of gas to allow two divers to
> surface safely.  Do we gain anything by excluding this possibility?
Hmm.. maybe 18m is really not a good decision. It's really arbitrary as
Robert is saying. What I would like to avoid is that we are printing a
minimum gas result in case we replan a dive and the calculated planner
points including all deco stops are still in the user entry table
(before one deletes them).
May I propose a compromise here: Lets use last/2nd last deco stop depth
(at ~6m) as limit. This is much less arbitrary than choosing 18m.

>
> Robert mentioned this his comments, so sorry to repeat, but where
> possible try to use the special unit types defined in core/dive.h:
> duration_t (unit seconds), depth_t (unit mm), pressure_t (unit mbar),
> volume_t (unit ml).  It's a little bit of extra work, but it makes the
> code clearer to read and debug.  Obviously the UI should present the
> values in friendly units.
For this and all the other comments from Robert I hope I understood
everything and there is nothing to discuss. Updated version will come soon.
BTW: The types are in units.h ;-)
A very important hint came from Robert about why we add the two new
variables to struct diveplan. This was clearly a mistake is not needed.
I will remove it.


Best regards
Stefan


-- 

Stefan Fuchs
E-Mail: sfuchs at gmx.de <mailto:sfuchs at gmx.de>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170215/293077b1/attachment.html>