From 85217405fe0c7194b61d7a75e1329912b9af310e Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Fri, 1 May 2026 00:39:00 +0200 Subject: [PATCH 01/12] Reconnect in case of network error. --- .../connectionplugins/connectiontcp.cpp | 128 +++++++++++++++--- .../connectionplugins/connectiontcp.h | 14 ++ 2 files changed, 122 insertions(+), 20 deletions(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index ce855e7c11..5d4e1a1701 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -76,6 +76,28 @@ TCP::TCP(INDI::DefaultDevice *dev, IPerm permission) : Interface(dev, CONNECTION IUFillSwitchVector(&LANSearchSP, LANSearchS, 2, dev->getDeviceName(), INDI::SP::DEVICE_LAN_SEARCH, "LAN Search", CONNECTION_TAB, IP_RW, ISR_1OFMANY, 60, IPS_IDLE); + // Retry/backoff configuration: number vector property + // Default values + IUFillNumber(&RetryN[TCP::RETRY_RETRIES], "CONNECT_RETRIES", "Connection retries", "%.0f", 0, 100, 1, + static_cast(m_ConnectRetries)); + IUFillNumber(&RetryN[TCP::RETRY_BACKOFF_MS], "BACKOFF_BASE_MS", "Backoff base (ms)", "%.0f", 0, 60000, 1, + static_cast(m_BackoffBaseMs)); + IUFillNumberVector(&RetryNP, RetryN, 2, getDeviceName(), "CONNECTION_RETRY", "Connection Retry", + CONNECTION_TAB, IP_RW, 60, IPS_IDLE); + + // Try to load persisted values from config (if any) + double dval = 0; + if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_RETRIES].name, &dval) == 0) + { + RetryN[TCP::RETRY_RETRIES].value = dval; + m_ConnectRetries = static_cast(dval); + } + if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_BACKOFF_MS].name, &dval) == 0) + { + RetryN[TCP::RETRY_BACKOFF_MS].value = dval; + m_BackoffBaseMs = static_cast(dval); + } + } ////////////////////////////////////////////////////////////////////////////////////////////////// @@ -139,6 +161,41 @@ bool TCP::ISNewSwitch(const char *dev, const char *name, ISState *states, char * return false; } +////////////////////////////////////////////////////////////////////////////////////////////////// +/// +////////////////////////////////////////////////////////////////////////////////////////////////// +bool TCP::ISNewNumber(const char *dev, const char *name, double values[], char *names[], int n) +{ + if (!strcmp(dev, m_Device->getDeviceName())) + { + // Connection retry/backoff configuration + if (!strcmp(name, RetryNP.name)) + { + IUUpdateNumber(&RetryNP, values, names, n); + RetryNP.s = IPS_OK; + + // Update runtime values + m_ConnectRetries = static_cast(RetryN[TCP::RETRY_RETRIES].value); + m_BackoffBaseMs = static_cast(RetryN[TCP::RETRY_BACKOFF_MS].value); + + // Persist the change + m_Device->saveConfig(true, RetryNP.name); + + // Log the new configuration for visibility + LOGF_INFO("Connection retry configuration updated: CONNECT_RETRIES=%d, BACKOFF_BASE_MS=%d", + m_ConnectRetries, m_BackoffBaseMs); + + // Notify clients of the change if initialization is complete + if (m_Device->isInitializationComplete()) + IDSetNumber(&RetryNP, nullptr); + + return true; + } + } + + return false; +} + ////////////////////////////////////////////////////////////////////////////////////////////////// /// ////////////////////////////////////////////////////////////////////////////////////////////////// @@ -219,41 +276,66 @@ bool TCP::Connect() return false; } - bool handshakeResult = true; + bool handshakeResult = false; std::string hostname = AddressT[0].text; std::string port = AddressT[1].text; - // Should just call handshake on simulation + // Use runtime-configurable retry/backoff parameters (can be changed via the Connection Retry property) + int connectRetries = m_ConnectRetries > 0 ? m_ConnectRetries : 1; + int backoffBaseMs = m_BackoffBaseMs > 0 ? m_BackoffBaseMs : 200; + + // Simulation devices bypass connection/retry logic and simply call the handshake if (m_Device->isSimulation()) + { handshakeResult = Handshake(); - + } else { - handshakeResult = false; std::regex ipv4("^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$"); const auto isIPv4 = regex_match(hostname, ipv4); - // Establish connection to host:port - if (establishConnection(hostname, port)) + // Try a small number of retries on the requested address before falling back to LAN search + for (int attempt = 1; attempt <= connectRetries; ++attempt) { - PortFD = m_SockFD; - LOGF_DEBUG("Connection to %s@%s is successful, attempting handshake...", hostname.c_str(), port.c_str()); - handshakeResult = Handshake(); + if (establishConnection(hostname, port)) + { + PortFD = m_SockFD; + LOGF_DEBUG("Connection to %s@%s is successful, attempting handshake...", hostname.c_str(), port.c_str()); + handshakeResult = Handshake(); + + if (handshakeResult) + break; + + // Handshake failed, close and retry according to policy (unless LAN search is disabled) + close(m_SockFD); + m_SockFD = -1; + PortFD = -1; + if (LANSearchS[INDI::DefaultDevice::INDI_ENABLED].s == ISS_OFF) + { + LOGF_DEBUG("Handshake failed on attempt %d/%d to %s@%s, will retry.", attempt, connectRetries, hostname.c_str(), port.c_str()); + } + else + { + LOGF_DEBUG("Handshake failed on attempt %d/%d to %s@%s, will retry before attempting LAN search.", attempt, connectRetries, hostname.c_str(), port.c_str()); + } + } + else + { + LOGF_DEBUG("Connection attempt %d/%d to %s@%s failed.", attempt, CONNECT_RETRIES, hostname.c_str(), port.c_str()); + } - // Auto search is disabled. - if (handshakeResult == false && LANSearchS[INDI::DefaultDevice::INDI_ENABLED].s == ISS_OFF) + if (attempt < connectRetries) { - LOG_DEBUG("Handshake failed."); - return false; + // backoff before retrying + int backoff = backoffBaseMs * (1 << (attempt - 1)); + LOGF_DEBUG("Waiting %d ms before next connect attempt.", backoff); + usleep(static_cast(backoff * 1000)); } } - // If connection failed; or - // handshake failed; then we can search LAN if the IP address is v4 - // LAN search is enabled. - if (handshakeResult == false && - LANSearchS[INDI::DefaultDevice::INDI_ENABLED].s == ISS_ON && - isIPv4) + // If all direct retries failed and LAN search is enabled and the provided address looks like an IPv4, + // proceed with the existing LAN search fallback behavior. + if (!handshakeResult && LANSearchS[INDI::DefaultDevice::INDI_ENABLED].s == ISS_ON && isIPv4) { size_t found = hostname.find_last_of("."); @@ -300,13 +382,16 @@ bool TCP::Connect() if (establishConnection(newAddress, port, 1)) { PortFD = m_SockFD; - LOGF_DEBUG("Connection to %s@%s is successful, attempting handshake...", hostname.c_str(), port.c_str()); + LOGF_DEBUG("Connection to %s@%s is successful, attempting handshake...", newAddress.c_str(), port.c_str()); handshakeResult = Handshake(); if (handshakeResult) { hostname = newAddress; break; } + close(m_SockFD); + m_SockFD = -1; + PortFD = -1; } } @@ -363,6 +448,7 @@ void TCP::Activated() { m_Device->defineProperty(&TcpUdpSP); m_Device->defineProperty(&LANSearchSP); + m_Device->defineProperty(&RetryNP); } } @@ -376,6 +462,7 @@ void TCP::Deactivated() { m_Device->deleteProperty(TcpUdpSP.name); m_Device->deleteProperty(LANSearchSP.name); + m_Device->deleteProperty(RetryNP.name); } } @@ -389,6 +476,7 @@ bool TCP::saveConfigItems(FILE * fp) IUSaveConfigText(fp, &AddressTP); IUSaveConfigSwitch(fp, &TcpUdpSP); IUSaveConfigSwitch(fp, &LANSearchSP); + IUSaveConfigNumber(fp, &RetryNP); } return true; diff --git a/libs/indibase/connectionplugins/connectiontcp.h b/libs/indibase/connectionplugins/connectiontcp.h index 807347f110..824e25f416 100644 --- a/libs/indibase/connectionplugins/connectiontcp.h +++ b/libs/indibase/connectionplugins/connectiontcp.h @@ -47,6 +47,7 @@ class TCP : public Interface virtual ~TCP() = default; virtual bool Connect() override; + virtual bool ISNewNumber(const char *dev, const char *name, double values[], char *names[], int n) override; virtual bool Disconnect() override; @@ -113,14 +114,27 @@ class TCP : public Interface // Auto search ISwitch LANSearchS[2]; ISwitchVectorProperty LANSearchSP; + // Retry/backoff configuration + enum RetryIndex + { + RETRY_RETRIES = 0, + RETRY_BACKOFF_MS = 1, + }; + INumber RetryN[2]; + INumberVectorProperty RetryNP; // Variables IPerm m_Permission = IP_RW; std::string m_ConfigHost; std::string m_ConfigPort; int m_ConfigConnectionType {-1}; + int m_ConfigConnectRetries {-1}; + int m_ConfigBackoffBaseMs {-1}; int m_SockFD {-1}; int PortFD = -1; static constexpr uint8_t SOCKET_TIMEOUT {5}; + // Runtime-configurable connection retry parameters + int m_ConnectRetries {3}; + int m_BackoffBaseMs {500}; // milliseconds }; } From af4d479948378df3f928e6c2fdc7a9ade02d9ee2 Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Fri, 1 May 2026 00:47:58 +0200 Subject: [PATCH 02/12] Fix wrong name. --- libs/indibase/connectionplugins/connectiontcp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index 5d4e1a1701..080b48638b 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -321,7 +321,7 @@ bool TCP::Connect() } else { - LOGF_DEBUG("Connection attempt %d/%d to %s@%s failed.", attempt, CONNECT_RETRIES, hostname.c_str(), port.c_str()); + LOGF_DEBUG("Connection attempt %d/%d to %s@%s failed.", attempt, connectRetries, hostname.c_str(), port.c_str()); } if (attempt < connectRetries) From 4175628d126f57366798f1d9c54fd5164a94bb0b Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 18:05:30 +0200 Subject: [PATCH 03/12] Add max backoff delay. --- libs/indibase/connectionplugins/connectiontcp.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/indibase/connectionplugins/connectiontcp.h b/libs/indibase/connectionplugins/connectiontcp.h index 824e25f416..bce680e512 100644 --- a/libs/indibase/connectionplugins/connectiontcp.h +++ b/libs/indibase/connectionplugins/connectiontcp.h @@ -37,6 +37,7 @@ namespace Connection class TCP : public Interface { public: + static const long long MAX_BACKOFF_DELAY = 60000LL; enum ConnectionType { TYPE_TCP = 0, From 7b96b18c6cc146c1a173d982dbfa9b6427834f38 Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 18:06:15 +0200 Subject: [PATCH 04/12] Fix backoff overflow in TCP::Connect by clamping retries/backoff --- libs/indibase/connectionplugins/connectiontcp.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index 080b48638b..d29eb2666c 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -27,6 +27,9 @@ #include #include #include +#include +#include +#include #if defined(__FreeBSD__) || defined(__OpenBSD__) #include @@ -327,9 +330,12 @@ bool TCP::Connect() if (attempt < connectRetries) { // backoff before retrying - int backoff = backoffBaseMs * (1 << (attempt - 1)); - LOGF_DEBUG("Waiting %d ms before next connect attempt.", backoff); - usleep(static_cast(backoff * 1000)); + int shift = std::min(attempt - 1, 30); + long long backoff_ms = static_cast(backoffBaseMs) * (1LL << shift); + // Clamp to maximum + backoff_ms = std::min(backoff_ms, MAX_BACKOFF_DELAY); + LOGF_DEBUG("Waiting %lld ms before next connect attempt.", backoff_ms); + std::this_thread::sleep_for(std::chrono::milliseconds(backoff_ms)); } } From 6a77a58eecc765592ebf2dfb589812c514d1ab30 Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 18:14:40 +0200 Subject: [PATCH 05/12] Add validation/clamping for config-loaded retry/backoff values in TCP constructor. --- libs/indibase/connectionplugins/connectiontcp.cpp | 14 ++++++++++++-- libs/indibase/connectionplugins/connectiontcp.h | 1 + 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index d29eb2666c..7dd4a1732b 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -81,9 +81,9 @@ TCP::TCP(INDI::DefaultDevice *dev, IPerm permission) : Interface(dev, CONNECTION // Retry/backoff configuration: number vector property // Default values - IUFillNumber(&RetryN[TCP::RETRY_RETRIES], "CONNECT_RETRIES", "Connection retries", "%.0f", 0, 100, 1, + IUFillNumber(&RetryN[TCP::RETRY_RETRIES], "CONNECT_RETRIES", "Connection retries", "%.0f", 0, TCP::MAX_CONNECT_RETRIES, 1, static_cast(m_ConnectRetries)); - IUFillNumber(&RetryN[TCP::RETRY_BACKOFF_MS], "BACKOFF_BASE_MS", "Backoff base (ms)", "%.0f", 0, 60000, 1, + IUFillNumber(&RetryN[TCP::RETRY_BACKOFF_MS], "BACKOFF_BASE_MS", "Backoff base (ms)", "%.0f", 0, TCP::MAX_BACKOFF_DELAY, 1, static_cast(m_BackoffBaseMs)); IUFillNumberVector(&RetryNP, RetryN, 2, getDeviceName(), "CONNECTION_RETRY", "Connection Retry", CONNECTION_TAB, IP_RW, 60, IPS_IDLE); @@ -92,11 +92,15 @@ TCP::TCP(INDI::DefaultDevice *dev, IPerm permission) : Interface(dev, CONNECTION double dval = 0; if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_RETRIES].name, &dval) == 0) { + // Clamp CONNECT_RETRIES to valid range [0, 100] + dval = std::max(0.0, std::min(dval, static_cast(TCP::MAX_CONNECT_RETRIES))); RetryN[TCP::RETRY_RETRIES].value = dval; m_ConnectRetries = static_cast(dval); } if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_BACKOFF_MS].name, &dval) == 0) { + // Clamp BACKOFF_BASE_MS to valid range [0, 60000] + dval = std::max(0.0, std::min(dval, static_cast(TCP::MAX_BACKOFF_DELAY))); RetryN[TCP::RETRY_BACKOFF_MS].value = dval; m_BackoffBaseMs = static_cast(dval); } @@ -177,6 +181,12 @@ bool TCP::ISNewNumber(const char *dev, const char *name, double values[], char * IUUpdateNumber(&RetryNP, values, names, n); RetryNP.s = IPS_OK; + // Validate and clamp CONNECT_RETRIES to [0, 100] + RetryN[TCP::RETRY_RETRIES].value = std::max(0.0, std::min(RetryN[TCP::RETRY_RETRIES].value, 100.0)); + // Validate and clamp BACKOFF_BASE_MS to [0, 60000] + RetryN[TCP::RETRY_BACKOFF_MS].value = std::max(0.0, std::min(RetryN[TCP::RETRY_BACKOFF_MS].value, + static_cast(TCP::MAX_BACKOFF_DELAY))); + // Update runtime values m_ConnectRetries = static_cast(RetryN[TCP::RETRY_RETRIES].value); m_BackoffBaseMs = static_cast(RetryN[TCP::RETRY_BACKOFF_MS].value); diff --git a/libs/indibase/connectionplugins/connectiontcp.h b/libs/indibase/connectionplugins/connectiontcp.h index bce680e512..932c4663b0 100644 --- a/libs/indibase/connectionplugins/connectiontcp.h +++ b/libs/indibase/connectionplugins/connectiontcp.h @@ -38,6 +38,7 @@ class TCP : public Interface { public: static const long long MAX_BACKOFF_DELAY = 60000LL; + static const int MAX_CONNECT_RETRIES = 100; enum ConnectionType { TYPE_TCP = 0, From f88d80c17972f274f138f9e4fe44659a6cb2553a Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 18:15:57 +0200 Subject: [PATCH 06/12] Fix TCP::Disconnect to close sockets for m_SockFD == 0 by using >=0 or != -1. --- libs/indibase/connectionplugins/connectiontcp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index 7dd4a1732b..fa26ce2d41 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -445,7 +445,7 @@ bool TCP::Connect() ////////////////////////////////////////////////////////////////////////////////////////////////// bool TCP::Disconnect() { - if (m_SockFD > 0) + if (m_SockFD != -1) { close(m_SockFD); m_SockFD = PortFD = -1; From c7af5531ea30dc3158096893399ac4231c09cfeb Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 18:59:34 +0200 Subject: [PATCH 07/12] Refactor TCP::Connect retry backoff to avoid long blocking usleep or cap total retry time. --- .../connectionplugins/connectiontcp.cpp | 43 +++++++++++++------ .../connectionplugins/connectiontcp.h | 14 ++++-- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index fa26ce2d41..231190ee53 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -83,7 +83,7 @@ TCP::TCP(INDI::DefaultDevice *dev, IPerm permission) : Interface(dev, CONNECTION // Default values IUFillNumber(&RetryN[TCP::RETRY_RETRIES], "CONNECT_RETRIES", "Connection retries", "%.0f", 0, TCP::MAX_CONNECT_RETRIES, 1, static_cast(m_ConnectRetries)); - IUFillNumber(&RetryN[TCP::RETRY_BACKOFF_MS], "BACKOFF_BASE_MS", "Backoff base (ms)", "%.0f", 0, TCP::MAX_BACKOFF_DELAY, 1, + IUFillNumber(&RetryN[TCP::RETRY_BACKOFF_MS], "BACKOFF_BASE_MS", "Backoff base (ms)", "%.0f", 0, TCP::MAX_BACKOFF_BASE_DELAY, 1, static_cast(m_BackoffBaseMs)); IUFillNumberVector(&RetryNP, RetryN, 2, getDeviceName(), "CONNECTION_RETRY", "Connection Retry", CONNECTION_TAB, IP_RW, 60, IPS_IDLE); @@ -100,7 +100,7 @@ TCP::TCP(INDI::DefaultDevice *dev, IPerm permission) : Interface(dev, CONNECTION if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_BACKOFF_MS].name, &dval) == 0) { // Clamp BACKOFF_BASE_MS to valid range [0, 60000] - dval = std::max(0.0, std::min(dval, static_cast(TCP::MAX_BACKOFF_DELAY))); + dval = std::max(0.0, std::min(dval, static_cast(TCP::MAX_BACKOFF_BASE_DELAY))); RetryN[TCP::RETRY_BACKOFF_MS].value = dval; m_BackoffBaseMs = static_cast(dval); } @@ -182,17 +182,17 @@ bool TCP::ISNewNumber(const char *dev, const char *name, double values[], char * RetryNP.s = IPS_OK; // Validate and clamp CONNECT_RETRIES to [0, 100] - RetryN[TCP::RETRY_RETRIES].value = std::max(0.0, std::min(RetryN[TCP::RETRY_RETRIES].value, 100.0)); + RetryN[TCP::RETRY_RETRIES].value = std::max(0.0, std::min(RetryN[TCP::RETRY_RETRIES].value, TCP::MAX_CONNECT_RETRIES)); // Validate and clamp BACKOFF_BASE_MS to [0, 60000] RetryN[TCP::RETRY_BACKOFF_MS].value = std::max(0.0, std::min(RetryN[TCP::RETRY_BACKOFF_MS].value, - static_cast(TCP::MAX_BACKOFF_DELAY))); + static_cast(TCP::MAX_BACKOFF_BASE_DELAY))); // Update runtime values m_ConnectRetries = static_cast(RetryN[TCP::RETRY_RETRIES].value); m_BackoffBaseMs = static_cast(RetryN[TCP::RETRY_BACKOFF_MS].value); - // Persist the change - m_Device->saveConfig(true, RetryNP.name); + // Mark config as dirty; actual save will be deferred + m_RetryConfigDirty = true; // Log the new configuration for visibility LOGF_INFO("Connection retry configuration updated: CONNECT_RETRIES=%d, BACKOFF_BASE_MS=%d", @@ -308,7 +308,10 @@ bool TCP::Connect() const auto isIPv4 = regex_match(hostname, ipv4); // Try a small number of retries on the requested address before falling back to LAN search - for (int attempt = 1; attempt <= connectRetries; ++attempt) + auto start_time = std::chrono::steady_clock::now(); + long long total_elapsed_ms = 0; + + for (int attempt = 1; attempt <= connectRetries && total_elapsed_ms < MAX_TOTAL_RETRY_TIME_MS; ++attempt) { if (establishConnection(hostname, port)) { @@ -337,15 +340,29 @@ bool TCP::Connect() LOGF_DEBUG("Connection attempt %d/%d to %s@%s failed.", attempt, connectRetries, hostname.c_str(), port.c_str()); } - if (attempt < connectRetries) + if (attempt < connectRetries && total_elapsed_ms < MAX_TOTAL_RETRY_TIME_MS) { - // backoff before retrying - int shift = std::min(attempt - 1, 30); + // Calculate backoff with capped exponential growth + int shift = std::min(attempt - 1, MAX_CONNECT_RETRIES); long long backoff_ms = static_cast(backoffBaseMs) * (1LL << shift); - // Clamp to maximum + // Cap individual backoff backoff_ms = std::min(backoff_ms, MAX_BACKOFF_DELAY); - LOGF_DEBUG("Waiting %lld ms before next connect attempt.", backoff_ms); - std::this_thread::sleep_for(std::chrono::milliseconds(backoff_ms)); + + // Ensure we don't exceed total retry time limit + long long remaining_time_ms = MAX_TOTAL_RETRY_TIME_MS - total_elapsed_ms; + backoff_ms = std::min(backoff_ms, remaining_time_ms); + + if (backoff_ms > 0) + { + LOGF_DEBUG("Waiting %lld ms before next connect attempt (total elapsed: %lld ms).", backoff_ms, total_elapsed_ms); + std::this_thread::sleep_for(std::chrono::milliseconds(backoff_ms)); + total_elapsed_ms += backoff_ms; + } + else + { + LOGF_DEBUG("Skipping backoff - total retry time limit reached."); + break; + } } } diff --git a/libs/indibase/connectionplugins/connectiontcp.h b/libs/indibase/connectionplugins/connectiontcp.h index 932c4663b0..6b5b3b6610 100644 --- a/libs/indibase/connectionplugins/connectiontcp.h +++ b/libs/indibase/connectionplugins/connectiontcp.h @@ -37,8 +37,14 @@ namespace Connection class TCP : public Interface { public: - static const long long MAX_BACKOFF_DELAY = 60000LL; - static const int MAX_CONNECT_RETRIES = 100; + // Max base delay for the exponential backoff retry + static const long long MAX_BACKOFF_BASE_DELAY = 10000LL; + // Max delay to wait for the next backoff retry + static const long long MAX_BACKOFF_DELAY = 30000LL; + // Max retry time + static const long long MAX_TOTAL_RETRY_TIME_MS = 300000LL; // 5 minutes + + static const int MAX_CONNECT_RETRIES = 10; enum ConnectionType { TYPE_TCP = 0, @@ -130,13 +136,13 @@ class TCP : public Interface std::string m_ConfigHost; std::string m_ConfigPort; int m_ConfigConnectionType {-1}; - int m_ConfigConnectRetries {-1}; - int m_ConfigBackoffBaseMs {-1}; int m_SockFD {-1}; int PortFD = -1; static constexpr uint8_t SOCKET_TIMEOUT {5}; // Runtime-configurable connection retry parameters int m_ConnectRetries {3}; int m_BackoffBaseMs {500}; // milliseconds + // Flag to defer config persistence (avoid frequent saveConfig calls) + bool m_RetryConfigDirty {false}; }; } From e0a31cf3a867db97827d6b2747f9ec809f650552 Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 19:15:32 +0200 Subject: [PATCH 08/12] Remove redundant and confusing constant casts. --- libs/indibase/connectionplugins/connectiontcp.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index 231190ee53..638a236aa1 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -93,14 +93,14 @@ TCP::TCP(INDI::DefaultDevice *dev, IPerm permission) : Interface(dev, CONNECTION if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_RETRIES].name, &dval) == 0) { // Clamp CONNECT_RETRIES to valid range [0, 100] - dval = std::max(0.0, std::min(dval, static_cast(TCP::MAX_CONNECT_RETRIES))); + dval = std::max(0.0, std::min(dval, TCP::MAX_CONNECT_RETRIES)); RetryN[TCP::RETRY_RETRIES].value = dval; m_ConnectRetries = static_cast(dval); } if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_BACKOFF_MS].name, &dval) == 0) { // Clamp BACKOFF_BASE_MS to valid range [0, 60000] - dval = std::max(0.0, std::min(dval, static_cast(TCP::MAX_BACKOFF_BASE_DELAY))); + dval = std::max(0.0, std::min(dval, TCP::MAX_BACKOFF_BASE_DELAY)); RetryN[TCP::RETRY_BACKOFF_MS].value = dval; m_BackoffBaseMs = static_cast(dval); } From 74bbb28867a7dc41002703c9618fe6da44772042 Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 19:19:54 +0200 Subject: [PATCH 09/12] Remove dirty config flgs. --- libs/indibase/connectionplugins/connectiontcp.cpp | 4 ++-- libs/indibase/connectionplugins/connectiontcp.h | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index 638a236aa1..e2a24e7625 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -191,8 +191,8 @@ bool TCP::ISNewNumber(const char *dev, const char *name, double values[], char * m_ConnectRetries = static_cast(RetryN[TCP::RETRY_RETRIES].value); m_BackoffBaseMs = static_cast(RetryN[TCP::RETRY_BACKOFF_MS].value); - // Mark config as dirty; actual save will be deferred - m_RetryConfigDirty = true; + // Persist the change + m_Device->saveConfig(true, RetryNP.name); // Log the new configuration for visibility LOGF_INFO("Connection retry configuration updated: CONNECT_RETRIES=%d, BACKOFF_BASE_MS=%d", diff --git a/libs/indibase/connectionplugins/connectiontcp.h b/libs/indibase/connectionplugins/connectiontcp.h index 6b5b3b6610..e6328861b8 100644 --- a/libs/indibase/connectionplugins/connectiontcp.h +++ b/libs/indibase/connectionplugins/connectiontcp.h @@ -141,8 +141,6 @@ class TCP : public Interface static constexpr uint8_t SOCKET_TIMEOUT {5}; // Runtime-configurable connection retry parameters int m_ConnectRetries {3}; - int m_BackoffBaseMs {500}; // milliseconds - // Flag to defer config persistence (avoid frequent saveConfig calls) - bool m_RetryConfigDirty {false}; -}; + int m_BackoffBaseMs {500}; // milliseconds}; + }; } From 18bf8c7f45e78c782701404208e781c721eb8352 Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 19:32:42 +0200 Subject: [PATCH 10/12] Fix elapsed time tracking in TCP::Connect to include connection/handshake time, not just sleep. --- .../connectionplugins/connectiontcp.cpp | 29 +++++++++++++++---- .../connectionplugins/connectiontcp.h | 3 +- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index e2a24e7625..bc23884eed 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -308,12 +308,25 @@ bool TCP::Connect() const auto isIPv4 = regex_match(hostname, ipv4); // Try a small number of retries on the requested address before falling back to LAN search - auto start_time = std::chrono::steady_clock::now(); - long long total_elapsed_ms = 0; + auto retry_start_time = std::chrono::steady_clock::now(); - for (int attempt = 1; attempt <= connectRetries && total_elapsed_ms < MAX_TOTAL_RETRY_TIME_MS; ++attempt) + for (int attempt = 1; attempt <= connectRetries; ++attempt) { - if (establishConnection(hostname, port)) + // Check if we've exceeded total retry time before starting this attempt + auto attempt_start = std::chrono::steady_clock::now(); + long long total_elapsed_ms = std::chrono::duration_cast( + attempt_start - retry_start_time).count(); + + if (total_elapsed_ms >= MAX_TOTAL_RETRY_TIME_MS) + { + LOGF_DEBUG("Total retry time limit (%lld ms) exceeded, stopping retry loop.", MAX_TOTAL_RETRY_TIME_MS); + break; + } + + // Attempt connection + bool connection_succeeded = establishConnection(hostname, port); + + if (connection_succeeded) { PortFD = m_SockFD; LOGF_DEBUG("Connection to %s@%s is successful, attempting handshake...", hostname.c_str(), port.c_str()); @@ -340,10 +353,15 @@ bool TCP::Connect() LOGF_DEBUG("Connection attempt %d/%d to %s@%s failed.", attempt, connectRetries, hostname.c_str(), port.c_str()); } + // Calculate elapsed time including connection/handshake attempt + auto attempt_end = std::chrono::steady_clock::now(); + total_elapsed_ms = std::chrono::duration_cast( + attempt_end - retry_start_time).count(); + if (attempt < connectRetries && total_elapsed_ms < MAX_TOTAL_RETRY_TIME_MS) { // Calculate backoff with capped exponential growth - int shift = std::min(attempt - 1, MAX_CONNECT_RETRIES); + int shift = std::min(attempt - 1, MAX_BACKOFF_SHIFT); long long backoff_ms = static_cast(backoffBaseMs) * (1LL << shift); // Cap individual backoff backoff_ms = std::min(backoff_ms, MAX_BACKOFF_DELAY); @@ -356,7 +374,6 @@ bool TCP::Connect() { LOGF_DEBUG("Waiting %lld ms before next connect attempt (total elapsed: %lld ms).", backoff_ms, total_elapsed_ms); std::this_thread::sleep_for(std::chrono::milliseconds(backoff_ms)); - total_elapsed_ms += backoff_ms; } else { diff --git a/libs/indibase/connectionplugins/connectiontcp.h b/libs/indibase/connectionplugins/connectiontcp.h index e6328861b8..1688ee651a 100644 --- a/libs/indibase/connectionplugins/connectiontcp.h +++ b/libs/indibase/connectionplugins/connectiontcp.h @@ -44,7 +44,8 @@ class TCP : public Interface // Max retry time static const long long MAX_TOTAL_RETRY_TIME_MS = 300000LL; // 5 minutes - static const int MAX_CONNECT_RETRIES = 10; + static constexpr double MAX_CONNECT_RETRIES = 10; + static constexpr double MAX_BACKOFF_SHIFT = 10; enum ConnectionType { TYPE_TCP = 0, From b0a967d8d2b4a088afaa226ad9eb67ac429ea8a7 Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 23:35:16 +0200 Subject: [PATCH 11/12] Fix types on std::min function --- libs/indibase/connectionplugins/connectiontcp.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index bc23884eed..18a2b5c75d 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -92,15 +92,15 @@ TCP::TCP(INDI::DefaultDevice *dev, IPerm permission) : Interface(dev, CONNECTION double dval = 0; if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_RETRIES].name, &dval) == 0) { - // Clamp CONNECT_RETRIES to valid range [0, 100] + // Clamp CONNECT_RETRIES to valid range dval = std::max(0.0, std::min(dval, TCP::MAX_CONNECT_RETRIES)); RetryN[TCP::RETRY_RETRIES].value = dval; m_ConnectRetries = static_cast(dval); } if (IUGetConfigNumber(dev->getDeviceName(), RetryNP.name, RetryN[TCP::RETRY_BACKOFF_MS].name, &dval) == 0) { - // Clamp BACKOFF_BASE_MS to valid range [0, 60000] - dval = std::max(0.0, std::min(dval, TCP::MAX_BACKOFF_BASE_DELAY)); + // Clamp BACKOFF_BASE_MS to valid range + dval = std::max(0.0, std::min(dval, static_cast(TCP::MAX_BACKOFF_BASE_DELAY))); RetryN[TCP::RETRY_BACKOFF_MS].value = dval; m_BackoffBaseMs = static_cast(dval); } @@ -181,9 +181,9 @@ bool TCP::ISNewNumber(const char *dev, const char *name, double values[], char * IUUpdateNumber(&RetryNP, values, names, n); RetryNP.s = IPS_OK; - // Validate and clamp CONNECT_RETRIES to [0, 100] + // Validate and clamp CONNECT_RETRIES to valid range RetryN[TCP::RETRY_RETRIES].value = std::max(0.0, std::min(RetryN[TCP::RETRY_RETRIES].value, TCP::MAX_CONNECT_RETRIES)); - // Validate and clamp BACKOFF_BASE_MS to [0, 60000] + // Validate and clamp BACKOFF_BASE_MS to valid range RetryN[TCP::RETRY_BACKOFF_MS].value = std::max(0.0, std::min(RetryN[TCP::RETRY_BACKOFF_MS].value, static_cast(TCP::MAX_BACKOFF_BASE_DELAY))); @@ -361,10 +361,10 @@ bool TCP::Connect() if (attempt < connectRetries && total_elapsed_ms < MAX_TOTAL_RETRY_TIME_MS) { // Calculate backoff with capped exponential growth - int shift = std::min(attempt - 1, MAX_BACKOFF_SHIFT); + int shift = std::min(attempt - 1, static_cast(MAX_BACKOFF_SHIFT)); long long backoff_ms = static_cast(backoffBaseMs) * (1LL << shift); // Cap individual backoff - backoff_ms = std::min(backoff_ms, MAX_BACKOFF_DELAY); + backoff_ms = std::min(backoff_ms, static_cast(MAX_BACKOFF_DELAY)); // Ensure we don't exceed total retry time limit long long remaining_time_ms = MAX_TOTAL_RETRY_TIME_MS - total_elapsed_ms; From d561deede10a6118f70cb8738ef60f1754fd4159 Mon Sep 17 00:00:00 2001 From: Orestes Sanchez Benavente Date: Sun, 10 May 2026 23:39:46 +0200 Subject: [PATCH 12/12] Use appropiate logger macro --- libs/indibase/connectionplugins/connectiontcp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/indibase/connectionplugins/connectiontcp.cpp b/libs/indibase/connectionplugins/connectiontcp.cpp index 18a2b5c75d..4bd6f814a5 100644 --- a/libs/indibase/connectionplugins/connectiontcp.cpp +++ b/libs/indibase/connectionplugins/connectiontcp.cpp @@ -377,7 +377,7 @@ bool TCP::Connect() } else { - LOGF_DEBUG("Skipping backoff - total retry time limit reached."); + LOG_DEBUG("Skipping backoff - total retry time limit reached."); break; } }