diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5bad3774d..8628fd233 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -85,9 +85,41 @@ jobs: env cmake -B build -DCMAKE_BUILD_TYPE=Release && cmake --build build --parallel --target ALL_BUILD --config Release cp build/Release/pingviewer.exe deploy/pingviewer.exe - if (($env:OPENSSL) -and (Test-Path $env:OPENSSL -pathType container)) { - Copy-Item ${env:OPENSSL}\*.dll -Destination deploy -Force + + # Qt 5.15.2 is built against OpenSSL 1.1 and loads libssl-1_1-x64.dll / libcrypto-1_1-x64.dll + # at runtime (QSslSocket, used by the firmware-update HTTPS check). The VS2026 runner no + # longer ships OpenSSL 1.1 at a predictable location (it has OpenSSL 3.x), so the previous + # hardcoded copy silently shipped a build with broken HTTPS. Search common locations, fall + # back to a pinned OpenSSL 1.1.1 download, then fail if the DLLs are still missing. + $opensslDlls = @('libssl-1_1-x64.dll', 'libcrypto-1_1-x64.dll') + $searchRoots = @( + $env:OPENSSL, + 'C:\Program Files\OpenSSL-Win64\bin', + 'C:\Program Files\OpenSSL\bin', + "$env:QT_ROOT_DIR\..\..\Tools\OpenSSL\Win_x64\bin" + ) | Where-Object { $_ -and (Test-Path $_) } + foreach ($dll in $opensslDlls) { + if (Test-Path "deploy\$dll") { continue } + foreach ($root in $searchRoots) { + $hit = Get-ChildItem -Path $root -Filter $dll -Recurse -ErrorAction SilentlyContinue | Select-Object -First 1 + if ($hit) { Copy-Item $hit.FullName -Destination deploy -Force; break } + } + } + if (-not (Test-Path 'deploy\libssl-1_1-x64.dll') -or -not (Test-Path 'deploy\libcrypto-1_1-x64.dll')) { + curl -L https://download.firedaemon.com/FireDaemon-OpenSSL/openssl-1.1.1w.zip -o openssl-1.1.zip + Expand-Archive -Path openssl-1.1.zip -DestinationPath openssl-1.1 -Force + foreach ($dll in $opensslDlls) { + $hit = Get-ChildItem -Path openssl-1.1 -Filter $dll -Recurse -ErrorAction SilentlyContinue | Select-Object -First 1 + if ($hit) { Copy-Item $hit.FullName -Destination deploy -Force } + } } + foreach ($dll in $opensslDlls) { + if (-not (Test-Path "deploy\$dll")) { + Write-Error "Required OpenSSL 1.1 runtime '$dll' is missing from the deploy directory." + exit 1 + } + } + curl -L https://github.com/bluerobotics/stm32flash-code/releases/download/continuous/stm32flash.exe -o deploy/stm32flash.exe foreach ($I in (${env:SYSTEM32_DLLS} -split ' ')) { copy ${env:SYSTEM32}\$I deploy\ } windeployqt --qmldir qml --release deploy/pingviewer.exe --verbose=2 diff --git a/CMakeLists.txt b/CMakeLists.txt index 201e0f190..9b8a38d2a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,6 +31,14 @@ add_compile_definitions( QT_USE_FAST_OPERATOR_PLUS ) +# VS2026's MSVC STL removed stdext::make_checked_array_iterator, which Qt 5.15.2's +# qlist.h still calls unconditionally. Force-include a shim that restores it as a +# raw-pointer passthrough on new MSVC so QList-using translation units compile. +# See tools/msvc_stdext_shim.h for details. +if(MSVC) + add_compile_options("$<$:/FI${CMAKE_CURRENT_SOURCE_DIR}/tools/msvc_stdext_shim.h>") +endif() + find_package(Qt5 ${QT_MIN_VERSION} REQUIRED NO_MODULE COMPONENTS Charts Concurrent @@ -58,6 +66,11 @@ include_directories( lib/ping-cpp/ping-cpp/src/message/ ) +# The bundled fmt/maddy submodules declare cmake_minimum_required(VERSION < 3.5), +# which CMake >= 4.0 rejects. Allow those older sub-projects to configure until the +# submodules are bumped. Must be set before add_subdirectory(lib/fmt/fmt). +set(CMAKE_POLICY_VERSION_MINIMUM 3.5) + add_subdirectory(lib/fmt/fmt) add_subdirectory(src) diff --git a/lib/fmt/fmt b/lib/fmt/fmt index 9f2e7edae..e69e5f977 160000 --- a/lib/fmt/fmt +++ b/lib/fmt/fmt @@ -1 +1 @@ -Subproject commit 9f2e7edaebccf8f271cec5e855cce1223ff3e6d6 +Subproject commit e69e5f977d458f2650bb346dadf2ad30c5320281 diff --git a/qml/DeviceManagerViewer.qml b/qml/DeviceManagerViewer.qml index 7432a01fc..1593391d1 100644 --- a/qml/DeviceManagerViewer.qml +++ b/qml/DeviceManagerViewer.qml @@ -216,7 +216,7 @@ PingPopup { if (!connection.isValid()) return DeviceManagerViewer.ConnectionStatus.InvalidConfiguration; - if (connection.deviceType() == PingEnumNamespace.PingDeviceType.PING360 && connection.type() == AbstractLinkNamespace.Udp && connection.isSubnetBroadcast()) + if (connection.deviceType() == PingEnumNamespace.PingDeviceType.PING360 && connection.type() == AbstractLinkNamespace.Udp && (connection.isSubnetBroadcast() || !connection.isInSubnet())) return DeviceManagerViewer.ConnectionStatus.ConfigurationIsRequired; if (available) diff --git a/src/devicemanager/devicemanager.cpp b/src/devicemanager/devicemanager.cpp index ad1235929..b7986f464 100644 --- a/src/devicemanager/devicemanager.cpp +++ b/src/devicemanager/devicemanager.cpp @@ -1,3 +1,4 @@ +#include #include #include "devicemanager.h" @@ -24,6 +25,11 @@ DeviceManager::DeviceManager() connect(_detector, &ProtocolDetector::availableLinksChanged, this, &DeviceManager::updateAvailableConnections); connect(Ping360HelperService::self(), &Ping360HelperService::availableLinkFound, this, &DeviceManager::updateAvailableConnections); + + // Periodically expire devices that have not been reported recently instead of dropping them + // on the first missed discovery cycle. The timer only runs while detecting (see + // startDetecting/stopDetecting) so connected devices are not expired after discovery stops. + connect(&_availabilityTimer, &QTimer::timeout, this, &DeviceManager::expireStaleConnections); } void DeviceManager::append(const LinkConfiguration& linkConf, const QString& deviceName, const QString& detectorName) @@ -34,6 +40,9 @@ void DeviceManager::append(const LinkConfiguration& linkConf, const QString& dev qCDebug(DEVICEMANAGER) << "Connection configuration already exist for:" << _sensors[Name][i] << linkConf << linkConf.argsAsConst(); _sensors[Available][i] = true; + if (i < _lastSeenMs.size()) { + _lastSeenMs[i] = QDateTime::currentMSecsSinceEpoch(); + } const auto indexRow = index(i); emit dataChanged(indexRow, indexRow, _roles); return; @@ -50,6 +59,7 @@ void DeviceManager::append(const LinkConfiguration& linkConf, const QString& dev _sensors[Connected].append(false); _sensors[DetectorName].append(detectorName); _sensors[Name].append(deviceName); + _lastSeenMs.append(QDateTime::currentMSecsSinceEpoch()); const auto& indexRow = index(line); endInsertRows(); @@ -62,6 +72,7 @@ void DeviceManager::startDetecting() qCDebug(DEVICEMANAGER) << "Start protocol detector service."; _detectorThread.start(); Ping360HelperService::self()->startBroadcastService(); + _availabilityTimer.start(1000); } void DeviceManager::stopDetecting() @@ -70,6 +81,7 @@ void DeviceManager::stopDetecting() Ping360HelperService::self()->stopBroadcastService(); _detector->stop(); _detectorThread.quit(); + _availabilityTimer.stop(); } void DeviceManager::connectLink(LinkConfiguration* linkConf) @@ -151,19 +163,42 @@ void DeviceManager::updateAvailableConnections( const QVector& availableLinkConfigurations, const QString& detector) { qCDebug(DEVICEMANAGER) << "Available devices:" << availableLinkConfigurations; - // Make all connections unavailable by default + + // Refresh availability using a last-seen timestamp (updated by append) instead of flipping + // every entry for this detector to unavailable on each cycle. That previous approach made the + // list flicker whenever a single discovery cycle missed a reply. Stale entries are dropped by + // expireStaleConnections() once they have not been seen for _availabilityTtlMs. + for (const auto& link : availableLinkConfigurations) { + append(link, PingHelper::nameFromDeviceType(link.deviceType()), detector); + } +} + +void DeviceManager::expireStaleConnections() +{ + const qint64 now = QDateTime::currentMSecsSinceEpoch(); for (int i {0}; i < _sensors[Available].size(); i++) { + if (!_sensors[Available][i].toBool() || i >= _lastSeenMs.size()) { + continue; + } + + // Never expire a device we are actively connected to. Discovery is stopped while + // connected, so its last-seen timestamp would otherwise always go stale. + if (_sensors[Connected][i].toBool()) { + continue; + } + + // Keep simulations and directly-connected (non-detected) entries available; only devices + // reported by a detector are subject to the last-seen TTL. auto linkConf = _sensors[Connection][i].value>(); - if (linkConf->isSimulation() || _sensors[DetectorName][i] != detector) { + if (linkConf->isSimulation() || _sensors[DetectorName][i].toString() == QStringLiteral("None")) { continue; } - _sensors[Available][i] = false; - const auto indexRow = index(i); - emit dataChanged(indexRow, indexRow, _roles); - } - for (const auto& link : availableLinkConfigurations) { - append(link, PingHelper::nameFromDeviceType(link.deviceType()), detector); + if (now - _lastSeenMs[i] > _availabilityTtlMs) { + _sensors[Available][i] = false; + const auto indexRow = index(i); + emit dataChanged(indexRow, indexRow, _roles); + } } } @@ -173,6 +208,7 @@ void DeviceManager::clear() for (const auto category : _roleNames.keys()) { _sensors[category].clear(); } + _lastSeenMs.clear(); endResetModel(); emit countChanged(); } diff --git a/src/devicemanager/devicemanager.h b/src/devicemanager/devicemanager.h index 1a93f68cb..ad87bed2d 100644 --- a/src/devicemanager/devicemanager.h +++ b/src/devicemanager/devicemanager.h @@ -3,6 +3,7 @@ #include #include #include +#include #include "abstractlinknamespace.h" #include "ping360helperservice.h" @@ -178,6 +179,14 @@ class DeviceManager : public QAbstractListModel { void updateAvailableConnections( const QVector& availableLinkConfigurations, const QString& detectorName); + /** + * @brief Mark detected devices as unavailable once they have not been seen for longer than + * the availability TTL. This replaces flipping every entry to unavailable on each scan + * cycle, which made the device list flicker whenever a single discovery cycle missed a reply. + * + */ + void expireStaleConnections(); + // Role and names enum Roles { Available = 0, @@ -201,4 +210,10 @@ class DeviceManager : public QAbstractListModel { // Model variables QVector _roles; QHash> _sensors; + + // Availability bookkeeping: last time (ms since epoch) each row was reported by a detector, + // kept in sync with the model rows (rows are only ever appended or cleared). + QVector _lastSeenMs; + QTimer _availabilityTimer; + static constexpr qint64 _availabilityTtlMs {5000}; }; diff --git a/src/link/abstractlink.h b/src/link/abstractlink.h index e592fa9c7..fe71257be 100644 --- a/src/link/abstractlink.h +++ b/src/link/abstractlink.h @@ -271,6 +271,15 @@ class AbstractLink : public QObject { void availableConnectionsChanged(); void configurationChanged(); void nameChanged(const QString& name); + + /** + * @brief Emitted when the link fails in a way the user should be told about, with an + * actionable, human-readable message. Unlike transient logging, this is throttled/escalated + * by the link implementation so upper layers can surface it once instead of on every retry. + * + * @param errorMessage + */ + void linkError(const QString& errorMessage); void autoConnectChanged(); void linkChanged(AbstractLinkNamespace::LinkType link); void newData(const QByteArray& data); diff --git a/src/link/linkconfiguration.h b/src/link/linkconfiguration.h index 5f1d4aa20..2d384a062 100644 --- a/src/link/linkconfiguration.h +++ b/src/link/linkconfiguration.h @@ -246,7 +246,7 @@ class LinkConfiguration : public QObject { * @return true * @return false if IP is not in the same subnet as the computer or is not an ethernet connection */ - bool isInSubnet() const; + Q_INVOKABLE bool isInSubnet() const; /** * @brief Check if IP has a valid and accessible broadcast IP. diff --git a/src/link/udplink.cpp b/src/link/udplink.cpp index a9bb42a63..f6a7e24d5 100644 --- a/src/link/udplink.cpp +++ b/src/link/udplink.cpp @@ -3,6 +3,7 @@ #include #include "logger.h" +#include "networkmanager.h" #include "udplink.h" PING_LOGGING_CATEGORY(PING_PROTOCOL_UDPLINK, "ping.protocol.udplink") @@ -17,16 +18,23 @@ UDPLink::UDPLink(QObject* parent) connect(_udpSocket, &QAbstractSocket::errorOccurred, this, [this](QAbstractSocket::SocketError /*socketError*/) { printErrorMessage(); }); + // Reset the failure counter and backoff once we are actually connected. + connect(_udpSocket, &QAbstractSocket::connected, this, [this] { resetConnectionState(); }); + // QUdpSocket fail to emit state signal // Here we use a timer to check if we are in a connect state, if not we try again connect(&_stateTimer, &QTimer::timeout, this, [this] { if (_udpSocket->state() == QAbstractSocket::UnconnectedState) { - printErrorMessage(); + handleConnectionFailure(); qDebug(PING_PROTOCOL_UDPLINK) << "Trying to reconnect with host again."; _udpSocket->connectToHost(_hostAddress, _port); + + // Back off the reconnect interval so we stop flooding the log and network while the + // target stays unreachable, capping at _maxReconnectIntervalMs. + _stateTimer.setInterval(qMin(_stateTimer.interval() * 2, _maxReconnectIntervalMs)); } }); - _stateTimer.start(1000); + _stateTimer.start(_baseReconnectIntervalMs); connect(this, &AbstractLink::sendData, this, [this](const QByteArray& data) { _udpSocket->write(data); }); } @@ -73,10 +81,36 @@ bool UDPLink::setConfiguration(const LinkConfiguration& linkConfiguration) void UDPLink::printErrorMessage() { qCWarning(PING_PROTOCOL_UDPLINK) << "An error has occurred with:" << _linkConfiguration; - QString errorMessage = QStringLiteral("Error (%1): %2.").arg(_udpSocket->state()).arg(_udpSocket->errorString()); + QString errorMessage = QStringLiteral("Error (%1): %2.").arg(_udpSocket->error()).arg(_udpSocket->errorString()); qCWarning(PING_PROTOCOL_UDPLINK) << errorMessage; } +void UDPLink::handleConnectionFailure() +{ + printErrorMessage(); + _connectionErrorCount++; + + // Escalate only once, with an actionable message, instead of silently retrying forever. + if (!_errorEscalated && _connectionErrorCount >= _errorEscalationThreshold) { + _errorEscalated = true; + + QString message = QStringLiteral("Unable to reach the device at %1:%2.").arg(_hostAddress).arg(_port); + if (!NetworkManager::isAddressInSubnet(_hostAddress)) { + message += QStringLiteral(" It is on a different network than your computer. Set the device IP " + "or your computer's network settings so they share a subnet."); + } + qCWarning(PING_PROTOCOL_UDPLINK) << message; + emit linkError(message); + } +} + +void UDPLink::resetConnectionState() +{ + _connectionErrorCount = 0; + _errorEscalated = false; + _stateTimer.setInterval(_baseReconnectIntervalMs); +} + bool UDPLink::finishConnection() { _udpSocket->close(); diff --git a/src/link/udplink.h b/src/link/udplink.h index 97264af4c..63f839241 100644 --- a/src/link/udplink.h +++ b/src/link/udplink.h @@ -77,8 +77,28 @@ class UDPLink : public AbstractLink { */ void printErrorMessage(); + /** + * @brief Handle a failed connection attempt: log it, count consecutive failures, and once the + * escalation threshold is reached emit a single actionable linkError instead of silently + * retrying forever. + * + */ + void handleConnectionFailure(); + + /** + * @brief Reset the failure counter and reconnect backoff after a successful connection. + * + */ + void resetConnectionState(); + QString _hostAddress; QTimer _stateTimer; QUdpSocket* _udpSocket; uint _port; + + int _connectionErrorCount {0}; + bool _errorEscalated {false}; + static constexpr int _baseReconnectIntervalMs {1000}; + static constexpr int _maxReconnectIntervalMs {8000}; + static constexpr int _errorEscalationThreshold {3}; }; diff --git a/src/logger/logger.cpp b/src/logger/logger.cpp index bb7c428dd..a547155f4 100644 --- a/src/logger/logger.cpp +++ b/src/logger/logger.cpp @@ -10,8 +10,13 @@ #include #include #include +#include #include +#ifdef Q_OS_WIN +#include +#endif + PING_LOGGING_CATEGORY(logger, "ping.logger") Logger::Logger() @@ -52,6 +57,9 @@ void Logger::logMessage(const QString& msg, const QtMsgType& type, const QMessag // Save the message into the file _fileStream << QString("%1 %2\n").arg(time, msg); + // QTextStream buffers, and nothing else flushes it during a normal run, so an abnormal exit + // would discard the buffer and leave a zero-byte Gui_Log. + _fileStream.flush(); _logModel.append(time, msg, _colors[type], _categoryIndexer[context.category]); } @@ -90,7 +98,31 @@ void Logger::handleMessage(QtMsgType type, const QMessageLogContext& context, co style = fmt::emphasis::bold | fg(fmt::color::yellow) | bg(fmt::color::red); break; } - fmt::print(style, "{}\n", qFormatLogMessage(type, context, logMsg).toStdString()); + + // Detect once whether stdout can actually be written to. A GUI-subsystem binary launched from + // Explorer has no console, so there is nothing to print to. + static const bool stdoutUsable = []() { +#ifdef Q_OS_WIN + const HANDLE handle = GetStdHandle(STD_OUTPUT_HANDLE); + return handle != nullptr && handle != INVALID_HANDLE_VALUE; +#else + return true; +#endif + }(); + + if (!stdoutUsable) { + return; + } + + // fmt >= 10 routes the styled print() through detail::print() -> fwrite_fully(), which throws + // std::system_error when the write fails. A Qt message handler must never throw: it is called + // from arbitrary contexts (including destructors), and an escaping exception reaches + // std::terminate(). A redirected stdout can still fail mid-run, so this stays as a backstop. + try { + fmt::print(style, "{}\n", qFormatLogMessage(type, context, logMsg).toStdString()); + } catch (const std::exception&) { + // There is no console to report to; dropping the line is the only option. + } } void Logger::registerCategory(const char* category) diff --git a/src/main.cpp b/src/main.cpp index ace96151d..64ef2d164 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -146,5 +146,12 @@ int main(int argc, char* argv[]) KCrash::initialize(); #endif - return app.exec(); + const int result = app.exec(); + + // Logger::self() is a function-local static, so it is destroyed during static destruction while + // the message handler is still installed. A log line from any later destructor would otherwise + // re-enter an already-destroyed Logger. + qInstallMessageHandler(nullptr); + + return result; } diff --git a/src/network/networkmanager.cpp b/src/network/networkmanager.cpp index a39625266..5b51b8e93 100644 --- a/src/network/networkmanager.cpp +++ b/src/network/networkmanager.cpp @@ -90,20 +90,40 @@ QHostAddress NetworkManager::addressToIp(const QString& address) return QHostInfo::fromName(address).addresses().first(); } +bool NetworkManager::isInterfaceUsable(const QNetworkInterface& interface) +{ + const auto flags = interface.flags(); + return flags.testFlag(QNetworkInterface::IsUp) && flags.testFlag(QNetworkInterface::IsRunning) + && !flags.testFlag(QNetworkInterface::IsLoopBack); +} + bool NetworkManager::isAddressInSubnet(const QString& address) { const QHostAddress testAddress = addressToIp(address); - if (testAddress.protocol() == QAbstractSocket::IPv6Protocol) { + if (testAddress.protocol() != QAbstractSocket::IPv4Protocol) { qCWarning(NETWORKMANAGER) << "Invalid network interface for ip:" << testAddress; return false; } for (const auto& interface : QNetworkInterface::allInterfaces()) { + // Skip interfaces that are not up and running (e.g. a disconnected Wi-Fi or Bluetooth + // adapter). These may still expose stale link-local addresses that would otherwise + // produce false matches and log noise. + if (!isInterfaceUsable(interface)) { + continue; + } + for (const auto& networkAddressEntry : interface.addressEntries()) { const auto address = networkAddressEntry.ip(); const auto netmask = networkAddressEntry.netmask(); + // Only IPv4 entries can be compared against an IPv4 target. Skip IPv6 (e.g. fe80::) + // entries and any entry without a valid netmask. + if (address.protocol() != QAbstractSocket::IPv4Protocol || netmask.isNull()) { + continue; + } + // Remove the last value of the IP address const bool sameSubnet = (address.toIPv4Address() & netmask.toIPv4Address()) == (testAddress.toIPv4Address() & netmask.toIPv4Address()); diff --git a/src/network/networkmanager.h b/src/network/networkmanager.h index fb9549a63..2cd25303d 100644 --- a/src/network/networkmanager.h +++ b/src/network/networkmanager.h @@ -7,6 +7,7 @@ class QJSEngine; class QQmlEngine; +class QNetworkInterface; Q_DECLARE_LOGGING_CATEGORY(NETWORKMANAGER) @@ -48,6 +49,19 @@ class NetworkManager : public QObject { */ static QHostAddress addressToIp(const QString& address); + /** + * @brief Check if a network interface is usable for IPv4 communication + * An interface is considered usable when it is administratively up, operationally running + * (carrier/link present), and is not the loopback interface. Down adapters such as a + * disconnected Wi-Fi or Bluetooth interface (which may still carry stale link-local + * addresses) must be skipped. + * + * @param interface + * @return true + * @return false + */ + static bool isInterfaceUsable(const QNetworkInterface& interface); + /** * @brief Check if an address is a valid IP of the host network interfaces * diff --git a/src/sensor/ping360.cpp b/src/sensor/ping360.cpp index 7b8c86c62..2693a50c9 100644 --- a/src/sensor/ping360.cpp +++ b/src/sensor/ping360.cpp @@ -151,18 +151,13 @@ void Ping360::checkBootloader() return; } - // bootloader may only be present on serial links, it does not communicate - // by ethernet - if (link()->type() != LinkType::Serial) { + // The bootloader may only be present on serial links, it does not communicate by ethernet. + // When the probe cannot run we must still fall through to the regular configuration path, + // otherwise the sensor stays unconfigured and silent. + if (link()->type() != LinkType::Serial || !link()->isOpen() || !link()->isWritable()) { startPreConfigurationProcess(); return; } - if (!link()->isOpen()) { - return; - } - if (!link()->isWritable()) { - return; - } qCWarning(PING_PROTOCOL_PING360) << "checking for bootloader..."; Ping360BootloaderPacket::packet_cmd_read_version_t readVersion @@ -184,7 +179,10 @@ void Ping360::checkBootloader() } }); - QTimer::singleShot(250, [=] { + // The context object is mandatory: DeviceManager::connectLink() destroys the current sensor + // whenever a new connection is made, and without a receiver Qt cannot cancel this callback. + // Firing it on a destroyed Ping360 dereferences a freed QObjectPrivate inside QTimer::stop(). + QTimer::singleShot(250, this, [=] { disconnect(blScanCallback); startConfiguration(); }); @@ -192,6 +190,10 @@ void Ping360::checkBootloader() void Ping360::startPreConfigurationProcess() { + if (!link()) { + return; + } + // Force the default settings resetSettings(); @@ -272,6 +274,13 @@ void Ping360::requestNextProfile() } } +void Ping360::requestNextProfileDeferred() +{ + // A zero-timer runs on the next event-loop iteration, so the "request next message ASAP" + // behaviour is preserved while never reconfiguring the serial port inside its own readyRead. + QTimer::singleShot(0, this, [this] { requestNextProfile(); }); +} + void Ping360::legacyProfileRequest() { // Calculate the next delta step @@ -344,8 +353,6 @@ void Ping360::asyncProfileRequest() void Ping360::handleMessage(const ping_message& msg) { - static uint8_t _waitRetryMessages = 1; - qCDebug(PING_PROTOCOL_PING360) << QStringLiteral("Handling Message: %1 [%2]") .arg(PingHelper::nameFromMessageId( static_cast(msg.message_id()))) @@ -367,7 +374,7 @@ void Ping360::handleMessage(const ping_message& msg) } else { _baudrateConfigurationTimer.stop(); _timeoutProfileMessage.start(); - requestNextProfile(); + requestNextProfileDeferred(); } return; } @@ -380,7 +387,7 @@ void Ping360::handleMessage(const ping_message& msg) _angle = deviceData.angle(); // Request next message ASAP - requestNextProfile(); + requestNextProfileDeferred(); // Restart timer, if the channel allows it if (link()->isWritable()) { @@ -484,7 +491,7 @@ void Ping360::handleMessage(const ping_message& msg) // before sending a new request. _waitRetryMessages = 5; qCDebug(PING_PROTOCOL_PING360) << "AUTO_DEVICE_DATA parameters do not match settings (invalid)"; - requestNextProfile(); + requestNextProfileDeferred(); } } else { _waitRetryMessages = 1; @@ -508,7 +515,7 @@ void Ping360::handleMessage(const ping_message& msg) set_number_of_points(_viewerDefaultNumberOfSamples); // request another transmission - requestNextProfile(); + requestNextProfileDeferred(); // restart timer _timeoutProfileMessage.start(); @@ -728,8 +735,10 @@ void Ping360::resetBaudrate() void Ping360::setBaudRate(int baudRate) { - // It's only possible to change baudrates in serial connections - if (link()->type() != LinkType::Serial) { + // It's only possible to change baudrates in serial connections. + // Sensor::link() returns nullptr once the link has been cleared, which happens while the + // sensor is being torn down. + if (!link() || link()->type() != LinkType::Serial) { return; } @@ -909,6 +918,12 @@ Ping360::~Ping360() { updateSensorConfigurationSettings(); + // The link may already be gone (or not writable) during teardown; the motor-off handshake + // below dereferences it, so bail out when there is nothing to talk to. + if (!link() || !link()->isOpen() || !link()->isWritable()) { + return; + } + ping360_motor_off message; message.updateChecksum(); diff --git a/src/sensor/ping360.h b/src/sensor/ping360.h index a86c139d1..811990824 100644 --- a/src/sensor/ping360.h +++ b/src/sensor/ping360.h @@ -190,7 +190,7 @@ class Ping360 : public PingSensor { // reduce _sample period until we are within operational parameters // maximize the number of points - while (_sensorSettings.sample_period < _firmwareMinSamplePeriod) { + while (_sensorSettings.sample_period < _firmwareMinSamplePeriod && _sensorSettings.num_points > 1) { _sensorSettings.num_points--; _sensorSettings.sample_period = calculateSamplePeriod(newRange); } @@ -254,7 +254,7 @@ class Ping360 : public PingSensor { // reduce _sample period until we are within operational parameters // maximize the number of points - while (_sensorSettings.sample_period < _firmwareMinSamplePeriod) { + while (_sensorSettings.sample_period < _firmwareMinSamplePeriod && _sensorSettings.num_points > 1) { _sensorSettings.num_points--; _sensorSettings.sample_period = calculateSamplePeriod(desiredRange); } @@ -661,7 +661,7 @@ class Ping360 : public PingSensor { } _sensorSettings; // Counter for recieving new messages before retrying a request - uint8_t _waitRetryMessages; + uint8_t _waitRetryMessages = 1; // This variables are not user configuration settings uint16_t _angle = 200; @@ -814,6 +814,16 @@ class Ping360 : public PingSensor { */ void requestNextProfile(); + /** + * @brief Queue a profile request on the event loop + * + * requestNextProfile() can reach resetBaudrate(), which closes and reopens the serial port. + * Message handling runs synchronously inside QSerialPort::readyRead (link -> parser -> + * handleMessage are all Qt::DirectConnection), and a QSerialPort must never be closed from + * inside its own readyRead emission. Deferring to the event loop avoids that reentrancy. + */ + void requestNextProfileDeferred(); + /** * @brief Legacy profile request * Used in firmwares 3.1 diff --git a/src/sensor/ping360helperservice.cpp b/src/sensor/ping360helperservice.cpp index 76c4fbc8f..1d7c6df5b 100644 --- a/src/sensor/ping360helperservice.cpp +++ b/src/sensor/ping360helperservice.cpp @@ -4,6 +4,7 @@ #include #include "logger.h" +#include "networkmanager.h" #include "ping360asciiprotocol.h" #include "ping360helperservice.h" @@ -20,12 +21,16 @@ Ping360HelperService::Ping360HelperService() QQmlEngine::setObjectOwnership(this, QQmlEngine::CppOwnership); const int randomPort = 0; // Force OS to give a random port - _broadcastSocket.bind(QHostAddress::AnyIPv4, randomPort, QAbstractSocket::ReuseAddressHint); + _broadcastSocket.bind( + QHostAddress::AnyIPv4, randomPort, QUdpSocket::ShareAddress | QAbstractSocket::ReuseAddressHint); + _broadcastSocket.setSocketOption(QAbstractSocket::MulticastTtlOption, 1); - // Bind all available interfaces + // Bind all available interfaces that are up, running and can broadcast. Down adapters + // (e.g. a disconnected Wi-Fi or Bluetooth interface) are skipped. const auto interfaces = QNetworkInterface::allInterfaces(); for (const QNetworkInterface& networkInterface : interfaces) { - if (networkInterface.flags() & QNetworkInterface::CanBroadcast) { + if (NetworkManager::isInterfaceUsable(networkInterface) + && networkInterface.flags().testFlag(QNetworkInterface::CanBroadcast)) { _broadcastSocket.joinMulticastGroup(QHostAddress(QHostAddress::Broadcast), networkInterface); } } @@ -55,8 +60,7 @@ void Ping360HelperService::doBroadcast() QList> ipBroadcastAddresses; QList interfaces = QNetworkInterface::allInterfaces(); for (const QNetworkInterface& interface : interfaces) { - if (interface.flags().testFlag(QNetworkInterface::IsUp) - && !interface.flags().testFlag(QNetworkInterface::IsLoopBack)) { + if (NetworkManager::isInterfaceUsable(interface)) { QList entries = interface.addressEntries(); for (const QNetworkAddressEntry& entry : entries) { if (entry.ip().protocol() == QAbstractSocket::IPv4Protocol) { @@ -79,33 +83,41 @@ void Ping360HelperService::doBroadcast() << "for IP address:" << ipAddress.toString(); } } + + // Also send to the limited broadcast address (255.255.255.255). A device on a link-local + // AutoIP address is not on any of our subnets, so a subnet-directed broadcast may never reach + // it; the limited broadcast improves the odds that such a device receives the discovery + // request and replies, making detection more reliable. + if (_broadcastSocket.writeDatagram(datagram, QHostAddress::Broadcast, Ping360AsciiProtocol::udpPort()) == -1) { + qDebug() << "Failed to send datagram to limited broadcast address: 255.255.255.255"; + } } void Ping360HelperService::processBroadcastResponses() { while (_broadcastSocket.hasPendingDatagrams()) { - QHostAddress sender, destination; - QNetworkDatagram datagram = _broadcastSocket.receiveDatagram(); - // Make sure we have an IPV4 address, and not something like "::ffff:192.168.1.1" - sender = QHostAddress(datagram.senderAddress().toIPv4Address()); - - // if the sender ip address starts with 169.254.x.x, then the ping360 has not - // been assigned an ip address and we will not be able to reach it on this address - // we need to broadcast on the destination subnet, where we will be able to communicate - if (sender.isLinkLocal()) { - sender = QHostAddress(datagram.destinationAddress().toIPv4Address() | 255); - } - const Ping360DiscoveryResponse decoded = Ping360AsciiProtocol::decodeDiscoveryResponse(datagram.data()); - if (decoded.deviceName.contains("PING360")) { - emit availableLinkFound( - {{LinkType::Udp, {decoded.ipAddress, "12345"}, "Ping360 Port", PingDeviceType::PING360}}, - QStringLiteral("Ping360 Ethernet Protocol Detector")); - } else { + if (!decoded.deviceName.contains("PING360")) { qCWarning(PING360HELPERSERVICE) << "Invalid message:" << datagram.data(); + continue; } + + // A Ping360 that has not been assigned an IP address falls back to a link-local + // (169.254.x.x) AutoIP address. It answers the discovery broadcast, but it cannot be + // reached by unicast unless the computer is on the same subnet. Detect that case so the + // device is flagged as needing IP configuration instead of being shown as ready to use. + const bool reachable = NetworkManager::isAddressInSubnet(decoded.ipAddress); + if (!reachable) { + qCWarning(PING360HELPERSERVICE) + << "Ping360 discovered on unreachable address:" << decoded.ipAddress + << "- the device and the computer are on different networks. Set a static IP on the device."; + } + + emit availableLinkFound( + {{LinkType::Udp, {decoded.ipAddress, "12345"}, "Ping360 Port", PingDeviceType::PING360}}, + QStringLiteral("Ping360 Ethernet Protocol Detector")); } } @@ -113,9 +125,13 @@ void Ping360HelperService::setDHCPServer(const QString& ip) { setStaticIP(ip, "0 void Ping360HelperService::setStaticIP(const QString& ip, const QString& staticIp) { + Q_UNUSED(ip) const QByteArray datagram = Ping360AsciiProtocol::staticIpAddressMessage(staticIp); - qCDebug(PING360HELPERSERVICE) << "Sending IP configuration message:" << datagram; - _broadcastSocket.writeDatagram(datagram, QHostAddress {ip}, Ping360AsciiProtocol::udpPort()); + const auto port = Ping360AsciiProtocol::udpPort(); + // Broadcast to the limited broadcast address so the message reaches a device that is currently + // on a different subnet (e.g. a link-local AutoIP address), which a unicast to `ip` could not. + qCDebug(PING360HELPERSERVICE) << "Broadcasting IP configuration message:" << datagram << "on port" << port; + _broadcastSocket.writeDatagram(datagram, QHostAddress::Broadcast, port); } QObject* Ping360HelperService::qmlSingletonRegister(QQmlEngine* engine, QJSEngine* scriptEngine) diff --git a/tools/msvc_stdext_shim.h b/tools/msvc_stdext_shim.h new file mode 100644 index 000000000..5585d1fcb --- /dev/null +++ b/tools/msvc_stdext_shim.h @@ -0,0 +1,54 @@ +// msvc_stdext_shim.h +// +// Visual Studio 2026 (MSVC toolset 14.5x, _MSC_VER >= 1950) REMOVED +// stdext::make_checked_array_iterator / stdext::make_unchecked_array_iterator +// from (see microsoft/STL PR #5817). +// +// Qt 5.15.2 still calls them from QtCore/qlist.h (and friends) via the +// QT_MAKE_CHECKED_ARRAY_ITERATOR / QT_MAKE_UNCHECKED_ARRAY_ITERATOR macros, +// which on MSVC expand to stdext::make_..._array_iterator unconditionally with +// no way to disable them. As a result every translation unit that instantiates a +// QList fails to compile: +// error C2653: 'stdext': is not a class or namespace name +// error C3861: 'make_checked_array_iterator': identifier not found +// +// GitHub's hosted windows-latest and windows-2025 images both rolled to VS2026 +// in mid-2026, and no VS2022 image remains, so this cannot be fixed at the +// runner level and _SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING no longer helps +// (the symbols are gone, not merely deprecated). Qt itself fixed this in 5.15.17, +// but the open-source aqt archive only offers 5.15.2. +// +// This header is force-included (/FI) before every Qt header on MSVC and +// provides a minimal stand-in that returns the raw pointer, byte-identical in +// behaviour to the QT_MAKE_CHECKED_ARRAY_ITERATOR(x, N) => (x) branch Qt already +// ships on Linux/macOS. Bounds are intentionally ignored (same as that path). +// +// Guarded to _MSC_VER >= 1950 (VS2026+) so it can never collide with an older +// toolchain's real stdext::make_checked_array_iterator, which is still present on +// VS2019 / VS2022 and returns a different type. + +#pragma once + +// Defense-in-depth: this header is only meaningful for C++. Guard against being +// force-included into a C translation unit (the project also compiles C sources). +#if defined(__cplusplus) && defined(_MSC_VER) && _MSC_VER >= 1950 + +#include + +namespace stdext { + +template +inline T* make_checked_array_iterator(T* ptr, std::size_t /*size*/, std::size_t /*index*/ = 0) noexcept +{ + return ptr; +} + +template +inline T* make_unchecked_array_iterator(T* ptr) noexcept +{ + return ptr; +} + +} // namespace stdext + +#endif // defined(__cplusplus) && defined(_MSC_VER) && _MSC_VER >= 1950