Bugfixes automatic source/receiver calibration

Software/PC_Application/Calibration/amplitudecaldialog.cpp

@@ -60,6 +60,8 @@ AmplitudeCalDialog::AmplitudeCalDialog(Device *dev, QWidget *parent) :
         model.dataChanged(model.index(0, model.ColIndexPort1), model.index(points.size(), model.ColIndexPort2));
     });
     connect(ui->automatic, &QPushButton::clicked, this, &AmplitudeCalDialog::AutomaticMeasurementDialog);
+
+    connect(dev, &Device::SpectrumResultReceived, this, &AmplitudeCalDialog::ReceivedMeasurement);
 }
 
 AmplitudeCalDialog::~AmplitudeCalDialog()
@@ -98,10 +100,12 @@ void AmplitudeCalDialog::setAmplitude(double amplitude, unsigned int point, bool
         // apply result from port 1 to port 2 as well
         points[point].correctionPort2 = points[point].correctionPort1;
         points[point].port2set = points[point].port1set;
+        points[point].amplitudePort2 = points[point].amplitudePort1;
     } else if(mode == CalibrationMode::OnlyPort2 && port2) {
         // apply result from port 2 to port 1 as well
         points[point].correctionPort1 = points[point].correctionPort2;
         points[point].port1set = points[point].port2set;
+        points[point].amplitudePort1 = points[point].amplitudePort2;
     }
     model.dataChanged(model.index(point, model.ColIndexCorrectionFactors), model.index(point, model.ColIndexPort2));
     UpdateSaveButton();
