Big change to serial code - moved out to its own class and thread. Serial comm is now run on a separate thread. This code now certainly leaks a small amount of memory but that'll get fixed.

SavvyCAN.pro

@@ -20,7 +20,8 @@ SOURCES += main.cpp\
     graphingwindow.cpp \
     frameinfowindow.cpp \
     newgraphdialog.cpp \
-    frameplaybackwindow.cpp
+    frameplaybackwindow.cpp \
+    serialworker.cpp
 
 HEADERS  += mainwindow.h \
     can_structs.h \
@@ -30,7 +31,8 @@ HEADERS  += mainwindow.h \
     graphingwindow.h \
     frameinfowindow.h \
     newgraphdialog.h \
-    frameplaybackwindow.h
+    frameplaybackwindow.h \
+    serialworker.h
 
 FORMS    += mainwindow.ui \
     graphingwindow.ui \

mainwindow.cpp

@@ -6,6 +6,7 @@
 #include <QtSerialPort/QSerialPortInfo>
 #include "canframemodel.h"
 #include "utility.h"
+#include "serialworker.h"
 
 /*
 This first order of business is to attempt to gain feature parity (roughly) with GVRET-PC so  that this
@@ -24,14 +25,6 @@ MainWindow::MainWindow(QWidget *parent) :
 {
     ui->setupUi(this);
 
-    connect(ui->btnConnect, SIGNAL(clicked(bool)), this, SLOT(connButtonPress()));
-    connect(ui->actionOpen_Log_File, SIGNAL(triggered(bool)), this, SLOT(handleLoadFile()));
-    connect(ui->actionGraph_Dta, SIGNAL(triggered(bool)), this, SLOT(showGraphingWindow()));
-    connect(ui->actionFrame_Data_Analysis, SIGNAL(triggered(bool)), this, SLOT(showFrameDataAnalysis()));
-    connect(ui->btnClearFrames, SIGNAL(clicked(bool)), this, SLOT(clearFrames()));
-    connect(ui->actionSave_Log_File, SIGNAL(triggered(bool)), this, SLOT(handleSaveFile()));
-    connect(ui->action_Playback, SIGNAL(triggered(bool)), this, SLOT(showPlaybackWindow()));
-
     model = new CANFrameModel();
     ui->canFramesView->setModel(model);
 
@@ -52,20 +45,43 @@ MainWindow::MainWindow(QWidget *parent) :
         ui->cbSerialPorts->addItem(ports[i].portName());
     }
 
-    port = new QSerialPort(this);
+    SerialWorker *worker = new SerialWorker();
-
+    worker->moveToThread(&serialWorkerThread);
-    rx_state = IDLE;
+    connect(&serialWorkerThread, &QThread::finished, worker, &QObject::deleteLater);
-    rx_step = 0;
+    //connect(this, &Controller::operate, worker, &Worker::doWork);
+    //connect(worker, &Worker::resultReady, this, &Controller::handleResults);
+    connect(this, &MainWindow::sendSerialPort, worker, &SerialWorker::setSerialPort, Qt::QueuedConnection);
+    connect(worker, &SerialWorker::receivedFrame, this, &MainWindow::gotFrame, Qt::QueuedConnection);
+    serialWorkerThread.start();
 
     graphingWindow = NULL;
     frameInfoWindow = NULL;
     playbackWindow = NULL;
+
+    connect(ui->btnConnect, SIGNAL(clicked(bool)), this, SLOT(connButtonPress()));
+    connect(ui->actionOpen_Log_File, SIGNAL(triggered(bool)), this, SLOT(handleLoadFile()));
+    connect(ui->actionGraph_Dta, SIGNAL(triggered(bool)), this, SLOT(showGraphingWindow()));
+    connect(ui->actionFrame_Data_Analysis, SIGNAL(triggered(bool)), this, SLOT(showFrameDataAnalysis()));
+    connect(ui->btnClearFrames, SIGNAL(clicked(bool)), this, SLOT(clearFrames()));
+    connect(ui->actionSave_Log_File, SIGNAL(triggered(bool)), this, SLOT(handleSaveFile()));
+    connect(ui->action_Playback, SIGNAL(triggered(bool)), this, SLOT(showPlaybackWindow()));
+
 }
 
 MainWindow::~MainWindow()
 {
     delete ui;
     if (graphingWindow) delete graphingWindow;
+
+    serialWorkerThread.quit();
+    serialWorkerThread.wait();
+}
+
+void MainWindow::gotFrame(CANFrame *frame)
+{
+    qDebug() << "got frame from serial side. ID was " << frame->ID;
+    addFrameToDisplay(*frame, true);
+    ui->lbNumFrames->setText(QString::number(model->rowCount()));
 }
 
 void MainWindow::addFrameToDisplay(CANFrame &frame, bool autoRefresh = false)
@@ -508,22 +524,7 @@ void MainWindow::handleSaveFile()
 
 void MainWindow::connButtonPress()
 {
-    if (port->isOpen())
+    emit sendSerialPort(ui->cbSerialPorts->currentText());
-    {
-        port->close();
-    }
-    else
-    {
-        port->setPortName(ui->cbSerialPorts->currentText());
-        port->open(QIODevice::ReadWrite);
-        QByteArray output;
-        output.append(0xE7);
-        output.append(0xE7);
-        port->write(output);
-        ///isConnected = true;
-        connect(port, SIGNAL(readyRead()), this, SLOT(readSerialData()));
-
-    }
 }
 
 void MainWindow::showGraphingWindow()
@@ -544,123 +545,3 @@ void MainWindow::showPlaybackWindow()
     if (!playbackWindow) playbackWindow = new FramePlaybackWindow(model->getListReference());
     playbackWindow->show();
 }
-
-void MainWindow::readSerialData()
-{
-    QByteArray data = port->readAll();
-    unsigned char c;
-    qDebug() << (tr("Got data from serial. Len = %0").arg(data.length()));
-    for (int i = 0; i < data.length(); i++)
-    {
-        c = data.at(i);
-        procRXChar(c);
-    }
-}
-
-void MainWindow::procRXChar(unsigned char c)
-{
-    switch (rx_state)
-    {
-    case IDLE:
-        if (c == 0xF1) rx_state = GET_COMMAND;
-        break;
-    case GET_COMMAND:
-        switch (c)
-        {
-        case 0: //receiving a can frame
-            rx_state = BUILD_CAN_FRAME;
-            rx_step = 0;
-            break;
-        case 1: //we don't accept time sync commands from the firmware
-            rx_state = IDLE;
-            break;
-        case 2: //process a return reply for digital input states.
-            rx_state = GET_DIG_INPUTS;
-            rx_step = 0;
-            break;
-        case 3: //process a return reply for analog inputs
-            rx_state = GET_ANALOG_INPUTS;
-            break;
-        case 4: //we set digital outputs we don't accept replies so nothing here.
-            rx_state = IDLE;
-            break;
-        case 5: //we set canbus specs we don't accept replies.
-            rx_state = IDLE;
-            break;
-        }
-        break;
-    case BUILD_CAN_FRAME:
-        switch (rx_step)
-        {
-        case 0:
-            buildFrame.timestamp = c;
-            break;
-        case 1:
-            buildFrame.timestamp |= (uint)(c << 8);
-            break;
-        case 2:
-            buildFrame.timestamp |= (uint)c << 16;
-            break;
-        case 3:
-            buildFrame.timestamp |= (uint)c << 24;
-            break;
-        case 4:
-            buildFrame.ID = c;
-            break;
-        case 5:
-            buildFrame.ID |= c << 8;
-            break;
-        case 6:
-            buildFrame.ID |= c << 16;
-            break;
-        case 7:
-            buildFrame.ID |= c << 24;
-            if ((buildFrame.ID & 1 << 31) == 1 << 31)
-            {
-                buildFrame.ID &= 0x7FFFFFFF;
-                buildFrame.extended = true;
-            }
-            else buildFrame.extended = false;
-            break;
-        case 8:
-            buildFrame.len = c & 0xF;
-            if (buildFrame.len > 8) buildFrame.len = 8;
-            buildFrame.bus = (c & 0xF0) >> 4;
-            break;
-        default:
-            if (rx_step < buildFrame.len + 9)
-            {
-                buildFrame.data[rx_step - 9] = c;
-            }
-            else
-            {
-                rx_state = IDLE;
-                rx_step = 0;
-                addFrameToDisplay(buildFrame, true);
-                ui->lbNumFrames->setText(QString::number(model->rowCount()));
-            }
-            break;
-        }
-        rx_step++;
-        break;
-    case GET_ANALOG_INPUTS: //get 9 bytes - 2 per analog input plus checksum
-        switch (rx_step)
-        {
-        case 0:
-            break;
-        }
-        rx_step++;
-        break;
-    case GET_DIG_INPUTS: //get two bytes. One for digital in status and one for checksum.
-        switch (rx_step)
-        {
-        case 0:
-            break;
-        case 1:
-            rx_state = IDLE;
-            break;
-        }
-        rx_step++;
-        break;
-    }
-}

mainwindow.h

@@ -13,18 +13,6 @@ namespace Ui {
 class MainWindow;
 }
 
-enum STATE //keep this enum synchronized with the Arduino firmware project
-{
-    IDLE,
-    GET_COMMAND,
-    BUILD_CAN_FRAME,
-    TIME_SYNC,
-    GET_DIG_INPUTS,
-    GET_ANALOG_INPUTS,
-    SET_DIG_OUTPUTS,
-    SETUP_CANBUS
-};
-
 class MainWindow : public QMainWindow
 {
     Q_OBJECT
@@ -37,20 +25,22 @@ private slots:
     void handleLoadFile();
     void handleSaveFile();
     void connButtonPress();
-    void readSerialData();
     void showGraphingWindow();
     void showFrameDataAnalysis();
     void clearFrames();
     void showPlaybackWindow();
 
+public slots:
+    void gotFrame(CANFrame *frame);
+
+signals:
+    void sendSerialPort(QString portName);
+
 private:
     Ui::MainWindow *ui;
     CANFrameModel *model;
-    QSerialPort *port;
+    QThread serialWorkerThread;
     QByteArray inputBuffer;
-    STATE rx_state;
-    int rx_step;
-    CANFrame buildFrame;
     GraphingWindow *graphingWindow;
     FrameInfoWindow *frameInfoWindow;
     FramePlaybackWindow *playbackWindow;
@@ -65,7 +55,6 @@ private:
     void saveLogFile(QString);
     void saveMicrochipFile(QString);
     void addFrameToDisplay(CANFrame &, bool);
-    void procRXChar(unsigned char);
 };
 
 #endif // MAINWINDOW_H

serialworker.cpp

@@ -0,0 +1,160 @@
+#include "serialworker.h"
+
+#include <QSerialPort>
+#include <QDebug>
+
+SerialWorker::SerialWorker(QObject *parent) : QObject(parent)
+{
+    serial = NULL;
+    rx_state = IDLE;
+    rx_step = 0;
+    buildFrame = new CANFrame;
+}
+
+SerialWorker::~SerialWorker()
+{
+    if (serial != NULL) delete serial;
+}
+
+void SerialWorker::setSerialPort(QString portName)
+{
+    if (serial == NULL) serial = new QSerialPort(this);
+
+    if (serial->isOpen())
+    {
+        serial->close();
+    }
+    else
+    {
+        qDebug() << "Serial port name is " << portName;
+        serial->setPortName(portName);
+        serial->open(QIODevice::ReadWrite);
+        QByteArray output;
+        output.append(0xE7);
+        output.append(0xE7);
+        serial->write(output);
+        ///isConnected = true;
+        connect(serial, SIGNAL(readyRead()), this, SLOT(readSerialData()));
+    }
+}
+
+void SerialWorker::readSerialData()
+{
+    QByteArray data = serial->readAll();
+    unsigned char c;
+    qDebug() << (tr("Got data from serial. Len = %0").arg(data.length()));
+    for (int i = 0; i < data.length(); i++)
+    {
+        c = data.at(i);
+        procRXChar(c);
+    }
+}
+
+void SerialWorker::procRXChar(unsigned char c)
+{
+    switch (rx_state)
+    {
+    case IDLE:
+        if (c == 0xF1) rx_state = GET_COMMAND;
+        break;
+    case GET_COMMAND:
+        switch (c)
+        {
+        case 0: //receiving a can frame
+            rx_state = BUILD_CAN_FRAME;
+            rx_step = 0;
+            break;
+        case 1: //we don't accept time sync commands from the firmware
+            rx_state = IDLE;
+            break;
+        case 2: //process a return reply for digital input states.
+            rx_state = GET_DIG_INPUTS;
+            rx_step = 0;
+            break;
+        case 3: //process a return reply for analog inputs
+            rx_state = GET_ANALOG_INPUTS;
+            break;
+        case 4: //we set digital outputs we don't accept replies so nothing here.
+            rx_state = IDLE;
+            break;
+        case 5: //we set canbus specs we don't accept replies.
+            rx_state = IDLE;
+            break;
+        }
+        break;
+    case BUILD_CAN_FRAME:
+        switch (rx_step)
+        {
+        case 0:
+            buildFrame->timestamp = c;
+            break;
+        case 1:
+            buildFrame->timestamp |= (uint)(c << 8);
+            break;
+        case 2:
+            buildFrame->timestamp |= (uint)c << 16;
+            break;
+        case 3:
+            buildFrame->timestamp |= (uint)c << 24;
+            break;
+        case 4:
+            buildFrame->ID = c;
+            break;
+        case 5:
+            buildFrame->ID |= c << 8;
+            break;
+        case 6:
+            buildFrame->ID |= c << 16;
+            break;
+        case 7:
+            buildFrame->ID |= c << 24;
+            if ((buildFrame->ID & 1 << 31) == 1 << 31)
+            {
+                buildFrame->ID &= 0x7FFFFFFF;
+                buildFrame->extended = true;
+            }
+            else buildFrame->extended = false;
+            break;
+        case 8:
+            buildFrame->len = c & 0xF;
+            if (buildFrame->len > 8) buildFrame->len = 8;
+            buildFrame->bus = (c & 0xF0) >> 4;
+            break;
+        default:
+            if (rx_step < buildFrame->len + 9)
+            {
+                buildFrame->data[rx_step - 9] = c;
+            }
+            else
+            {
+                rx_state = IDLE;
+                rx_step = 0;
+                qDebug() << "emit from serial handler to main form id: " << buildFrame->ID;
+                emit receivedFrame(buildFrame);
+                buildFrame = new CANFrame;
+            }
+            break;
+        }
+        rx_step++;
+        break;
+    case GET_ANALOG_INPUTS: //get 9 bytes - 2 per analog input plus checksum
+        switch (rx_step)
+        {
+        case 0:
+            break;
+        }
+        rx_step++;
+        break;
+    case GET_DIG_INPUTS: //get two bytes. One for digital in status and one for checksum.
+        switch (rx_step)
+        {
+        case 0:
+            break;
+        case 1:
+            rx_state = IDLE;
+            break;
+        }
+        rx_step++;
+        break;
+    }
+}

serialworker.h

@@ -0,0 +1,49 @@
+#ifndef SERIALWORKER_H
+#define SERIALWORKER_H
+
+#include <QObject>
+#include <QSerialPort>
+#include "can_structs.h"
+
+enum STATE //keep this enum synchronized with the Arduino firmware project
+{
+    IDLE,
+    GET_COMMAND,
+    BUILD_CAN_FRAME,
+    TIME_SYNC,
+    GET_DIG_INPUTS,
+    GET_ANALOG_INPUTS,
+    SET_DIG_OUTPUTS,
+    SETUP_CANBUS
+};
+
+class SerialWorker : public QObject
+{
+    Q_OBJECT
+
+public:
+    SerialWorker(QObject *parent = 0);
+    ~SerialWorker();
+
+signals: //we emit signals
+    void error(const QString &s);
+    void receivedFrame(CANFrame *frame);
+
+private slots: //we receive things in slots
+    void readSerialData();
+
+public slots:
+    void setSerialPort(QString portName);
+
+private:
+    QString portName;
+    bool quit;
+    QSerialPort *serial;
+    STATE rx_state;
+    int rx_step;
+    CANFrame *buildFrame;
+
+    void procRXChar(unsigned char c);
+};
+
+#endif // SERIALTHREAD_H