Skip to content

Commit d540c2a

Browse files
committed
LP-597 Progress bar for GCS log replay - ensure the index vectors are emptied before adding elements to them. Caused problem when replaying multiple logfiles successively.
- Bugfix: ensure the index vectors are emptied before adding elements to them. Caused problem when replaying multiple logfiles successively. - extra validations while indexing the logfile - Define TIMESTAMP_SIZE_BYTES and use that instead of the hardcoded timestamp size of 4 bytes. - modify variable case: updateBeginAndEndtimes -> updateBeginAndEndTimes - improve qWarning messages for logfile.cpp: add the name of the function because we read the logfile in two separate functions.
1 parent ef3cb8b commit d540c2a

4 files changed

Lines changed: 58 additions & 31 deletions

File tree

ground/gcs/src/libs/utils/logfile.cpp

Lines changed: 54 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
#include <QDataStream>
3030
#include <QThread> // DEBUG: to display the thread ID
3131

32+
#define TIMESTAMP_SIZE_BYTES 4
33+
3234
LogFile::LogFile(QObject *parent) : QIODevice(parent),
3335
m_timer(this),
3436
m_previousTimeStamp(0),
@@ -156,7 +158,7 @@ void LogFile::timerFired()
156158
}
157159
m_timer_tick++;
158160