@@ -254,9 +258,9 @@ void AmplitudeCalDialog::AddPointDialog()
 
 void AmplitudeCalDialog::AutomaticMeasurementDialog()
 {
-    bool isSourceCal = pointType() == Protocol::PacketType::SourceCalPoint;
+    automatic.isSourceCal = pointType() == Protocol::PacketType::SourceCalPoint;
-    const QString ownCal = isSourceCal ? "Source" : "Receiver";
+    const QString ownCal = automatic.isSourceCal ? "Source" : "Receiver";
-    const QString otherCal = isSourceCal ? "Receiver" : "Source";
+    const QString otherCal = automatic.isSourceCal ? "Receiver" : "Source";
 
     // compile info text
     QString info = "It is possible to determine the " + ownCal + " Calibration if a valid " + otherCal
@@ -264,22 +268,26 @@ void AmplitudeCalDialog::AutomaticMeasurementDialog()
             + " between the ports. For each point in the " + otherCal + " Calibration, the device will take"
             + " a measurement and determine the correction factors for the " + ownCal + " Calibration.";
 
-    auto d = new QDialog(this);
+    automatic.dialog = new QDialog(this);
     auto ui = new Ui::AutomaticAmplitudeDialog();
-    ui->setupUi(d);
+    ui->setupUi(automatic.dialog);
+    automatic.progress = ui->progress;
     ui->explanation->setText(info);
     ui->status->setText("Gathering information about "+otherCal+" Calibration...");
 
     automatic.points.clear();
 
-    connect(d, &QDialog::rejected, ui->abort, &QPushButton::click);
+    connect(automatic.dialog, &QDialog::rejected, ui->abort, &QPushButton::click);
     connect(ui->abort, &QPushButton::clicked, [=](){
         // aborted, clean up
-        delete d;
+        disconnect(dev, &Device::SpectrumResultReceived, this, &AmplitudeCalDialog::ReceivedAutomaticMeasurementResult);
+        dev->SetIdle();
+        delete ui;
+        delete automatic.dialog;
     });
 
     dev->SetIdle();
-    auto receiveConn = connect(dev, &Device::AmplitudeCorrectionPointReceived, this, [this, ui, otherCal](Protocol::AmplitudeCorrectionPoint p) {
+    connect(dev, &Device::AmplitudeCorrectionPointReceived, this, [this, ui, otherCal](Protocol::AmplitudeCorrectionPoint p) {
         CorrectionPoint c;
         c.frequency = p.freq * 10.0;
         c.correctionPort1 = p.port1;
@@ -294,11 +302,12 @@ void AmplitudeCalDialog::AutomaticMeasurementDialog()
             ui->status->setText(otherCal + " Calibration contains " +QString::number(p.totalPoints)+" points, ready to start measurement");
             ui->start->setEnabled(true);
             disconnect(dev, &Device::AmplitudeCorrectionPointReceived, this, nullptr);
+            qDebug() << "Received" << p.totalPoints << "points for automatic calibration";
         }
     });
     // request points of otherCal
     // switch between source/receiver calibration
-    auto request = isSourceCal ? Protocol::PacketType::RequestReceiverCal : Protocol::PacketType::RequestSourceCal;
+    auto request = automatic.isSourceCal ? Protocol::PacketType::RequestReceiverCal : Protocol::PacketType::RequestSourceCal;
     dev->SendCommandWithoutPayload(request);
 
     connect(ui->start, &QPushButton::clicked, [=](){
@@ -308,58 +317,26 @@ void AmplitudeCalDialog::AutomaticMeasurementDialog()
             AddPoint(p.frequency);
         }
         // intialize measurement state machine
-        automatic.resultConnection = connect(dev, &Device::SpectrumResultReceived, [=](Protocol::SpectrumAnalyzerResult res) {
+        connect(dev, &Device::SpectrumResultReceived, this, &AmplitudeCalDialog::ReceivedAutomaticMeasurementResult);
-            if(res.pointNum != 1) {
-                // ignore first and last point, only use the middle one
-                return;
-            }
-            if(automatic.settlingCount > 0) {
-                automatic.settlingCount--;
-                return;
-            }
-            // Grab correct measurement
-            double measurement;
-            if(isSourceCal) {
-                // For a source calibration we need the measurement of the other port (measuring the ouput power of the port to calibrate)
-                measurement = automatic.measuringPort2 ? res.port1 : res.port2;
-            } else {
-                // For a receiver calibration we need the measurement of the port to calibrate
-                measurement = automatic.measuringPort2 ? res.port2 : res.port1;
-            }
-            // convert to dbm
-            double reported_dbm = 20*log10(measurement);
-            if(isSourceCal) {
-                // receiver cal already done, the measurement result is accurate and can be used to determine actual output power
-                setAmplitude(reported_dbm, automatic.measuringCount, automatic.measuringPort2);
-            } else {
-                // source cal already done, the output power is accurate while the measurement might be off
-                setAmplitude(excitationAmplitude, automatic.measuringCount, automatic.measuringPort2);
-            }
-            // advance state machine
-            // switch ports
-            automatic.measuringPort2 = !automatic.measuringPort2;
-            if(!automatic.measuringPort2) {
-                // now measuring port1, this means we have to advance to the next point
-                automatic.measuringCount++;
-                if(automatic.measuringCount >= points.size()) {
-                    // all done, disconnect this lambda and close dialog
-                    disconnect(automatic.resultConnection);
-                    delete d;
-                    dev->SetIdle();
-                } else {
-                    // update progress bar
-                    ui->progress->setValue(100 * automatic.measuringCount / points.size());
-                    // Start next measurement
-                    SetupNextAutomaticPoint(isSourceCal);
-                }
-            }
-        });
         automatic.measuringPort2 = false;
         automatic.measuringCount = 0;
-        SetupNextAutomaticPoint(isSourceCal);
+        ui->status->setText("Taking measurements...");
+        SetupNextAutomaticPoint(automatic.isSourceCal);
     });
 
-    d->show();
+    automatic.dialog->show();
+}
+
+void AmplitudeCalDialog::ReceivedMeasurement(Protocol::SpectrumAnalyzerResult res)
+{
+    if(res.pointNum == 1) {
+        // store result in center of sweep of 3 points
+        if(measured.size() >= averages) {
+            measured.pop_front();
+        }
+        MeasurementResult m = {.port1 = 20*log10(res.port1), .port2 = 20*log10(res.port2)};
+        measured.push_back(m);
+    }
 }
 
 bool AmplitudeCalDialog::ConfirmActionIfEdited()
