[Patch] Fix planner notes gas change output logic

[Robert C. Helling]

Hi everybody,

let me first say this: This code is quite convoluted and has some history over which I tried several times to get it right. Too bad, I failed with it so far. Since yesterday, I have already spent several hours on this and still what I have does not satisfy me (one could say: It is still buggy). In particular, I also came to the conclusion that the whole „postposed“ logic is simply wrong and I threw it out.

The root of the problem (as with many things with the UI of the planner) is the distinction between segments of the dive and waypoints: The dive is stored in segments, they correspond to intervals of time and we store the depth and the time at the end of the segment and the gas breathed during the segment.

Waypoints on the other hand are moments of time: „At this time I am at this depth“. At first sight they seem to be identical to the endpoints of segments, but they are not: I believe for example if one associated a gas to a waypoint the correct semantics would be „we switch to that gas at that point in time or we keep breathing this gas“. So while the time and depth come from the segment preceding the waypoint, the gas is that of the following segment.

Then comes the question „What exactly do we mean when we print a gas in a row of the dive plan?“.

Note that a row has a depth and a duration and a runtime. In my understanding, a row is therefore like a segment above (actually, it can be several segments as we might not explicitly show transition segments or we join several transition segments into one).

So, if a row corresponds to a segment at a constant depth (or at least the last part of the segment is at constant depth, the row might include the transition to that depth), then clearly, the gas should be the gas during that segment (or actually it should mean: switch to that gas once your reach that depth). I.e. The gas should be displayed if before (maybe in the transition to that depth that is included in the same row), a different gas was breathed. Actually, it should be displayed if during one of the segments composing a row a different gas was breathed than in the previous row. In particular, we have to print the gas of the current, constant depth segment. (If there are several gases at the same constant depth, every change should trigger a row to be displayed, they should not be joint into one row).

This is different, however, for transitions. Imagine that the plan is to transition to 9m, switch to EAN80 there and then directly proceed to 6m. In that case, I would expect the EAN80 to be printed in the row with the depth 9m, even though that row as a segment indicates the transition leading to 9m (with still the previous gas being breathed), and the EAN80 being the gas of the following segment. So here, the row (with 9m depth and EAN80 displayed) has more of the meaning of a waypoint (with the gas of the next segment and also the decision about whether at all a gas should be shown being a transition at the end rather at the beginning of the segment).

Again, this is different for a transition to 9m and then a stop at 9m: There the gas should be displayed with the row indicating the stop, not transitioning to the depth of the gas change.

So, when we decide if the transition to 9m should be displayed (with a  gas) it depends on the next segment, whether it is a stop or a transition. That is why we have to look ahead. And what complicates this looking ahead is that we have „fake“ segments interspersed, segments of very short duration (0 or 1sec) or transitions split into various parts. So, we really have to look ahead not for to the next segment, but to the next real segment. So this is why this is so complicated.

At least that is my understanding what _should_ be displayed. Please voice your concerns if you want to see something else.

> On 17 Jun 2015, at 21:26, Joakim Bygdell <j.bygdell at gmail.com> wrote:
> 
> Robert, care to explain the postponed logic?
> 
> This set is from master.
> 
> Transition to 30 m in 1:40 min - runtime 1:40 on EAN32
> Stay at 30 m for 28:20 min - runtime 30:00 on EAN32
> Switch gas to air
> Stay at 30 m for 30:00 min - runtime 60:00 on air
> Switch gas to EAN32
> Stay at 30 m for 30:00 min - runtime 90:00 on EAN32
> Switch gas to EAN80
> Stay at 9.0 m for 6:00 min - runtime 99:39 on EAN80
> Stay at 6.0 m for 9:00 min - runtime 109:39 on EAN80
> Stay at 3.0 m for 17:00 min - runtime 128:12 on EAN80
> Transition to 0.0 m in 1:33 min - runtime 129:45 on EAN80
> 

I think there is no problem with the verbatim plan.


> 
> depth
> duration
> runtime
> gas
> 30m
> 2min
> 2min
> EAN32
> 30m
> 28min
> 30min
> 
> 30m
> 30min
> 60min
> EAN32
> 30m
> 30min
> 90min
> 
> 9m
> 10min
> 100min
> 6m
> 10min
> 110min
> EAN80
> 3m
> 19min
> 128min
> 
> 0m
> 2min
> 130min
> 
> 

This is wrong on many levels: Displaying EAN32 twice is of course nonsense, the second segment should be air and EAN80 should be printed on the 9m line, not at 6m.


> If we ignore the check for postponed on line 653 - 655 of planner.c this is the corresponding planner notes.
> This looks much more like what I would expect from the verbatim notes on top.
> 
> depth
> duration
> runtime
> gas
> 30m
> 2min
> 2min
> EAN32
> 30m
> 28min
> 30min
> air
> 30m
> 30min
> 60min
> EAN32
> 30m
> 30min
> 90min
> 
> 9m
> 10min
> 100min
> EAN80
> 6m
> 10min
> 110min
> 
> 3m
> 19min
> 128min
> 
> 0m
> 2min
> 130min
> 
> 
> 

Now the beginning is wrong: We switch back to EAN32 at 60min runtime, the air should be printed one row later. Same for the next EAN32.

> 
> Another thing, if the stop time is less than 1min at the switch depth the switch is displayed at the correct depth.
> 
> depth
> duration
> runtime
> gas
> 40m
> 2min
> 2min
> (24/25)
> 40m
> 23min
> 25min
> 
> 9m
> 6min
> 31min
> 
> 9m
> 1min
> 32min
> 
> 6m
> 1min
> 33min
> 
> 6m
> 22min
> 55min
> 
> 0m
> 3min
> 58min
> 
> 
> depth
> duration
> runtime
> gas
> 40m
> 2min
> 2min
> (24/25)
> 40m
> 23min
> 25min
> 
> 9m
> 6min
> 31min
> EAN80
> 6m
> 8min
> 39min
> 
> 0m
> 3min
> 42min
> 
> 
> 
> 2min stoptime @9m == wrong switch depth.
> depth
> duration
> runtime
> gas
> 40m
> 2min
> 2min
> (24/25)
> 40m
> 24min
> 26min
> 
> 9m
> 6min
> 32min
> 
> 9m
> 2min
> 34min
> 
> 6m
> 1min
> 35min
> 
> 6m
> 24min
> 59min
> 
> 0m
> 3min
> 62min
> 
> 
> depth
> duration
> runtime
> gas
> 40m
> 2min
> 2min
> (24/25)
> 40m
> 24min
> 26min
> 
> 9m
> 7min
> 33min
> 6m
> 8min
> 41min
> EAN80
> 0m
> 3min
> 44min
> 
> 

I agree this is weird and wrong.

As I said, The above explanation is what I came up with after spending far too much time on this problem and I am still in the process of implementing this properly.

Best
Robert

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20150617/8a5e611f/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20150617/8a5e611f/attachment-0001.sig>