[Patch] Fix planner notes gas change output logic
Rick Walsh
rickmwalsh at gmail.com
Wed Jun 17 17:09:40 PDT 2015
Robert,
On 18 June 2015 at 06:53, Robert C. Helling <helling at atdotde.de> wrote:
> 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).
>
My understanding of how it *should* work is very similar to yours, but with
one slight distinction: the gas printed (or implied = same as above) in
that segment is the gas to be breathed up to the printed runtime. This was
the intent of my patch last night (Australian time). E.g. copied from my
previous email without transitions:
depth dur. runtime gas
20m 1min 1min air
20m 5min 5min
20m 10min 15min EAN80
20m 5min 20min air
9m 1min 21min <-for the segment of 1min duration leading up to the 21min
runtime, we are still breathing air
6m 17min 39min EAN80 <-for the segment of 17min duration leading up to the
39min runtime, we have changed to EAN80
0m 2min 41min
I don't think this is 'wrong', but as you have correctly pointed out, it
doesn't explicitly state that the gas change is at 9m, and it is easy to
interpret that it should be at 6m. As there is no stop at 9m, it isn't
really missing much. However, I think a more intuitive output would be:
depth dur. runtime gas
20m 1min 1min air
20m 5min 5min
20m 10min 15min EAN80
20m 5min 20min air
9m 1min 21min <-for the segment of 1min duration leading up to the 21min
runtime (we ascend to 9m), we are still breathing air
9m 0min 21min EAN80 <-there is zero time at 9m, but this is where to change
to EAN80
6m 17min 39min <-keep breathing EAN80
0m 2min 41min
Do you think this is a clear and reasonable approach?
Unless I messed other things up, it should not be hard to do on top of my
last patch. I nearly made the change last night, but messed up table
formatting and it was way past my bedtime. My approach was:
In the section around line 660 after printing the gas
}
lastprintsetpoint = dp->setpoint;
lastprintgasmix = gasmix;
*+ gaschange = false;*
} else {
len += snprintf(buffer + len, sizeof(buffer) - len, "<td> </td>");
}
In the section currently used to print the gas switch line when using the
verbatim output around line 680. Note that this will not be run if there's
a real stop so we've already printed the new gas in the table, and set
gaschange = false:
674
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l674>
if (gaschange) {
675
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l675>
if(dp->depth == lastdepth && !postponed) {
676
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l676>
postponed = true;
677
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l677>
} else {
678
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l678>
// gas switch at this waypoint
679
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l679>
if (plan_verbatim) {
680
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l680>
if (lastsetpoint >= 0) {
681
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l681>
if (nextdp && nextdp->setpoint)
682
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l682>
snprintf(temp,
sizeof(temp), translate("gettextFromC", "Switch gas to %s (SP =
%.1fbar)"), gasname(&newgasmix), (double) nextdp->setpoint / 1000.0);
683
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l683>
else
684
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l684>
snprintf(temp,
sizeof(temp), translate("gettextFromC", "Switch gas to %s"),
gasname(&newgasmix));
685
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l685>
686
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l686>
len += snprintf(buffer + len,
sizeof(buffer) - len, "%s<br>", temp);
687
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l687>
}
688
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l688>
gaschange = false;
689
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l689>
postponed = true;
- 690
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l690>
}
+ 690
<http://git.subsurface-divelog.org/index.cgi?p=subsurface.git;a=blob;f=planner.c;h=9b2d355faac88b7aa32ddfdf6244cc7918466fed;hb=f66ea4cbb03c126fce3805d778c9f197fb6fa5e8#l690>
} else {
Insert code to print a new line to the
table for the gas switch: 0 min duration, and at the same depth and runtime
as the last line
+ gaschange = false;
+ }
Sorry for the ugly pseudo-patch formatted email. I'm at work at the moment.
The distinction between segments and waypoints is very important, but as
your (I assume it is yours) code for the verbatim output demonstrates,
cycling through waypoints isn't 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.
>
If you have worked out a better way then that is excellent
>
> Best
> Robert
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20150618/593080c7/attachment-0001.html>
More information about the subsurface
mailing list