@@ -393,6 +370,7 @@ void AmplitudeCalDialog::UpdateSaveButton()
 
 void AmplitudeCalDialog::SetupNextAutomaticPoint(bool isSourceCal)
 {
+    qDebug() << "Starting automatic calibration measurement for point" << automatic.measuringCount <<", measuring port2:" << automatic.measuringPort2;
     auto point = automatic.points[automatic.measuringCount];
     Protocol::PacketInfo p = {};
     p.type = Protocol::PacketType::SpectrumAnalyzerSettings;
@@ -410,7 +388,19 @@ void AmplitudeCalDialog::SetupNextAutomaticPoint(bool isSourceCal)
     p.spectrumSettings.trackingGeneratorOffset = 0;
     // For a source cal, the tracking generator has to be enabled at the measurement port (calibrating the output power)
     // For a receiver cal, the tracking generator has to be enabled at the other port (calibrating received power)
-    p.spectrumSettings.trackingGeneratorPort = isSourceCal ? automatic.measuringPort2 : !automatic.measuringPort2;
+    if(isSourceCal) {
+        if(automatic.measuringPort2) {
+            p.spectrumSettings.trackingGeneratorPort = 1;
+        } else {
+            p.spectrumSettings.trackingGeneratorPort = 0;
+        }
+    } else {
+        if(automatic.measuringPort2) {
+            p.spectrumSettings.trackingGeneratorPort = 0;
+        } else {
+            p.spectrumSettings.trackingGeneratorPort = 1;
+        }
+    }
     if(isSourceCal) {
         // Calibrating the source which means the receiver is already calibrated -> use receiver corrections but no source corrections
         p.spectrumSettings.applyReceiverCorrection = 1;
@@ -420,10 +410,64 @@ void AmplitudeCalDialog::SetupNextAutomaticPoint(bool isSourceCal)
         p.spectrumSettings.applyReceiverCorrection = 0;
         p.spectrumSettings.applySourceCorrection = 1;
     }
-    automatic.settlingCount = 30;
+    automatic.settlingCount = averages + automaticSettling;
     dev->SendPacket(p);
 }
 
+void AmplitudeCalDialog::ReceivedAutomaticMeasurementResult(Protocol::SpectrumAnalyzerResult res)
+{
+    if(res.pointNum != 1) {
+        // ignore first and last point, only use the middle one
+        return;
+    }
+    if(automatic.settlingCount > 0) {
+        automatic.settlingCount--;
+        return;
+    }
+    if(automatic.isSourceCal) {
+        // grab correct measurement and convert to dbm
+        auto m = averageMeasurement();
+        double measurement = automatic.measuringPort2 ? m.port1 : m.port2;
+        // receiver cal already done, the measurement result is accurate and can be used to determine actual output power
+        setAmplitude(measurement, automatic.measuringCount, automatic.measuringPort2);
+    } else {
+        // source cal already done, the output power is accurate while the measurement might be off.
+        // The measurement result is already extracted by the ReceiverCalDialog
+        setAmplitude(excitationAmplitude, automatic.measuringCount, automatic.measuringPort2);
+    }
+    // update progress bar
+    automatic.progress->setValue(100 * (automatic.measuringCount * 2 + automatic.measuringPort2 + 1) / (points.size() * 2));
+    // advance state machine
+    // switch ports
+    automatic.measuringPort2 = !automatic.measuringPort2;
+    if(!automatic.measuringPort2) {
+        // now measuring port1, this means we have to advance to the next point
+        automatic.measuringCount++;
+        if(automatic.measuringCount >= points.size()) {
+            // all done, close dialog
+            // Closing the dialog will disconnect the connection to this function and return the device into the idle state
+            automatic.dialog->reject();
+            automatic.dialog = nullptr;
+            qDebug() << "Automatic source/receiver calibration complete";
+            return;
+        }
+    }
+    // Start next measurement
+    SetupNextAutomaticPoint(automatic.isSourceCal);
+}
+
+AmplitudeCalDialog::MeasurementResult AmplitudeCalDialog::averageMeasurement()
+{
+    MeasurementResult ret = {};
+    for(auto m : measured) {
+        ret.port1 += m.port1;
+        ret.port2 += m.port2;
+    }
+    ret.port1 /= measured.size();
+    ret.port2 /= measured.size();
+    return ret;
+}
+
 AmplitudeCalDialog::CalibrationMode AmplitudeCalDialog::getMode() const
 {
     return mode;

Software/PC_Application/Calibration/amplitudecaldialog.h

@@ -75,11 +75,14 @@ protected slots:
     void AddPoint(double frequency);
     void AddPointDialog();
     void AutomaticMeasurementDialog();
+    void ReceivedMeasurement(Protocol::SpectrumAnalyzerResult res);
 signals:
     void pointsUpdated();
     void newPointCreated(CorrectionPoint& p);
 protected:
     static constexpr double excitationAmplitude = -20.0;
+    static constexpr int averages = 20;
+    static constexpr int automaticSettling = 10;
 
     bool ConfirmActionIfEdited();
     void UpdateSaveButton();
@@ -100,13 +103,24 @@ protected:
     CalibrationMode mode;
 
     void SetupNextAutomaticPoint(bool isSourceCal);
+    void ReceivedAutomaticMeasurementResult(Protocol::SpectrumAnalyzerResult res);
     struct {
+        QDialog *dialog;
         std::vector<CorrectionPoint> points;
         bool measuringPort2; // true if port2 is being calibrated
         unsigned int measuringCount; // number of calibration point
         unsigned int settlingCount; // number of measurements still to ignore before taking measurement
-        QMetaObject::Connection resultConnection;
+        bool isSourceCal;
+        QProgressBar *progress;
     } automatic;
+
+    // raw (uncorrected) measurements from device.
+    // Used in reveicer calibration as well as in automatic calibrations
+    struct MeasurementResult {
+        double port1, port2;
+    };
+    std::deque<MeasurementResult> measured;
+    MeasurementResult averageMeasurement();
 };
 
 #endif // SOURCECALDIALOG_H

