Hi Rick,
I am currently looking at the patch and write down things I notice. The gas change looks good for our case of a switch at a depth without a stop (even when mandatory stop time is set to 0). But when I turn on displayed transitions I would expect the gas change to be shown on the stop rather than on the upwards transition that leads to it (as the later is only the place to print it if there is no stop for it). I attached a patch (to be applied on top of yours or for yours to be amended with it) to fix that. |
From 778f8afd3e7a413491de88176e67a7e196bf1ab5 Mon Sep 17 00:00:00 2001 From: "Robert C. Helling" <[email protected]> Date: Mon, 22 Jun 2015 22:45:56 +0200 Subject: [PATCH] When displaying transitions show the gaschange at the stop if there is one
Signed-off-by: Robert C. Helling <[email protected]> --- planner.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner.c b/planner.c index 49b0990..481d10c 100644 --- a/planner.c +++ b/planner.c @@ -654,7 +654,7 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool len += snprintf(buffer + len, sizeof(buffer) - len, "<td style='padding-left: 10px; float: right;'>%s</td>", temp); } - if (isascent && gaschange) { + if (isascent && gaschange && dp->next && dp->depth != nextdp->depth) { if (dp->setpoint) { snprintf(temp, sizeof(temp), translate("gettextFromC", "(SP = %.1fbar)"), (double) nextdp->setpoint / 1000.0); len += snprintf(buffer + len, sizeof(buffer) - len, "<td style='padding-left: 10px; color: red; float: left;'><b>%s %s</b></td>", gasname(&newgasmix), -- 1.9.5 (Apple Git-50.3)
My compiler gives a warning that in that very long if statement there is || next to && and thus some parenthesis might add clarity. I would like to chime in here and also suggest to break that condition over several more lines essentially with one part of the || statement per line and to add comments there so we will not be as confused again when we look at this in a few months. Another stylistic thing: The bool gaschange is not a good name anymore and should be renamed to something like gaschange_after and a similar abbreviation gaschange_before could be introduced. Here is a second patch for that: |
From cd617348596fb723db56234d72dd4403c76f6f8c Mon Sep 17 00:00:00 2001 From: "Robert C. Helling" <[email protected]> Date: Mon, 22 Jun 2015 23:07:43 +0200 Subject: [PATCH 2/2] Refactor to use gaschage_before and gaschange_after ... for better readability. Signed-off-by: Robert C. Helling <[email protected]> --- planner.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/planner.c b/planner.c index 481d10c..afcbfa6 100644 --- a/planner.c +++ b/planner.c @@ -524,7 +524,8 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool int len, lastdepth = 0, lasttime = 0, lastsetpoint = -1, newdepth = 0, lastprintdepth = 0, lastprintsetpoint = -1; struct gasmix lastprintgasmix = { -1, -1 }; struct divedatapoint *dp = diveplan->dp; - bool gaschange = !plan_verbatim; + bool gaschange_after = !plan_verbatim; + bool gaschange_before; struct divedatapoint *nextdp = NULL; disclaimer = translate("gettextFromC", "DISCLAIMER / WARNING: THIS IS A NEW IMPLEMENTATION OF THE BUHLMANN " @@ -587,17 +588,18 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool dp->setpoint == nextdp->setpoint && dp->depth != lastdepth && nextdp->depth != dp->depth - && !gaschange) + && !gaschange_after) continue; - if (dp->time - lasttime < 10 && !(gaschange && dp->next && dp->depth != dp->next->depth)) + if (dp->time - lasttime < 10 && !(gaschange_after && dp->next && dp->depth != dp->next->depth)) continue; len = strlen(buffer); if (nextdp && (gasmix_distance(&gasmix, &newgasmix) || dp->setpoint != nextdp->setpoint)) - gaschange = true; + gaschange_after = true; + gaschange_before = (gasmix_distance(&lastprintgasmix, &gasmix) || lastprintsetpoint != dp->setpoint); if (plan_verbatim) { if (dp->depth != lastprintdepth) { - if (plan_display_transitions || dp->entered || !dp->next || (gaschange && dp->next && dp->depth != nextdp->depth)) { + if (plan_display_transitions || dp->entered || !dp->next || (gaschange_after && dp->next && dp->depth != nextdp->depth)) { if (dp->setpoint) snprintf(temp, sizeof(temp), translate("gettextFromC", "Transition to %.*f %s in %d:%02d min - runtime %d:%02u on %s (SP = %.1fbar)"), decimals, depthvalue, depth_unit, @@ -618,7 +620,7 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool newdepth = dp->depth; lasttime = dp->time; } else { - if (dp->depth != nextdp->depth || gasmix_distance(&gasmix, &newgasmix) != 0 || dp->setpoint != nextdp->setpoint) { + if (dp->depth != nextdp->depth || gaschange_after) { if (dp->setpoint) snprintf(temp, sizeof(temp), translate("gettextFromC", "Stay at %.*f %s for %d:%02d min - runtime %d:%02u on %s (SP = %.1fbar)"), decimals, depthvalue, depth_unit, @@ -640,9 +642,9 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool } } else { if (plan_display_transitions || dp->entered || !dp->next || dp->depth != nextdp->depth || - !isascent && (gasmix_distance(&lastprintgasmix, &gasmix) || lastprintsetpoint != dp->setpoint) - && (dp->depth != nextdp->depth || gasmix_distance(&gasmix, &newgasmix) || dp->setpoint != nextdp->setpoint) - || (isascent && gaschange && dp->depth != nextdp->depth )) { + !isascent && gaschange_before + && (dp->depth != nextdp->depth || gaschange_after) + || (isascent && gaschange_after && dp->depth != nextdp->depth )) { snprintf(temp, sizeof(temp), translate("gettextFromC", "%3.0f%s"), depthvalue, depth_unit); len += snprintf(buffer + len, sizeof(buffer) - len, "<tr><td style='padding-left: 10px; float: right;'>%s</td>", temp); if (plan_display_duration) { @@ -654,7 +656,7 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool len += snprintf(buffer + len, sizeof(buffer) - len, "<td style='padding-left: 10px; float: right;'>%s</td>", temp); } - if (isascent && gaschange && dp->next && dp->depth != nextdp->depth) { + if (isascent && gaschange_after && dp->next && dp->depth != nextdp->depth) { if (dp->setpoint) { snprintf(temp, sizeof(temp), translate("gettextFromC", "(SP = %.1fbar)"), (double) nextdp->setpoint / 1000.0); len += snprintf(buffer + len, sizeof(buffer) - len, "<td style='padding-left: 10px; color: red; float: left;'><b>%s %s</b></td>", gasname(&newgasmix), @@ -664,8 +666,8 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool } lastprintsetpoint = nextdp->setpoint; lastprintgasmix = newgasmix; - gaschange = false; - } else if (gasmix_distance(&lastprintgasmix, &gasmix) != 0 || lastprintsetpoint != dp->setpoint) { + gaschange_after = false; + } else if (gaschange_before) { if (dp->setpoint) { snprintf(temp, sizeof(temp), translate("gettextFromC", "(SP = %.1fbar)"), (double) dp->setpoint / 1000.0); len += snprintf(buffer + len, sizeof(buffer) - len, "<td style='padding-left: 10px; color: red; float: left;'><b>%s %s</b></td>", gasname(&gasmix), @@ -675,7 +677,7 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool } lastprintsetpoint = dp->setpoint; lastprintgasmix = gasmix; - gaschange = false; + gaschange_after = false; } else { len += snprintf(buffer + len, sizeof(buffer) - len, "<td> </td>"); } @@ -684,7 +686,7 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool lasttime = dp->time; } } - if (gaschange) { + if (gaschange_after) { // gas switch at this waypoint if (plan_verbatim) { if (lastsetpoint >= 0) { @@ -695,7 +697,7 @@ static void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool len += snprintf(buffer + len, sizeof(buffer) - len, "%s<br>", temp); } - gaschange = false; + gaschange_after = false; gasmix = newgasmix; } } -- 1.9.5 (Apple Git-50.3)
But apart from that, this finally seems to do the right thing. Thanks a lot for sorting this out! Best Robert |
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
