mobile/dive summary: rewrite statistics code

There were quite a few issues with the code - clearly a complete failure of
code review.

- all values were '??' if a period contained no dives
- imperial units were not calculated at all
- significant truncation and data loss in the way totals were added as meters
  and minutes instead of the higher precision data that is available
- several issues in striing conversion methodology, e.g. missing zero padding
  for minutes
- missing maxSac
- incorrectly calculated avgSac
- incorrectly claculated number of EANx dives
- hard to read code with most variables named 'temp'

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

backend-shared/divesummary.cpp

@@ -9,9 +9,9 @@ QStringList diveSummary::diveSummaryText;
 
 timestamp_t diveSummary::firstDive, diveSummary::lastDive;
 int diveSummary::dives[2], diveSummary::divesEAN[2], diveSummary::divesDeep[2], diveSummary::diveplans[2];
-long diveSummary::divetime[2], diveSummary::depth[2], diveSummary::sac[2];
+long diveSummary::divetime[2], diveSummary::depth[2];
-long diveSummary::divetimeMax[2], diveSummary::depthMax[2], diveSummary::sacMin[2];
+long diveSummary::divetimeMax[2], diveSummary::depthMax[2], diveSummary::sacMin[2], diveSummary::sacMax[2];
-int diveSummary::divesSAC[2];
+long diveSummary::totalSACTime[2], diveSummary::totalSacVolume[2];
 
 void diveSummary::summaryCalculation(int primaryPeriod, int secondaryPeriod)
 {
@@ -28,12 +28,12 @@ void diveSummary::summaryCalculation(int primaryPeriod, int secondaryPeriod)
 
 	// prepare stringlist
 	diveSummaryText.clear();
-	diveSummaryText << "??" << "??" << "??" << "??" << "??" <<
+	diveSummaryText << "??" << "??" << QObject::tr("no dives in period") << QObject::tr("no divies in period")  << "??" <<
 						 "??" << "??" << "??" <<
 						 "?:??" << "?:??" << "?:??" <<
 						 "?:??" << "?:??" << "?:??" <<
 						 "??" << "??" << "??" << "??" << "??" <<
-						 "??" << "??" << "??" << "??" << "??";
+						 "??" << "??" << "??" << "??" << "??" << "??" << "??";
 
 	// set oldest/newest date
 	if (firstDive) {
@@ -62,10 +62,11 @@ void diveSummary::loopDives(timestamp_t primaryStart, timestamp_t secondaryStart
 	dives[0] = dives[1] = divesEAN[0] = divesEAN[1] = 0;
 	divesDeep[0] = divesDeep[1] = diveplans[0] = diveplans[1] = 0;
 	divetime[0] = divetime[1] = depth[0] = depth[1] = 0;
-	sac[0] = sac[1] = 0;
 	divetimeMax[0] = divetimeMax[1] = depthMax[0] = depthMax[1] = 0;
 	sacMin[0] = sacMin[1] = 99999;
-	divesSAC[0] = divesSAC[1] = 0;
+	sacMax[0] = sacMax[1] = 0;
+	totalSACTime[0] = totalSACTime[1] = 0;
+	totalSacVolume[0] = totalSacVolume[1] = 0;
 
 	for_each_dive (i, dive) {
 		// remember time of oldest and newest dive
@@ -94,48 +95,66 @@ void diveSummary::loopDives(timestamp_t primaryStart, timestamp_t secondaryStart
 
 void diveSummary::calculateDive(int inx, struct dive *dive)
 {
-	long temp;
-
 	// one more real dive
 	dives[inx]++;
 
 	// sum dive in minutes and check for new max.
-	temp = dive->duration.seconds / 60;
+	divetime[inx] += dive->duration.seconds;
-	divetime[inx] += temp;
+	if (dive->duration.seconds > divetimeMax[inx])
-	if (temp > divetimeMax[inx])
+		divetimeMax[inx] = dive->duration.seconds;
-		divetimeMax[inx] = temp;
 
 	// sum depth in meters, check for new max. and if dive is a deep dive
-	temp = dive->maxdepth.mm / 1000;
+	depth[inx] += dive->maxdepth.mm;
-	depth[inx] += temp;
+	if (dive->maxdepth.mm > depthMax[inx])
-	if (temp > depthMax[inx])
+		depthMax[inx] = dive->maxdepth.mm;
-		depthMax[inx] = temp;
+	if (dive->maxdepth.mm > 39000)
-	if (temp > 39)
 		divesDeep[inx]++;
 
-	// sum SAC in liters, check for new max.
+	// sum SAC, check for new min/max.
-	temp = dive->sac / 1000;
+	if (dive->sac) {
-	if (temp) {
+		totalSACTime[inx] += dive->duration.seconds;
-		divesSAC[inx]++;
+		totalSacVolume[inx] += dive->sac * dive->duration.seconds;
-		sac[inx] += temp;
+		if (dive->sac < sacMin[inx])
-		if (temp < sacMin[inx])
+			sacMin[inx] = dive->sac;
-			sacMin[inx] = temp;
+		if (dive->sac > sacMax[inx])
+			sacMax[inx] = dive->sac;
 	}
 
 	// EAN dive ?
 	for (int j = 0; j < dive->cylinders.nr; ++j) {
-		if (dive->cylinders.cylinders[j].gasmix.o2.permille > 210)
+		if (dive->cylinders.cylinders[j].gasmix.o2.permille > 210) {
 			divesEAN[inx]++;
+			break;
+		}
 	}
 }
 
+static QString timeString(long duration)
+{
+	long hours = duration / 3600;
+	long minutes = (duration - hours * 3600) / 60;
+	if (hours >= 100)
+		return QStringLiteral("%1h").arg(hours);
+	else
+		return QStringLiteral("%1:%2").arg(hours).arg(minutes, 2, 10, QChar('0'));
+}
+
+static QString depthString(long depth)
+{
+	return QString("%1").arg((qPrefUnits::length() == units::METERS) ? depth / 1000 : lrint(mm_to_feet(depth)));
+}
+
+static QString volumeString(long volume)
+{
+	return QString("%1").arg((qPrefUnits::volume() == units::LITER) ? volume / 1000 : round(100.0 * ml_to_cuft(volume)) / 100.0);
+}
+
 void diveSummary::buildStringList(int inx)
 {
-	int temp1, temp2;
+	if (!dives[inx]) {
-	QString tempStr;
+		diveSummaryText[2+inx] = QObject::tr("no dives in period");
-
-	if (!dives[inx])
 		return;
+	}
 
 	// dives
 	diveSummaryText[2+inx] = QStringLiteral("%1").arg(dives[inx]);
@@ -143,34 +162,23 @@ void diveSummary::buildStringList(int inx)
 	diveSummaryText[6+inx] = QStringLiteral("%1").arg(divesDeep[inx]);
 
 	// time
-	temp1 = divetime[inx] / 60;
+	diveSummaryText[8+inx] = timeString(divetime[inx]);
-	temp2 = divetime[inx] - temp1 * 60;
+	diveSummaryText[10+inx] = timeString(divetimeMax[inx]);
-	if (temp1 >= 100)
+	diveSummaryText[12+inx] = timeString(divetime[inx] / dives[inx]);
-		diveSummaryText[8+inx] = QStringLiteral("%1h").arg(temp1);
-	else
-		diveSummaryText[8+inx] = QStringLiteral("%1:%2").arg(temp1).arg(temp2);
-	temp1 = divetimeMax[inx] / 60;
-	temp2 = divetimeMax[inx] - temp1 * 60;
-	diveSummaryText[10+inx] = QStringLiteral("%1:%2").arg(temp1).arg(temp2);
-	temp2 = divetime[inx] / dives[inx];
-	temp1 = temp2 / 60;
-	temp2 = temp2 - temp1 * 60;
-	diveSummaryText[12+inx] = QStringLiteral("%1:%2").arg(temp1).arg(temp2);
 
 	// depth
-	tempStr = (qPrefUnits::length() == units::METERS) ? "m" : "ft";
+	QString unitText = (qPrefUnits::length() == units::METERS) ? " m" : " ft";
-	diveSummaryText[14+inx] = QStringLiteral("%1%2").arg(depthMax[inx]).arg(tempStr);
+	diveSummaryText[14+inx] = depthString(depthMax[inx]) + unitText;
-	temp1 = depth[inx] / dives[inx];
+	diveSummaryText[16+inx] = depthString(depth[inx] / dives[inx]) + unitText;
-	diveSummaryText[16+inx] = QStringLiteral("%1%2").arg(temp1).arg(tempStr);
 
 	// SAC
-	tempStr = (qPrefUnits::volume() == units::LITER) ? "l/min" : "cuft/min";
+	unitText = (qPrefUnits::volume() == units::LITER) ? " l/min" : " cuft/min";
-	diveSummaryText[18+inx] = QStringLiteral("%1 %2").arg(sacMin[inx]).arg(tempStr);
+	diveSummaryText[18+inx] = volumeString(sacMin[inx]) + unitText;
-	if (divesSAC[inx]) {
+	diveSummaryText[20+inx] = volumeString(sacMax[inx]) + unitText;
-		temp1 = depth[inx] / divesSAC[inx];
+	// finally the weighted average
-		diveSummaryText[20+inx] = QStringLiteral("%1%2").arg(temp1).arg(tempStr);
+	long avgSac = totalSacVolume[inx] / totalSACTime[inx];
-	}
+	diveSummaryText[22+inx] = volumeString(avgSac) + unitText;
 
 	// Diveplan(s)
-	diveSummaryText[22+inx] = QStringLiteral("%1").arg(diveplans[inx]);
+	diveSummaryText[24+inx] = QStringLiteral("%1").arg(diveplans[inx]);
 }

backend-shared/divesummary.h

@@ -21,9 +21,9 @@ private:
 
 	static timestamp_t firstDive, lastDive;
 	static int dives[2], divesEAN[2], divesDeep[2], diveplans[2];
-	static long divetime[2], depth[2], sac[2];
+	static long divetime[2], depth[2];
-	static long divetimeMax[2], depthMax[2], sacMin[2];
+	static long divetimeMax[2], depthMax[2], sacMin[2], sacMax[2];
 	static long divetimeAvg[2], depthAvg[2], sacAvg[2];
-	static int divesSAC[2];
+	static long totalSACTime[2], totalSacVolume[2];
 };
 #endif // DIVESUMMARY_H

mobile-widgets/qml/DiveSummary.qml

@@ -181,7 +181,7 @@ Kirigami.ScrollablePage {
 			text: Backend.diveSummaryText[19]
 		}
 		TemplateLabel {
-			text: qsTr("avg. SAC")
+			text: qsTr("max. SAC")
 			font.bold: true
 		}
 		TemplateLabel {
@@ -190,6 +190,16 @@ Kirigami.ScrollablePage {
 		TemplateLabel {
 			text: Backend.diveSummaryText[21]
 		}
+		TemplateLabel {
+			text: qsTr("avg. SAC")
+			font.bold: true
+		}
+		TemplateLabel {
+			text: Backend.diveSummaryText[22]
+		}
+		TemplateLabel {
+			text: Backend.diveSummaryText[23]
+		}
 
 		TemplateLabel {
 			Layout.columnSpan: 3
@@ -200,10 +210,10 @@ Kirigami.ScrollablePage {
 			font.bold: true
 		}
 		TemplateLabel {
-			text: Backend.diveSummaryText[22]
+			text: Backend.diveSummaryText[24]
 		}
 		TemplateLabel {
-			text: Backend.diveSummaryText[23]
+			text: Backend.diveSummaryText[25]
 		}
 
 	}