Software/PC_Application/Calibration/automaticamplitudedialog.ui

@@ -61,7 +61,10 @@
      <item>
       <widget class="QLineEdit" name="status">
        <property name="enabled">
-        <bool>false</bool>
+        <bool>true</bool>
+       </property>
+       <property name="readOnly">
+        <bool>true</bool>
        </property>
       </widget>
      </item>

Software/PC_Application/Calibration/receivercaldialog.cpp

@@ -5,13 +5,6 @@ ReceiverCalDialog::ReceiverCalDialog(Device *dev)
 {
     setWindowTitle("Receiver Calibration Dialog");
     LoadFromDevice();
-    connect(dev, &Device::SpectrumResultReceived, [=](Protocol::SpectrumAnalyzerResult res) {
-        if(res.pointNum == 1) {
-            // store result in center of sweep of 3 points
-            port1_result = 20*log10(res.port1);
-            port2_result = 20*log10(res.port2);
-        }
-    });
 }
 
 void ReceiverCalDialog::SelectedPoint(double frequency, bool)
@@ -38,11 +31,12 @@ void ReceiverCalDialog::SelectedPoint(double frequency, bool)
 
 void ReceiverCalDialog::AmplitudeChanged(AmplitudeCalDialog::CorrectionPoint &point, bool port2)
 {
+    auto m = averageMeasurement();
     auto *factor = port2 ? &point.correctionPort2 : &point.correctionPort1;
     const auto *amplitude = port2 ? &point.amplitudePort2 : &point.amplitudePort1;
-    const auto *measured = port2 ? &port2_result : &port1_result;
+    const auto *measured = port2 ? &m.port2 : &m.port1;
     // calculate correction factor by comparing expected with measured amplitude
-    *factor = (*measured - *amplitude) * 100.0;
+    *factor = (*amplitude - *measured) * 100.0;
 }
 
 void ReceiverCalDialog::UpdateAmplitude(AmplitudeCalDialog::CorrectionPoint &point)

Software/PC_Application/Calibration/receivercaldialog.h

@@ -14,9 +14,6 @@ protected:
     void SelectedPoint(double frequency, bool port2) override;
     void AmplitudeChanged(CorrectionPoint &point, bool port2) override;
     void UpdateAmplitude(CorrectionPoint& point) override;
-private:
-    static constexpr double excitationAmplitude = -20.0;
-    double port1_result, port2_result; // raw (uncorrected) measurements from device
 };
 
 #endif // RECEIVERCALDIALOG_H

Software/PC_Application/CustomWidgets/informationbox.cpp

@@ -3,14 +3,19 @@
 #include <QSettings>
 #include <QDebug>
 
-void InformationBox::ShowMessage(QString title, QString message, QWidget *parent)
+void InformationBox::ShowMessage(QString title, QString message, QString messageID)
 {
     // check if the user still wants to see this message
-    auto hash = qHash(message);
+    unsigned int hash;
+    if(messageID.isEmpty()) {
+        hash = qHash(message);
+    } else {
+        hash = qHash(messageID);
+    }
 
     QSettings s;
     if(!s.contains(hashToSettingsKey(hash))) {
-        auto box = new InformationBox(title, message, hash, parent);
+        auto box = new InformationBox(title, message, hash, nullptr);
         box->exec();
     }
 }

