From 53a8167787acf4618317c926c8d3bbd2dfb25e9e Mon Sep 17 00:00:00 2001 From: Mark Liversedge Date: Sat, 9 Apr 2011 08:20:06 +0100 Subject: [PATCH 1/2] Fix Weekly Summary Crash Refactoring of WeeklySummary to use the metricDB (speed optimisation) introduced a bug where the ridefile iterator was erroneously used whilst computing daily averages. This patch fixes this by using the daily iterator instead. Many thanks to John Ehrlinger for identifying, reporting and helping to diagnose this defect. --- src/WeeklySummaryWindow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WeeklySummaryWindow.cpp b/src/WeeklySummaryWindow.cpp index 87e4fc7fe..f77e96e2d 100644 --- a/src/WeeklySummaryWindow.cpp +++ b/src/WeeklySummaryWindow.cpp @@ -216,7 +216,7 @@ WeeklySummaryWindow::refresh() // average RI for the day if (dailyRides[day] > 1) - dailyRI[day] = ((dailyRI[day] * (dailyRides[i]-1)) + dailyRI[day] = ((dailyRI[day] * (dailyRides[day]-1)) + metrics.getForSymbol("skiba_relative_intensity")) / dailyRides[day]; else dailyRI[day] = metrics.getForSymbol("skiba_relative_intensity"); From 6af6b347bf2c8d77f14392624f3a2e912e43ecb7 Mon Sep 17 00:00:00 2001 From: Mark Liversedge Date: Sat, 9 Apr 2011 11:24:40 +0100 Subject: [PATCH 2/2] Version 3 - No Ridefiles Bugs Bonanza When no ridefiles are available (new cyclist) or the last ridefile is deleted the current ride will be null. In addition the ride importer deletes the memory for a ride imported to ensure VM is not exhausted on large imports. This patch fixes a whole host of null errors across the codebase. They were identified by creating a new cyclist, executing every menu option and tab/chart and then importing a file choosing everything and then deleting the file and choosing every option again. This negative testing should be performed before every stable release since it has identified at least 6 bugs which are almost certainly present in the current V2 code. --- src/GcCalendarModel.h | 2 +- src/GoogleMapControl.cpp | 2 +- src/MainWindow.cpp | 20 ++++++++++++++++---- src/RideEditor.cpp | 14 +++++++++----- src/RideFile.cpp | 1 + src/RideFile.h | 1 + src/RideFileTableModel.cpp | 8 ++++++++ src/RideFileTableModel.h | 6 ++++++ src/RideImportWizard.cpp | 4 +++- src/RideNavigator.cpp | 7 ++++++- 10 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/GcCalendarModel.h b/src/GcCalendarModel.h index b8cd61a81..bcdafce81 100644 --- a/src/GcCalendarModel.h +++ b/src/GcCalendarModel.h @@ -202,7 +202,7 @@ public: if (arr) { foreach (int i, *arr) // we have rides on this day... - if (sourceModel()->data(index(i, dateIndex, QModelIndex())).toDateTime() == mainWindow->rideItem()->dateTime) + if (mainWindow->rideItem() && sourceModel()->data(index(i, dateIndex, QModelIndex())).toDateTime() == mainWindow->rideItem()->dateTime) colors.append(GColor(CCALCURRENT)); else colors.append(GColor(CCALACTUAL)); diff --git a/src/GoogleMapControl.cpp b/src/GoogleMapControl.cpp index 253e5b091..bc0cc711d 100644 --- a/src/GoogleMapControl.cpp +++ b/src/GoogleMapControl.cpp @@ -291,7 +291,7 @@ void GoogleMapControl::createHtml() } } RideItem * ride = myRideItem; - if(!ride->ride() || ride->ride()->areDataPresent()->lat == false || + if(!ride || !ride->ride() || ride->ride()->areDataPresent()->lat == false || ride->ride()->areDataPresent()->lon == false) { currentPage << tr("No GPS Data Present").toStdString(); diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index 6cdc05d79..3d7d2fc94 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -1054,8 +1054,10 @@ MainWindow::exportKML() void MainWindow::uploadTP() { - TPUploadDialog uploader(cyclist, currentRide(), this); - uploader.exec(); + if (ride) { + TPUploadDialog uploader(cyclist, currentRide(), this); + uploader.exec(); + } } void @@ -1158,6 +1160,9 @@ MainWindow::addIntervalForPowerPeaksForSecs(RideFile *ride, int windowSizeSecs, void MainWindow::findPowerPeaks() { + + if (!ride) return; + QTreeWidgetItem *which = treeWidget->selectedItems().first(); if (which->type() != RIDE_TYPE) { return; @@ -1644,7 +1649,14 @@ void MainWindow::showWorkoutWizard() void MainWindow::saveRide() { - saveRideSingleDialog(ride); // will signal save to everyone + if (ride) + saveRideSingleDialog(ride); // will signal save to everyone + else { + QMessageBox oops(QMessageBox::Critical, tr("No Ride To Save"), + tr("There is no currently selected ride to save.")); + oops.exec(); + return; + } } void @@ -1661,7 +1673,7 @@ MainWindow::revertRide() void MainWindow::splitRide() { - (new SplitRideDialog(this))->exec(); + if (ride) (new SplitRideDialog(this))->exec(); } void diff --git a/src/RideEditor.cpp b/src/RideEditor.cpp index 53645762b..0b67f0800 100644 --- a/src/RideEditor.cpp +++ b/src/RideEditor.cpp @@ -256,7 +256,7 @@ RideEditor::isColumnSelected() void RideEditor::saveFile() { - if (ride->isDirty()) { + if (ride && ride->isDirty()) { main->saveRideSingleDialog(ride); } } @@ -264,13 +264,15 @@ RideEditor::saveFile() void RideEditor::undo() { - ride->ride()->command->undoCommand(); + if (ride && ride->ride() && ride->ride()->command) + ride->ride()->command->undoCommand(); } void RideEditor::redo() { - ride->ride()->command->redoCommand(); + if (ride && ride->ride() && ride->ride()->command) + ride->ride()->command->redoCommand(); } void @@ -1080,7 +1082,10 @@ void RideEditor::rideSelected() { RideItem *current = myRideItem; - if (!current || !current->ride()) return; + if (!current || !current->ride()) { + model->setRide(NULL); + return; + } ride = current; @@ -1092,7 +1097,6 @@ RideEditor::rideSelected() data = ride->ride()->editorData(); data->found.clear(); // search is not active, so clear } - model->setRide(ride->ride()); // reset the save icon on the toolbar diff --git a/src/RideFile.cpp b/src/RideFile.cpp index a6408f6bd..05c3881fd 100644 --- a/src/RideFile.cpp +++ b/src/RideFile.cpp @@ -50,6 +50,7 @@ RideFile::RideFile() : recIntSecs_(0.0), deviceType_("unknown"), data(NULL) RideFile::~RideFile() { + emit deleted(); foreach(RideFilePoint *point, dataPoints_) delete point; delete command; diff --git a/src/RideFile.h b/src/RideFile.h index f5f35a422..ad93ce909 100644 --- a/src/RideFile.h +++ b/src/RideFile.h @@ -174,6 +174,7 @@ class RideFile : public QObject // QObject to emit signals void saved(); void reverted(); void modified(); + void deleted(); protected: void emitSaved(); diff --git a/src/RideFileTableModel.cpp b/src/RideFileTableModel.cpp index 714bb01f7..8e826999c 100644 --- a/src/RideFileTableModel.cpp +++ b/src/RideFileTableModel.cpp @@ -45,12 +45,20 @@ RideFileTableModel::setRide(RideFile *newride) connection = ride->command; connect(ride->command, SIGNAL(beginCommand(bool,RideCommand*)), this, SLOT(beginCommand(bool,RideCommand*))); connect(ride->command, SIGNAL(endCommand(bool,RideCommand*)), this, SLOT(endCommand(bool,RideCommand*))); + connect(ride, SIGNAL(deleted()), this, SLOT(deleted())); // refresh emit layoutChanged(); } } +void +RideFileTableModel::deleted() +{ + // we don't need to disconnect since they're free'd up by QT + ride = NULL; +} + void RideFileTableModel::setHeadings(RideFile::SeriesType series) { diff --git a/src/RideFileTableModel.h b/src/RideFileTableModel.h index 95946ad72..732d9e2d6 100644 --- a/src/RideFileTableModel.h +++ b/src/RideFileTableModel.h @@ -83,6 +83,12 @@ class RideFileTableModel : public QAbstractTableModel void beginCommand(bool undo, RideCommand *); void endCommand(bool undo, RideCommand *); + // Import wizard frees the ridefile memory to + // stop exhausting memory, but we need to know + // coz we reference the ride file and not the + // ride item. long story. + void deleted(); + // force redraw - used by anomaly detection and find void forceRedraw(); diff --git a/src/RideImportWizard.cpp b/src/RideImportWizard.cpp index af1652e1d..8795daca8 100644 --- a/src/RideImportWizard.cpp +++ b/src/RideImportWizard.cpp @@ -687,7 +687,6 @@ RideImportWizard::abortClicked() } } else { - // for native file formats the filename IS the ride date time so // no need to write -- we just copy @@ -725,6 +724,9 @@ RideImportWizard::abortClicked() mainWindow->addRide(QFileInfo(fulltarget).fileName(), true); // add to tree view // free immediately otherwise all imported rides are cached // and with large imports this can lead to memory exhaustion + // BUT! Some charts/windows will hava snaffled away the ridefile + // pointer which is now invalid so once all the rides have been imported + // we need to select the last one... see below mainWindow->rideItem()->freeMemory(); } else tableWidget->item(i,5)->setText(tr("Error - copy failed")); diff --git a/src/RideNavigator.cpp b/src/RideNavigator.cpp index 287c00e6c..fcada613f 100644 --- a/src/RideNavigator.cpp +++ b/src/RideNavigator.cpp @@ -536,7 +536,12 @@ RideNavigator::rideTreeSelectionChanged() if (active == true) return; else active = true; - QTreeWidgetItem *which = main->rideTreeWidget()->selectedItems().first(); + QTreeWidgetItem *which; + if (main->rideTreeWidget()->selectedItems().count()) + which = main->rideTreeWidget()->selectedItems().first(); + else // no rides slected + which = NULL; + if (which && which->type() == RIDE_TYPE) { RideItem *rideItem = static_cast(which);