From c0e4f41115f640b5b1e171350fdfc9f36cda751e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=A4berich?= Date: Thu, 29 Oct 2020 19:27:04 +0100 Subject: [PATCH] Various small bugfixes - Improved device communication (callbacks for transmissions working properly now) - Honor averaging when calibrating - Ignore delayed points from last sweep during calibration - Stop the sweep when disconnecting --- .../Calibration/calkitdialog.ui | 8 +- Software/PC_Application/Device/device.cpp | 77 +++++++++++++------ Software/PC_Application/Device/device.h | 12 ++- .../SpectrumAnalyzer/spectrumanalyzer.cpp | 2 +- Software/PC_Application/VNA/vna.cpp | 76 +++++++++++------- Software/PC_Application/VNA/vna.h | 6 +- Software/PC_Application/appwindow.cpp | 1 + Software/PC_Application/averaging.cpp | 16 +++- Software/PC_Application/averaging.h | 7 +- 9 files changed, 142 insertions(+), 63 deletions(-) diff --git a/Software/PC_Application/Calibration/calkitdialog.ui b/Software/PC_Application/Calibration/calkitdialog.ui index df9e004..e3bfb0c 100644 --- a/Software/PC_Application/Calibration/calkitdialog.ui +++ b/Software/PC_Application/Calibration/calkitdialog.ui @@ -484,7 +484,7 @@ - Offset delay [ps]: + Delay [ps]: @@ -494,7 +494,7 @@ - Offset loss [GΩ/s]: + Loss [GΩ/s]: @@ -903,10 +903,10 @@ - + + - diff --git a/Software/PC_Application/Device/device.cpp b/Software/PC_Application/Device/device.cpp index 2591cda..7961454 100644 --- a/Software/PC_Application/Device/device.cpp +++ b/Software/PC_Application/Device/device.cpp @@ -35,7 +35,10 @@ USBInBuffer::~USBInBuffer() // wait for cancellation to complete mutex mtx; unique_lock lck(mtx); - cv.wait(lck); + using namespace std::chrono_literals; + if(cv.wait_for(lck, 100ms) == cv_status::timeout) { + qWarning() << "Timed out waiting for mutex acquisition during disconnect"; + } } delete buffer; } @@ -69,10 +72,13 @@ void USBInBuffer::Callback(libusb_transfer *transfer) inCallback = false; break; case LIBUSB_TRANSFER_ERROR: - case LIBUSB_TRANSFER_NO_DEVICE: - case LIBUSB_TRANSFER_OVERFLOW: - case LIBUSB_TRANSFER_STALL: qCritical() << "LIBUSB_TRANSFER_ERROR"; + case LIBUSB_TRANSFER_NO_DEVICE: + qCritical() << "LIBUSB_TRANSFER_NO_DEVICE"; + case LIBUSB_TRANSFER_OVERFLOW: + qCritical() << "LIBUSB_TRANSFER_OVERFLOW"; + case LIBUSB_TRANSFER_STALL: + qCritical() << "LIBUSB_TRANSFER_STALL"; libusb_free_transfer(transfer); this->transfer = nullptr; emit TransferError(); @@ -167,12 +173,13 @@ Device::Device(QString serial) qInfo() << "USB connection established" << flush; m_connected = true; m_receiveThread = new std::thread(&Device::USBHandleThread, this); - dataBuffer = new USBInBuffer(m_handle, EP_Data_In_Addr, 2048); - logBuffer = new USBInBuffer(m_handle, EP_Log_In_Addr, 2048); + dataBuffer = new USBInBuffer(m_handle, EP_Data_In_Addr, 65536); + logBuffer = new USBInBuffer(m_handle, EP_Log_In_Addr, 65536); connect(dataBuffer, &USBInBuffer::DataReceived, this, &Device::ReceivedData, Qt::DirectConnection); connect(dataBuffer, &USBInBuffer::TransferError, this, &Device::ConnectionLost); connect(logBuffer, &USBInBuffer::DataReceived, this, &Device::ReceivedLog, Qt::DirectConnection); connect(&transmissionTimer, &QTimer::timeout, this, &Device::transmissionTimeout); + connect(this, &Device::receivedAnswer, this, &Device::transmissionFinished, Qt::QueuedConnection); transmissionTimer.setSingleShot(true); transmissionActive = false; // got a new connection, request limits @@ -182,6 +189,7 @@ Device::Device(QString serial) Device::~Device() { if(m_connected) { + SetIdle(); delete dataBuffer; delete logBuffer; m_connected = false; @@ -197,25 +205,26 @@ Device::~Device() } } -bool Device::SendPacket(Protocol::PacketInfo packet, std::function cb, unsigned int timeout) +bool Device::SendPacket(const Protocol::PacketInfo& packet, std::function cb, unsigned int timeout) { Transmission t; t.packet = packet; t.timeout = timeout; t.callback = cb; transmissionQueue.enqueue(t); +// qDebug() << "Enqueued packet, queue at " << transmissionQueue.size(); if(!transmissionActive) { startNextTransmission(); } return true; } -bool Device::Configure(Protocol::SweepSettings settings) +bool Device::Configure(Protocol::SweepSettings settings, std::function cb) { Protocol::PacketInfo p; p.type = Protocol::PacketType::SweepSettings; p.settings = settings; - return SendPacket(p); + return SendPacket(p, cb); } bool Device::Configure(Protocol::SpectrumAnalyzerSettings settings) @@ -234,6 +243,14 @@ bool Device::SetManual(Protocol::ManualControl manual) return SendPacket(p); } +bool Device::SetIdle() +{ + Protocol::SweepSettings s; + s.excitePort1 = 0; + s.excitePort2 = 0; + return Configure(s); +} + bool Device::SendFirmwareChunk(Protocol::FirmwarePacket &fw) { Protocol::PacketInfo p; @@ -402,11 +419,11 @@ void Device::ReceivedData() break; case Protocol::PacketType::Ack: emit AckReceived(); -// transmissionFinished(TransmissionResult::Ack); + emit receivedAnswer(TransmissionResult::Ack); break; case Protocol::PacketType::Nack: emit NackReceived(); -// transmissionFinished(TransmissionResult::Nack); + emit receivedAnswer(TransmissionResult::Nack); break; case Protocol::PacketType::DeviceLimits: limits = packet.limits; @@ -437,12 +454,12 @@ QString Device::serial() const return m_serial; } -void Device::startNextTransmission() +bool Device::startNextTransmission() { if(transmissionQueue.isEmpty() || !m_connected) { // nothing more to transmit transmissionActive = false; - return; + return false; } transmissionActive = true; auto t = transmissionQueue.head(); @@ -450,29 +467,45 @@ void Device::startNextTransmission() unsigned int length = Protocol::EncodePacket(t.packet, buffer, sizeof(buffer)); if(!length) { qCritical() << "Failed to encode packet"; - transmissionFinished(TransmissionResult::InternalError); + return false; } int actual_length; auto ret = libusb_bulk_transfer(m_handle, EP_Data_Out_Addr, buffer, length, &actual_length, 0); if(ret < 0) { qCritical() << "Error sending data: " << libusb_strerror((libusb_error) ret); - transmissionFinished(TransmissionResult::InternalError); + return false; } transmissionTimer.start(t.timeout); +// qDebug() << "Transmission started, queue at " << transmissionQueue.size(); + return true; } void Device::transmissionFinished(TransmissionResult result) { - transmissionTimer.stop(); // remove transmitted packet - if(!transmissionQueue.isEmpty()) { - auto t = transmissionQueue.dequeue(); - if(t.callback) { - t.callback(result); +// qDebug() << "Transmission finsished (" << result << "), queue at " << transmissionQueue.size(); + if(transmissionQueue.empty()) { + qWarning() << "transmissionFinished with empty transmission queue, stray Ack?"; + return; + } + auto t = transmissionQueue.dequeue(); + if(t.callback) { + t.callback(result); + } + transmissionTimer.stop(); + bool success = false; + while(!transmissionQueue.isEmpty() && !success) { + success = startNextTransmission(); + if(!success) { + // failed to send this packet + auto t = transmissionQueue.dequeue(); + if(t.callback) { + t.callback(TransmissionResult::InternalError); + } } - startNextTransmission(); - } else { + } + if(transmissionQueue.isEmpty()) { transmissionActive = false; } } diff --git a/Software/PC_Application/Device/device.h b/Software/PC_Application/Device/device.h index 28f0cb0..a1905ae 100644 --- a/Software/PC_Application/Device/device.h +++ b/Software/PC_Application/Device/device.h @@ -52,14 +52,16 @@ public: Timeout, InternalError, }; + Q_ENUM(TransmissionResult) // connect to a VNA device. If serial is specified only connecting to this device, otherwise to the first one found Device(QString serial = QString()); ~Device(); - bool SendPacket(Protocol::PacketInfo packet, std::function cb = nullptr, unsigned int timeout = 10); - bool Configure(Protocol::SweepSettings settings); + bool SendPacket(const Protocol::PacketInfo& packet, std::function cb = nullptr, unsigned int timeout = 200); + bool Configure(Protocol::SweepSettings settings, std::function cb = nullptr); bool Configure(Protocol::SpectrumAnalyzerSettings settings); bool SetManual(Protocol::ManualControl manual); + bool SetIdle(); bool SendFirmwareChunk(Protocol::FirmwarePacket &fw); bool SendCommandWithoutPayload(Protocol::PacketType type); QString serial() const; @@ -84,6 +86,9 @@ private slots: void transmissionTimeout() { transmissionFinished(TransmissionResult::Timeout); } + void transmissionFinished(TransmissionResult result); +signals: + void receivedAnswer(TransmissionResult result); private: static constexpr int EP_Data_Out_Addr = 0x01; @@ -107,8 +112,7 @@ private: }; QQueue transmissionQueue; - void startNextTransmission(); - void transmissionFinished(TransmissionResult result); + bool startNextTransmission(); QTimer transmissionTimer; bool transmissionActive; diff --git a/Software/PC_Application/SpectrumAnalyzer/spectrumanalyzer.cpp b/Software/PC_Application/SpectrumAnalyzer/spectrumanalyzer.cpp index b170984..3d676a5 100644 --- a/Software/PC_Application/SpectrumAnalyzer/spectrumanalyzer.cpp +++ b/Software/PC_Application/SpectrumAnalyzer/spectrumanalyzer.cpp @@ -254,7 +254,7 @@ void SpectrumAnalyzer::SettingsChanged() if(window->getDevice()) { window->getDevice()->Configure(settings); } - average.reset(); + average.reset(settings.pointNum); UpdateAverageCount(); traceModel.clearVNAData(); emit traceModel.SpanChanged(settings.f_start, settings.f_stop); diff --git a/Software/PC_Application/VNA/vna.cpp b/Software/PC_Application/VNA/vna.cpp index 916da79..0f896aa 100644 --- a/Software/PC_Application/VNA/vna.cpp +++ b/Software/PC_Application/VNA/vna.cpp @@ -95,11 +95,7 @@ VNA::VNA(AppWindow *window) calMenu->addSeparator(); auto calData = calMenu->addAction("Calibration Data"); connect(calData, &QAction::triggered, [=](){ - auto dialog = new CalibrationTraceDialog(&cal); - connect(dialog, &CalibrationTraceDialog::triggerMeasurement, this, &VNA::StartCalibrationMeasurement); - connect(dialog, &CalibrationTraceDialog::applyCalibration, this, &VNA::ApplyCalibration); - connect(this, &VNA::CalibrationMeasurementComplete, dialog, &CalibrationTraceDialog::measurementComplete); - dialog->show(); + StartCalibrationDialog(); }); auto calImport = calMenu->addAction("Import error terms as traces"); @@ -402,21 +398,26 @@ using namespace std; void VNA::NewDatapoint(Protocol::Datapoint d) { + d = average.process(d); if(calMeasuring) { - if(!calWaitFirst || d.pointNum == 0) { - calWaitFirst = false; - cal.addMeasurement(calMeasurement, d); - if(d.pointNum == settings.points - 1) { - calMeasuring = false; - emit CalibrationMeasurementComplete(calMeasurement); + + if(average.currentSweep() == averages) { + // this is the last averaging sweep, use values for calibration + if(!calWaitFirst || d.pointNum == 0) { + calWaitFirst = false; + cal.addMeasurement(calMeasurement, d); + if(d.pointNum == settings.points - 1) { + calMeasuring = false; + emit CalibrationMeasurementComplete(calMeasurement); + } } - calDialog.setValue(d.pointNum + 1); } + int percentage = (((average.currentSweep() - 1) * 100) + (d.pointNum + 1) * 100 / settings.points) / averages; + calDialog.setValue(percentage); } if(calValid) { cal.correctMeasurement(d); } - d = average.process(d); traceModel.addVNAData(d); emit dataChanged(); if(d.pointNum == settings.points - 1) { @@ -430,13 +431,13 @@ void VNA::UpdateAverageCount() lAverages->setText(QString::number(average.getLevel()) + "/"); } -void VNA::SettingsChanged() +void VNA::SettingsChanged(std::function cb) { settings.suppressPeaks = Preferences::getInstance().Acquisition.suppressPeaks ? 1 : 0; if(window->getDevice()) { - window->getDevice()->Configure(settings); + window->getDevice()->Configure(settings, cb); } - average.reset(); + average.reset(settings.points); traceModel.clearVNAData(); UpdateAverageCount(); emit traceModel.SpanChanged(settings.f_start, settings.f_stop); @@ -586,7 +587,7 @@ void VNA::DisableCalibration(bool force) if(calValid || force) { calValid = false; emit CalibrationDisabled(); - average.reset(); + average.reset(settings.points); } } @@ -596,7 +597,7 @@ void VNA::ApplyCalibration(Calibration::Type type) try { if(cal.constructErrorTerms(type)) { calValid = true; - average.reset(); + average.reset(settings.points); emit CalibrationApplied(type); } } catch (runtime_error e) { @@ -608,27 +609,25 @@ void VNA::ApplyCalibration(Calibration::Type type) // TODO start tracedata dialog with required traces QMessageBox::information(this, "Missing calibration traces", "Not all calibration traces for this type of calibration have been measured. The calibration can be enabled after the missing traces have been acquired."); DisableCalibration(true); - auto traceDialog = new CalibrationTraceDialog(&cal, type); - connect(traceDialog, &CalibrationTraceDialog::triggerMeasurement, this, &VNA::StartCalibrationMeasurement); - connect(traceDialog, &CalibrationTraceDialog::applyCalibration, this, &VNA::ApplyCalibration); - connect(this, &VNA::CalibrationMeasurementComplete, traceDialog, &CalibrationTraceDialog::measurementComplete); - traceDialog->show(); + StartCalibrationDialog(type); } } void VNA::StartCalibrationMeasurement(Calibration::Measurement m) { - // Trigger sweep to start from beginning - SettingsChanged(); - calMeasurement = m; + auto device = window->getDevice(); + if(!device) { + return; + } + // Stop sweep + StopSweep(); + calMeasurement = m; // Delete any already captured data of this measurement cal.clearMeasurement(m); calWaitFirst = true; - calMeasuring = true; QString text = "Measuring \""; text.append(Calibration::MeasurementToString(m)); text.append("\" parameters."); - calDialog.setRange(0, settings.points); calDialog.setLabelText(text); calDialog.setCancelButtonText("Abort"); calDialog.setWindowTitle("Taking calibration measurement..."); @@ -641,6 +640,11 @@ void VNA::StartCalibrationMeasurement(Calibration::Measurement m) calMeasuring = false; cal.clearMeasurement(calMeasurement); }); + // Trigger sweep to start from beginning + SettingsChanged([=](Device::TransmissionResult){ + // enable calibration measurement only in transmission callback (prevents accidental sampling of data which was still being processed) + calMeasuring = true; + }); } void VNA::ConstrainAndUpdateFrequencies() @@ -684,3 +688,19 @@ void VNA::StoreSweepSettings() s.setValue("SweepAveraging", averages); s.setValue("SweepLevel", (double) settings.cdbm_excitation / 100.0); } + +void VNA::StopSweep() +{ + if(window->getDevice()) { + window->getDevice()->SetIdle(); + } +} + +void VNA::StartCalibrationDialog(Calibration::Type type) +{ + auto traceDialog = new CalibrationTraceDialog(&cal, type); + connect(traceDialog, &CalibrationTraceDialog::triggerMeasurement, this, &VNA::StartCalibrationMeasurement); + connect(traceDialog, &CalibrationTraceDialog::applyCalibration, this, &VNA::ApplyCalibration); + connect(this, &VNA::CalibrationMeasurementComplete, traceDialog, &CalibrationTraceDialog::measurementComplete); + traceDialog->show(); +} diff --git a/Software/PC_Application/VNA/vna.h b/Software/PC_Application/VNA/vna.h index b44cf81..48f6917 100644 --- a/Software/PC_Application/VNA/vna.h +++ b/Software/PC_Application/VNA/vna.h @@ -6,6 +6,8 @@ #include "appwindow.h" #include "mode.h" #include "CustomWidgets/tilewidget.h" +#include "Device/device.h" +#include class VNA : public Mode { @@ -43,10 +45,12 @@ signals: private: void UpdateAverageCount(); - void SettingsChanged(); + void SettingsChanged(std::function cb = nullptr); void ConstrainAndUpdateFrequencies(); void LoadSweepSettings(); void StoreSweepSettings(); + void StopSweep(); + void StartCalibrationDialog(Calibration::Type type = Calibration::Type::None); Protocol::SweepSettings settings; unsigned int averages; diff --git a/Software/PC_Application/appwindow.cpp b/Software/PC_Application/appwindow.cpp index 9c8b79e..1549bcc 100644 --- a/Software/PC_Application/appwindow.cpp +++ b/Software/PC_Application/appwindow.cpp @@ -139,6 +139,7 @@ AppWindow::AppWindow(QWidget *parent) void AppWindow::closeEvent(QCloseEvent *event) { + delete device; QSettings settings; settings.setValue("geometry", saveGeometry()); // deactivate currently used mode (stores mode state in settings) diff --git a/Software/PC_Application/averaging.cpp b/Software/PC_Application/averaging.cpp index a84180b..8a5cffa 100644 --- a/Software/PC_Application/averaging.cpp +++ b/Software/PC_Application/averaging.cpp @@ -7,15 +7,18 @@ Averaging::Averaging() averages = 1; } -void Averaging::reset() +void Averaging::reset(unsigned int points) { avg.clear(); + for(unsigned int i = 0;i, 4>>()); + } } void Averaging::setAverages(unsigned int a) { averages = a; - reset(); + reset(avg.size()); } Protocol::Datapoint Averaging::process(Protocol::Datapoint d) @@ -110,3 +113,12 @@ unsigned int Averaging::getLevel() return 0; } } + +unsigned int Averaging::currentSweep() +{ + if(avg.size() > 0) { + return avg.front().size(); + } else { + return 0; + } +} diff --git a/Software/PC_Application/averaging.h b/Software/PC_Application/averaging.h index 488c00e..af9499a 100644 --- a/Software/PC_Application/averaging.h +++ b/Software/PC_Application/averaging.h @@ -10,11 +10,16 @@ class Averaging { public: Averaging(); - void reset(); + void reset(unsigned int points); void setAverages(unsigned int a); Protocol::Datapoint process(Protocol::Datapoint d); Protocol::SpectrumAnalyzerResult process(Protocol::SpectrumAnalyzerResult d); + // Returns the number of averaged sweeps. Value is incremented whenever the last point of the sweep is added. + // Returned values are in range 0 to averages unsigned int getLevel(); + // Returns the number of the currently active sweep. Value is incremented whenever the the first point of the sweep is added + // Returned values are in range 0 (when no data has been added yet) to averages + unsigned int currentSweep(); private: std::vector, 4>>> avg; int maxPoints;