Software/PC_Application/CustomWidgets/informationbox.h

@@ -7,7 +7,7 @@ class InformationBox : public QMessageBox
 {
     Q_OBJECT
 public:
-    static void ShowMessage(QString title, QString message, QWidget *parent = nullptr);
+    static void ShowMessage(QString title, QString message, QString messageID = QString());
 private:
     InformationBox(QString title, QString message, unsigned int hash, QWidget *parent);
     ~InformationBox();

Software/PC_Application/SpectrumAnalyzer/spectrumanalyzer.cpp

@@ -221,7 +221,7 @@ SpectrumAnalyzer::SpectrumAnalyzer(AppWindow *window)
     window->addToolBar(tb_trackgen);
     toolbars.insert(tb_trackgen);
 
-    markerModel = new TraceMarkerModel(traceModel);
+    markerModel = new TraceMarkerModel(traceModel, this);
 
     auto tracesDock = new QDockWidget("Traces");
     tracesDock->setWidget(new TraceWidget(traceModel, window, true));
@@ -305,6 +305,32 @@ void SpectrumAnalyzer::SettingsChanged()
         settings.UseDFT = 0;
     }
 
+    if(settings.trackingGenerator && settings.f_stop >= 25000000) {
+        // Check point spacing.
+        // The highband PLL used as the tracking generator is not able to reach every frequency exactly. This
+        // could lead to sharp drops in the spectrum at certain frequencies. If the span is wide enough with
+        // respect to the point number, it is ensured that every displayed point has at least one sample with
+        // a reachable PLL frequency in it. Display a warning message if this is not the case with the current
+        // settings.
+        auto pointSpacing = (settings.f_stop - settings.f_start) / (settings.pointNum - 1);
+        // The frequency resolution of the PLL is frequency dependent (due to PLL divider).
+        // This code assumes some knowledge of the actual hardware and probably should be moved
+        // onto the device at some point
+        double minSpacing = 25000;
+        auto stop = settings.f_stop;
+        while(stop <= 3000000000) {
+            minSpacing /= 2;
+            stop *= 2;
+        }
+        if(pointSpacing < minSpacing) {
+            auto requiredMinSpan = minSpacing * (settings.pointNum - 1);
+            auto message = QString() + "Due to PLL limitations, the tracking generator can not reach every frequency exactly. "
+                            "With your current span, this could result in the signal not being detected at some bands. A minimum"
+                            " span of " + Unit::ToString(requiredMinSpan, "Hz", " kMG") + " is recommended at this stop frequency.";
+            InformationBox::ShowMessage("Warning", message, "TrackingGeneratorSpanTooSmallWarning");
+        }
+    }
+
     if(window->getDevice()) {
         window->getDevice()->Configure(settings);
     }

Software/PC_Application/Traces/tracemarkermodel.cpp

@@ -15,6 +15,11 @@ TraceMarkerModel::TraceMarkerModel(TraceModel &model, QObject *parent)
     root = new TraceMarker(this);
 }
 
+TraceMarkerModel::~TraceMarkerModel()
+{
+    delete root;
+}
+
 QModelIndex TraceMarkerModel::index(int row, int column, const QModelIndex &parent) const
 {
     if (!hasIndex(row, column, parent)) {

Software/PC_Application/Traces/tracemarkermodel.h

@@ -37,6 +37,7 @@ class TraceMarkerModel : public QAbstractItemModel
     Q_OBJECT
 public:
     TraceMarkerModel(TraceModel &model, QObject *parent = 0);
+    ~TraceMarkerModel();
 
     enum {
         ColIndexNumber = 0,

Software/PC_Application/VNA/vna.cpp

@@ -651,7 +651,7 @@ void VNA::ApplyCalibration(Calibration::Type type)
         }
     } else {
         // Not all required traces available
-        InformationBox::ShowMessage("Missing calibration measurements", "Not all calibration measurements for this type of calibration have been taken. The calibration can be enabled after the missing measurements have been acquired.", window);
+        InformationBox::ShowMessage("Missing calibration measurements", "Not all calibration measurements for this type of calibration have been taken. The calibration can be enabled after the missing measurements have been acquired.");
         DisableCalibration(true);
         StartCalibrationDialog(type);
     }