[PATCH 6/4] Fix another cylinder pressure plotting special case

[Linus Torvalds]

On Sun, Jul 30, 2017 at 10:51 AM, Linus Torvalds
<torvalds at linux-foundation.org> wrote:
>
> So I think the code now does exactly the right thing for your case. Except
> for the equipment tab side, which doesn't show your two unused cylinders,
> even though you filled in information for them. But that looks like a
> pre-existing bug and probably has nothing to do with the recent pressure
> graph changes.

Oh. And that's not a bug, it's a "feature".

We have a "display_unused_tanks" preference setting, which disables
showing of tanks that aren't actively used during the dive.

So the crazy code will initially show them as you add them (because it
has logic to always show manually added cylinders), but then as you
read the xml file again, it will stop showing them, because now they
are no longer manually added, and their gas use indicates they weren't
used during the dive.

That's not a great feature.

This attached patch may or may not be an improvement. Dirk?

The new logic is basically:

 - if the cylinder is "used", it is always shown (so gas pressure has
changed, or we have gas switch events to it, or it's the first gas and
we had no switches away from it or whatever)

 - if the cylinder has no data at all, we don't show it

 - if it has pressure information - even if that pressure information
indicates that it wasn't used - it's shown

 - I left the "manually added" case

 - if it's not used, and has no pressure information, but does have a
type name and size, we fall back to the "prefs.display_unused_tanks"
thing.

I personally think this is an improvement, and it now shows all four
cylinders that Gaetan had on his CCR dive, even if the bailout
cylinders weren't actually used.

Gaetan had filled in cylinder pressures and everything, I think we
should show them by default.

I *think* that display_unused_tanks preference setting came from some
dive computers always showing every cylinder they could support, so
you ended up with 8 (or whatever) cylinders with just the default
cylinder information, even though none of them were actually used. But
having pressure data definitely means that they are more than just
placeholders..

                     Linus
-------------- next part --------------
 qt-models/cylindermodel.cpp | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/qt-models/cylindermodel.cpp b/qt-models/cylindermodel.cpp
index 5da0d42a..594df658 100644
--- a/qt-models/cylindermodel.cpp
+++ b/qt-models/cylindermodel.cpp
@@ -484,15 +484,33 @@ void CylindersModel::clear()
 	}
 }
 
+static bool show_cylinder(struct dive *dive, int i)
+{
+	cylinder_t *cyl = dive->cylinder + i;
+
+	if (is_cylinder_used(dive, i))
+		return true;
+	if (cylinder_none(cyl))
+		return false;
+	if (cyl->start.mbar || cyl->sample_start.mbar ||
+	    cyl->end.mbar || cyl->sample_end.mbar)
+		return true;
+	if (cyl->manually_added)
+		return true;
+
+	/*
+	 * The cylinder has some data, but none of it is very interesting,
+	 * it has no pressures and no gas switches. Do we want to show it?
+	 */
+	return prefs.display_unused_tanks;
+}
+
 void CylindersModel::updateDive()
 {
 	clear();
 	rows = 0;
 	for (int i = 0; i < MAX_CYLINDERS; i++) {
-		if (!cylinder_none(&displayed_dive.cylinder[i]) &&
-				(prefs.display_unused_tanks ||
-				 is_cylinder_used(&displayed_dive, i) ||
-				 displayed_dive.cylinder[i].manually_added))
+		if (show_cylinder(&displayed_dive, i))
 			rows = i + 1;
 	}
 	if (rows > 0) {
@@ -507,10 +525,8 @@ void CylindersModel::copyFromDive(dive *d)
 		return;
 	rows = 0;
 	for (int i = 0; i < MAX_CYLINDERS; i++) {
-		if (!cylinder_none(&d->cylinder[i]) &&
-				(is_cylinder_used(d, i) || prefs.display_unused_tanks)) {
+		if (show_cylinder(d, i))
 			rows = i + 1;
-		}
 	}
 	if (rows > 0) {
 		beginInsertRows(QModelIndex(), 0, rows - 1);