diff options
author | Dirk Hohndel <dirk@hohndel.org> | 2020-01-29 11:26:22 +0200 |
---|---|---|
committer | Dirk Hohndel <dirk@hohndel.org> | 2020-01-30 07:45:25 +0200 |
commit | 08e39f9d2d293ac22e6174e7b723abb805650552 (patch) | |
tree | c669bf41f8e3df46e273367283abe1cf966bbabb | |
parent | a534f1a25c36e4dbd27cc7d341d0c0d1a92d7cc7 (diff) | |
download | subsurface-08e39f9d2d293ac22e6174e7b723abb805650552.tar.gz |
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>
-rw-r--r-- | backend-shared/divesummary.cpp | 116 | ||||
-rw-r--r-- | backend-shared/divesummary.h | 6 | ||||
-rw-r--r-- | mobile-widgets/qml/DiveSummary.qml | 16 |
3 files changed, 78 insertions, 60 deletions
diff --git a/backend-shared/divesummary.cpp b/backend-shared/divesummary.cpp index 9f3c749cb..12d7ff0e2 100644 --- a/backend-shared/divesummary.cpp +++ b/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::divetimeMax[2], diveSummary::depthMax[2], diveSummary::sacMin[2]; -int diveSummary::divesSAC[2]; +long diveSummary::divetime[2], diveSummary::depth[2]; +long diveSummary::divetimeMax[2], diveSummary::depthMax[2], diveSummary::sacMin[2], diveSummary::sacMax[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] += temp; - if (temp > divetimeMax[inx]) - divetimeMax[inx] = temp; + divetime[inx] += dive->duration.seconds; + if (dive->duration.seconds > divetimeMax[inx]) + divetimeMax[inx] = dive->duration.seconds; // sum depth in meters, check for new max. and if dive is a deep dive - temp = dive->maxdepth.mm / 1000; - depth[inx] += temp; - if (temp > depthMax[inx]) - depthMax[inx] = temp; - if (temp > 39) + depth[inx] += dive->maxdepth.mm; + if (dive->maxdepth.mm > depthMax[inx]) + depthMax[inx] = dive->maxdepth.mm; + if (dive->maxdepth.mm > 39000) divesDeep[inx]++; - // sum SAC in liters, check for new max. - temp = dive->sac / 1000; - if (temp) { - divesSAC[inx]++; - sac[inx] += temp; - if (temp < sacMin[inx]) - sacMin[inx] = temp; + // sum SAC, check for new min/max. + if (dive->sac) { + totalSACTime[inx] += dive->duration.seconds; + totalSacVolume[inx] += dive->sac * dive->duration.seconds; + if (dive->sac < sacMin[inx]) + sacMin[inx] = dive->sac; + 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; + } } } -void diveSummary::buildStringList(int inx) +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) { - int temp1, temp2; - QString tempStr; + return QString("%1").arg((qPrefUnits::volume() == units::LITER) ? volume / 1000 : round(100.0 * ml_to_cuft(volume)) / 100.0); +} - if (!dives[inx]) +void diveSummary::buildStringList(int inx) +{ + if (!dives[inx]) { + diveSummaryText[2+inx] = QObject::tr("no dives in period"); 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; - temp2 = divetime[inx] - temp1 * 60; - if (temp1 >= 100) - 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); + diveSummaryText[8+inx] = timeString(divetime[inx]); + diveSummaryText[10+inx] = timeString(divetimeMax[inx]); + diveSummaryText[12+inx] = timeString(divetime[inx] / dives[inx]); // depth - tempStr = (qPrefUnits::length() == units::METERS) ? "m" : "ft"; - diveSummaryText[14+inx] = QStringLiteral("%1%2").arg(depthMax[inx]).arg(tempStr); - temp1 = depth[inx] / dives[inx]; - diveSummaryText[16+inx] = QStringLiteral("%1%2").arg(temp1).arg(tempStr); + QString unitText = (qPrefUnits::length() == units::METERS) ? " m" : " ft"; + diveSummaryText[14+inx] = depthString(depthMax[inx]) + unitText; + diveSummaryText[16+inx] = depthString(depth[inx] / dives[inx]) + unitText; // SAC - tempStr = (qPrefUnits::volume() == units::LITER) ? "l/min" : "cuft/min"; - diveSummaryText[18+inx] = QStringLiteral("%1 %2").arg(sacMin[inx]).arg(tempStr); - if (divesSAC[inx]) { - temp1 = depth[inx] / divesSAC[inx]; - diveSummaryText[20+inx] = QStringLiteral("%1%2").arg(temp1).arg(tempStr); - } + unitText = (qPrefUnits::volume() == units::LITER) ? " l/min" : " cuft/min"; + diveSummaryText[18+inx] = volumeString(sacMin[inx]) + unitText; + diveSummaryText[20+inx] = volumeString(sacMax[inx]) + unitText; + // finally the weighted average + 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]); } diff --git a/backend-shared/divesummary.h b/backend-shared/divesummary.h index 986024b44..a477bb0a3 100644 --- a/backend-shared/divesummary.h +++ b/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 divetimeMax[2], depthMax[2], sacMin[2]; + static long divetime[2], depth[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 diff --git a/mobile-widgets/qml/DiveSummary.qml b/mobile-widgets/qml/DiveSummary.qml index b690109e4..d55443815 100644 --- a/mobile-widgets/qml/DiveSummary.qml +++ b/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] } } |