diff --git a/src/core/DvkWavTransfer.cpp b/src/core/DvkWavTransfer.cpp index df88bcdadb..978a56d401 100644 --- a/src/core/DvkWavTransfer.cpp +++ b/src/core/DvkWavTransfer.cpp @@ -23,6 +23,9 @@ DvkWavTransfer::DvkWavTransfer(RadioModel* model, QObject* parent) finish(false, "Timed out waiting for radio connection", true); } else if (m_direction == Upload && m_client && m_client->state() != QAbstractSocket::ConnectedState) { + if (retryUploadOnDefaultPort(m_connectTimeoutGeneration)) { + return; + } finish(false, "Timed out connecting to radio upload port", false); } }); @@ -256,8 +259,11 @@ void DvkWavTransfer::handleNewConnection(quint64 generation, QTcpServer* server) // Anyone on the LAN can reach the listening port; only the radio may // write the operator's file. Others are dropped and the wait continues. if (m_model && !isRadioPeer(pending->peerAddress(), m_model->radioAddress())) { - qWarning() << "DvkWavTransfer: rejected export connection from" - << pending->peerAddress().toString(); + const QString peer = pending->peerAddress().toString(); + qWarning() << "DvkWavTransfer: rejected export connection from" << peer + << "expected" << m_model->radioAddress().toString(); + emit statusChanged(QString("Ignored a connection from %1; waiting for the radio at %2…") + .arg(peer, m_model->radioAddress().toString())); pending->abort(); pending->deleteLater(); return; @@ -383,6 +389,12 @@ void DvkWavTransfer::upload(int slotId, const QString& filePath) emit finished(false, "The radio connection was lost before the transfer could start."); return; } + // Checked here, not only when the menu opened: the file dialog is modal, + // and the DVK may have started recording or playing while it was up. + if (m_dvk && !m_dvk->canStartOperation()) { + emit finished(false, "The voice keyer is busy; stop it before importing"); + return; + } QFile f(filePath); if (!f.open(QIODevice::ReadOnly)) { @@ -488,6 +500,8 @@ void DvkWavTransfer::handleUploadPortReceived(quint64 generation, quint64 reques bool ok = false; int port = body.trimmed().toInt(&ok); if (!ok || port <= 0 || port > 65535) { + qWarning() << "DvkWavTransfer: no usable port in file upload reply" << body.trimmed() + << "- using" << DEFAULT_UPLOAD_PORT; port = DEFAULT_UPLOAD_PORT; } @@ -635,6 +649,24 @@ void DvkWavTransfer::handleUploadBytesWritten(quint64 generation, QTcpSocket* so sendNextChunk(generation, socket); } +bool DvkWavTransfer::retryUploadOnDefaultPort(quint64 generation) +{ + if (!isCurrent(generation) || m_uploadFallbackTried || m_uploadConnected + || m_uploadPort == 0 || m_uploadPort == DEFAULT_UPLOAD_PORT) { + return false; + } + m_uploadFallbackTried = true; + if (m_client) { + m_client->disconnect(this); + m_client->abort(); + m_client->deleteLater(); + m_client = nullptr; + } + stopConnectTimeout(); + openUploadSocket(generation, DEFAULT_UPLOAD_PORT); + return true; +} + void DvkWavTransfer::handleUploadError(quint64 generation, QTcpSocket* socket) { if (!isCurrentSocket(generation, socket)) return; @@ -642,15 +674,7 @@ void DvkWavTransfer::handleUploadError(quint64 generation, QTcpSocket* socket) // As FirmwareUploader: a port the radio named but would not connect gets // one retry on the file server's usual 42607. An error after connecting is // a failed upload, not a wrong port. - if (!m_uploadFallbackTried && !m_uploadConnected && m_uploadPort != 0 - && m_uploadPort != DEFAULT_UPLOAD_PORT) { - m_uploadFallbackTried = true; - m_client->disconnect(this); - m_client->abort(); - m_client->deleteLater(); - m_client = nullptr; - stopConnectTimeout(); - openUploadSocket(generation, DEFAULT_UPLOAD_PORT); + if (retryUploadOnDefaultPort(generation)) { return; } @@ -704,7 +728,9 @@ void DvkWavTransfer::cleanup(bool discardDownload) // already queued for this transfer must not match a replacement started // from the finished() handler below. invalidateOperation(); - if (m_direction == Upload) { + // Only an upload that reached the file server leaves it busy; a refused + // `dvk upload` or a port that never connected must not mask the next try. + if (m_direction == Upload && m_uploadConnected) { m_sinceUpload.start(); } m_transferring = false; diff --git a/src/core/DvkWavTransfer.h b/src/core/DvkWavTransfer.h index 4d4d6b2f93..e8aeb8bd72 100644 --- a/src/core/DvkWavTransfer.h +++ b/src/core/DvkWavTransfer.h @@ -84,6 +84,9 @@ class DvkWavTransfer : public QObject { quint64 requestId); void handleUploadSlotAccepted(quint64 generation, quint64 requestId, int code); void openUploadSocket(quint64 generation, int port); + // One retry on 42607 for a named port that refused or never answered, + // before any connection was made. False when it does not apply. + bool retryUploadOnDefaultPort(quint64 generation); // A refusal on any transfer leg also reaches the DVK license latch. void reportRefusal(int code); std::function makeUploadPortCallback(quint64 generation, diff --git a/src/core/backends/flex/CommandParser.cpp b/src/core/backends/flex/CommandParser.cpp index e44cd2bed6..f6bf6d3abd 100644 --- a/src/core/backends/flex/CommandParser.cpp +++ b/src/core/backends/flex/CommandParser.cpp @@ -10,8 +10,9 @@ QMap CommandParser::parseKVs(const QString& body) QMap result; // Body may look like: "freq=14.225000 mode=USB filter_lo=-1500 filter_hi=1500" // A quoted value may contain spaces (`dvk … name="CQ Contest"`) and is kept - // whole, quotes included, up to the next token holding a quote. A quote - // left open with no later quote on the line splits as before. + // whole, quotes included, up to the token that closes it. A quote left + // open (no closing token before the next `key="` or the end of the line) + // splits as before, so a malformed value cannot swallow the keys after it. const QStringList tokens = body.split(' ', Qt::SkipEmptyParts); for (qsizetype i = 0; i < tokens.size(); ++i) { QString token = tokens[i]; @@ -20,7 +21,7 @@ QMap CommandParser::parseKVs(const QString& body) while (end < tokens.size() && !tokens[end].contains('"')) { ++end; } - if (end < tokens.size()) { + if (end < tokens.size() && !tokens[end].contains(QLatin1String("=\""))) { for (qsizetype j = i + 1; j <= end; ++j) { token += QLatin1Char(' ') + tokens[j]; } diff --git a/src/gui/DvkPanel.cpp b/src/gui/DvkPanel.cpp index 4b09b0f86f..fcb29f4dbe 100644 --- a/src/gui/DvkPanel.cpp +++ b/src/gui/DvkPanel.cpp @@ -359,6 +359,11 @@ DvkPanel::DvkPanel(DvkModel* model, QWidget* parent) stopActiveOperation(); }); + m_announceTimer = new QTimer(this); + m_announceTimer->setSingleShot(true); + m_announceTimer->setInterval(150); + connect(m_announceTimer, &QTimer::timeout, this, &DvkPanel::flushSlotAnnouncement); + // Elapsed timer for recording/playback/preview progress m_elapsedTimer = new QTimer(this); m_elapsedTimer->setInterval(100); @@ -401,12 +406,27 @@ void DvkPanel::updateSlotAccessibility(int id, const QString& name, int duration play->setAccessibleDescription(durationMs > 0 ? QString("%1 recording. Transmits on the transmit slice (F%2).").arg(length).arg(id) : QStringLiteral("Empty slot, nothing to play.")); - // Announce a real change the operator can see; slot loads and resets on - // connect and disconnect would otherwise be a burst of a dozen. - if (changed && play->isVisible()) { - QAccessibleEvent ev(play, QAccessible::NameChanged); - QAccessible::updateAccessibility(&ev); + // Announce a real change the operator can see, one slot at a time. A + // load or reset changes many slots at once; flushSlotAnnouncement() stays + // silent for those instead of reading a dozen names back to back. + if (changed && m_announceTimer) { + if (!m_announceSlots.contains(id)) { + m_announceSlots.append(id); + } + m_announceTimer->start(); + } +} + +void DvkPanel::flushSlotAnnouncement() +{ + if (m_announceSlots.size() == 1) { + QPushButton* play = m_fkeyBtns[m_announceSlots.first() - 1]; + if (play->isVisible()) { + QAccessibleEvent ev(play, QAccessible::NameChanged); + QAccessible::updateAccessibility(&ev); + } } + m_announceSlots.clear(); } void DvkPanel::announceStatus(const QString& text, bool error) diff --git a/src/gui/DvkPanel.h b/src/gui/DvkPanel.h index b94d4a9f60..6e2e1e3a43 100644 --- a/src/gui/DvkPanel.h +++ b/src/gui/DvkPanel.h @@ -70,6 +70,12 @@ private slots: // (#2464, #2582). QVector m_shortcuts; + // F-key name changes collected for one announcement; a burst (slot load, + // disconnect) of more than one is not announced. + QVector m_announceSlots; + QTimer* m_announceTimer{nullptr}; + void flushSlotAnnouncement(); + void selectSlot(int id); // Row and F-key names carry the slot's name and length for screen readers. void updateSlotAccessibility(int id, const QString& name, int durationMs); diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 913b6ac716..82ea69b3eb 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -10301,9 +10301,11 @@ void MainWindow::updateToolsMenuState() if (m_swrScanAction) { m_swrScanAction->setEnabled(txReady); - m_swrScanAction->setToolTip(txReady ? QString() + const QString reason = txReady ? QString() : tr("Requires an idle, TX-capable radio with this client holding " - "the interlock")); + "the interlock"); + m_swrScanAction->setToolTip(reason); + m_swrScanAction->setStatusTip(reason); // a tooltip is never announced } // Every disabling condition names itself. A greyed control with no stated @@ -10311,7 +10313,7 @@ void MainWindow::updateToolsMenuState() if (m_preTuneAction) { m_preTuneAction->setEnabled(txReady && memories && tx.memoriesEnabled() && hasTxApplet); - m_preTuneAction->setToolTip( + const QString reason = !caps.hasTunerMemories ? tr("ATU memory controls are unavailable for this radio") : tgxlOperate @@ -10323,11 +10325,13 @@ void MainWindow::updateToolsMenuState() : !txReady ? tr("Requires an idle, TX-capable radio with this client " "holding the interlock") - : QString()); + : QString(); + m_preTuneAction->setToolTip(reason); + m_preTuneAction->setStatusTip(reason); } if (m_clearAtuAction) { m_clearAtuAction->setEnabled(connected && memories && hasTxApplet); - m_clearAtuAction->setToolTip( + const QString reason = !caps.hasTunerMemories ? tr("ATU memory controls are unavailable for this radio") : tgxlOperate @@ -10336,7 +10340,9 @@ void MainWindow::updateToolsMenuState() ? tr("The transmit applet is unavailable in this build") : !connected ? tr("Connect to a radio first") - : QString()); + : QString(); + m_clearAtuAction->setToolTip(reason); + m_clearAtuAction->setStatusTip(reason); } if (m_gpsDashboardAction) { @@ -10349,7 +10355,7 @@ void MainWindow::updateToolsMenuState() const bool externalRx = active && active->externalReceiveReplacementActive(); m_agcTCalibrationMenuAction->setEnabled(connected && active && !externalRx); - m_agcTCalibrationMenuAction->setToolTip( + const QString reason = !connected ? tr("Connect to a radio first") : !active @@ -10357,7 +10363,9 @@ void MainWindow::updateToolsMenuState() : externalRx ? tr("Not available while an external receive source " "replaces this slice's RX") - : QString()); + : QString(); + m_agcTCalibrationMenuAction->setToolTip(reason); + m_agcTCalibrationMenuAction->setStatusTip(reason); } } diff --git a/src/models/DvkModel.cpp b/src/models/DvkModel.cpp index 0146e60664..8f8987676a 100644 --- a/src/models/DvkModel.cpp +++ b/src/models/DvkModel.cpp @@ -19,30 +19,36 @@ DvkModel::DvkModel(QObject* parent) : QObject(parent) {} void DvkModel::recStart(int id) { + m_pendingId = id; setPending(Recording); emit replyCommandReady(QString("dvk rec_start id=%1").arg(id), "rec_start", id); } void DvkModel::recStop() { - emit replyCommandReady(QStringLiteral("dvk rec_stop"), "rec_stop", m_activeId); + emit replyCommandReady(QStringLiteral("dvk rec_stop"), "rec_stop", + m_activeId > 0 ? m_activeId : m_pendingId); } void DvkModel::previewStart(int id) { + m_pendingId = id; setPending(Preview); emit replyCommandReady(QString("dvk preview_start id=%1").arg(id), "preview_start", id); } void DvkModel::previewStop() { - emit replyCommandReady(QStringLiteral("dvk preview_stop"), "preview_stop", m_activeId); + emit replyCommandReady(QStringLiteral("dvk preview_stop"), "preview_stop", + m_activeId > 0 ? m_activeId : m_pendingId); } void DvkModel::playbackStart(int id) { + m_pendingId = id; setPending(Playback); emit replyCommandReady(QString("dvk playback_start id=%1").arg(id), "playback_start", id); } void DvkModel::playbackStop() { - emit replyCommandReady(QStringLiteral("dvk playback_stop"), "playback_stop", m_activeId); + emit replyCommandReady(QStringLiteral("dvk playback_stop"), "playback_stop", + m_activeId > 0 ? m_activeId : m_pendingId); } void DvkModel::clear(int id) { @@ -131,6 +137,14 @@ void DvkModel::setTransferActive(bool active) emit admissionChanged(); } +void DvkModel::clearRefusal() +{ + if (m_licenseRefused) { + m_licenseRefused = false; + emit licenseRefusedChanged(false); + } +} + void DvkModel::noteRefusal(uint code) { if (code == kNotLicensed && !m_licenseRefused) { @@ -254,7 +268,9 @@ void DvkModel::reset() m_enabled = false; m_licenseRefused = false; m_pending = Unknown; - m_transferActive = false; + m_pendingId = -1; + // m_transferActive belongs to DvkWavTransfer, which clears it when its own + // transfer ends; a transfer can outlive the connection by its timeout. const QVector gone = std::exchange(m_recordings, {}); emit statusChanged(m_status, m_activeId); for (const DvkRecording& r : gone) { diff --git a/src/models/DvkModel.h b/src/models/DvkModel.h index 9f991a3995..b06d8a678f 100644 --- a/src/models/DvkModel.h +++ b/src/models/DvkModel.h @@ -3,6 +3,7 @@ #include #include #include +#include #include namespace AetherSDR { @@ -41,6 +42,11 @@ class DvkModel : public QObject { bool licenseRefused() const { return m_licenseRefused; } // Latches a 50004001 refusal from any dvk path, including WAV transfers. void noteRefusal(uint code); + // The radio later reported the feature licensed (its status outranks an + // earlier refusal, e.g. a subscription activated mid-session). + void clearRefusal(); + // The radio's name for the DVK entitlement in `license feature` status. + static constexpr QLatin1String kLicenseFeature{"digital_voice_keyer"}; const QVector& recordings() const { return m_recordings; } static QString defaultName(int id); @@ -99,6 +105,7 @@ class DvkModel : public QObject { bool m_enabled{false}; bool m_licenseRefused{false}; Status m_pending{Unknown}; + int m_pendingId{-1}; bool m_transferActive{false}; void setPending(Status pending); diff --git a/src/models/RadioModel.cpp b/src/models/RadioModel.cpp index 308d4134fb..31b86ef9f5 100644 --- a/src/models/RadioModel.cpp +++ b/src/models/RadioModel.cpp @@ -10952,6 +10952,10 @@ void RadioModel::onStatusReceived(const QString& object, m_licenseFeatures.insert(name, next); emit licenseFeaturesChanged(); } + // The radio's licensed status outranks an earlier 50004001 refusal. + if (next.enabled && name == DvkModel::kLicenseFeature) { + m_dvkModel.clearRefusal(); + } emit infoChanged(); return; } diff --git a/tests/dvk_model_status_test.cpp b/tests/dvk_model_status_test.cpp index a5bfcb5c05..736daf0b7b 100644 --- a/tests/dvk_model_status_test.cpp +++ b/tests/dvk_model_status_test.cpp @@ -1,8 +1,10 @@ // #6244 — DvkModel against the SmartSDR API wiki (TCPIP-dvk) and lines // captured from a FLEX-8600 on fw 4.2.20. Status lines go through the real // CommandParser, so quoted names with spaces are covered end to end. +#include "TestSettingsProfile.h" #include "core/backends/flex/CommandParser.h" #include "models/DvkModel.h" +#include "models/RadioModel.h" #include #include @@ -67,6 +69,10 @@ void capture(DvkModel& model, Sent& sent) int main(int argc, char* argv[]) { + TestSettingsProfile profile(QStringLiteral("dvk-model-status-test")); + if (!profile.isValid()) { + return 1; + } QCoreApplication app(argc, argv); // ── parseKVs keeps quoted values whole ────────────────────────────────── @@ -94,6 +100,14 @@ int main(int argc, char* argv[]) && kvs.value("name") == QStringLiteral("\"Solo\"") && kvs.contains("removed") && kvs.value("removed").isEmpty()); } + { + // Review #6247: an unterminated value must not swallow a later quoted key. + const auto kvs = CommandParser::parseKVs( + QStringLiteral("name=\"abc foo=1 bar=\"x\" baz=2")); + report("an unterminated quote stops at the next quoted key", + kvs.value("foo") == QStringLiteral("1") && kvs.value("bar") == QStringLiteral("\"x\"") + && kvs.value("baz") == QStringLiteral("2")); + } { const auto kvs = CommandParser::parseKVs(QStringLiteral("name=\"open ended x=1")); report("unterminated quote splits exactly as before", @@ -215,8 +229,45 @@ int main(int argc, char* argv[]) model.playbackStart(3); model.setTransferActive(true); model.reset(); - report("disconnect clears a pending start and transfer", - model.pendingOperation() == DvkModel::Unknown && model.canStartOperation()); + report("disconnect clears a pending start", + model.pendingOperation() == DvkModel::Unknown); + report("a transfer still running keeps admission closed after disconnect", + !model.canStartOperation()); + model.setTransferActive(false); + report("its owner's end of transfer reopens it", model.canStartOperation()); + } + { + // Review #6247: STOP before the echo names the slot it stops. + DvkModel model; + int stopId = 0; + QObject::connect(&model, &DvkModel::replyCommandReady, &model, + [&stopId](const QString&, const QString& verb, int id) { + if (verb == QLatin1String("rec_stop")) { + stopId = id; + } + }); + model.recStart(4); + model.recStop(); + report("a stop before the echo carries the pending start's slot", stopId == 4); + } + { + // The radio's licensed status outranks an earlier 50004001 refusal. + RadioModel radio; + using Kvs = QMap; // Q_ARG cannot take the comma + const auto status = [&radio](const QString& name, const QString& enabled) { + const Kvs kvs{{QStringLiteral("name"), name}, {QStringLiteral("enabled"), enabled}}; + QMetaObject::invokeMethod(&radio, "onStatusReceived", Qt::DirectConnection, + Q_ARG(QString, QStringLiteral("license feature")), + Q_ARG(Kvs, kvs)); + }; + radio.dvkModel().noteRefusal(0x50004001u); + report("a refusal reaches RadioModel's entitlement input", radio.dvkLicenseRefused()); + status(QStringLiteral("digital_voice_keyer"), QStringLiteral("0")); + report("a not-licensed status keeps the refusal", radio.dvkLicenseRefused()); + status(QStringLiteral("some_other_feature"), QStringLiteral("1")); + report("another feature's licence does not clear it", radio.dvkLicenseRefused()); + status(QStringLiteral("digital_voice_keyer"), QStringLiteral("1")); + report("the radio reporting DVK licensed clears the refusal", !radio.dvkLicenseRefused()); } { DvkModel model; diff --git a/tests/dvk_wav_transfer_generation_test.cpp b/tests/dvk_wav_transfer_generation_test.cpp index 60cf6185c3..30979857e8 100644 --- a/tests/dvk_wav_transfer_generation_test.cpp +++ b/tests/dvk_wav_transfer_generation_test.cpp @@ -88,6 +88,9 @@ class DvkWavTransferTestAccess { transfer.handleUploadError(transfer.m_operationGeneration, transfer.m_client); } static void finishOk(DvkWavTransfer& transfer) { transfer.finish(true, QString(), false); } + static void markConnected(DvkWavTransfer& transfer) { transfer.m_uploadConnected = true; } + // Fires the connect timeout now instead of after CONNECT_TIMEOUT_MS. + static void expireConnectTimeout(DvkWavTransfer& transfer) { transfer.m_timeout->start(1); } static qint64 uploadSize(const DvkWavTransfer& transfer) { return transfer.m_uploadData.size(); } @@ -428,6 +431,7 @@ int main(int argc, char* argv[]) TA::attachDvk(transfer, &dvk); transfer.upload(3, path); ok &= expect(!dvk.canStartOperation(), "a running upload blocks a DVK start"); + TA::markConnected(transfer); TA::finishOk(transfer); ok &= expect(dvk.canStartOperation(), "the upload's end admits DVK starts again"); @@ -442,6 +446,60 @@ int main(int argc, char* argv[]) "a transfer inside the settle window is refused locally"); } + { + // A refused `dvk upload` never reached the file server, so it leaves no + // busy window: an immediate retry reaches the radio and its real answer. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("cq.wav")); + ok &= expect(writeFile(path, stereo48kWav()), "fixture WAV written"); + AetherSDR::DvkWavTransfer transfer(nullptr); + QStringList sent; + std::vector replies; + TA::recordCommands(transfer, sent, replies); + transfer.upload(3, path); + replies.at(0)(static_cast(0x50004001u), QString()); + ok &= expect(!transfer.isBusy(), "a refused upload leaves no settle window"); + transfer.upload(3, path); + ok &= expect(sent.size() == 2 && sent.at(1) == QStringLiteral("dvk upload id=3"), + "a retry after a refusal goes to the radio"); + } + + { + // Review #6247: Import re-checks DVK admission when it actually starts; + // the menu was enabled before a modal file dialog. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("cq.wav")); + ok &= expect(writeFile(path, stereo48kWav()), "fixture WAV written"); + AetherSDR::DvkModel dvk; + dvk.previewStart(1); + AetherSDR::DvkWavTransfer transfer(nullptr); + QStringList sent; + std::vector replies; + TA::recordCommands(transfer, sent, replies); + TA::attachDvk(transfer, &dvk); + QString message; + QObject::connect(&transfer, &AetherSDR::DvkWavTransfer::finished, &transfer, + [&message](bool, const QString& m) { message = m; }); + transfer.upload(3, path); + ok &= expect(sent.isEmpty() && message.contains(QStringLiteral("busy")), + "an import while the DVK is busy is refused before any command"); + } + + { + // A named port that never answers retries on 42607 at the connect timeout. + AetherSDR::DvkWavTransfer transfer(nullptr); + TA::begin(transfer, TA::Direction::Upload, 1); + auto reply = TA::armUploadPortReply(transfer); + reply(0, QStringLiteral("4995")); + TA::expireConnectTimeout(transfer); + waitForMilliseconds(50); + ok &= expect(transfer.isTransferring() && TA::uploadPort(transfer) == 42607, + "a silent named port retries on 42607 at the timeout"); + TA::expireConnectTimeout(transfer); + waitForMilliseconds(50); + ok &= expect(!transfer.isTransferring(), "a silent 42607 ends the upload"); + } + { // A named port that refuses the connection gets one retry on 42607. AetherSDR::DvkWavTransfer transfer(nullptr); diff --git a/tests/tests.cmake b/tests/tests.cmake index 245a47e203..00ad3e070c 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -3167,7 +3167,7 @@ add_test(NAME dvk_wav_converter_test COMMAND dvk_wav_converter_test) add_executable(dvk_model_status_test tests/dvk_model_status_test.cpp ) -target_include_directories(dvk_model_status_test PRIVATE src) +target_include_directories(dvk_model_status_test PRIVATE src tests) target_link_libraries(dvk_model_status_test PRIVATE aethercore Qt6::Core) add_test(NAME dvk_model_status_test COMMAND dvk_model_status_test)