From 57ba591d985559db8b430acbad21825b6aafc5a9 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 30 Sep 2026 00:36:33 +0700 Subject: [PATCH 1/4] fix: keep a bulk IN pending between requests on Panorama-family displays Panorama, Panorama SE and Panorama WB displays on AMD 800-series chipset controllers vanish from the bus during a session and return only after the power supply is switched off. Testers traced it to the idle link between exchanges: the official bridge keeps a bulk IN pending almost all the time and survives for days, while a client that arms the IN only around each request died within minutes and survived hours once it kept the IN pending. This runtime armed the IN only around requests as well. Add a keepBulkInPending product profile flag, enabled for the Panorama family, and leave the IN pending after every exchange. An idle IN that the firmware ends with an error or an empty packet is logged as tryx_usb_idle_input, never counts toward the persistent input failure that ends the session, and is re-armed only after the next request. Turris keeps the request-scoped IN until there is data for it. Document the change and the usbcore quirk workaround in the README. --- README.md | 23 +++++- src/printerproductprofile.cpp | 1 + src/printerproductprofile.h | 5 ++ src/printerprotocol.cpp | 5 ++ src/printerprotocol.h | 12 +++ src/printertransactionchannel.cpp | 3 + src/usbprintertransport.cpp | 120 ++++++++++++++++++++++++++++++ src/usbprintertransport.h | 8 ++ tests/printerprotocol_tests.cpp | 70 +++++++++++++++++ 9 files changed, 246 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 20f54be..1c4b2b8 100644 --- a/README.md +++ b/README.md @@ -696,6 +696,27 @@ sudo dnf install -y android-tools unzip e2fsprogs ffmpeg mesa-demos - Supported printer-class devices use `391a:1011` for Panorama, `391a:1021` for Panorama SE / PASE, `391a:1031` for Panorama WB, and `391a:2011` for Turris 620; direct libusb access uses `/dev/bus/usb/*/*` and requires the `lp` group or a seat ACL from `TAG+="uaccess"` - Fedora's generic printer rule must not start CUPS `configure-printer` for this vendor protocol. The qmake install target places an early access rule and a late printer-suppression rule in `/usr/lib/udev/rules.d`; do not create same-named overrides in `/etc/udev/rules.d`, because they would shadow packaged updates. +### Display drops off USB and only returns after a power-off + +Some Panorama `391a:1011`, Panorama SE `391a:1021` and Panorama WB +`391a:1031` displays disappear from the bus during a session and come back +only after the power supply is switched off. Reports come from AMD 800-series +chipset USB controllers on Linux and Windows. The firmware fails when the host +stops polling the device between exchanges; the official bridge keeps a bulk +IN pending at all times and survives for days. Since this build the runtime +does the same for the Panorama family. + +If a display still drops, a kernel parameter that disables USB link power +management for these devices is a known workaround: + +```text +usbcore.quirks=391a:1011:k,391a:1021:k,391a:1031:k +``` + +Add it to the kernel command line of your boot loader, reboot, and switch the +power supply off once so the display recovers. Details and measurements are in +issue #28. + ## Firmware Updates Firmware updates are initiated from the firmware panel in Quick Settings, but @@ -727,7 +748,7 @@ verification. A new flash cannot be started from an unidentified Loader-only device; the runtime must first identify a firmware-capable TRYX product before authorizing the transition into Loader mode. -After updating to the new KANALI firmware, the cooler no longer exposes ADB by default. It appears as `391a:1021 RK PASE` with a bidirectional printer interface. The app generates C++ types from three minimal, project-owned schemas under `protocol/wire-v1`; recovered vendor descriptor sources are not a build or release dependency. The production path does not read or write `/dev/usb/lp*`: it claims the `07/01/02` interface through usbfs, temporarily detaches `usblp`, arms one bulk IN before each request, never re-arms that endpoint while the matching bulk OUT is still active, drains optional periodic responses to a complete frame boundary after OUT, and releases the interface on shutdown. +After updating to the new KANALI firmware, the cooler no longer exposes ADB by default. It appears as `391a:1021 RK PASE` with a bidirectional printer interface. The app generates C++ types from three minimal, project-owned schemas under `protocol/wire-v1`; recovered vendor descriptor sources are not a build or release dependency. The production path does not read or write `/dev/usb/lp*`: it claims the `07/01/02` interface through usbfs, temporarily detaches `usblp`, arms one bulk IN before each request and, on the Panorama family, leaves one bulk IN pending between requests as the official bridge does (an idle link made these displays drop off the bus until power was removed, see issue #28), never re-arms that endpoint while the matching bulk OUT is still active, drains optional periodic responses to a complete frame boundary after OUT, and releases the interface on shutdown. Turris 620 exposes the supported `391a:2011` printer-class identity and runs on the same configuration pipeline as PASE with a Turris product profile. Media is diff --git a/src/printerproductprofile.cpp b/src/printerproductprofile.cpp index 9315438..81409ca 100644 --- a/src/printerproductprofile.cpp +++ b/src/printerproductprofile.cpp @@ -13,6 +13,7 @@ PrinterProductProfile paseFamilyProfile(quint16 productId, bool firmwareFlash) { profile.overlayLayout = PrinterOverlayLayoutKind::PaseDualArea2240; profile.orientationModel = PrinterDisplayOrientationModel::RotationFields; profile.fileTransferTrackId = 0; + profile.keepBulkInPending = true; profile.mediaUploadSupported = true; profile.mediaCatalogSupported = true; profile.mediaPullSupported = true; diff --git a/src/printerproductprofile.h b/src/printerproductprofile.h index a0111d8..9ab15da 100644 --- a/src/printerproductprofile.h +++ b/src/printerproductprofile.h @@ -43,6 +43,11 @@ struct PrinterProductProfile { // 0 keeps the full transaction timeout. Turris firmware never // acknowledges these writes and the official app does not wait for them. int readbackConfirmedWriteAckWindowMs = 0; + // Keep one bulk IN transfer pending between requests, as the official + // bridge does. With the IN armed only around each request, the idle link + // between exchanges makes Panorama-family firmware drop off the bus until + // power is removed (issue #28). + bool keepBulkInPending = false; // Written into UserConfiguration when the device reports no such section. QString defaultPowerOnMedia; QString defaultStandbyMedia; diff --git a/src/printerprotocol.cpp b/src/printerprotocol.cpp index 83d52b9..1710ef9 100644 --- a/src/printerprotocol.cpp +++ b/src/printerprotocol.cpp @@ -292,6 +292,11 @@ bool PrinterProtocol::validateMediaPullPathForTesting(const QByteArray &rawPath, return validateMediaPullPath(rawPath, mediaName); } +PrinterProtocol::IdleInputTestResult PrinterProtocol::runIdleInputScenarioForTesting( + const QList &events, int cycles) { + return UsbPrinterTransport::runIdleInputScenarioForTesting(events, cycles); +} + PrinterProtocol::DuplexTestResult PrinterProtocol::runDuplexTransportScenarioForTesting( const QList &events, const QByteArray &request, int writeTimeoutMs, int readTimeoutMs) { diff --git a/src/printerprotocol.h b/src/printerprotocol.h index 362efdf..fd48ba9 100644 --- a/src/printerprotocol.h +++ b/src/printerprotocol.h @@ -427,6 +427,16 @@ class PrinterProtocol { bool persistentInputFailure = false; }; + struct IdleInputTestResult { + QString error; + int completedCycles = 0; + int inputSubmissions = 0; + int idleInputErrors = 0; + int inputErrors = 0; + bool persistentInputFailure = false; + bool inputPendingAtEnd = false; + }; + bool trackedPingForTesting(const QString &devicePath, QString *payload, QString *errorMessage, const OperationContext &context); @@ -451,6 +461,8 @@ class PrinterProtocol { const QString &devRoot, quint16 expectedProductId, QString *errorMessage); + static IdleInputTestResult runIdleInputScenarioForTesting( + const QList &events, int cycles); static DuplexTestResult runDuplexTransportScenarioForTesting( const QList &events, const QByteArray &request, diff --git a/src/printertransactionchannel.cpp b/src/printertransactionchannel.cpp index e57b1de..0c7a4c5 100644 --- a/src/printertransactionchannel.cpp +++ b/src/printertransactionchannel.cpp @@ -1,4 +1,5 @@ #include "printertransactionchannel.h" +#include "printerproductprofile.h" #include "printerframecodec_p.h" #include "printeroperation_p.h" #include "turrismediaformat.h" @@ -40,6 +41,8 @@ PrinterTransactionChannel::PrinterTransactionChannel(quint16 expectedProductId, if (nextTrackId_ == 0) { nextTrackId_ = 1; } + const auto profile = printerProductProfileForId(expectedProductId_); + libusbTransport_.setKeepInputPending(profile && profile->keepBulkInPending); } PrinterTransactionChannel::~PrinterTransactionChannel() { closeDevice(); } diff --git a/src/usbprintertransport.cpp b/src/usbprintertransport.cpp index 27434d0..08e386c 100644 --- a/src/usbprintertransport.cpp +++ b/src/usbprintertransport.cpp @@ -254,6 +254,8 @@ class UsbPrinterTransport::Impl { libusb_transfer *transfer = nullptr; bool active = false; bool abandoned = false; + // Armed after an exchange, with no response expected. + bool idleArm = false; libusb_transfer_status lastStatus = LIBUSB_TRANSFER_COMPLETED; int lastActualLength = 0; std::array buffer{}; @@ -486,6 +488,7 @@ class UsbPrinterTransport::Impl { receiveQueue_.clear(); consecutiveInputTransferErrors_ = 0; consecutiveRetryableInputCompletions_ = 0; + idleRearmSuppressed_ = false; inputCompletionGeneration_ = 0; zeroLengthInputCompletionGeneration_ = 0; inputTransferErrorGeneration_ = 0; @@ -623,6 +626,7 @@ class UsbPrinterTransport::Impl { receiveQueue_.clear(); consecutiveInputTransferErrors_ = 0; consecutiveRetryableInputCompletions_ = 0; + idleRearmSuppressed_ = false; closing_ = false; fatalError_.clear(); #ifdef TRYX_PROTOCOL_TESTING @@ -668,6 +672,7 @@ class UsbPrinterTransport::Impl { receiveQueue_.clear(); consecutiveInputTransferErrors_ = 0; consecutiveRetryableInputCompletions_ = 0; + idleRearmSuppressed_ = false; inputCompletionGeneration_ = 0; zeroLengthInputCompletionGeneration_ = 0; inputTransferErrorGeneration_ = 0; @@ -694,6 +699,18 @@ class UsbPrinterTransport::Impl { int inputRearmCountForTesting() const { return static_cast(inputRearmsDuringOutput_); } + + int idleInputErrorCountForTesting() const { + return static_cast(idleInputErrorGeneration_); + } + + bool inputPendingForTesting() const { + return inputState_ && inputState_->active; + } + + bool serviceEventsForTesting(int timeoutMs, QString *errorMessage) { + return serviceEvents(timeoutMs, errorMessage); + } #endif WriteResult write(const QByteArray &data, int timeoutMs, @@ -739,6 +756,8 @@ class UsbPrinterTransport::Impl { const quint64 zeroLengthInputCompletionGenerationBeforeWrite = zeroLengthInputCompletionGeneration_; const quint64 inputErrorGenerationBeforeWrite = inputTransferErrorGeneration_; + // A new request gives a suppressed idle IN another chance afterwards. + idleRearmSuppressed_ = false; if (!ensureInputActive(&result.error)) { return result; } @@ -935,6 +954,7 @@ class UsbPrinterTransport::Impl { *bytes = std::move(receiveQueue_); } receiveQueue_.clear(); + rearmIdleInput(); return ReadResult::Data; } if (!fatalError_.isEmpty()) { @@ -990,6 +1010,7 @@ class UsbPrinterTransport::Impl { return ReadResult::Error; } } + rearmIdleInput(); return ReadResult::Timeout; } @@ -1035,9 +1056,31 @@ class UsbPrinterTransport::Impl { } receiveQueue_.clear(); } + rearmIdleInput(); return fatalError_.isEmpty(); } + void setKeepInputPending(bool keep) { keepInputPending_ = keep; } + + // After an exchange, leave the bulk IN pending so the host keeps polling + // the device between requests, as the official bridge does. Never re-arm + // while an idle IN error is being backed off or the session is failing. + void rearmIdleInput() { + if (!keepInputPending_ || !sessionOpen_ || closing_ || !inputState_ || + !inputState_->transfer || !fatalError_.isEmpty() || + persistentInputFailureLatched_ || idleRearmSuppressed_ || + consecutiveRetryableInputCompletions_ > 0) { + return; + } + if (!inputState_->active) { + QString ignored; + if (!ensureInputActive(&ignored)) { + return; + } + } + inputState_->idleArm = true; + } + private: static void LIBUSB_CALL inputTransferCompleted(libusb_transfer *transfer) { auto *state = static_cast(transfer->user_data); @@ -1051,16 +1094,39 @@ class UsbPrinterTransport::Impl { return; } const bool hasInputBytes = transfer->actual_length > 0; + const bool idleArm = state->idleArm; + state->idleArm = false; transport->eventCompletionObserved_ = 1; ++transport->inputCompletionGeneration_; state->lastStatus = transfer->status; state->lastActualLength = transfer->actual_length; + if (idleArm && !hasInputBytes && + (transfer->status == LIBUSB_TRANSFER_ERROR || + transfer->status == LIBUSB_TRANSFER_COMPLETED)) { + // An idle IN that ends without data proves nothing about the + // session. Record it, keep it out of the persistent failure + // counters and wait for the next request before re-arming. + ++transport->idleInputErrorGeneration_; + transport->idleRearmSuppressed_ = true; + const quint64 count = transport->idleInputErrorGeneration_; + if (count <= 3 || count % 100 == 0) { + qInfo().noquote() + << QStringLiteral( + "tryx_usb_idle_input outcome=%1 count=%2") + .arg(transfer->status == LIBUSB_TRANSFER_ERROR + ? QStringLiteral("error") + : QStringLiteral("empty")) + .arg(count); + } + return; + } if (hasInputBytes) { transport->receiveQueue_.append( reinterpret_cast(transfer->buffer), transfer->actual_length); transport->consecutiveInputTransferErrors_ = 0; transport->consecutiveRetryableInputCompletions_ = 0; + transport->idleRearmSuppressed_ = false; if (transport->receiveQueue_.size() > kMaxLibusbReceiveQueueSize) { transport->fatalError_ = QObject::tr("TRYX libusb receive queue exceeded its bounded size"); @@ -1177,6 +1243,7 @@ class UsbPrinterTransport::Impl { return false; } if (inputState_->active) { + inputState_->idleArm = false; return true; } if (!fatalError_.isEmpty()) { @@ -1204,6 +1271,7 @@ class UsbPrinterTransport::Impl { return false; } inputState_->active = true; + inputState_->idleArm = false; return true; } @@ -1265,6 +1333,9 @@ class UsbPrinterTransport::Impl { QString fatalError_; int consecutiveInputTransferErrors_ = 0; bool persistentInputFailureLatched_ = false; + bool keepInputPending_ = false; + bool idleRearmSuppressed_ = false; + quint64 idleInputErrorGeneration_ = 0; int consecutiveRetryableInputCompletions_ = 0; quint64 inputCompletionGeneration_ = 0; quint64 zeroLengthInputCompletionGeneration_ = 0; @@ -1489,7 +1560,56 @@ bool UsbPrinterTransport::takeAvailable(QByteArray *bytes, QString *errorMessage return impl_->takeAvailable(bytes, errorMessage, timeoutMs); } +void UsbPrinterTransport::setKeepInputPending(bool keep) { + impl_->setKeepInputPending(keep); +} + #ifdef TRYX_PROTOCOL_TESTING +PrinterProtocol::IdleInputTestResult UsbPrinterTransport::runIdleInputScenarioForTesting( + const QList &events, int cycles) { + PrinterProtocol::IdleInputTestResult result; + Impl transport; + QString transportError; + ScriptedLibusbEventBackend *backend = transport.adoptScriptedSessionForTesting( + events, QStringLiteral("scripted-usb"), 0x1021, &transportError); + if (!backend) { + result.error = transportError; + return result; + } + transport.setKeepInputPending(true); + PrinterProtocol::OperationContext context; + for (int cycle = 0; cycle < cycles; ++cycle) { + const Impl::WriteResult write = + transport.write(QByteArrayLiteral("request"), 100, context); + if (!write.success) { + result.error = write.error; + break; + } + QByteArray response; + if (transport.readSome(&response, 100, context, &transportError) != + Impl::ReadResult::Data) { + result.error = transportError.isEmpty() + ? QStringLiteral("no response in cycle %1").arg(cycle) + : transportError; + break; + } + // Idle time between requests: dispatch whatever the device does with + // the pending IN, without any request expecting it. + if (!transport.serviceEventsForTesting(10, &transportError)) { + result.error = transportError; + break; + } + ++result.completedCycles; + } + result.inputSubmissions = backend->inputSubmissions(); + result.idleInputErrors = transport.idleInputErrorCountForTesting(); + result.inputErrors = transport.inputErrorCountForTesting(); + result.persistentInputFailure = transport.persistentUsbInputFailure(); + result.inputPendingAtEnd = transport.inputPendingForTesting(); + transport.close(); + return result; +} + PrinterProtocol::DuplexTestResult UsbPrinterTransport::runScenarioForTesting( const QList &events, const QByteArray &request, int writeTimeoutMs, int readTimeoutMs, const QString &deviceId, diff --git a/src/usbprintertransport.h b/src/usbprintertransport.h index 5490864..340b2df 100644 --- a/src/usbprintertransport.h +++ b/src/usbprintertransport.h @@ -57,11 +57,19 @@ class UsbPrinterTransport final { const PrinterProtocol::OperationContext &context, QString *errorMessage); bool takeAvailable(QByteArray *bytes, QString *errorMessage, int timeoutMs = 0); + // Leave one bulk IN pending after every exchange instead of only around a + // request. An error on such an idle IN never counts toward the persistent + // input failure; it only postpones the next idle IN to the next request. + void setKeepInputPending(bool keep); #ifdef TRYX_PROTOCOL_TESTING void adoptFileDescriptorForTesting(int fd, const QString &devicePath); void setPersistentUsbInputFailureForTesting(bool persistent) { persistentUsbInputFailureLatched_ = persistent; } + // Runs request/response cycles with the idle IN kept pending. Each cycle + // writes the request, reads the reply and then dispatches one idle event. + static PrinterProtocol::IdleInputTestResult runIdleInputScenarioForTesting( + const QList &events, int cycles); static PrinterProtocol::DuplexTestResult runScenarioForTesting(const QList &events, const QByteArray &request, int writeTimeoutMs, diff --git a/tests/printerprotocol_tests.cpp b/tests/printerprotocol_tests.cpp index bb8fb59..853c409 100644 --- a/tests/printerprotocol_tests.cpp +++ b/tests/printerprotocol_tests.cpp @@ -2706,6 +2706,8 @@ private slots: void usbTransportOwnsOnlyItsDataDescriptor(); void modelClientsBorrowOneBufferedChannel(); void duplexInputReceivesAckDuringOutput(); + void idleInputStaysPendingBetweenExchanges(); + void idleInputErrorsNeverLatchPersistentFailure(); void duplexZeroLengthInputDefersRearmUntilOutputCompletes(); void duplexInputErrorDefersRearmWithoutStarvingOutput(); void duplexInputRetryBudgetIsBounded(); @@ -11096,6 +11098,10 @@ void PrinterProtocolTests::productProfilesExposeExactCapabilities() { QVERIFY(!pano->firmwareFlashSupported); QVERIFY(!panoWb->firmwareFlashSupported); QCOMPARE(panoWb->productId, quint16{0x1031}); + QVERIFY(pase->keepBulkInPending); + QVERIFY(pano->keepBulkInPending); + QVERIFY(panoWb->keepBulkInPending); + QVERIFY(!turris->keepBulkInPending); QCOMPARE(turris->mediaWidth, 1280); QCOMPARE(turris->mediaHeight, 720); @@ -45251,6 +45257,70 @@ void PrinterProtocolTests::modelClientsBorrowOneBufferedChannel() { QCOMPARE(errno, EBADF); } +// Issue #28: Panorama-family firmware drops off the bus when the host stops +// polling between exchanges. After every reply the IN goes pending again. +void PrinterProtocolTests::idleInputStaysPendingBetweenExchanges() { + using Direction = PrinterProtocol::DuplexTestDirection; + QList events; + for (int cycle = 0; cycle < 3; ++cycle) { + PrinterProtocol::DuplexTestEvent output; + output.direction = Direction::Output; + events.append(output); + PrinterProtocol::DuplexTestEvent reply; + reply.direction = Direction::Input; + reply.payload = QByteArrayLiteral("reply"); + events.append(reply); + } + const auto result = PrinterProtocol::runIdleInputScenarioForTesting(events, 3); + QVERIFY2(result.error.isEmpty(), qPrintable(result.error)); + QCOMPARE(result.completedCycles, 3); + // One IN per reply plus the idle IN left pending after the last one. + QCOMPARE(result.inputSubmissions, 4); + QVERIFY(result.inputPendingAtEnd); + QCOMPARE(result.idleInputErrors, 0); + QVERIFY(!result.persistentInputFailure); + + const auto pase = printerProductProfileForId(0x1021); + const auto turris = printerProductProfileForId(0x2011); + QVERIFY(pase && pase->keepBulkInPending); + QVERIFY(turris && !turris->keepBulkInPending); +} + +// An idle IN that the firmware ends with EPROTO or an empty packet must not +// accumulate into the persistent input failure that ends the session, however +// often it happens; it only waits for the next request before re-arming. +void PrinterProtocolTests::idleInputErrorsNeverLatchPersistentFailure() { + using Direction = PrinterProtocol::DuplexTestDirection; + using Status = PrinterProtocol::DuplexTestStatus; + constexpr int kCycles = 25; + QList events; + for (int cycle = 0; cycle < kCycles; ++cycle) { + PrinterProtocol::DuplexTestEvent output; + output.direction = Direction::Output; + events.append(output); + PrinterProtocol::DuplexTestEvent reply; + reply.direction = Direction::Input; + reply.payload = QByteArrayLiteral("reply"); + events.append(reply); + PrinterProtocol::DuplexTestEvent idle; + idle.direction = Direction::Input; + idle.status = cycle % 2 == 0 ? Status::Error : Status::Completed; + idle.actualLength = 0; + events.append(idle); + } + const auto result = + PrinterProtocol::runIdleInputScenarioForTesting(events, kCycles); + QVERIFY2(result.error.isEmpty(), qPrintable(result.error)); + QCOMPARE(result.completedCycles, kCycles); + QCOMPARE(result.idleInputErrors, kCycles); + QCOMPARE(result.inputErrors, 0); + QVERIFY(!result.persistentInputFailure); + // Each cycle: one IN for the reply and one idle IN that the device ended; + // after the last idle error the IN waits for the next request. + QCOMPARE(result.inputSubmissions, 2 * kCycles); + QVERIFY(!result.inputPendingAtEnd); +} + void PrinterProtocolTests::duplexInputReceivesAckDuringOutput() { QList events; PrinterProtocol::DuplexTestEvent input; From b87fbcf0ab6301a258a12a5b83b9146c1b408ad8 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 30 Sep 2026 03:27:30 +0700 Subject: [PATCH 2/4] fix: re-arm an idle bulk IN between requests instead of at the next one Between requests no transport call serviced libusb, so an idle IN that the firmware ended stayed unnoticed until the next request, and an idle IN error suppressed the re-arm until then. With a 2 s keepalive the link could stay idle for up to 2 s, the condition that drops Panorama-family displays. Sessions that keep the IN pending now hand libusb's poll descriptors to the owning thread's event loop, service completions as soon as they are ready, and re-arm the IN right after data or after a short backoff (20 ms doubling to 200 ms) when it ended without data. Data that arrives between requests is queued and the IN re-armed at once. Reported in review by @groovg on #28. --- src/printerprotocol.cpp | 4 +- src/printerprotocol.h | 4 +- src/printerprotocolconstants_p.h | 5 + src/usbprintertransport.cpp | 250 ++++++++++++++++++++++++++++--- src/usbprintertransport.h | 3 +- tests/printerprotocol_tests.cpp | 84 ++++++++++- 6 files changed, 318 insertions(+), 32 deletions(-) diff --git a/src/printerprotocol.cpp b/src/printerprotocol.cpp index 1710ef9..6e03a4b 100644 --- a/src/printerprotocol.cpp +++ b/src/printerprotocol.cpp @@ -293,8 +293,8 @@ bool PrinterProtocol::validateMediaPullPathForTesting(const QByteArray &rawPath, } PrinterProtocol::IdleInputTestResult PrinterProtocol::runIdleInputScenarioForTesting( - const QList &events, int cycles) { - return UsbPrinterTransport::runIdleInputScenarioForTesting(events, cycles); + const QList &events, int cycles, int idleMs) { + return UsbPrinterTransport::runIdleInputScenarioForTesting(events, cycles, idleMs); } PrinterProtocol::DuplexTestResult PrinterProtocol::runDuplexTransportScenarioForTesting( diff --git a/src/printerprotocol.h b/src/printerprotocol.h index fd48ba9..0e3fea1 100644 --- a/src/printerprotocol.h +++ b/src/printerprotocol.h @@ -435,6 +435,8 @@ class PrinterProtocol { int inputErrors = 0; bool persistentInputFailure = false; bool inputPendingAtEnd = false; + QList idleRearmDelaysMs; + int queuedInputBytesAtEnd = 0; }; bool trackedPingForTesting(const QString &devicePath, QString *payload, @@ -462,7 +464,7 @@ class PrinterProtocol { quint16 expectedProductId, QString *errorMessage); static IdleInputTestResult runIdleInputScenarioForTesting( - const QList &events, int cycles); + const QList &events, int cycles, int idleMs = 100); static DuplexTestResult runDuplexTransportScenarioForTesting( const QList &events, const QByteArray &request, diff --git a/src/printerprotocolconstants_p.h b/src/printerprotocolconstants_p.h index ec4ff0a..1a5e4e6 100644 --- a/src/printerprotocolconstants_p.h +++ b/src/printerprotocolconstants_p.h @@ -38,6 +38,11 @@ constexpr int kMaxInputTransferErrorRetries = 10; constexpr int kInputTransferErrorRearmInitialBackoffMs = 5; constexpr int kInputTransferErrorRearmMaxBackoffMs = 100; constexpr int kPersistentInputTransferErrorThreshold = kMaxInputTransferErrorRetries; +// An idle bulk IN that the firmware ends without data is re-armed after this +// delay, doubled for each consecutive idle failure up to the maximum. An idle +// IN that stayed pending longer than the maximum starts the backoff over. +constexpr int kIdleInputRearmInitialDelayMs = 20; +constexpr int kIdleInputRearmMaxDelayMs = 200; constexpr qsizetype kMaxLibusbReceiveQueueSize = (PrinterFrameCodec::MaxPayloadSize + 8) * 4; diff --git a/src/usbprintertransport.cpp b/src/usbprintertransport.cpp index 08e386c..8f214e6 100644 --- a/src/usbprintertransport.cpp +++ b/src/usbprintertransport.cpp @@ -7,14 +7,19 @@ #include #endif +#include #include #include #include #include #include +#include +#include +#include #include #include +#include #include #include @@ -488,7 +493,8 @@ class UsbPrinterTransport::Impl { receiveQueue_.clear(); consecutiveInputTransferErrors_ = 0; consecutiveRetryableInputCompletions_ = 0; - idleRearmSuppressed_ = false; + idleRearmFailures_ = 0; + idleRearmDueAtMs_ = -1; inputCompletionGeneration_ = 0; zeroLengthInputCompletionGeneration_ = 0; inputTransferErrorGeneration_ = 0; @@ -497,10 +503,13 @@ class UsbPrinterTransport::Impl { close(); return false; } + startIdleWatch(); return true; } void close() { + TransportCallScope scope(transportCallDepth_); + stopIdleWatch(); closing_ = true; if (outputState_ && !outputState_->completed) { const int cancelResult = @@ -626,7 +635,8 @@ class UsbPrinterTransport::Impl { receiveQueue_.clear(); consecutiveInputTransferErrors_ = 0; consecutiveRetryableInputCompletions_ = 0; - idleRearmSuppressed_ = false; + idleRearmFailures_ = 0; + idleRearmDueAtMs_ = -1; closing_ = false; fatalError_.clear(); #ifdef TRYX_PROTOCOL_TESTING @@ -672,7 +682,8 @@ class UsbPrinterTransport::Impl { receiveQueue_.clear(); consecutiveInputTransferErrors_ = 0; consecutiveRetryableInputCompletions_ = 0; - idleRearmSuppressed_ = false; + idleRearmFailures_ = 0; + idleRearmDueAtMs_ = -1; inputCompletionGeneration_ = 0; zeroLengthInputCompletionGeneration_ = 0; inputTransferErrorGeneration_ = 0; @@ -708,13 +719,31 @@ class UsbPrinterTransport::Impl { return inputState_ && inputState_->active; } - bool serviceEventsForTesting(int timeoutMs, QString *errorMessage) { - return serviceEvents(timeoutMs, errorMessage); + // Stands in for the event loop between requests: the idle watch services + // libusb once a completion is ready, and its timer re-arms the IN when the + // backoff is due. Scripted time advances by the slice per dispatch. + bool idleForTesting(int durationMs, QString *errorMessage) { + constexpr int kSliceMs = 5; + const qint64 end = eventBackend_->monotonicMilliseconds() + durationMs; + while (eventBackend_->monotonicMilliseconds() < end) { + if (!serviceEvents(kSliceMs, errorMessage)) { + return false; + } + rearmIdleInput(); + } + return true; + } + + QList idleRearmDelaysForTesting() const { return idleRearmDelaysForTesting_; } + + int queuedInputBytesForTesting() const { + return static_cast(receiveQueue_.size()); } #endif WriteResult write(const QByteArray &data, int timeoutMs, const PrinterProtocol::OperationContext &context) { + TransportCallScope scope(transportCallDepth_); WriteResult result; if (!grantIsCurrent(&result.error)) { result.cancelled = true; @@ -756,8 +785,6 @@ class UsbPrinterTransport::Impl { const quint64 zeroLengthInputCompletionGenerationBeforeWrite = zeroLengthInputCompletionGeneration_; const quint64 inputErrorGenerationBeforeWrite = inputTransferErrorGeneration_; - // A new request gives a suppressed idle IN another chance afterwards. - idleRearmSuppressed_ = false; if (!ensureInputActive(&result.error)) { return result; } @@ -925,6 +952,7 @@ class UsbPrinterTransport::Impl { ReadResult readSome(QByteArray *bytes, int timeoutMs, const PrinterProtocol::OperationContext &context, QString *errorMessage) { + TransportCallScope scope(transportCallDepth_); if (bytes) { bytes->clear(); } @@ -1015,6 +1043,7 @@ class UsbPrinterTransport::Impl { } bool takeAvailable(QByteArray *bytes, QString *errorMessage, int timeoutMs = 0) { + TransportCallScope scope(transportCallDepth_); if (bytes) { bytes->clear(); } @@ -1032,6 +1061,7 @@ class UsbPrinterTransport::Impl { *bytes = std::move(receiveQueue_); } receiveQueue_.clear(); + rearmIdleInput(); return true; } // A completed bulk-IN transfer is request-scoped and is not rearmed by @@ -1062,26 +1092,172 @@ class UsbPrinterTransport::Impl { void setKeepInputPending(bool keep) { keepInputPending_ = keep; } - // After an exchange, leave the bulk IN pending so the host keeps polling - // the device between requests, as the official bridge does. Never re-arm - // while an idle IN error is being backed off or the session is failing. + // After an exchange, and whenever an idle IN has ended, leave the bulk IN + // pending so the host keeps polling the device between requests, as the + // official bridge does. An idle IN that ended without data is re-armed + // once its backoff delay has passed; the idle watch's timer calls back + // here when it is due. Never re-arm while the session is failing. void rearmIdleInput() { if (!keepInputPending_ || !sessionOpen_ || closing_ || !inputState_ || !inputState_->transfer || !fatalError_.isEmpty() || - persistentInputFailureLatched_ || idleRearmSuppressed_ || + persistentInputFailureLatched_ || consecutiveRetryableInputCompletions_ > 0) { return; } + const qint64 now = eventBackend_->monotonicMilliseconds(); if (!inputState_->active) { + if (idleRearmDueAtMs_ > now) { + scheduleIdleRearm(static_cast(idleRearmDueAtMs_ - now)); + return; + } QString ignored; if (!ensureInputActive(&ignored)) { return; } } - inputState_->idleArm = true; + idleRearmDueAtMs_ = -1; + if (!inputState_->idleArm) { + idleArmedAtMs_ = now; + inputState_->idleArm = true; + } } private: + // Keeps a transport call from being re-entered by the idle watch. + struct TransportCallScope { + explicit TransportCallScope(int &depth) : depth_(depth) { ++depth_; } + ~TransportCallScope() { --depth_; } + int &depth_; + }; + + // Between requests no transport call services libusb, so an idle IN that + // ends would stay unnoticed, and the link idle, until the next request. + // The idle watch hands libusb's poll descriptors to the event loop of the + // thread that opened the session and services completions as soon as they + // are ready. Only sessions that keep the IN pending use it, and only when + // that thread has an event loop. + void startIdleWatch() { + stopIdleWatch(); + if (!keepInputPending_ || testingSession_ || !context_) { + return; + } + if (!QAbstractEventDispatcher::instance(QThread::currentThread())) { + qInfo().noquote() << QStringLiteral("tryx_usb_idle_watch state=unavailable"); + return; + } + idleWatchToken_ = std::make_shared(0); + if (const libusb_pollfd **pollfds = libusb_get_pollfds(context_)) { + for (const libusb_pollfd **entry = pollfds; *entry; ++entry) { + watchPollfd((*entry)->fd, (*entry)->events); + } + libusb_free_pollfds(pollfds); + } + libusb_set_pollfd_notifiers(context_, &Impl::pollfdAdded, &Impl::pollfdRemoved, + this); + idleRearmTimer_ = new QTimer; + idleRearmTimer_->setSingleShot(true); + const std::weak_ptr token = idleWatchToken_; + QObject::connect(idleRearmTimer_, &QTimer::timeout, idleRearmTimer_, + [this, token]() { + if (!token.expired()) { + serviceIdleEvents(); + } + }); + qInfo().noquote() << QStringLiteral("tryx_usb_idle_watch state=started descriptors=%1") + .arg(idleNotifiers_.size()); + } + + void stopIdleWatch() { + if (context_ && idleWatchToken_) { + libusb_set_pollfd_notifiers(context_, nullptr, nullptr, nullptr); + } + idleWatchToken_.reset(); + for (QSocketNotifier *notifier : std::as_const(idleNotifiers_)) { + retireIdleWatchObject(notifier); + } + idleNotifiers_.clear(); + if (idleRearmTimer_) { + retireIdleWatchObject(idleRearmTimer_); + idleRearmTimer_ = nullptr; + } + } + + // A notifier may be retired from inside its own activation, so it is + // disabled at once and deleted by its event loop. + static void retireIdleWatchObject(QObject *object) { + if (object->thread() == QThread::currentThread()) { + if (auto *notifier = qobject_cast(object)) { + notifier->setEnabled(false); + } else if (auto *timer = qobject_cast(object)) { + timer->stop(); + } + } + object->deleteLater(); + } + + void watchPollfd(int fd, short events) { + const std::weak_ptr token = idleWatchToken_; + const auto watch = [this, fd, &token](QSocketNotifier::Type type) { + auto *notifier = new QSocketNotifier(fd, type); + QObject::connect(notifier, &QSocketNotifier::activated, notifier, + [this, token]() { + if (!token.expired()) { + serviceIdleEvents(); + } + }); + idleNotifiers_.append(notifier); + }; + if (events & POLLIN) { + watch(QSocketNotifier::Read); + } + if (events & POLLOUT) { + watch(QSocketNotifier::Write); + } + } + + static void LIBUSB_CALL pollfdAdded(int fd, short events, void *userData) { + static_cast(userData)->watchPollfd(fd, events); + } + + // libusb stops polling a disconnected device's descriptor, which then + // reports an error to every poll; its notifier must go with it. + static void LIBUSB_CALL pollfdRemoved(int fd, void *userData) { + auto *transport = static_cast(userData); + for (qsizetype index = transport->idleNotifiers_.size() - 1; index >= 0; --index) { + QSocketNotifier *notifier = transport->idleNotifiers_.at(index); + if (notifier->socket() == fd) { + retireIdleWatchObject(notifier); + transport->idleNotifiers_.removeAt(index); + } + } + } + + void serviceIdleEvents() { + if (transportCallDepth_ > 0 || !sessionOpen_ || closing_) { + return; + } + TransportCallScope scope(transportCallDepth_); + QString ignored; + if (serviceEvents(0, &ignored)) { + rearmIdleInput(); + } + } + + void scheduleIdleRearm(int delayMs) { + if (idleRearmTimer_ && !idleRearmTimer_->isActive()) { + idleRearmTimer_->start(qMax(0, delayMs)); + } + } + + int idleRearmDelayMs(int failures) const { + int delayMs = kIdleInputRearmInitialDelayMs; + for (int failure = 1; failure < failures && delayMs < kIdleInputRearmMaxDelayMs; + ++failure) { + delayMs = qMin(delayMs * 2, kIdleInputRearmMaxDelayMs); + } + return delayMs; + } + static void LIBUSB_CALL inputTransferCompleted(libusb_transfer *transfer) { auto *state = static_cast(transfer->user_data); state->active = false; @@ -1105,18 +1281,31 @@ class UsbPrinterTransport::Impl { transfer->status == LIBUSB_TRANSFER_COMPLETED)) { // An idle IN that ends without data proves nothing about the // session. Record it, keep it out of the persistent failure - // counters and wait for the next request before re-arming. + // counters and re-arm it after a short backoff, so a firmware + // that ends every IN at once cannot make the host spin. ++transport->idleInputErrorGeneration_; - transport->idleRearmSuppressed_ = true; + const qint64 now = transport->eventBackend_->monotonicMilliseconds(); + if (now - transport->idleArmedAtMs_ > kIdleInputRearmMaxDelayMs) { + transport->idleRearmFailures_ = 0; + } + if (transport->idleRearmFailures_ < kMaxInputTransferErrorRetries) { + ++transport->idleRearmFailures_; + } + const int delayMs = transport->idleRearmDelayMs(transport->idleRearmFailures_); + transport->idleRearmDueAtMs_ = now + delayMs; +#ifdef TRYX_PROTOCOL_TESTING + transport->idleRearmDelaysForTesting_.append(delayMs); +#endif const quint64 count = transport->idleInputErrorGeneration_; if (count <= 3 || count % 100 == 0) { qInfo().noquote() << QStringLiteral( - "tryx_usb_idle_input outcome=%1 count=%2") + "tryx_usb_idle_input outcome=%1 count=%2 rearm_ms=%3") .arg(transfer->status == LIBUSB_TRANSFER_ERROR ? QStringLiteral("error") : QStringLiteral("empty")) - .arg(count); + .arg(count) + .arg(delayMs); } return; } @@ -1126,7 +1315,8 @@ class UsbPrinterTransport::Impl { transfer->actual_length); transport->consecutiveInputTransferErrors_ = 0; transport->consecutiveRetryableInputCompletions_ = 0; - transport->idleRearmSuppressed_ = false; + transport->idleRearmFailures_ = 0; + transport->idleRearmDueAtMs_ = -1; if (transport->receiveQueue_.size() > kMaxLibusbReceiveQueueSize) { transport->fatalError_ = QObject::tr("TRYX libusb receive queue exceeded its bounded size"); @@ -1174,9 +1364,10 @@ class UsbPrinterTransport::Impl { transport->consecutiveRetryableInputCompletions_ = 0; } - // Do not leave a speculative IN URB armed after a completed fragment. - // The firmware reports EPROTO for idle reads. The protocol reader will - // re-arm this transfer before waiting for the next frame fragment. + // Never re-arm from the callback. The protocol reader re-arms this + // transfer before waiting for the next frame fragment; sessions that + // keep the IN pending re-arm it once the current transport call or the + // idle watch has seen the completion. } static void LIBUSB_CALL outputTransferCompleted(libusb_transfer *transfer) { @@ -1244,6 +1435,7 @@ class UsbPrinterTransport::Impl { } if (inputState_->active) { inputState_->idleArm = false; + idleRearmDueAtMs_ = -1; return true; } if (!fatalError_.isEmpty()) { @@ -1272,6 +1464,7 @@ class UsbPrinterTransport::Impl { } inputState_->active = true; inputState_->idleArm = false; + idleRearmDueAtMs_ = -1; return true; } @@ -1334,8 +1527,17 @@ class UsbPrinterTransport::Impl { int consecutiveInputTransferErrors_ = 0; bool persistentInputFailureLatched_ = false; bool keepInputPending_ = false; - bool idleRearmSuppressed_ = false; + int idleRearmFailures_ = 0; + qint64 idleRearmDueAtMs_ = -1; + qint64 idleArmedAtMs_ = 0; quint64 idleInputErrorGeneration_ = 0; + int transportCallDepth_ = 0; + std::shared_ptr idleWatchToken_; + QList idleNotifiers_; + QTimer *idleRearmTimer_ = nullptr; +#ifdef TRYX_PROTOCOL_TESTING + QList idleRearmDelaysForTesting_; +#endif int consecutiveRetryableInputCompletions_ = 0; quint64 inputCompletionGeneration_ = 0; quint64 zeroLengthInputCompletionGeneration_ = 0; @@ -1566,7 +1768,7 @@ void UsbPrinterTransport::setKeepInputPending(bool keep) { #ifdef TRYX_PROTOCOL_TESTING PrinterProtocol::IdleInputTestResult UsbPrinterTransport::runIdleInputScenarioForTesting( - const QList &events, int cycles) { + const QList &events, int cycles, int idleMs) { PrinterProtocol::IdleInputTestResult result; Impl transport; QString transportError; @@ -1595,7 +1797,7 @@ PrinterProtocol::IdleInputTestResult UsbPrinterTransport::runIdleInputScenarioFo } // Idle time between requests: dispatch whatever the device does with // the pending IN, without any request expecting it. - if (!transport.serviceEventsForTesting(10, &transportError)) { + if (!transport.idleForTesting(idleMs, &transportError)) { result.error = transportError; break; } @@ -1606,6 +1808,8 @@ PrinterProtocol::IdleInputTestResult UsbPrinterTransport::runIdleInputScenarioFo result.inputErrors = transport.inputErrorCountForTesting(); result.persistentInputFailure = transport.persistentUsbInputFailure(); result.inputPendingAtEnd = transport.inputPendingForTesting(); + result.idleRearmDelaysMs = transport.idleRearmDelaysForTesting(); + result.queuedInputBytesAtEnd = transport.queuedInputBytesForTesting(); transport.close(); return result; } diff --git a/src/usbprintertransport.h b/src/usbprintertransport.h index 340b2df..cd45587 100644 --- a/src/usbprintertransport.h +++ b/src/usbprintertransport.h @@ -69,7 +69,8 @@ class UsbPrinterTransport final { // Runs request/response cycles with the idle IN kept pending. Each cycle // writes the request, reads the reply and then dispatches one idle event. static PrinterProtocol::IdleInputTestResult runIdleInputScenarioForTesting( - const QList &events, int cycles); + const QList &events, int cycles, + int idleMs); static PrinterProtocol::DuplexTestResult runScenarioForTesting(const QList &events, const QByteArray &request, int writeTimeoutMs, diff --git a/tests/printerprotocol_tests.cpp b/tests/printerprotocol_tests.cpp index 853c409..856facb 100644 --- a/tests/printerprotocol_tests.cpp +++ b/tests/printerprotocol_tests.cpp @@ -2708,6 +2708,8 @@ private slots: void duplexInputReceivesAckDuringOutput(); void idleInputStaysPendingBetweenExchanges(); void idleInputErrorsNeverLatchPersistentFailure(); + void idleInputRearmBacksOffAndRecovers(); + void idleInputDataIsQueuedAndRearmed(); void duplexZeroLengthInputDefersRearmUntilOutputCompletes(); void duplexInputErrorDefersRearmWithoutStarvingOutput(); void duplexInputRetryBudgetIsBounded(); @@ -45288,7 +45290,7 @@ void PrinterProtocolTests::idleInputStaysPendingBetweenExchanges() { // An idle IN that the firmware ends with EPROTO or an empty packet must not // accumulate into the persistent input failure that ends the session, however -// often it happens; it only waits for the next request before re-arming. +// often it happens, and must be re-armed before the next request. void PrinterProtocolTests::idleInputErrorsNeverLatchPersistentFailure() { using Direction = PrinterProtocol::DuplexTestDirection; using Status = PrinterProtocol::DuplexTestStatus; @@ -45315,10 +45317,82 @@ void PrinterProtocolTests::idleInputErrorsNeverLatchPersistentFailure() { QCOMPARE(result.idleInputErrors, kCycles); QCOMPARE(result.inputErrors, 0); QVERIFY(!result.persistentInputFailure); - // Each cycle: one IN for the reply and one idle IN that the device ended; - // after the last idle error the IN waits for the next request. - QCOMPARE(result.inputSubmissions, 2 * kCycles); - QVERIFY(!result.inputPendingAtEnd); + // The first request arms one IN. In every cycle the reply ends it, the + // idle IN armed after the reply is ended by the device, and the backoff + // re-arm leaves an IN pending for the next request to reuse. + QCOMPARE(result.inputSubmissions, 1 + 2 * kCycles); + QVERIFY(result.inputPendingAtEnd); + // Each reply resets the backoff. + QCOMPARE(result.idleRearmDelaysMs, QList(kCycles, 20)); +} + +// A firmware that ends every idle IN at once gets a growing, capped re-arm +// delay instead of a spinning host. An idle IN that stayed pending for a while +// before it failed starts the backoff over. +void PrinterProtocolTests::idleInputRearmBacksOffAndRecovers() { + using Direction = PrinterProtocol::DuplexTestDirection; + using Status = PrinterProtocol::DuplexTestStatus; + QList events; + PrinterProtocol::DuplexTestEvent output; + output.direction = Direction::Output; + events.append(output); + PrinterProtocol::DuplexTestEvent reply; + reply.direction = Direction::Input; + reply.payload = QByteArrayLiteral("reply"); + events.append(reply); + for (int failure = 0; failure < 8; ++failure) { + PrinterProtocol::DuplexTestEvent idle; + idle.direction = Direction::Input; + idle.status = Status::Error; + idle.actualLength = 0; + events.append(idle); + } + PrinterProtocol::DuplexTestEvent lateIdle; + lateIdle.direction = Direction::Input; + lateIdle.status = Status::Error; + lateIdle.actualLength = 0; + // About 750 ms of scripted time, most of it with the IN pending. + lateIdle.deferredDispatches = 150; + events.append(lateIdle); + + const auto result = PrinterProtocol::runIdleInputScenarioForTesting(events, 1, 3000); + QVERIFY2(result.error.isEmpty(), qPrintable(result.error)); + QCOMPARE(result.completedCycles, 1); + QCOMPARE(result.idleInputErrors, 9); + QCOMPARE(result.inputErrors, 0); + QVERIFY(!result.persistentInputFailure); + QCOMPARE(result.idleRearmDelaysMs, + QList({20, 40, 80, 160, 200, 200, 200, 200, 20})); + // The request's IN, the idle IN after the reply and one re-arm per failure. + QCOMPARE(result.inputSubmissions, 2 + 9); + QVERIFY(result.inputPendingAtEnd); +} + +// Data that arrives on the idle IN between requests is kept for the next +// reader, and the IN is re-armed at once instead of waiting for a request. +void PrinterProtocolTests::idleInputDataIsQueuedAndRearmed() { + using Direction = PrinterProtocol::DuplexTestDirection; + QList events; + PrinterProtocol::DuplexTestEvent output; + output.direction = Direction::Output; + events.append(output); + PrinterProtocol::DuplexTestEvent reply; + reply.direction = Direction::Input; + reply.payload = QByteArrayLiteral("reply"); + events.append(reply); + PrinterProtocol::DuplexTestEvent unsolicited; + unsolicited.direction = Direction::Input; + unsolicited.payload = QByteArrayLiteral("unsolicited"); + events.append(unsolicited); + + const auto result = PrinterProtocol::runIdleInputScenarioForTesting(events, 1); + QVERIFY2(result.error.isEmpty(), qPrintable(result.error)); + QCOMPARE(result.completedCycles, 1); + QCOMPARE(result.idleInputErrors, 0); + QVERIFY(result.idleRearmDelaysMs.isEmpty()); + QCOMPARE(result.queuedInputBytesAtEnd, 11); + QCOMPARE(result.inputSubmissions, 3); + QVERIFY(result.inputPendingAtEnd); } void PrinterProtocolTests::duplexInputReceivesAckDuringOutput() { From fb4d34b5c330c61e2bf2b28d66515a8d9038778a Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 4 Oct 2026 02:41:35 +0700 Subject: [PATCH 3/4] fix: create runtime directories owner-only instead of from the umask QDir::mkpath() creates directories with 0777 minus the process umask. With the 0002 umask common on Ubuntu and Mint desktops, the shared data root, its media-catalog and the prepared-media retry-cache root became group-writable, and the stores' own safety checks then rejected them on the next start. A rejected retry-cache root keeps the PASE display session from starting at all, while the status claimed that stored retry media was still being validated. Directories the runtime creates for itself now get 0700 for every missing component. Existing directories are left as they are and the safety checks are unchanged. When a check rejects a directory, the warning names it with its owner or mode and the chmod command that fixes the common case, and the session status says that the display session cannot start instead of describing a wait. Reported in #32. --- src/deleteintentstore.cpp | 3 +- src/devicemanager.cpp | 12 +- src/firmwarerecoveryjournal.cpp | 4 +- src/mediacatalogstore.cpp | 3 +- src/pasemetricsconfigstore.cpp | 3 +- src/printermediapreparer.cpp | 3 +- src/printeroperationcoordinator.cpp | 18 ++- src/printeroperationcoordinator.h | 1 + src/printersessioncontroller.cpp | 32 ++++- src/printersessioncontroller.h | 2 + src/privatedirectorypath.h | 85 ++++++++++++ src/runtimedowngradestore.cpp | 4 +- src/runtimepresentationpreferencesstore.cpp | 3 +- src/savedlayoutstore.cpp | 3 +- tests/printerprotocol_tests.cpp | 141 ++++++++++++++++++++ tests/printerprotocol_tests.pro | 1 + tests/runtime_downgradestore_tests.pro | 1 + tests/savedlayoutstore_tests.pro | 1 + translations/tryx-panorama_ru.ts | 8 ++ tryx-cli.pro | 1 + tryx-panorama.pro | 1 + 21 files changed, 313 insertions(+), 17 deletions(-) create mode 100644 src/privatedirectorypath.h diff --git a/src/deleteintentstore.cpp b/src/deleteintentstore.cpp index 4e36c43..b63c08c 100644 --- a/src/deleteintentstore.cpp +++ b/src/deleteintentstore.cpp @@ -2,6 +2,7 @@ #include "devicemanagermessages.h" #include "printermediaidentity.h" +#include "privatedirectorypath.h" #include #include @@ -424,7 +425,7 @@ bool ensureDirectory(const QString &path, QString *errorMessage) { systemErrorText(tryx::DeviceManagerMessages::tr("Cannot inspect the delete intent directory"), errorNumber)); } - if (!QDir().mkpath(directory)) { + if (!tryx::makePrivateDirectoryPath(directory)) { return setError( errorMessage, tryx::DeviceManagerMessages::tr("Cannot create the delete intent directory")); diff --git a/src/devicemanager.cpp b/src/devicemanager.cpp index 45161f2..582c840 100644 --- a/src/devicemanager.cpp +++ b/src/devicemanager.cpp @@ -8,6 +8,7 @@ #include "mediatransform.h" #include "paseoverlayconfig.h" #include "pasemetricsconfigstore.h" +#include "privatedirectorypath.h" #include "privateruntimepaths.h" #include "printermediafileintegrity.h" #include "printermediaidentity.h" @@ -2241,7 +2242,13 @@ void DeviceManager::loadRuntimePresentationPreferences() { presentationPreferences_ = result.preferences; presentationPreferences_.revision = 1; if (!result.detail.isEmpty()) { - qWarning().noquote() << result.detail; + QString detail = result.detail; + if (const QString problem = tryx::privateDirectoryProblem( + runtimePresentationPreferencesStore_->directory()); + !problem.isEmpty()) { + detail += QStringLiteral(" (%1)").arg(problem); + } + qWarning().noquote() << detail; } } const TryxRuntimePresentationPreferencesV1 loaded = @@ -2946,6 +2953,9 @@ PrinterSessionController::Callbacks DeviceManager::sessionCallbacks() { callbacks.retryCacheValidationPending = [this]() { return operationCoordinator_.retryCacheValidationPending(); }; + callbacks.retryCacheStartupFailureDetail = [this]() { + return operationCoordinator_.retryCacheStartupFailureDetail(); + }; callbacks.hasUnresolvedRetryOutcomeForFirmware = [this]() { return operationCoordinator_.hasUnresolvedRetryOutcomeForFirmware(); }; diff --git a/src/firmwarerecoveryjournal.cpp b/src/firmwarerecoveryjournal.cpp index 08ba3c9..a7ea733 100644 --- a/src/firmwarerecoveryjournal.cpp +++ b/src/firmwarerecoveryjournal.cpp @@ -1,5 +1,7 @@ #include "firmwarerecoveryjournal.h" +#include "privatedirectorypath.h" + #include #include #include @@ -173,7 +175,7 @@ bool openVerifiedDirectory( const QString parentPath = QFileInfo(directoryPath) .absolutePath(); - if (!QDir().mkpath(parentPath)) { + if (!tryx::makePrivateDirectoryPath(parentPath)) { return setError( errorMessage, QStringLiteral( diff --git a/src/mediacatalogstore.cpp b/src/mediacatalogstore.cpp index 09deb6e..736940e 100644 --- a/src/mediacatalogstore.cpp +++ b/src/mediacatalogstore.cpp @@ -4,6 +4,7 @@ #include "devicemanagermessages.h" #include "printermediafileintegrity.h" #include "printermediaidentity.h" +#include "privatedirectorypath.h" #include #include @@ -202,7 +203,7 @@ bool MediaCatalogStore::ensureDirectories(QString *errorMessage) const { } return false; } - if (!info.exists() && !QDir().mkpath(path)) { + if (!info.exists() && !tryx::makePrivateDirectoryPath(path)) { if (errorMessage) { *errorMessage = tryx::DeviceManagerMessages::tr( "Cannot create the media catalog directory"); diff --git a/src/pasemetricsconfigstore.cpp b/src/pasemetricsconfigstore.cpp index 733eb71..b66c49a 100644 --- a/src/pasemetricsconfigstore.cpp +++ b/src/pasemetricsconfigstore.cpp @@ -4,6 +4,7 @@ #include "configurationformatbackup.h" #include "devicemanagermessages.h" #include "paseoverlayconfig.h" +#include "privatedirectorypath.h" #include #include @@ -252,7 +253,7 @@ bool PaseMetricsConfigStore::ensureDirectory( } return false; } - if (!info.exists() && !QDir().mkpath(directory_)) { + if (!info.exists() && !tryx::makePrivateDirectoryPath(directory_)) { if (errorMessage) { *errorMessage = tryx::DeviceManagerMessages::tr( "Cannot create the PASE metrics configuration directory"); diff --git a/src/printermediapreparer.cpp b/src/printermediapreparer.cpp index 9313d5d..4fe88bd 100644 --- a/src/printermediapreparer.cpp +++ b/src/printermediapreparer.cpp @@ -4,6 +4,7 @@ #include "printermediafileintegrity.h" #include "printermediaidentity.h" #include "printerprotocol.h" +#include "privatedirectorypath.h" #include "turrismediaformat.h" #include @@ -106,7 +107,7 @@ QString printerTempPath(const QString &fileName) { const QString directory = QDir(QStandardPaths::writableLocation(QStandardPaths::CacheLocation)) .filePath(QStringLiteral("prepared-media")); - QDir().mkpath(directory); + tryx::makePrivateDirectoryPath(directory); return QDir(directory).filePath(fileName); } diff --git a/src/printeroperationcoordinator.cpp b/src/printeroperationcoordinator.cpp index a6e75c8..9d03ac2 100644 --- a/src/printeroperationcoordinator.cpp +++ b/src/printeroperationcoordinator.cpp @@ -5,6 +5,7 @@ #include "printermediaidentity.h" #include "mediatransform.h" #include "paseoverlayconfig.h" +#include "privatedirectorypath.h" #include "privateruntimepaths.h" #include "runtimeapplyrequestcodec.h" @@ -314,6 +315,10 @@ bool PrinterOperationCoordinator::retryCacheValidationPending() const { !retryCacheLoadComplete_; } +QString PrinterOperationCoordinator::retryCacheStartupFailureDetail() const { + return retryCacheStartupFailure_ ? retryCacheFailureDetail_ : QString(); +} + PrinterOperationCoordinator::RuntimeDowngradeAssessment PrinterOperationCoordinator::runtimeDowngradeAssessment() const { RuntimeDowngradeAssessment assessment; @@ -1528,6 +1533,11 @@ void PrinterOperationCoordinator::loadRetryCache( retryCacheFailureDetail_ = loaded.detail.isEmpty() ? tryx::DeviceManagerMessages::tr("Retry-cache state is invalid or unsafe") : loaded.detail; + if (const QString problem = + tryx::privateDirectoryProblem(retryCacheDirectory()); + !problem.isEmpty()) { + retryCacheFailureDetail_ += QStringLiteral(" (%1)").arg(problem); + } qWarning().noquote() << tryx::DeviceManagerMessages::tr("Retry-cache startup remains fail-closed: %1") .arg(retryCacheFailureDetail_); @@ -5237,9 +5247,15 @@ void PrinterOperationCoordinator::loadDeleteIntent() { pendingDeleteIntent_.reset(); pendingDeleteOperationId_ = QStringLiteral("invalid-delete-intent"); + QString detail = loaded.detail; + if (const QString problem = tryx::privateDirectoryProblem( + QFileInfo(deleteIntentPath()).absolutePath()); + !problem.isEmpty()) { + detail += QStringLiteral(" (%1)").arg(problem); + } qWarning().noquote() << "Delete intent was not accepted; deletes remain blocked:" - << loaded.detail; + << detail; return; } const tryx::DeleteIntentRecord &intent = loaded.record; diff --git a/src/printeroperationcoordinator.h b/src/printeroperationcoordinator.h index 510fe4b..e683b3c 100644 --- a/src/printeroperationcoordinator.h +++ b/src/printeroperationcoordinator.h @@ -157,6 +157,7 @@ class PrinterOperationCoordinator final : public QObject { bool hasPendingDeleteRecovery() const; bool hasPendingReplaceRecovery() const; bool retryCacheValidationPending() const; + QString retryCacheStartupFailureDetail() const; RuntimeDowngradeAssessment runtimeDowngradeAssessment() const; SupportState supportState() const; void initializeDeviceMediaOutbox(); diff --git a/src/printersessioncontroller.cpp b/src/printersessioncontroller.cpp index 580d681..1f3ac98 100644 --- a/src/printersessioncontroller.cpp +++ b/src/printersessioncontroller.cpp @@ -429,9 +429,7 @@ void PrinterSessionController::handlePrinterSnapshot( } else if (!retryCacheValidationComplete && !state_.printerDisplaySessionLost) { state_.printerDisplaySessionLost = false; - emit uploadStatus(tryx::DeviceManagerMessages::tr( - "Stored retry media is still being validated; the PASE display " - "session will start only after validation finishes")); + emit uploadStatus(retryCacheSessionGateStatusText()); } else { state_.printerDisplaySessionLost = !restrictedRecoverySession; emit uploadStatus(printerMutationUnavailableStatusText()); @@ -649,10 +647,7 @@ void PrinterSessionController::connectDevice(const QString &port) { } else if (!retryCacheValidationComplete && !state_.printerDisplaySessionLost) { state_.printerDisplaySessionLost = false; - emit uploadStatus(tryx::DeviceManagerMessages::tr( - "Stored retry media is still being validated; the PASE " - "display session will start only after validation " - "finishes")); + emit uploadStatus(retryCacheSessionGateStatusText()); } else { state_.printerDisplaySessionLost = !restrictedRecoverySession; emit uploadStatus(printerMutationUnavailableStatusText()); @@ -989,6 +984,29 @@ QString PrinterSessionController::printerUnavailableStatusText() const { return state_.printerSnapshot.statusText(); } +// The startup gate holds the session back both while stored retry media is +// validated and when the retry cache cannot be used at all. Only the first is +// a wait; the second lasts until the cause is fixed and the runtime restarts. +QString PrinterSessionController::retryCacheSessionGateStatusText() const { + if (callbacks_.retryCacheStoreBlocksMutations()) { + const QString detail = callbacks_.retryCacheStartupFailureDetail + ? callbacks_.retryCacheStartupFailureDetail() + : QString(); + return detail.isEmpty() + ? tryx::DeviceManagerMessages::tr( + "The PASE display session cannot start because the stored " + "retry state is invalid or unsafe. Keep the cache and check " + "the runtime log.") + : tryx::DeviceManagerMessages::tr( + "The PASE display session cannot start because the stored " + "retry state cannot be used: %1") + .arg(detail); + } + return tryx::DeviceManagerMessages::tr( + "Stored retry media is still being validated; the PASE display " + "session will start only after validation finishes"); +} + QString PrinterSessionController::printerMutationUnavailableStatusText() const { if (callbacks_.runtimeDowngradePrepared()) { return tryx::DeviceManagerMessages::tr( diff --git a/src/printersessioncontroller.h b/src/printersessioncontroller.h index 3e32f8e..49b9346 100644 --- a/src/printersessioncontroller.h +++ b/src/printersessioncontroller.h @@ -74,6 +74,7 @@ class PrinterSessionController final : public QObject { std::function retryCacheRestrictedRecoveryActive; std::function retryCacheMutationGateActive; std::function retryCacheValidationPending; + std::function retryCacheStartupFailureDetail; std::function hasUnresolvedRetryOutcomeForFirmware; std::function hasPendingDeleteRecovery; std::function hasPendingReplaceRecovery; @@ -136,6 +137,7 @@ class PrinterSessionController final : public QObject { bool firmwareFlashAllowedForCurrentDevice(QString *errorMessage) const; QString printerUnavailableStatusText() const; QString printerMutationUnavailableStatusText() const; + QString retryCacheSessionGateStatusText() const; QString firmwareExclusiveStatusText() const; void resumePrinterSessionAfterRetryCacheValidation(); void requirePrinterRecovery(const QString &message); diff --git a/src/privatedirectorypath.h b/src/privatedirectorypath.h new file mode 100644 index 0000000..19cbaf8 --- /dev/null +++ b/src/privatedirectorypath.h @@ -0,0 +1,85 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace tryx { + +// QDir::mkpath() creates directories with 0777 minus the process umask. With +// the 0002 umask many desktop users have, runtime data and cache directories +// become group-writable, and the stores' own safety checks then reject them +// on the next start. This creates every missing component owner-only (0700) +// and leaves existing components as they are. Like mkpath(), it returns true +// when the path is a directory afterwards. +inline bool makePrivateDirectoryPath(const QString &path) { + if (path.isEmpty()) { + return false; + } + const QString cleanPath = QDir::cleanPath(QFileInfo(path).absoluteFilePath()); + QString current; + const QStringList components = cleanPath.split(QLatin1Char('/'), Qt::SkipEmptyParts); + for (const QString &component : components) { + current += QLatin1Char('/') + component; + const QByteArray encoded = QFile::encodeName(current); + struct stat status {}; + if (::stat(encoded.constData(), &status) == 0) { + if (!S_ISDIR(status.st_mode)) { + return false; + } + continue; + } + if (errno != ENOENT) { + return false; + } + if (::mkdir(encoded.constData(), S_IRWXU) != 0 && errno != EEXIST) { + return false; + } + if (::stat(encoded.constData(), &status) != 0 || !S_ISDIR(status.st_mode)) { + return false; + } + } + return true; +} + +// Explains why an existing directory fails the runtime's private-directory +// checks, with the command that fixes the common case. Empty when the path is +// a directory of this user that others cannot write, or cannot be inspected. +inline QString privateDirectoryProblem(const QString &path) { + struct stat status {}; + if (path.isEmpty() || ::lstat(QFile::encodeName(path).constData(), &status) != 0) { + return {}; + } + if (S_ISLNK(status.st_mode)) { + return QStringLiteral("%1 is a symbolic link; it must be a real directory") + .arg(path); + } + if (!S_ISDIR(status.st_mode)) { + return QStringLiteral("%1 is not a directory").arg(path); + } + if (status.st_uid != ::geteuid()) { + return QStringLiteral("%1 is owned by uid %2, but the TRYX runtime runs as uid %3") + .arg(path) + .arg(status.st_uid) + .arg(::geteuid()); + } + if ((status.st_mode & (S_IWGRP | S_IWOTH)) != 0) { + QString quoted = path; + quoted.replace(QLatin1Char('\''), QStringLiteral("'\\''")); + return QStringLiteral( + "%1 has mode %2, so other users can write to it; " + "make it private with: chmod go-w '%3'") + .arg(path) + .arg(status.st_mode & 07777, 4, 8, QLatin1Char('0')) + .arg(quoted); + } + return {}; +} + +} // namespace tryx diff --git a/src/runtimedowngradestore.cpp b/src/runtimedowngradestore.cpp index f62fbd4..a4d13cf 100644 --- a/src/runtimedowngradestore.cpp +++ b/src/runtimedowngradestore.cpp @@ -1,5 +1,7 @@ #include "runtimedowngradestore.h" +#include "privatedirectorypath.h" + #include #include #include @@ -462,7 +464,7 @@ DirectoryOpenResult openStoreDirectory(const QString &directory, return {DirectoryOpenStatus::Missing, {}, {}}; } if (errorNumber == ENOENT && create) { - if (!QDir().mkpath(parentPath) || + if (!tryx::makePrivateDirectoryPath(parentPath) || ::lstat(encodedParent.constData(), &parentStatus) != 0) { return {DirectoryOpenStatus::IoError, {}, diff --git a/src/runtimepresentationpreferencesstore.cpp b/src/runtimepresentationpreferencesstore.cpp index f0fbb9e..ebbe119 100644 --- a/src/runtimepresentationpreferencesstore.cpp +++ b/src/runtimepresentationpreferencesstore.cpp @@ -1,6 +1,7 @@ #include "runtimepresentationpreferencesstore.h" #include "applicationpaths.h" +#include "privatedirectorypath.h" #include #include @@ -110,7 +111,7 @@ bool RuntimePresentationPreferencesStore::ensureDirectory( } return false; } - if (!info.exists() && !QDir().mkpath(directory_)) { + if (!info.exists() && !tryx::makePrivateDirectoryPath(directory_)) { if (errorMessage) { *errorMessage = QStringLiteral( "Cannot create the runtime presentation preferences directory"); diff --git a/src/savedlayoutstore.cpp b/src/savedlayoutstore.cpp index 60decc7..9558697 100644 --- a/src/savedlayoutstore.cpp +++ b/src/savedlayoutstore.cpp @@ -2,6 +2,7 @@ #include "applicationpaths.h" #include "configurationformatbackup.h" +#include "privatedirectorypath.h" #include "runtimeapplyrequestcodec.h" #include @@ -1005,7 +1006,7 @@ bool SavedLayoutStore::ensureDirectory(QString *detail) const { return false; } const bool create = !info.exists(); - if (create && !QDir().mkpath(directory_)) { + if (create && !tryx::makePrivateDirectoryPath(directory_)) { if (detail) { *detail = QStringLiteral( "Cannot create the saved layouts directory"); diff --git a/tests/printerprotocol_tests.cpp b/tests/printerprotocol_tests.cpp index 856facb..9244b45 100644 --- a/tests/printerprotocol_tests.cpp +++ b/tests/printerprotocol_tests.cpp @@ -26,6 +26,7 @@ #include "privateruntimepaths.h" #include "runtimeapplyrequestcodec.h" #include "paseoverlayconfig.h" +#include "privatedirectorypath.h" #include "runtimebridge.h" #include "runtimedowngradestore.h" #include "runtimepresentationpreferencesstore.h" @@ -2710,6 +2711,9 @@ private slots: void idleInputErrorsNeverLatchPersistentFailure(); void idleInputRearmBacksOffAndRecovers(); void idleInputDataIsQueuedAndRearmed(); + void privateDirectoryPathIgnoresGroupWritableUmask(); + void runtimeStoresStayUsableUnderGroupWritableUmask(); + void retryCacheStartupFailureNamesTheUnsafeDirectory(); void duplexZeroLengthInputDefersRearmUntilOutputCompletes(); void duplexInputErrorDefersRearmWithoutStarvingOutput(); void duplexInputRetryBudgetIsBounded(); @@ -45395,6 +45399,143 @@ void PrinterProtocolTests::idleInputDataIsQueuedAndRearmed() { QVERIFY(result.inputPendingAtEnd); } +namespace { +mode_t directoryModeForTesting(const QString &path) { + struct stat status {}; + if (::lstat(QFile::encodeName(path).constData(), &status) != 0) { + return 0; + } + return status.st_mode & 07777; +} +} // namespace + +// #32: a 0002 umask made QDir::mkpath() create group-writable runtime +// directories that the stores then rejected. Every component the helper +// creates is owner-only, and existing components keep their mode. +void PrinterProtocolTests::privateDirectoryPathIgnoresGroupWritableUmask() { + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const mode_t previousMask = ::umask(0002); + const auto restoreMask = qScopeGuard([previousMask]() { ::umask(previousMask); }); + + const QString existing = directory.filePath(QStringLiteral("existing")); + QVERIFY(QDir().mkdir(existing)); + QVERIFY(::chmod(QFile::encodeName(existing).constData(), 0755) == 0); + const QString target = existing + QStringLiteral("/a/b/c"); + QVERIFY(tryx::makePrivateDirectoryPath(target)); + QCOMPARE(directoryModeForTesting(existing), mode_t(0755)); + QCOMPARE(directoryModeForTesting(existing + QStringLiteral("/a")), mode_t(0700)); + QCOMPARE(directoryModeForTesting(existing + QStringLiteral("/a/b")), mode_t(0700)); + QCOMPARE(directoryModeForTesting(target), mode_t(0700)); + QVERIFY(tryx::makePrivateDirectoryPath(target)); + QVERIFY(!tryx::makePrivateDirectoryPath(QString())); + + const QString file = directory.filePath(QStringLiteral("file")); + QFile regular(file); + QVERIFY(regular.open(QIODevice::WriteOnly)); + regular.close(); + QVERIFY(!tryx::makePrivateDirectoryPath(file + QStringLiteral("/below"))); + + QVERIFY(tryx::privateDirectoryProblem(target).isEmpty()); + QVERIFY(tryx::privateDirectoryProblem(existing).isEmpty()); + QVERIFY(tryx::privateDirectoryProblem( + directory.filePath(QStringLiteral("missing"))).isEmpty()); + QVERIFY(tryx::privateDirectoryProblem(file).contains( + QStringLiteral("is not a directory"))); + const QString link = directory.filePath(QStringLiteral("link")); + QVERIFY(QFile::link(target, link)); + QVERIFY(tryx::privateDirectoryProblem(link).contains( + QStringLiteral("symbolic link"))); + + const QString shared = directory.filePath(QStringLiteral("it's shared")); + QVERIFY(QDir().mkdir(shared)); + QVERIFY(::chmod(QFile::encodeName(shared).constData(), 0775) == 0); + const QString problem = tryx::privateDirectoryProblem(shared); + QVERIFY2(problem.contains(QStringLiteral("mode 0775")), qPrintable(problem)); + QString quoted = shared; + quoted.replace(QLatin1Char('\''), QStringLiteral("'\\''")); + QVERIFY2(problem.contains(QStringLiteral("chmod go-w '%1'").arg(quoted)), + qPrintable(problem)); +} + +// The #32 sequence under a 0002 umask: the media catalog creates the shared +// data root, preferences and delete intent are read from it on the next start, +// and media preparation creates the retry-cache root. None of them may end up +// rejected as unsafe. +void PrinterProtocolTests::runtimeStoresStayUsableUnderGroupWritableUmask() { + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const mode_t previousMask = ::umask(0002); + const auto restoreMask = qScopeGuard([previousMask]() { ::umask(previousMask); }); + + const QString dataRoot = + directory.filePath(QStringLiteral("data/DXVSI/TRYX Panorama Manager")); + const QString catalogRoot = dataRoot + QStringLiteral("/media-catalog"); + const QString cacheRoot = directory.filePath( + QStringLiteral("cache/DXVSI/TRYX Panorama Runtime/prepared-media")); + + tryx::MediaCatalogStore catalog(catalogRoot); + const auto catalogResult = catalog.load(); + QCOMPARE(catalogResult.status, tryx::MediaCatalogStore::LoadStatus::Empty); + QCOMPARE(directoryModeForTesting(dataRoot), mode_t(0700)); + QCOMPARE(directoryModeForTesting(catalogRoot), mode_t(0700)); + + const auto preferences = tryx::RuntimePresentationPreferencesStore(dataRoot).load(); + QCOMPARE(preferences.status, + tryx::RuntimePresentationPreferencesStore::LoadStatus::Empty); + const auto deleteIntent = + tryx::DeleteIntentStore(catalogRoot + QStringLiteral("/delete-intent.json")).load(); + QCOMPARE(deleteIntent.status, tryx::DeleteIntentStore::LoadStatus::Missing); + + // printerTempPath() creates the retry-cache root with this helper. + QVERIFY(tryx::makePrivateDirectoryPath(cacheRoot)); + QCOMPARE(directoryModeForTesting(cacheRoot), mode_t(0700)); + tryx::RetryCacheStore retryCache(cacheRoot); + const auto retry = retryCache.load(); + QCOMPARE(retry.status, tryx::RetryCacheStore::LoadStatus::Missing); +} + +// A retry-cache root that fails the safety check blocks the display session +// until it is fixed. The status must say so and name the directory with the +// command that fixes it, not claim that validation is still running. +void PrinterProtocolTests::retryCacheStartupFailureNamesTheUnsafeDirectory() { + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString retryRoot = directory.filePath(QStringLiteral("retry-cache")); + QVERIFY(QDir().mkdir(retryRoot)); + QVERIFY(::chmod(QFile::encodeName(retryRoot).constData(), 0775) == 0); + + std::unique_ptr manager(DeviceManager::createForTesting( + directory.filePath(QStringLiteral("sys")), + directory.filePath(QStringLiteral("dev")))); + QVERIFY(manager->retryCacheStartupSessionGateActive()); + const QString detail = + manager->operationCoordinator_.retryCacheStartupFailureDetail(); + QVERIFY2(detail.contains(QStringLiteral( + "Retry-cache root directory has unsafe ownership or permissions")), + qPrintable(detail)); + QVERIFY2(detail.contains(QStringLiteral("chmod go-w '%1'").arg(retryRoot)), + qPrintable(detail)); + + const QString blocked = + manager->sessionController_.retryCacheSessionGateStatusText(); + QVERIFY2(blocked.contains(QStringLiteral("cannot start")), qPrintable(blocked)); + QVERIFY2(blocked.contains(detail), qPrintable(blocked)); + QVERIFY(!blocked.contains(QStringLiteral("still being validated"))); + + // The documented fix: once the directory is private, the next load opens + // the gate. + QVERIFY(::chmod(QFile::encodeName(retryRoot).constData(), 0700) == 0); + manager->loadRetryCache(); + QVERIFY(!manager->retryCacheStartupSessionGateActive()); + QVERIFY(manager->operationCoordinator_.retryCacheStartupFailureDetail().isEmpty()); + + // A load that has not finished yet is still reported as a wait. + manager->operationCoordinator_.retryCacheLoadComplete_ = false; + QVERIFY(manager->sessionController_.retryCacheSessionGateStatusText().contains( + QStringLiteral("still being validated"))); +} + void PrinterProtocolTests::duplexInputReceivesAckDuringOutput() { QList events; PrinterProtocol::DuplexTestEvent input; diff --git a/tests/printerprotocol_tests.pro b/tests/printerprotocol_tests.pro index 89d517e..d446ca2 100644 --- a/tests/printerprotocol_tests.pro +++ b/tests/printerprotocol_tests.pro @@ -100,6 +100,7 @@ HEADERS += \ $$PWD/../src/mediatransform.h \ $$PWD/../src/paseoverlayconfig.h \ $$PWD/../src/pasemetricsconfigstore.h \ + $$PWD/../src/privatedirectorypath.h \ $$PWD/../src/privateruntimepaths.h \ $$PWD/../src/printermediafileintegrity.h \ $$PWD/../src/printermediaidentity.h \ diff --git a/tests/runtime_downgradestore_tests.pro b/tests/runtime_downgradestore_tests.pro index 54c8677..57508dd 100644 --- a/tests/runtime_downgradestore_tests.pro +++ b/tests/runtime_downgradestore_tests.pro @@ -18,6 +18,7 @@ OBJECTS_DIR = $$PWD/../build/runtime-downgradestore-tests/obj MOC_DIR = $$PWD/../build/runtime-downgradestore-tests/moc HEADERS += \ + $$PWD/../src/privatedirectorypath.h \ $$PWD/../src/runtimedowngradestore.h SOURCES += \ diff --git a/tests/savedlayoutstore_tests.pro b/tests/savedlayoutstore_tests.pro index e86d198..9b6db15 100644 --- a/tests/savedlayoutstore_tests.pro +++ b/tests/savedlayoutstore_tests.pro @@ -14,6 +14,7 @@ MOC_DIR = $$PWD/../build/savedlayoutstore-tests/moc HEADERS += \ $$PWD/../src/applicationpaths.h \ + $$PWD/../src/privatedirectorypath.h \ $$PWD/../src/runtimeapplyrequestcodec.h \ $$PWD/../src/runtimecontract.h \ $$PWD/../src/savedlayoutstore.h diff --git a/translations/tryx-panorama_ru.ts b/translations/tryx-panorama_ru.ts index becde8f..b4aafc9 100644 --- a/translations/tryx-panorama_ru.ts +++ b/translations/tryx-panorama_ru.ts @@ -935,6 +935,14 @@ Stored retry media is still being validated; the PASE display session will start only after validation finishes Сохранённый файл для повтора ещё проверяется; сессия дисплея PASE запустится только после завершения проверки + + The PASE display session cannot start because the stored retry state is invalid or unsafe. Keep the cache and check the runtime log. + Сессия дисплея PASE не может запуститься, потому что сохранённое состояние кэша повтора недействительно или небезопасно. Не удаляйте кэш и проверьте журнал службы. + + + The PASE display session cannot start because the stored retry state cannot be used: %1 + Сессия дисплея PASE не может запуститься, потому что сохранённое состояние кэша повтора нельзя использовать: %1 + A TRYX printer-class or Rockchip gadget device is present; use Auto connection. diff --git a/tryx-cli.pro b/tryx-cli.pro index 6764530..301cadc 100644 --- a/tryx-cli.pro +++ b/tryx-cli.pro @@ -22,6 +22,7 @@ MOC_DIR = $$PWD/build/cli/moc RCC_DIR = $$PWD/build/cli/rcc HEADERS += \ + src/privatedirectorypath.h \ src/runtimecontract.h \ src/runtimedowngradestore.h \ src/supportsnapshot.h \ diff --git a/tryx-panorama.pro b/tryx-panorama.pro index 2c85fd5..7a41a04 100644 --- a/tryx-panorama.pro +++ b/tryx-panorama.pro @@ -126,6 +126,7 @@ HEADERS += \ src/mediatransform.h \ src/paseoverlayconfig.h \ src/pasemetricsconfigstore.h \ + src/privatedirectorypath.h \ src/privateruntimepaths.h \ src/printermediafileintegrity.h \ src/printermediaidentity.h \ From 02a45a707161ab2785308389c99f475d43a663ba Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 7 Oct 2026 13:51:20 +0700 Subject: [PATCH 4/4] release: prepare 2.5.3 The release keeps Panorama-family displays on USB by holding a bulk IN pending between requests, and creates the runtime's directories owner-only so a 0002 umask no longer blocks the display session. Add the 2.5.3 entries to every changelog and the AppStream metadata, point the README install commands at the new package names, and describe the fix for directories created by earlier versions. --- README.md | 44 ++++++++++++++++--- VERSION | 2 +- debian/changelog | 8 ++++ packaging/arch/PKGBUILD | 2 +- ...b.dxvsi.tryx_panorama_manager.metainfo.xml | 9 ++++ packaging/rpm/tryx-panorama-manager.spec | 7 ++- packaging/tryx-panorama-manager.1 | 2 +- packaging/tryx.1 | 2 +- 8 files changed, 65 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 1c4b2b8..98c86e0 100644 --- a/README.md +++ b/README.md @@ -248,6 +248,21 @@ XDG Autostart entry for the GUI. Login start hides the initial window only when Hide to tray is selected and a tray host is actually available; otherwise the window is shown. This switch never changes the background runtime service. +## What's new in 2.5.3 + +- Panorama `391a:1011`, Panorama SE `391a:1021` and Panorama WB `391a:1031` + no longer drop off USB on systems where they used to disappear until the + power supply was switched off. The runtime keeps a bulk IN transfer pending + between requests, like the official bridge, and re-arms it as soon as it + ends. A reporter who had the drops ran the display for hours without a + disconnect (#28). +- The runtime creates its data and cache directories owner-only. With the + 0002 umask common on Ubuntu and Linux Mint they used to become + group-writable; after the first media upload the runtime refused them and + the display session no longer started. Directories created by earlier + versions are not changed automatically: the status and the log name the + directory and the `chmod go-w` command that fixes it (#32). + ## What's new in 2.5.2 - Panorama WB (`391a:1031`) is supported with the same feature set as the @@ -548,7 +563,7 @@ runtime remote, and a working USB portal backend are required. Stop any other TRYX runtime before starting this build. ```fish -flatpak install --user ./tryx-panorama-manager-2.5.2-experimental-x86_64.flatpak +flatpak install --user ./tryx-panorama-manager-2.5.3-experimental-x86_64.flatpak flatpak run io.github.dxvsi.tryx_panorama_manager//experimental ``` @@ -580,13 +595,13 @@ Install a downloaded package with the package manager for your distribution: # Fedora. Enable RPM Fusion Free first because media conversion requires the # full ffmpeg package with the libx264 encoder. set tryx_fedora_release (rpm -E %fedora) -sudo dnf install --allowerasing ./tryx-panorama-manager-2.5.2-1.fc$tryx_fedora_release.x86_64.rpm +sudo dnf install --allowerasing ./tryx-panorama-manager-2.5.3-1.fc$tryx_fedora_release.x86_64.rpm # Ubuntu 24.04 or Linux Mint 22 -sudo apt install ./tryx-panorama-manager_2.5.2-1_amd64.deb +sudo apt install ./tryx-panorama-manager_2.5.3-1_amd64.deb # Arch Linux -sudo pacman -U ./tryx-panorama-manager-2.5.2-1-x86_64.pkg.tar.zst +sudo pacman -U ./tryx-panorama-manager-2.5.3-1-x86_64.pkg.tar.zst ``` These commands use the distribution package manager to resolve and download @@ -703,8 +718,8 @@ Some Panorama `391a:1011`, Panorama SE `391a:1021` and Panorama WB only after the power supply is switched off. Reports come from AMD 800-series chipset USB controllers on Linux and Windows. The firmware fails when the host stops polling the device between exchanges; the official bridge keeps a bulk -IN pending at all times and survives for days. Since this build the runtime -does the same for the Panorama family. +IN pending at all times and survives for days. Since 2.5.3 the runtime does +the same for the Panorama family. If a display still drops, a kernel parameter that disables USB link power management for these devices is a known workaround: @@ -717,6 +732,23 @@ Add it to the kernel command line of your boot loader, reboot, and switch the power supply off once so the display recovers. Details and measurements are in issue #28. +### The display session never starts after the first media upload + +Before 2.5.3 the runtime created its directories with permissions from the +umask. With a 0002 umask they became group-writable, and the runtime then +refused them, so the display stayed on "Waiting for Device". Since 2.5.3 the +status and the service log name the affected directory with the command that +fixes it, for example: + +```text +chmod go-w '/home/user/.cache/DXVSI/TRYX Panorama Runtime/prepared-media' +``` + +Run the command for each directory the log names, without `-R`, then restart +the service with `systemctl --user restart tryx-panorama.service`. Do not +delete the directories: they can hold an interrupted upload that the runtime +still has to finish. + ## Firmware Updates Firmware updates are initiated from the firmware panel in Quick Settings, but diff --git a/VERSION b/VERSION index f225a78..aedc15b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.5.2 +2.5.3 diff --git a/debian/changelog b/debian/changelog index 7270f9e..2d2ade0 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +tryx-panorama-manager (2.5.3-1) noble; urgency=medium + + * Keep a bulk IN pending between requests on the Panorama, Panorama SE and Panorama WB and re-arm it as soon as it ends, as the official bridge does, so the display no longer drops off USB until a power-off. + * Create the runtime's data and cache directories owner-only regardless of the umask, so a 0002 umask no longer stops the display session from starting after the first media upload. + * When stored retry media cannot be used, report that the display session cannot start and name the directory with the command that fixes it, instead of a validation that never finishes. + + -- DXVSI Wed, 07 Oct 2026 10:00:00 +0000 + tryx-panorama-manager (2.5.2-1) noble; urgency=medium * Add Panorama WB (391a:1031) support with the Panorama feature set, confirmed on real hardware. diff --git a/packaging/arch/PKGBUILD b/packaging/arch/PKGBUILD index 687a484..675e99c 100644 --- a/packaging/arch/PKGBUILD +++ b/packaging/arch/PKGBUILD @@ -2,7 +2,7 @@ # PKGBUILD metadata and directory variables are consumed or provided by makepkg. # shellcheck disable=SC2034,SC2154 pkgname=tryx-panorama-manager -pkgver=2.5.2 +pkgver=2.5.3 pkgrel=1 pkgdesc='Control compatible TRYX cooler displays on Linux' arch=('x86_64') diff --git a/packaging/metainfo/io.github.dxvsi.tryx_panorama_manager.metainfo.xml b/packaging/metainfo/io.github.dxvsi.tryx_panorama_manager.metainfo.xml index 72e3dc1..d07943a 100644 --- a/packaging/metainfo/io.github.dxvsi.tryx_panorama_manager.metainfo.xml +++ b/packaging/metainfo/io.github.dxvsi.tryx_panorama_manager.metainfo.xml @@ -27,6 +27,15 @@ https://github.com/DXVSI/Tryx-Linux-GUI/issues + + +
    +
  • Keep a bulk IN pending between requests on the Panorama, Panorama SE and Panorama WB and re-arm it as soon as it ends, as the official bridge does, so the display no longer drops off USB until a power-off.
    • +
  • Create the runtime's data and cache directories owner-only regardless of the umask, so a 0002 umask no longer stops the display session from starting after the first media upload.
    • +
  • When stored retry media cannot be used, report that the display session cannot start and name the directory with the command that fixes it, instead of a validation that never finishes.
    • +
