diff --git a/CMakeLists.txt b/CMakeLists.txt index cbf04dd8c1..25de277c2c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1425,6 +1425,7 @@ set(GUI_SOURCES src/gui/HealthApplet.cpp src/gui/PersistentDialog.cpp src/gui/CanonWindow.cpp + src/gui/CanonMessageBox.cpp src/gui/CanonSurface.cpp src/gui/PanadapterHeader.cpp src/gui/FramelessMessageBox.cpp diff --git a/docs/ctr2-usb-relay-design.md b/docs/ctr2-usb-relay-design.md index c1aa8ebe18..7647b26639 100644 --- a/docs/ctr2-usb-relay-design.md +++ b/docs/ctr2-usb-relay-design.md @@ -415,6 +415,13 @@ maker's firmware and its settings survive every update. SWITCH asks the device to restart into that firmware; how a device selects its image is its own business. +A device answers SWITCH and BOOTLOADER with STATUS before it leaves the bus. +AetherSDR also counts the request done if the device leaves while the answer +is outstanding, once the port has written the request, so a restart that beats +its answer, or an answer naming the wrong op, is not reported as a lost +device. A port that fails before writing it (a write error) still reports the +device lost. + **Host implementation.** `Ctr2FirmwareUpdater` runs the session and checks the image before sending anything: an ESP32-S3 application image with an application description (`readImage`), not a merged flash image or a diff --git a/docs/style/dialog-patterns.md b/docs/style/dialog-patterns.md index 01920a8980..831bc4b2eb 100644 --- a/docs/style/dialog-patterns.md +++ b/docs/style/dialog-patterns.md @@ -276,6 +276,13 @@ has moved to it (Waveforms, Network Diagnostics, AetherRX, AetherTX, AetherModem installs `FramelessResizer`, since `CanonWindow` itself only moves. Other tool and workspace dialogs keep the persistent pattern above. +A question or notice from a canon window uses `CanonMessageBox` +(`src/gui/CanonMessageBox.{h,cpp}`) in place of `QMessageBox`: a canon dialog +with the title as a canon header, the text, optional further text, and the +buttons at the bottom right. `clickedButton()` names the button clicked and is +null when the box is closed any other way; the default button is outlined in +cyan and a destructive one is drawn in gold. The CTR2 window's questions use it. + ## Common pitfalls These are the issues that have hit real PRs in the project's history. diff --git a/docs/user/docs/ctr2-proxy.md b/docs/user/docs/ctr2-proxy.md index 3e67d2c8bc..089c014d33 100644 --- a/docs/user/docs/ctr2-proxy.md +++ b/docs/user/docs/ctr2-proxy.md @@ -263,7 +263,9 @@ date, including any saved with **Back Up…** in earlier versions of AetherSDR it. Installing one restores it: the whole flash goes back exactly as it was, both firmwares, the settings and the registration included, and anything since is lost. A backup restores only onto the unit it came from (matched by -its MAC address), and only if the file is unchanged since it was made. +its MAC address), and only if the file is unchanged since it was made. The +question before a restore has **Cancel** as its default, so pressing Return +keeps what is on the unit; restoring takes a click on **Restore**. ### Cancelling diff --git a/src/core/Ctr2FirmwareUpdater.cpp b/src/core/Ctr2FirmwareUpdater.cpp index 83ccab3e36..34e51ee2f2 100644 --- a/src/core/Ctr2FirmwareUpdater.cpp +++ b/src/core/Ctr2FirmwareUpdater.cpp @@ -202,7 +202,11 @@ bool Ctr2FirmwareUpdater::openPort(QString* error) port->setParent(nullptr); m_reassembler.reset(); m_reassembler.setExtensions(capabilityBit(MessageType::Firmware)); + m_reportsQueued = 0; + m_reportsWritten = 0; + m_restartQueuedTo = 0; connect(port, &Ctr2HidPort::extensionsNegotiated, this, &Ctr2FirmwareUpdater::onExtensions); + connect(port, &Ctr2HidPort::reportsSent, this, [this](int count) { m_reportsWritten += count; }); connect(port, &Ctr2HidPort::reportsReceived, this, &Ctr2FirmwareUpdater::onReports); connect(port, &Ctr2HidPort::failed, this, &Ctr2FirmwareUpdater::onPortFailed); return true; @@ -270,15 +274,19 @@ void Ctr2FirmwareUpdater::sendHello() std::vector out; m_encoder.reset(); m_encoder.encodeControl(MessageType::Hello, &out); + m_reportsQueued += qint64(out.size()); m_port->send(out); } -void Ctr2FirmwareUpdater::sendExtension(const QByteArray& payload) +bool Ctr2FirmwareUpdater::sendExtension(const QByteArray& payload) { std::vector out; - if (m_port && m_encoder.encodeExtension(MessageType::Firmware, payload, &out)) { - m_port->send(out); + if (!m_port || !m_encoder.encodeExtension(MessageType::Firmware, payload, &out)) { + return false; } + m_reportsQueued += qint64(out.size()); + m_port->send(out); + return true; } void Ctr2FirmwareUpdater::onReports(const QByteArray& reports) @@ -316,7 +324,11 @@ void Ctr2FirmwareUpdater::onMessage(const Message& message) const bool loader = *m_restart == Restart::Bootloader; setPhase(Phase::Commanding, loader ? tr("Restarting into download mode") : tr("Restarting into the other firmware")); - sendExtension(loader ? firmware::encodeBootloader() : firmware::encodeSwitch()); + if (!sendExtension(loader ? firmware::encodeBootloader() : firmware::encodeSwitch())) { + fail(tr("The restart request could not be sent.")); + return; + } + m_restartQueuedTo = m_reportsQueued; armTimeout(m_timing.answerTimeoutMs); } else if (m_phase == Phase::Calling) { m_retry->stop(); @@ -358,13 +370,8 @@ void Ctr2FirmwareUpdater::onStatus(const firmware::Reply& reply) : tr("The device has no other firmware to start.")); return; } - m_timeout->stop(); closePort(); // it is leaving the bus - m_phase = Phase::Idle; - m_restart.reset(); - emit finished(true, loader ? tr("The device is restarting into download mode.") - : tr("The device is restarting into its other firmware."), - QString()); + restarted(); return; } if (reply.status != firmware::Status::Ok) { @@ -462,12 +469,34 @@ void Ctr2FirmwareUpdater::onPortFailed(const QString& message) fail(tr("The device left during the final check (%1). Reconnect it: it runs " "either the new firmware or the old one.").arg(message)); break; + case Phase::Commanding: + // Leaving the bus is what the request asks for. A device that does it + // after the request was written, before its STATUS arrives or with a + // STATUS naming another op, has still restarted. A port that failed + // before writing it (a write error) has not. + if (m_reportsWritten >= m_restartQueuedTo) { + restarted(); + break; + } + fail(tr("Lost the device: %1.").arg(message) + kKeepsFirmware); + break; default: fail(tr("Lost the device: %1.").arg(message) + kKeepsFirmware); break; } } +void Ctr2FirmwareUpdater::restarted() +{ + const bool loader = m_restart && *m_restart == Restart::Bootloader; + m_timeout->stop(); + m_phase = Phase::Idle; + m_restart.reset(); + emit finished(true, loader ? tr("The device is restarting into download mode.") + : tr("The device is restarting into its other firmware."), + QString()); +} + void Ctr2FirmwareUpdater::onTimeout() { switch (m_phase) { diff --git a/src/core/Ctr2FirmwareUpdater.h b/src/core/Ctr2FirmwareUpdater.h index a1b90c5593..36dacc2823 100644 --- a/src/core/Ctr2FirmwareUpdater.h +++ b/src/core/Ctr2FirmwareUpdater.h @@ -105,11 +105,14 @@ class Ctr2FirmwareUpdater : public QObject { void onPortFailed(const QString& message); void onTimeout(); void sendHello(); - void sendExtension(const QByteArray& payload); + // False when nothing could be queued (no port, or the payload would not encode). + bool sendExtension(const QByteArray& payload); void sendChunks(); void lookForDevice(); void fail(const QString& message); void succeed(const QString& version); + // A restart request done: the device is leaving the bus for its target. + void restarted(); void armTimeout(int ms); DeviceSource m_devices; @@ -130,6 +133,12 @@ class Ctr2FirmwareUpdater : public QObject { qint64 m_sent{0}; // image bytes sent qint64 m_stored{0}; // image bytes the device has acknowledged int m_inFlight{0}; + // Reports this port has queued and written, and the queued count once the + // restart request was queued: it has reached the device's endpoint when + // the written count gets there. + qint64 m_reportsQueued{0}; + qint64 m_reportsWritten{0}; + qint64 m_restartQueuedTo{0}; QString m_previousVersion; }; diff --git a/src/core/Ctr2HidThreadPort.cpp b/src/core/Ctr2HidThreadPort.cpp index bd9553e130..04d5e0c480 100644 --- a/src/core/Ctr2HidThreadPort.cpp +++ b/src/core/Ctr2HidThreadPort.cpp @@ -255,6 +255,13 @@ class Ctr2HidIoWorker : public QObject { } const int n = writeReports(m_outbox, 0); if (n < 0) { + // What this tick did write reached the device: count it + // before the failure, which arrives after it. + if (sent > 0) { + QMetaObject::invokeMethod(m_port, [port = m_port, sent, generation] { + port->deliverSent(sent, generation); + }, Qt::QueuedConnection); + } fail(QStringLiteral("CTR2 USB write failed: %1").arg(errorText())); return; } diff --git a/src/gui/CanonMessageBox.cpp b/src/gui/CanonMessageBox.cpp new file mode 100644 index 0000000000..ce80b38146 --- /dev/null +++ b/src/gui/CanonMessageBox.cpp @@ -0,0 +1,131 @@ +#include "CanonMessageBox.h" + +#include "CanonIndicators.h" +#include "ModemChrome.h" + +#include +#include +#include +#include +#include + +namespace AetherSDR { + +namespace { + +// The texts wrap at the narrower width; one long unbreakable token (a path in +// a command to copy) widens the box up to the wider one rather than clipping. +constexpr int kTextWidth = 420; +constexpr int kMaxTextWidth = 720; + +} // namespace + +CanonMessageBox::CanonMessageBox(const QString& title, const QString& text, QWidget* parent) + : CanonWindow(title, parent) +{ + setObjectName(QStringLiteral("canonMessageBox")); + applyCanonSheet(bodyWidget(), [] { + return ModemChrome::styleSheet(ModemChrome::Scale::Dialog, ModemChrome::Look::Canon) + + QStringLiteral( + "QLabel#canonMessageText { color: {{color.canon.ink}}; }" + "QLabel#canonMessageInfo { color: {{color.canon.inkSoft}}; }" + "QPushButton { padding: 6px 18px; }" + "QPushButton:default { color: {{color.canon.cyan}}; border-color: {{color.canon.cyan}}; }" + "QPushButton[role=\"destructive\"] { color: {{color.canon.sparkGold}}; }"); + }); + + auto* root = new QVBoxLayout(bodyWidget()); + root->setContentsMargins(18, 14, 18, 16); + root->setSpacing(12); + // Texts carry manifest and device strings (a product name, a version, a + // path): never render them as rich text, the title included. + QWidget* header = makeCanonHeader(title); + if (auto* heading = header->findChild(QStringLiteral("canonHeaderTitle"))) + heading->setTextFormat(Qt::PlainText); + root->addWidget(header); + + // A screen reader announces a QMessageBox's text when it opens; this + // dialog is told the same through its description. + setAccessibleDescription(text); + m_text = new QLabel(text); + m_text->setObjectName(QStringLiteral("canonMessageText")); + m_text->setTextFormat(Qt::PlainText); + m_text->setWordWrap(true); + m_text->setMinimumWidth(kTextWidth); + m_text->setMaximumWidth(kMaxTextWidth); + root->addWidget(m_text); + + m_info = new QLabel; + m_info->setObjectName(QStringLiteral("canonMessageInfo")); + m_info->setTextFormat(Qt::PlainText); + m_info->setWordWrap(true); + m_info->setMinimumWidth(kTextWidth); + m_info->setMaximumWidth(kMaxTextWidth); + m_info->hide(); + root->addWidget(m_info); + + root->addSpacing(4); + m_buttons = new QHBoxLayout; + m_buttons->setSpacing(8); + m_buttons->addStretch(1); + root->addLayout(m_buttons); +} + +void CanonMessageBox::setInformativeText(const QString& text) +{ + m_info->setText(text); + m_info->setVisible(!text.isEmpty()); + setAccessibleDescription(text.isEmpty() ? m_text->text() + : m_text->text() + QLatin1Char('\n') + text); +} + +void CanonMessageBox::setTextSelectable(bool on) +{ + const Qt::TextInteractionFlags flags = on ? Qt::TextSelectableByMouse : Qt::NoTextInteraction; + m_text->setTextInteractionFlags(flags); + m_info->setTextInteractionFlags(flags); +} + +QPushButton* CanonMessageBox::addButton(const QString& text, Role role) +{ + // Auto-default, as in QMessageBox: a focused button takes Return itself, + // and the default button takes it only while no other button has focus. + auto* button = new QPushButton(text); + button->setAutoDefault(true); + if (role == Role::Destructive) { + button->setProperty("role", QStringLiteral("destructive")); + } + connect(button, &QPushButton::clicked, this, [this, button, role] { + m_clicked = button; + if (role == Role::Reject) { + reject(); + } else { + accept(); + } + }); + m_buttons->addWidget(button); + m_added.append(button); + return button; +} + +void CanonMessageBox::setDefaultButton(QPushButton* button) +{ + for (QPushButton* b : std::as_const(m_added)) { + b->setDefault(b == button); + } + m_default = button; +} + +void CanonMessageBox::showEvent(QShowEvent* event) +{ + CanonWindow::showEvent(event); + m_clicked = nullptr; // this showing's answer, not a previous one's + if (m_default) { + m_default->setFocus(); + } + // As QMessageBox does, so a screen reader reads the texts as it opens. + QAccessibleEvent alert(this, QAccessible::Alert); + QAccessible::updateAccessibility(&alert); +} + +} // namespace AetherSDR diff --git a/src/gui/CanonMessageBox.h b/src/gui/CanonMessageBox.h new file mode 100644 index 0000000000..916dde2556 --- /dev/null +++ b/src/gui/CanonMessageBox.h @@ -0,0 +1,52 @@ +#pragma once + +#include "CanonWindow.h" + +#include + +class QLabel; +class QHBoxLayout; +class QPushButton; + +namespace AetherSDR { + +// A question or a notice in the style guide's window, for windows built on +// CanonWindow in place of QMessageBox: the title as a canon header, the text in +// canon ink, optional further text below it in the softer ink, and the buttons +// in a row at the bottom right. Run it with exec() and read clickedButton(), +// which stays null when the operator closes it instead (the corner button, +// Escape, ⌘W / Ctrl+W). +class CanonMessageBox : public CanonWindow { + Q_OBJECT + +public: + // Accept and Reject buttons close it as accepted or rejected; Destructive + // accepts too, and is drawn in the warning gold, for a step that replaces + // what is on the device. + enum class Role { Accept, Reject, Destructive }; + + CanonMessageBox(const QString& title, const QString& text, QWidget* parent = nullptr); + + void setInformativeText(const QString& text); + // Lets the operator select and copy the texts (commands to paste). + void setTextSelectable(bool on); + + QPushButton* addButton(const QString& text, Role role); + // The button Return clicks; also takes the focus when the box opens. + // Every box sets one; without it nothing decides where the focus starts. + void setDefaultButton(QPushButton* button); + QPushButton* clickedButton() const { return m_clicked; } + +protected: + void showEvent(QShowEvent* event) override; + +private: + QLabel* m_text{nullptr}; + QLabel* m_info{nullptr}; + QHBoxLayout* m_buttons{nullptr}; + QList m_added; + QPushButton* m_default{nullptr}; + QPushButton* m_clicked{nullptr}; +}; + +} // namespace AetherSDR diff --git a/src/gui/Ctr2ProxyPanel.cpp b/src/gui/Ctr2ProxyPanel.cpp index 3cb9700d0b..aa2e20bb1d 100644 --- a/src/gui/Ctr2ProxyPanel.cpp +++ b/src/gui/Ctr2ProxyPanel.cpp @@ -1,5 +1,6 @@ #include "Ctr2ProxyPanel.h" +#include "CanonMessageBox.h" #include "GuardedSlider.h" #include "ScopedChildWidget.h" #include "models/Ctr2ProxyModel.h" @@ -12,7 +13,6 @@ #include #include #include -#include #include #include #include @@ -294,15 +294,15 @@ bool ruleAlreadyInstalled(const QString& rule) void Ctr2ProxyPanel::offerUsbAccessRule() { if (ruleAlreadyInstalled(loadRule())) { - ScopedChildWidget box( - QMessageBox::Information, tr("CTR2 USB access"), + ScopedChildWidget box( + tr("CTR2 USB access"), tr("The CTR2 access rule is already installed, but this login still cannot " - "open the CTR2."), - QMessageBox::Ok, this); + "open the CTR2."), this); box.get()->setInformativeText( tr("Unplug the CTR2, plug it back in, then press Start. The rule only opens " "the CTR2 to whoever is logged in at this computer's own screen, so a " "remote or background session cannot use it.")); + box.get()->setDefaultButton(box.get()->addButton(tr("OK"), CanonMessageBox::Role::Accept)); box.get()->exec(); return; } @@ -311,18 +311,18 @@ void Ctr2ProxyPanel::offerUsbAccessRule() return; } - ScopedChildWidget box( - QMessageBox::Question, tr("Allow AetherSDR to use the CTR2?"), + ScopedChildWidget box( + tr("Allow AetherSDR to use the CTR2?"), tr("Linux only lets the administrator open USB devices such as the CTR2, so " "AetherSDR cannot talk to it yet."), - QMessageBox::NoButton, this); + this); box.get()->setInformativeText( tr("AetherSDR can install a small system rule that lets whoever is logged in " "at this computer open CTR2 controllers, and only CTR2 controllers. You " "will be asked for your administrator password once; after that the CTR2 " "works every time.\n\nThe rule is written to %1.").arg(kRuleTarget)); - QPushButton* install = box.get()->addButton(tr("Install rule"), QMessageBox::AcceptRole); - box.get()->addButton(tr("Not now"), QMessageBox::RejectRole); + box.get()->addButton(tr("Not now"), CanonMessageBox::Role::Reject); + QPushButton* install = box.get()->addButton(tr("Install rule"), CanonMessageBox::Role::Accept); box.get()->setDefaultButton(install); box.get()->exec(); if (box && box.get()->clickedButton() == install) { @@ -342,28 +342,28 @@ void Ctr2ProxyPanel::showManualRuleInstructions(const QString& why) out.close(); if (!wrote) { qCWarning(lcDevices) << "CTR2: could not save the udev rule to" << saved; - ScopedChildWidget box( - QMessageBox::Warning, tr("CTR2 USB access"), + ScopedChildWidget box( + tr("CTR2 USB access"), tr("Linux only lets the administrator open USB devices such as the CTR2. " "AetherSDR could not ask for permission (%1), and could not save the " - "access rule to %2 for you to install by hand.").arg(why, saved), - QMessageBox::Ok, this); + "access rule to %2 for you to install by hand.").arg(why, saved), this); + box.get()->setDefaultButton(box.get()->addButton(tr("OK"), CanonMessageBox::Role::Accept)); box.get()->exec(); return; } - ScopedChildWidget box( - QMessageBox::Warning, tr("CTR2 USB access"), + ScopedChildWidget box( + tr("CTR2 USB access"), tr("Linux only lets the administrator open USB devices such as the CTR2, " "and AetherSDR could not ask for permission: %1.\n\n" "To allow it, run these commands in a terminal, then unplug and " - "replug the CTR2:").arg(why), - QMessageBox::Ok, this); + "replug the CTR2:").arg(why), this); box.get()->setInformativeText( QStringLiteral("sudo cp '%1' '%2'\n" "sudo udevadm control --reload-rules\n" "sudo udevadm trigger --subsystem-match=hidraw") .arg(saved, kRuleTarget)); - box.get()->setTextInteractionFlags(Qt::TextSelectableByMouse); + box.get()->setTextSelectable(true); + box.get()->setDefaultButton(box.get()->addButton(tr("OK"), CanonMessageBox::Role::Accept)); box.get()->exec(); } @@ -375,12 +375,13 @@ void Ctr2ProxyPanel::installUsbAccessRule() auto fail = [this](const QString& detail) { m_installingRule = false; syncConfiguration(); - ScopedChildWidget box( - QMessageBox::Warning, tr("CTR2 USB access"), - tr("The CTR2 access rule could not be installed."), QMessageBox::Ok, this); + ScopedChildWidget box( + tr("CTR2 USB access"), + tr("The CTR2 access rule could not be installed."), this); if (!detail.isEmpty()) { box.get()->setInformativeText(detail); } + box.get()->setDefaultButton(box.get()->addButton(tr("OK"), CanonMessageBox::Role::Accept)); box.get()->exec(); }; @@ -458,11 +459,11 @@ void Ctr2ProxyPanel::startAfterRuleInstalled(int attemptsLeft) return; // the Start was for USB; never start another transport from here } if (m_model->usbDeviceNeedsAccessRule()) { - ScopedChildWidget box( - QMessageBox::Information, tr("CTR2 USB access"), + ScopedChildWidget box( + tr("CTR2 USB access"), tr("The access rule is installed. Unplug the CTR2, plug it back in, " - "then press Start."), - QMessageBox::Ok, this); + "then press Start."), this); + box.get()->setDefaultButton(box.get()->addButton(tr("OK"), CanonMessageBox::Role::Accept)); box.get()->exec(); return; } diff --git a/src/gui/FirmwareInstallDialog.cpp b/src/gui/FirmwareInstallDialog.cpp index 42f6abac3e..5111765823 100644 --- a/src/gui/FirmwareInstallDialog.cpp +++ b/src/gui/FirmwareInstallDialog.cpp @@ -1,6 +1,7 @@ #include "FirmwareInstallDialog.h" #include "CanonIndicators.h" +#include "CanonMessageBox.h" #include "ComboStyle.h" #include "Ctr2ProxyPanel.h" #include "ModemChrome.h" @@ -19,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -686,29 +686,31 @@ bool FirmwareInstallDialog::askDownloadMode(bool buttons) if (!buttons) { return true; } - ScopedChildWidget ask(QMessageBox::Information, - tr("Put the CTR2-Max in download mode"), - tr("Put the CTR2-Max in download mode now."), - QMessageBox::Ok | QMessageBox::Cancel, this); + ScopedChildWidget ask(tr("Put the CTR2-Max in download mode"), + tr("Put the CTR2-Max in download mode now."), this); ask.get()->setInformativeText( tr("Hold BOOT (through the hole in the back), press and release RESET, then release " "BOOT. Its screen goes dark; that is normal. AetherSDR carries on by itself once it " "finds it there.")); - ask.get()->setDefaultButton(QMessageBox::Ok); - return ask.get()->exec() == QMessageBox::Ok; + ask.get()->addButton(tr("Cancel"), CanonMessageBox::Role::Reject); + QPushButton* ok = ask.get()->addButton(tr("OK"), CanonMessageBox::Role::Accept); + ask.get()->setDefaultButton(ok); + ask.get()->exec(); + return ask && ask.get()->clickedButton() == ok; } void FirmwareInstallDialog::confirmRestore(const QString& path) { FirmwareInstallModel::Target target = m_fw->target(); - ScopedChildWidget ask(QMessageBox::Warning, tr("Restore the backup?"), - backupText(path), QMessageBox::Cancel, this); + ScopedChildWidget ask(tr("Restore the backup?"), backupText(path), this); ask.get()->setInformativeText( tr("A backup goes back only onto the CTR2-Max it came from, and only if the file is " "unchanged since it was made.")); - QPushButton* go = ask.get()->addButton(tr("Restore"), QMessageBox::DestructiveRole); + // Return keeps what is on the device; restoring takes a click. + ask.get()->setDefaultButton(ask.get()->addButton(tr("Cancel"), CanonMessageBox::Role::Reject)); + QPushButton* go = ask.get()->addButton(tr("Restore"), CanonMessageBox::Role::Destructive); ask.get()->exec(); - if (ask.get()->clickedButton() != go || !askDownloadMode(needsButtons(*m_fw, target))) + if (!ask || ask.get()->clickedButton() != go || !askDownloadMode(needsButtons(*m_fw, target))) return; target = afterAlert(*m_fw, target); const QString done = tr("The backup is restored and the CTR2-Max is restarting."); @@ -737,18 +739,19 @@ void FirmwareInstallDialog::onInstall() const QString product = m_fw->releaseProduct(); const QString version = m_fw->releaseVersion(); const QString folder = QDir::toNativeSeparators(FirmwareInstallModel::backupFolder()); - ScopedChildWidget ask(QMessageBox::Question, tr("Install %1?").arg(product), - tr("Install %1 %2 on this CTR2-Max?").arg(product, version), - QMessageBox::Cancel, this); + ScopedChildWidget ask(tr("Install %1?").arg(product), + tr("Install %1 %2 on this CTR2-Max?").arg(product, version), + this); ask.get()->setInformativeText( tr("Its whole flash is backed up first, to %1; that takes about a minute. The install " "writes only what the firmware's manifest lists: the stock firmware, its settings and " "its registration are kept.") .arg(folder)); - QPushButton* go = ask.get()->addButton(tr("Install"), QMessageBox::AcceptRole); + ask.get()->addButton(tr("Cancel"), CanonMessageBox::Role::Reject); + QPushButton* go = ask.get()->addButton(tr("Install"), CanonMessageBox::Role::Accept); ask.get()->setDefaultButton(go); ask.get()->exec(); - if (ask.get()->clickedButton() != go || !askDownloadMode(needsButtons(*m_fw, target))) + if (!ask || ask.get()->clickedButton() != go || !askDownloadMode(needsButtons(*m_fw, target))) return; target = afterAlert(*m_fw, target); const QString done = @@ -770,18 +773,19 @@ void FirmwareInstallDialog::installOverLink() } const QString product = m_fw->releaseProduct(); const QString version = m_fw->releaseVersion(); - ScopedChildWidget ask( - QMessageBox::Question, tr("Install %1?").arg(product), + ScopedChildWidget ask( + tr("Install %1?").arg(product), tr("Install %1 %2 over the USB cable? The device runs %3 now.") .arg(product, version, m_fw->linkVersion()), - QMessageBox::Cancel, this); + this); ask.get()->setInformativeText( tr("It takes about three minutes. The device checks the firmware's signature, and keeps " "the firmware it has if anything goes wrong.")); - QPushButton* go = ask.get()->addButton(tr("Install"), QMessageBox::AcceptRole); + ask.get()->addButton(tr("Cancel"), CanonMessageBox::Role::Reject); + QPushButton* go = ask.get()->addButton(tr("Install"), CanonMessageBox::Role::Accept); ask.get()->setDefaultButton(go); ask.get()->exec(); - if (ask.get()->clickedButton() != go) + if (!ask || ask.get()->clickedButton() != go) return; const QString done = tr("%1 %2 is installed and running.").arg(product, version); m_step->setText(tr("Starting the update…")); @@ -830,9 +834,8 @@ void FirmwareInstallDialog::installStockFirmware() } const QString name = m_fw->stockFirmwareName(); const QString folder = QDir::toNativeSeparators(FirmwareInstallModel::backupFolder()); - ScopedChildWidget ask(QMessageBox::Question, tr("Update the CTR2-Max?"), - tr("Install %1 on this CTR2-Max?").arg(name), - QMessageBox::Cancel, this); + ScopedChildWidget ask(tr("Update the CTR2-Max?"), + tr("Install %1 on this CTR2-Max?").arg(name), this); QString info = tr("Only the firmware itself is written: the CTR2-Max keeps its settings, its " "registration and AetherCTR2. Its whole flash is backed up first, to %1; " "that takes about a minute.") @@ -842,10 +845,11 @@ void FirmwareInstallDialog::installStockFirmware() + tr("AetherCTR2 restarts into download mode for the update; the relay stops " "while it runs."); ask.get()->setInformativeText(info); - QPushButton* go = ask.get()->addButton(tr("Update"), QMessageBox::AcceptRole); + ask.get()->addButton(tr("Cancel"), CanonMessageBox::Role::Reject); + QPushButton* go = ask.get()->addButton(tr("Update"), CanonMessageBox::Role::Accept); ask.get()->setDefaultButton(go); ask.get()->exec(); - if (ask.get()->clickedButton() != go || !askDownloadMode(needsButtons(*m_fw, target))) + if (!ask || ask.get()->clickedButton() != go || !askDownloadMode(needsButtons(*m_fw, target))) return; target = afterAlert(*m_fw, target); const QString done = diff --git a/tests/canon_message_box_test.cpp b/tests/canon_message_box_test.cpp new file mode 100644 index 0000000000..d88e682b82 --- /dev/null +++ b/tests/canon_message_box_test.cpp @@ -0,0 +1,191 @@ +// CanonMessageBox (RFC #6226): the canon window's question box. The caller +// acts on clickedButton(), so it must name the button the operator clicked +// (or Return clicked for them), and be null when the box is closed instead. + +#include "TestSettingsProfile.h" +#include "core/AppSettings.h" +#include "core/ThemeManager.h" +#include "gui/CanonMessageBox.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace AetherSDR; + +static int g_failures = 0; + +#define EXPECT_TRUE(cond) do { \ + if (!(cond)) { \ + std::fprintf(stderr, "FAIL %s:%d expected (%s) to be true\n", \ + __FILE__, __LINE__, #cond); \ + ++g_failures; \ + } \ +} while (0) + +namespace { + +struct Box { + CanonMessageBox box{QStringLiteral("Install AetherCTR2?"), + QStringLiteral("Install AetherCTR2 0.9.6 over the USB cable?")}; + QPushButton* cancel{box.addButton(QStringLiteral("Cancel"), CanonMessageBox::Role::Reject)}; + QPushButton* install{box.addButton(QStringLiteral("Install"), CanonMessageBox::Role::Accept)}; + + Box() { box.setDefaultButton(install); } + + // Runs the box modally, doing `act` once it is showing. + int run(std::function act) + { + QTimer::singleShot(0, &box, [this, act] { + QTest::qWaitForWindowExposed(&box); + act(box); + }); + return box.exec(); + } +}; + +void pressKey(CanonMessageBox& box, Qt::Key key) +{ + QWidget* target = QApplication::focusWidget() ? QApplication::focusWidget() : &box; + QTest::keyClick(target, key); +} + +} // namespace + +int main(int argc, char** argv) +{ + TestSettingsProfile settingsProfile(QStringLiteral("aether-canon-message-box-test")); + if (!settingsProfile.isValid()) { + std::fprintf(stderr, "FAIL could not create isolated settings profile\n"); + return 1; + } + if (qEnvironmentVariableIsEmpty("QT_QPA_PLATFORM")) { + qputenv("QT_QPA_PLATFORM", "offscreen"); + } + QApplication app(argc, argv); + AppSettings::instance().load(); + ThemeManager::instance(); + + // ---- a click on an accepting button names it ---- + { + Box b; + const int result = b.run([&b](CanonMessageBox&) { b.install->click(); }); + EXPECT_TRUE(result == QDialog::Accepted); + EXPECT_TRUE(b.box.clickedButton() == b.install); + } + + // ---- a click on a rejecting button names it, and rejects ---- + { + Box b; + const int result = b.run([&b](CanonMessageBox&) { b.cancel->click(); }); + EXPECT_TRUE(result == QDialog::Rejected); + EXPECT_TRUE(b.box.clickedButton() == b.cancel); + } + + // ---- Escape closes it with no button clicked ---- + { + Box b; + const int result = b.run([](CanonMessageBox& box) { pressKey(box, Qt::Key_Escape); }); + EXPECT_TRUE(result == QDialog::Rejected); + EXPECT_TRUE(b.box.clickedButton() == nullptr); + } + + // ---- the corner close button closes it with no button clicked ---- + { + Box b; + const int result = b.run([](CanonMessageBox& box) { + // The corner button is the window's only QToolButton. + if (auto* close = box.findChild()) + close->click(); + else + box.done(-1); + }); + EXPECT_TRUE(result == QDialog::Rejected); + EXPECT_TRUE(b.box.clickedButton() == nullptr); + } + + // ---- Return clicks the default button, which opens with the focus ---- + { + Box b; + bool focused = false; + const int result = b.run([&b, &focused](CanonMessageBox& box) { + focused = b.install->hasFocus(); + pressKey(box, Qt::Key_Return); + }); + EXPECT_TRUE(focused); + EXPECT_TRUE(result == QDialog::Accepted); + EXPECT_TRUE(b.box.clickedButton() == b.install); + EXPECT_TRUE(b.install->isDefault() && !b.cancel->isDefault()); + } + + // ---- a box shown again forgets the previous answer ---- + { + Box b; + b.run([&b](CanonMessageBox&) { b.install->click(); }); + const int result = b.run([](CanonMessageBox& box) { pressKey(box, Qt::Key_Escape); }); + EXPECT_TRUE(result == QDialog::Rejected); + EXPECT_TRUE(b.box.clickedButton() == nullptr); + } + + // ---- markup in a manifest string is shown as text, never rendered ---- + { + CanonMessageBox box(QStringLiteral("Install X?"), + QStringLiteral("Install X 1.0?")); + box.setInformativeText(QStringLiteral("y")); + int plain = 0; + for (QLabel* l : box.findChildren()) { + if (l->text().contains(QLatin1Char('<'))) + plain += l->textFormat() == Qt::PlainText ? 1 : -100; + } + EXPECT_TRUE(plain == 3); // title, text and further text + } + + // ---- Return on a focused button clicks that button, not the default ---- + // Buttons are auto-default for this. A platform theme whose button-press + // keys include Return (as here, offscreen) gives the same result without + // it; one that leaves Return out would click the default instead. + { + Box b; + bool cancelFocused = false; + const int result = b.run([&b, &cancelFocused](CanonMessageBox& box) { + b.cancel->setFocus(); + cancelFocused = QApplication::focusWidget() == b.cancel; + pressKey(box, Qt::Key_Return); + }); + EXPECT_TRUE(cancelFocused); + EXPECT_TRUE(result == QDialog::Rejected); + EXPECT_TRUE(b.box.clickedButton() == b.cancel); + } + + // ---- the further text shows only when there is some; a destructive + // button accepts and is marked for the warning color ---- + { + CanonMessageBox box(QStringLiteral("Restore the backup?"), QStringLiteral("Restore it?")); + auto* info = box.findChild(QStringLiteral("canonMessageInfo")); + EXPECT_TRUE(info && info->isHidden()); + EXPECT_TRUE(box.accessibleDescription() == QStringLiteral("Restore it?")); + box.setInformativeText(QStringLiteral("Only onto the CTR2-Max it came from.")); + EXPECT_TRUE(info && !info->isHidden()); + // A screen reader reads both texts when the box opens. + EXPECT_TRUE(box.accessibleDescription() + == QStringLiteral("Restore it?\nOnly onto the CTR2-Max it came from.")); + QPushButton* restore = + box.addButton(QStringLiteral("Restore"), CanonMessageBox::Role::Destructive); + EXPECT_TRUE(restore->property("role").toString() == QStringLiteral("destructive")); + QTimer::singleShot(0, &box, [restore] { restore->click(); }); + EXPECT_TRUE(box.exec() == QDialog::Accepted); + EXPECT_TRUE(box.clickedButton() == restore); + } + + if (g_failures == 0) { + std::printf("canon_message_box_test: all checks passed\n"); + return 0; + } + std::fprintf(stderr, "canon_message_box_test: %d failure(s)\n", g_failures); + return 1; +} diff --git a/tests/ctr2_firmware_updater_test.cpp b/tests/ctr2_firmware_updater_test.cpp index c3ec87c070..df39ef6a6c 100644 --- a/tests/ctr2_firmware_updater_test.cpp +++ b/tests/ctr2_firmware_updater_test.cpp @@ -22,6 +22,7 @@ extern "C" { #include #include +#include #include #include #include @@ -73,6 +74,9 @@ struct Device { bool hasOtherFirmware{true}; // SWITCH has somewhere to go bool hasBootloaderOp{true}; // firmware that answers BOOTLOADER QString restartedInto; // "other" or "bootloader" after a restart request + std::uint8_t restartStatusOp{0}; // not 0: the op its restart STATUS names instead + bool restartsSilently{false}; // leaves for a restart without answering it + bool failsFirmwareWrite{false}; // the write of a FIRMWARE message fails (EIO) int dropsOnOpen{0}; // this open (1 = the first) fails before it negotiates }; @@ -118,11 +122,34 @@ class SimPort final : public Ctr2HidPort { bool isOpen() const override { return m_open; } void send(const std::vector& reports) override { + if (!m_open) { + return; + } + // As the USB worker does: a write that fails closes the port before + // the device sees the reports, and nothing is counted as sent. + const bool firmware = std::any_of(reports.begin(), reports.end(), [](const Report& r) { + return r.data()[0] == 0xFF && r.data()[3] == std::uint8_t(MessageType::Firmware); + }); + if (firmware && m_dev->failsFirmwareWrite) { + m_open = false; + QTimer::singleShot(0, this, [this] { + emit failed(QStringLiteral("CTR2 USB write failed: injected EIO")); + }); + return; + } for (const Report& r : reports) { if (m_open) { onReport(r.data()); } } + // Written reports are acknowledged before anything that follows. + const int count = int(reports.size()); + QPointer self(this); + QTimer::singleShot(0, this, [self, count] { + if (self) { + emit self->reportsSent(count); + } + }); } void discardQueued() override {} void shutdown(const std::vector& finalReports) override @@ -272,7 +299,8 @@ class SimPort final : public Ctr2HidPort { status(d[0], CTR2_FW_BAD_STATE); break; } - status(d[0], CTR2_FW_OK); + if (!m_dev->restartsSilently) + status(m_dev->restartStatusOp ? m_dev->restartStatusOp : d[0], CTR2_FW_OK); m_dev->restartedInto = loader ? QStringLiteral("bootloader") : QStringLiteral("other"); m_dev->gone = true; QTimer::singleShot(5, this, [this] { @@ -581,6 +609,30 @@ void testRestartRequests() run = restart(alone, R::OtherFirmware); check(!run.ok && alone.restartedInto.isEmpty(), "SWITCH with no other firmware fails"); check(alone.closes == 1, "and the session is closed"); + + // Lynovation's v1.02.00 betas named the op as a character ('5', 0x35) + // and left the bus two seconds later. + Device misnamed; + misnamed.restartStatusOp = 0x30 | CTR2_FW_SWITCH; + run = restart(misnamed, R::OtherFirmware); + check(run.ok && misnamed.restartedInto == QStringLiteral("other") + && run.message.contains(QStringLiteral("restarting")), + "a device that leaves after a STATUS naming another op has restarted"); + + Device silent; + silent.restartsSilently = true; + run = restart(silent, R::Bootloader); + check(run.ok && run.message.contains(QStringLiteral("download mode")), + "a device that leaves for download mode without answering has restarted"); + + // A port that fails writing the request has not sent it: no restart. + Device unwritten; + unwritten.failsFirmwareWrite = true; + run = restart(unwritten, R::OtherFirmware); + check(!run.ok && unwritten.restartedInto.isEmpty() + && run.message.contains(QStringLiteral("write failed")) + && run.message.contains(QStringLiteral("keeps the firmware")), + "a restart request whose write fails is reported as lost, not as a restart"); } // AetherSDR quitting mid-update destroys the updater: the device must still diff --git a/tests/ctr2_hid_thread_port_test.cpp b/tests/ctr2_hid_thread_port_test.cpp index 0a62342a51..1394880057 100644 --- a/tests/ctr2_hid_thread_port_test.cpp +++ b/tests/ctr2_hid_thread_port_test.cpp @@ -54,6 +54,7 @@ struct FakeDevice { std::atomic writeDelayMs{0}; std::atomic writesBegun{0}; std::atomic failWrites{false}; + std::atomic failWritesAfter{-1}; // not -1: writes after this many fail std::atomic closed{false}; std::function onWritten; // after a write completes, on the I/O thread QByteArray featureReply; // Get Feature 0x02 answer, ID first; empty = stall @@ -83,6 +84,9 @@ struct FakeDevice { return -1; } std::lock_guard g(lock); + if (failWritesAfter >= 0 && static_cast(writes.size()) >= failWritesAfter) { + return -1; + } writes.emplace_back(reinterpret_cast(buffer), size); if (onWritten) { onWritten(); @@ -224,6 +228,23 @@ void testReadsAndFailures() "a failed write fails the port with the device's reason"); port2->shutdown({}); waitUntil([] { return false; }, 100); + + // Writes that went out before a failing one in the same tick are counted, + // and reported before the failure: the firmware updater relies on it. + auto dev3 = std::make_shared(); + dev3->failWritesAfter = 2; + auto* port3 = new Ctr2HidThreadPort(dev3->io(), QStringLiteral("fake")); + int sent3 = 0; + int sentAtFailure = -1; + QObject::connect(port3, &Ctr2HidPort::reportsSent, port3, [&](int n) { sent3 += n; }); + QObject::connect(port3, &Ctr2HidPort::failed, port3, + [&](const QString&) { sentAtFailure = sent3; }); + port3->send(reports(3, 0x50)); + check(waitUntil([&] { return sentAtFailure >= 0; }) && dev3->writeCount() == 2 + && sentAtFailure == 2, + "writes before a failing one in the same tick are reported sent, before the failure"); + port3->shutdown({}); + waitUntil([] { return false; }, 100); } // Stop then Start on one device: the old port's slow final CLOSED and the new diff --git a/tests/tests.cmake b/tests/tests.cmake index 6741f9d36c..843a80ff10 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -2344,6 +2344,27 @@ add_test(NAME canon_window_test COMMAND canon_window_test) set_tests_properties(canon_window_test PROPERTIES ENVIRONMENT "QT_QPA_PLATFORM=offscreen") +# CanonMessageBox (RFC #6226): the canon question box reports the button +# clicked, and none when it is closed instead. +add_executable(canon_message_box_test + tests/canon_message_box_test.cpp + src/gui/CanonMessageBox.cpp + src/gui/CanonWindow.cpp + src/gui/ModemChrome.cpp + src/core/ThemeManager.cpp + src/core/ThemeSeedGenerated.cpp + $ + $ + $ + ${THEME_TEST_RESOURCES} +) +target_include_directories(canon_message_box_test PRIVATE src) +target_link_libraries(canon_message_box_test PRIVATE Qt6::Core Qt6::Gui Qt6::Widgets Qt6::Test) +set_target_properties(canon_message_box_test PROPERTIES AUTOMOC ON) +add_test(NAME canon_message_box_test COMMAND canon_message_box_test) +set_tests_properties(canon_message_box_test PROPERTIES + ENVIRONMENT "QT_QPA_PLATFORM=offscreen") + # CanonApplet, CanonPan and CanonCanvas (RFC #6226): the canon ground with # square corners inside the main window; the close button only requests. add_executable(canon_surface_test @@ -9823,6 +9844,7 @@ set(AETHER_SETTINGS_CONSUMERS aether_rx_profiles_test theme_manager_test canon_window_test + canon_message_box_test canon_surface_test kpa500_applet_test kxpa100_applet_test