159-
if (m_file.bytesAvailable() > 4) {
161+
if (m_file.bytesAvailable() > TIMESTAMP_SIZE_BYTES) {
160162
int time;
161163
time = m_myTime.elapsed();
162164

@@ -182,22 +184,22 @@ void LogFile::timerFired()
182184
// read data size
183185
qint64 dataSize;
184186
if (m_file.bytesAvailable() < (qint64)sizeof(dataSize)) {
185-
qDebug() << "LogFile - end of log file reached";
187+
qDebug() << "LogFile replay - end of log file reached";
186188
stopReplay();
187189
return;
188190
}
189191
m_file.read((char *)&dataSize, sizeof(dataSize));
190192

191193
// check size consistency
192194
if (dataSize < 1 || dataSize > (1024 * 1024)) {
193-
qWarning() << "LogFile - corrupted log file! Unlikely packet size:" << dataSize;
195+
qWarning() << "LogFile replay - corrupted log file! Unlikely packet size:" << dataSize;
194196
stopReplay();
195197
return;
196198
}
197199

198200
// read data
199201
if (m_file.bytesAvailable() < dataSize) {
200-
qDebug() << "LogFile - end of log file reached";
202+
qDebug() << "LogFile replay - end of log file reached";
201203
stopReplay();
202204
return;
203205
}
@@ -216,7 +218,7 @@ void LogFile::timerFired()
216218
}
217219
// read next timestamp
218220
if (m_file.bytesAvailable() < (qint64)sizeof(m_nextTimeStamp)) {
219-
qDebug() << "LogFile - end of log file reached";
221+
qDebug() << "LogFile replay - end of log file reached";
220222
stopReplay();
221223
return;
222224
}
@@ -226,7 +228,7 @@ void LogFile::timerFired()
226228
// some validity checks
227229
if ((m_nextTimeStamp < m_previousTimeStamp) // logfile goes back in time
228230
|| ((m_nextTimeStamp - m_previousTimeStamp) > 60 * 60 * 1000)) { // gap of more than 60 minutes
229-
qWarning() << "LogFile - corrupted log file! Unlikely timestamp:" << m_nextTimeStamp << "after" << m_previousTimeStamp;
231+
qWarning() << "LogFile replay - corrupted log file! Unlikely timestamp:" << m_nextTimeStamp << "after" << m_previousTimeStamp;
230232
stopReplay();
231233
return;
232234
}
@@ -235,7 +237,7 @@ void LogFile::timerFired()
235237
time = m_myTime.elapsed(); // number of milliseconds since start of playback
236238
}
237239
} else {
238-
qDebug() << "LogFile - end of log file reached";
240+
qDebug() << "LogFile replay - end of log file reached";
239241
stopReplay();
240242
}
241243
}
@@ -324,12 +326,12 @@ bool LogFile::stopReplay()
324326
* If no position is given, resumes from the last position
325327
*
326328
*/
327-
328329
bool LogFile::resumeReplay(quint32 desiredPosition)
329330
{
330331
if (m_timer.isActive()) {
331332
return false;
332333
}
334+
qDebug() << "LogFile - resumeReplay";
333335

334336
// Clear the playout buffer:
335337
m_mutex.lock();
@@ -346,7 +348,11 @@ bool LogFile::resumeReplay(quint32 desiredPosition)
346348
for (int i = 0; i < m_timeStamps.size(); i++) {
347349
if (m_timeStamps.at(i) >= desiredPosition) {
348350

349-
m_file.seek(m_timeStampPositions.at(i));
351+
int bytesToSkip = m_timeStampPositions.at(i);
352+
bool seek_ok = m_file.seek(bytesToSkip);
353+
if (!seek_ok) {
354+
qWarning() << "LogFile resumeReplay - an error occurred while seeking through the logfile.";
355+
}
350356
m_lastPlayed = m_timeStamps.at(i);
351357
break;
352358
}
@@ -401,6 +407,7 @@ bool LogFile::pauseAndResetPosition()
401407
if (!m_file.isOpen() || !m_timer.isActive()) {
402408
return false;
403409
}
410+
qDebug() << "LogFile - pauseAndResetPosition";
404411
m_timer.stop();
405412
m_replayStatus = STOPPED;
406413

@@ -426,11 +433,11 @@ ReplayState LogFile::getReplayStatus()
426433
/**
427434
* FUNCTION: buildIndex()
428435
*
429-
* Walk through the opened logfile and sets the start and end position timestamps
436+
* Walk through the opened logfile and stores the first and last position timestamps.
430437
* Also builds an index for quickly skipping to a specific position in the logfile.
431438
*
432-
* returns true when indexing has completed successfully
433-
* returns false when a problem was encountered
439+
* Returns true when indexing has completed successfully.
440+
* Returns false when a problem was encountered.
434441
*
435442
*/
436443
bool LogFile::buildIndex()
@@ -439,18 +446,28 @@ bool LogFile::buildIndex()
439446
qint64 totalSize;
440447
qint64 readPointer = 0;
441448
quint64 index = 0;
449+
int bytesRead = 0;
442450

443-
QByteArray arr = m_file.readAll();
451+
qDebug() << "LogFile - buildIndex";
444452

453+
// Ensure empty vectors:
454+
m_timeStampPositions.clear();
455+
m_timeStamps.clear();
456+
457+
QByteArray arr = m_file.readAll();
445458
totalSize = arr.size();
446459
QDataStream dataStream(&arr, QIODevice::ReadOnly);
447460

448-
// set the start timestamp
449-
if (totalSize - readPointer >= 4) {
450-
dataStream.readRawData((char *)&timeStamp, 4);
461+
// set the first timestamp
462+
if (totalSize - readPointer >= TIMESTAMP_SIZE_BYTES) {
463+
bytesRead = dataStream.readRawData((char *)&timeStamp, TIMESTAMP_SIZE_BYTES);
464+
if (bytesRead != TIMESTAMP_SIZE_BYTES) {
465+
qWarning() << "LogFile buildIndex - read first timeStamp: readRawData returned unexpected number of bytes:" << bytesRead << "at position" << readPointer << "\n";
466+
return false;
467+
}
451468
m_timeStamps.append(timeStamp);
452469
m_timeStampPositions.append(readPointer);
453-
readPointer += 4;
470+
readPointer += TIMESTAMP_SIZE_BYTES;
454471
index++;
455472
m_beginTimeStamp = timeStamp;
456473
m_endTimeStamp = timeStamp;
@@ -461,41 +478,51 @@ bool LogFile::buildIndex()
461478

462479
// Check if there are enough bytes remaining for a correct "dataSize" field
463480
if (totalSize - readPointer < (qint64)sizeof(dataSize)) {
464-
qDebug() << "Error: Logfile corrupted! Unexpected end of file";
481+
qWarning() << "LogFile buildIndex - logfile corrupted! Unexpected end of file";
482+
return false;
483+
}
484+
485+
// Read the dataSize field and check for I/O errors
486+
bytesRead = dataStream.readRawData((char *)&dataSize, sizeof(dataSize));
487+
if (bytesRead != sizeof(dataSize)) {
488+
qWarning() << "LogFile buildIndex - read dataSize: readRawData returned unexpected number of bytes:" << bytesRead << "at position" << readPointer << "\n";
465489
return false;
466490
}
467491

468-
// Read the dataSize field
469-
dataStream.readRawData((char *)&dataSize, sizeof(dataSize));
470492
readPointer += sizeof(dataSize);
471493

472494
if (dataSize < 1 || dataSize > (1024 * 1024)) {
473-
qDebug() << "Error: Logfile corrupted! Unlikely packet size: " << dataSize << "\n";
495+
qWarning() << "LogFile buildIndex - logfile corrupted! Unlikely packet size: " << dataSize << "\n";
474496
return false;
475497
}
476498

477499
// Check if there are enough bytes remaining
478500
if (totalSize - readPointer < dataSize) {
479-
qDebug() << "Error: Logfile corrupted! Unexpected end of file";
501+
qWarning() << "LogFile buildIndex - logfile corrupted! Unexpected end of file";
480502
return false;
481503
}
482504

483505
// skip reading the data (we don't need it at this point)
484506
readPointer += dataStream.skipRawData(dataSize);
485507

486508
// read the next timestamp
487-
if (totalSize - readPointer >= 4) {
488-
dataStream.readRawData((char *)&timeStamp, 4);
509+
if (totalSize - readPointer >= TIMESTAMP_SIZE_BYTES) {
510+
bytesRead = dataStream.readRawData((char *)&timeStamp, TIMESTAMP_SIZE_BYTES);
511+
if (bytesRead != TIMESTAMP_SIZE_BYTES) {
512+
qWarning() << "LogFile buildIndex - read timeStamp, readRawData returned unexpected number of bytes:" << bytesRead << "at position" << readPointer << "\n";
513+
return false;
514+
}
489515

490516
// some validity checks
491517
if (timeStamp < m_endTimeStamp // logfile goes back in time
492518
|| (timeStamp - m_endTimeStamp) > (60 * 60 * 1000)) { // gap of more than 60 minutes)
493-
qDebug() << "Error: Logfile corrupted! Unlikely timestamp " << timeStamp << " after " << m_endTimeStamp;
519+
qWarning() << "LogFile buildIndex - logfile corrupted! Unlikely timestamp " << timeStamp << " after " << m_endTimeStamp;
520+
return false;
494521
}
495522

496523
m_timeStamps.append(timeStamp);
497524
m_timeStampPositions.append(readPointer);
498-
readPointer += 4;
525+
readPointer += TIMESTAMP_SIZE_BYTES;
499526
index++;
500527
m_endTimeStamp = timeStamp;
501528
} else {
@@ -504,7 +531,7 @@ bool LogFile::buildIndex()
504531
}
505532
}
506533

507-
emit updateBeginAndEndtimes(m_beginTimeStamp, m_endTimeStamp);
534+
emit updateBeginAndEndTimes(m_beginTimeStamp, m_endTimeStamp);
508535

509536
// reset the read pointer to the start of the file
510537
m_file.seek(0);

ground/gcs/src/libs/utils/logfile.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ protected slots:
100100
void replayStarted();
101101
void replayFinished();
102102
void playbackPosition(quint32);
103-
void updateBeginAndEndtimes(quint32, quint32);
103+
void updateBeginAndEndTimes(quint32, quint32);
104104

105105
protected:
106106
QByteArray m_dataBuffer;

ground/gcs/src/plugins/logging/logginggadgetwidget.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ void LoggingGadgetWidget::setPlugin(LoggingPlugin *p)
7979

8080
// Feedback from logfile to GUI
8181
connect(loggingPlugin, &LoggingPlugin::stateChanged, this, &LoggingGadgetWidget::stateChanged);
82-
connect(logFile, &LogFile::updateBeginAndEndtimes, this, &LoggingGadgetWidget::updateBeginAndEndtimes);
82+
connect(logFile, &LogFile::updateBeginAndEndTimes, this, &LoggingGadgetWidget::updateBeginAndEndTimes);
8383
connect(logFile, &LogFile::playbackPosition, this, &LoggingGadgetWidget::playbackPosition);
8484
connect(logFile, &LogFile::replayStarted, this, &LoggingGadgetWidget::enableButtons);
8585
connect(logFile, &LogFile::replayFinished, this, &LoggingGadgetWidget::disableButtons);
@@ -174,7 +174,7 @@ void LoggingGadgetWidget::stateChanged(LoggingPlugin::State state)
174174
}
175175
}
176176

177-
void LoggingGadgetWidget::updateBeginAndEndtimes(quint32 startTimeStamp, quint32 endTimeStamp)
177+
void LoggingGadgetWidget::updateBeginAndEndTimes(quint32 startTimeStamp, quint32 endTimeStamp)
178178
{
179179
int startSec, startMin, endSec, endMin;
180180

ground/gcs/src/plugins/logging/logginggadgetwidget.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class LoggingGadgetWidget : public QWidget {
4949

5050
protected slots:
5151
void stateChanged(LoggingPlugin::State state);
52-
void updateBeginAndEndtimes(quint32 startTimeStamp, quint32 endTimeStamp);
52+
void updateBeginAndEndTimes(quint32 startTimeStamp, quint32 endTimeStamp);
5353
void playbackPosition(quint32 positionTimeStamp);
5454
void playPauseButtonAction();
5555
void stopButtonAction();

0 commit comments

Comments
 (0)