+ +
    diff --git a/packaging/rpm/tryx-panorama-manager.spec b/packaging/rpm/tryx-panorama-manager.spec index 3c8dbff..4b6c9b1 100644 --- a/packaging/rpm/tryx-panorama-manager.spec +++ b/packaging/rpm/tryx-panorama-manager.spec @@ -1,5 +1,5 @@ Name: tryx-panorama-manager -Version: 2.5.2 +Version: 2.5.3 Release: 1%{?dist} Summary: Linux manager for supported TRYX cooler displays @@ -120,6 +120,11 @@ udevadm verify --resolve-names=never \ %{_mandir}/man1/tryx.1* %changelog +* Wed Oct 07 2026 DXVSI - 2.5.3-1 +- Keep a bulk IN pending between requests on the Panorama, Panorama SE and Panorama WB and re-arm it as soon as it ends, as the official bridge does, so the display no longer drops off USB until a power-off +- Create the runtime's data and cache directories owner-only regardless of the umask, so a 0002 umask no longer stops the display session from starting after the first media upload +- When stored retry media cannot be used, report that the display session cannot start and name the directory with the command that fixes it, instead of a validation that never finishes + * Tue Sep 29 2026 DXVSI - 2.5.2-1 - Add Panorama WB (391a:1031) support with the Panorama feature set, confirmed on real hardware - Show GPU names with their vendor on the hardware badge, for example NVIDIA GeForce RTX 4090 on the NVIDIA colours, also inside the Flatpak diff --git a/packaging/tryx-panorama-manager.1 b/packaging/tryx-panorama-manager.1 index 840b24c..c784055 100644 --- a/packaging/tryx-panorama-manager.1 +++ b/packaging/tryx-panorama-manager.1 @@ -1,4 +1,4 @@ -.TH TRYX-PANORAMA-MANAGER 1 "2026-09-29" "TRYX Panorama Manager 2.5.2" "User Commands" +.TH TRYX-PANORAMA-MANAGER 1 "2026-10-07" "TRYX Panorama Manager 2.5.3" "User Commands" .SH NAME tryx-panorama-manager \- control compatible TRYX cooler displays .SH SYNOPSIS diff --git a/packaging/tryx.1 b/packaging/tryx.1 index 15f3b90..ac76762 100644 --- a/packaging/tryx.1 +++ b/packaging/tryx.1 @@ -1,4 +1,4 @@ -.TH TRYX 1 "2026-09-29" "TRYX Panorama Manager 2.5.2" "User Commands" +.TH TRYX 1 "2026-10-07" "TRYX Panorama Manager 2.5.3" "User Commands" .SH NAME tryx \- inspect and manage the local TRYX runtime from a terminal .SH SYNOPSIS