Keep Alive timeout with auto reconnect by LucHeart · Pull Request #469 · OpenShock/Firmware

LucHeart · 2026-05-25T15:24:17Z

Our ping pong keep alive should have a timeout timer, which forces a reconnect if the backend websocket stops sending pings, which would indicate a connection failure.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7d7abd1566

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions · 2026-05-25T15:27:22Z

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-format (v21.1.8) reports: 2 file(s) not formatted

src/GatewayClient.cpp
src/GatewayConnectionManager.cpp

clang-tidy (v21.1.8) reports: 55 concern(s)

include/GatewayClient.h:1:1: warning: [portability-avoid-pragma-once]

avoid 'pragma once' directive; use include guards instead
```
    1 | #pragma once
      | ^
```

include/GatewayClient.h:16:5: warning: [modernize-use-trailing-return-type]

   16 |     DISABLE_COPY(GatewayClient);
      |     ^
/home/runner/work/Firmware/Firmware/include/Common.h:9:13: note: expanded from macro 'DISABLE_COPY'
    9 |   TypeName& operator=(const TypeName&) = delete
      |             ^

include/GatewayClient.h:17:5: warning: [modernize-use-trailing-return-type]

   17 |     DISABLE_MOVE(GatewayClient);
      |     ^
/home/runner/work/Firmware/Firmware/include/Common.h:12:13: note: expanded from macro 'DISABLE_MOVE'
   12 |   TypeName& operator=(TypeName&&) = delete
      |             ^

include/GatewayClient.h:23:5: warning: [modernize-use-nodiscard]

   23 |     inline GatewayClientState state() const { return m_state; }
      |     ^
      |     [[nodiscard]]

include/GatewayClient.h:23:5: warning: [readability-redundant-inline-specifier]

function 'state' has inline specifier but is implicitly inlined
```
   23 |     inline GatewayClientState state() const { return m_state; }
      |     ^~~~~~
```

include/GatewayClient.h:23:31: warning: [modernize-use-trailing-return-type]

   23 |     inline GatewayClientState state() const { return m_state; }
      |            ~~~~~~~~~~~~~~~~~~ ^
      |            auto                             -> GatewayClientState

include/GatewayClient.h:28:10: warning: [modernize-use-trailing-return-type]

   28 |     bool sendMessageTXT(std::string_view data);
      |     ~~~~ ^                                    
      |     auto                                       -> bool

include/GatewayClient.h:29:10: warning: [modernize-use-trailing-return-type]

   29 |     bool sendMessageBIN(tcb::span<const uint8_t> data);
      |     ~~~~ ^                                            
      |     auto                                               -> bool

include/GatewayClient.h:33:10: warning: [modernize-use-trailing-return-type]

use a trailing return type for this function
```
   33 |     bool loop();
      |     ~~~~ ^     
      |     auto        -> bool
```
include/GatewayConnectionManager.h:1:1: warning: [portability-avoid-pragma-once]

avoid 'pragma once' directive; use include guards instead
```
    1 | #pragma once
      | ^
```

include/GatewayConnectionManager.h:11:22: warning: [modernize-use-trailing-return-type]

   11 |   [[nodiscard]] bool Init();
      |                 ~~~~ ^     
      |                 auto        -> bool

include/GatewayConnectionManager.h:13:8: warning: [modernize-use-trailing-return-type]

use a trailing return type for this function
```
   13 |   bool IsConnected();
      |   ~~~~ ^            
      |   auto               -> bool
```
include/GatewayConnectionManager.h:15:8: warning: [modernize-use-trailing-return-type]

use a trailing return type for this function
```
   15 |   bool IsLinked();
      |   ~~~~ ^         
      |   auto            -> bool
```

include/GatewayConnectionManager.h:16:25: warning: [modernize-use-trailing-return-type]

   16 |   AccountLinkResultCode Link(std::string_view linkCode);
      |   ~~~~~~~~~~~~~~~~~~~~~ ^                              
      |   auto                                                  -> AccountLinkResultCode

include/GatewayConnectionManager.h:19:8: warning: [modernize-use-trailing-return-type]

   19 |   bool SendMessageTXT(std::string_view data);
      |   ~~~~ ^                                    
      |   auto                                       -> bool

include/GatewayConnectionManager.h:20:8: warning: [modernize-use-trailing-return-type]

   20 |   bool SendMessageBIN(tcb::span<const uint8_t> data);
      |   ~~~~ ^                                            
      |   auto                                               -> bool

src/GatewayClient.cpp:19:13: warning: [cppcoreguidelines-avoid-non-const-global-variables]

variable 's_bootStatusSent' is non-const and globally accessible, consider making it const
```
   19 | static bool s_bootStatusSent = false;
      |             ^
```
src/GatewayClient.cpp:21:1: warning: [cppcoreguidelines-pro-type-member-init]

constructor does not initialize these fields: m_lastPingTimestamp
```
   21 | GatewayClient::GatewayClient(const std::string& authToken)
      | ^
```

src/GatewayClient.cpp:44:29: warning: [bugprone-easily-swappable-parameters]

   44 | void GatewayClient::connect(const std::string& host, uint16_t port, const std::string& path)
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/GatewayClient.cpp:44:48: note: the first parameter in the range is 'host'
   44 | void GatewayClient::connect(const std::string& host, uint16_t port, const std::string& path)
      |                                                ^~~~
src/GatewayClient.cpp:44:88: note: the last parameter in the range is 'path'
   44 | void GatewayClient::connect(const std::string& host, uint16_t port, const std::string& path)
      |                                                                                        ^~~~
src/GatewayClient.cpp:44:54: note: 'const int &' and 'int' parameters accept and bind the same kind of values
   44 | void GatewayClient::connect(const std::string& host, uint16_t port, const std::string& path)
      |                                                      ^

src/GatewayClient.cpp:78:21: warning: [modernize-use-trailing-return-type]

   78 | bool GatewayClient::sendMessageTXT(std::string_view data)
      | ~~~~                ^                                    
      | auto                                                      -> bool

src/GatewayClient.cpp:87:21: warning: [modernize-use-trailing-return-type]

   87 | bool GatewayClient::sendMessageBIN(tcb::span<const uint8_t> data)
      | ~~~~                ^                                            
      | auto                                                              -> bool

src/GatewayClient.cpp:101:21: warning: [modernize-use-trailing-return-type]

use a trailing return type for this function
```
  101 | bool GatewayClient::loop()
      | ~~~~                ^     
      | auto                       -> bool
```

src/GatewayClient.cpp:133:3: warning: [cppcoreguidelines-avoid-do-while]

  133 |   ESP_ERROR_CHECK(esp_event_post(OPENSHOCK_EVENTS, OPENSHOCK_EVENT_GATEWAY_CLIENT_STATE_CHANGED, &m_state, sizeof(m_state), portMAX_DELAY));
      |   ^
/home/runner/.platformio/packages/framework-arduinoespressif32/tools/sdk/esp32s3/include/esp_common/include/esp_err.h:115:28: note: expanded from macro 'ESP_ERROR_CHECK'
  115 | #define ESP_ERROR_CHECK(x) do {                                         \
      |                            ^

src/GatewayClient.cpp:138:24: warning: [readability-braces-around-statements]

  138 |   if (s_bootStatusSent) return;
      |                        ^       
      |                         {

src/GatewayClient.cpp:148:28: warning: [cppcoreguidelines-init-variables]

variable 'updateStep' is not initialized
```
  148 |   OpenShock::OtaUpdateStep updateStep;
      |                            ^
```

src/GatewayClient.cpp:162:124: warning: [modernize-use-trailing-return-type]

  162 |   s_bootStatusSent = Serialization::Gateway::SerializeBootStatusMessage(updateId, OtaUpdateManager::GetFirmwareBootType(), [this](tcb::span<const uint8_t> data) { return m_webSocket.sendBIN(data.data(), data.size()); });
      |                                                                                                                            ^
      |                                                                                                                                                                  -> void

src/GatewayConnectionManager.cpp:43:29: warning: [cppcoreguidelines-avoid-non-const-global-variables]

variable 's_flags' is non-const and globally accessible, consider making it const
```
   43 | static std::atomic<uint8_t> s_flags                 = 0;
      |                             ^
```
src/GatewayConnectionManager.cpp:44:29: warning: [cppcoreguidelines-avoid-non-const-global-variables]

variable 's_lastAuthFailure' is non-const and globally accessible, consider making it const
```
   44 | static std::atomic<int64_t> s_lastAuthFailure       = 0;
      |                             ^
```
src/GatewayConnectionManager.cpp:45:29: warning: [cppcoreguidelines-avoid-non-const-global-variables]

variable 's_lastConnectionAttempt' is non-const and globally accessible, consider making it const
```
   45 | static std::atomic<int64_t> s_lastConnectionAttempt = 0;
      |                             ^
```
src/GatewayConnectionManager.cpp:46:25: warning: [cppcoreguidelines-avoid-non-const-global-variables]

variable 's_isInitializing' is non-const and globally accessible, consider making it const
```
   46 | static std::atomic_flag s_isInitializing            = ATOMIC_FLAG_INIT;
      |                         ^
```
src/GatewayConnectionManager.cpp:47:31: warning: [cppcoreguidelines-avoid-non-const-global-variables]

variable 's_clientMutex' is non-const and globally accessible, consider making it const
```
   47 | static OpenShock::SimpleMutex s_clientMutex;
      |                               ^
```
src/GatewayConnectionManager.cpp:48:50: warning: [cppcoreguidelines-avoid-non-const-global-variables]

variable 's_wsClient' is non-const and globally accessible, consider making it const
```
   48 | static std::shared_ptr<OpenShock::GatewayClient> s_wsClient = nullptr;
      |                                                  ^
```

src/GatewayConnectionManager.cpp:50:50: warning: [modernize-use-trailing-return-type]

   50 | static std::shared_ptr<OpenShock::GatewayClient> GetClient()
      |                                                  ^

src/GatewayConnectionManager.cpp:52:25: warning: [bugprone-reserved-identifier]

declaration uses identifier 'lock__', which is a reserved identifier
```
   52 |   OpenShock::ScopedLock lock__(&s_clientMutex);
      |                         ^~~~~~
      |                         lock_
```
src/GatewayConnectionManager.cpp:57:25: warning: [bugprone-reserved-identifier]

declaration uses identifier 'lock__', which is a reserved identifier
```
   57 |   OpenShock::ScopedLock lock__(&s_clientMutex);
      |                         ^~~~~~
      |                         lock_
```
src/GatewayConnectionManager.cpp:62:25: warning: [bugprone-reserved-identifier]

declaration uses identifier 'lock__', which is a reserved identifier
```
   62 |   OpenShock::ScopedLock lock__(&s_clientMutex);
      |                         ^~~~~~
      |                         lock_
```

src/GatewayConnectionManager.cpp:83:13: warning: [modernize-use-trailing-return-type]

   83 | static bool checkIsDeAuthRateLimited(int64_t millis)
      |        ~~~~ ^                                       
      |        auto                                          -> bool

src/GatewayConnectionManager.cpp:85:67: warning: [cppcoreguidelines-avoid-magic-numbers]

   85 |   return s_lastAuthFailure != 0 && (millis - s_lastAuthFailure) < 300'000;  // 5 Minutes
      |                                                                   ^

src/GatewayConnectionManager.cpp:87:13: warning: [modernize-use-trailing-return-type]

   87 | static bool checkIsConnectionRateLimited(int64_t millis)
      |        ~~~~ ^                                           
      |        auto                                              -> bool

src/GatewayConnectionManager.cpp:89:79: warning: [cppcoreguidelines-avoid-magic-numbers]

   89 |   return s_lastConnectionAttempt != 0 && (millis - s_lastConnectionAttempt) < 20'000;  // 20 seconds
      |                                                                               ^

src/GatewayConnectionManager.cpp:95:32: warning: [modernize-use-trailing-return-type]

   95 | bool GatewayConnectionManager::Init()
      | ~~~~                           ^     
      | auto                                  -> bool

src/GatewayConnectionManager.cpp:104:32: warning: [modernize-use-trailing-return-type]

  104 | bool GatewayConnectionManager::IsConnected()
      | ~~~~                           ^            
      | auto                                         -> bool

src/GatewayConnectionManager.cpp:114:32: warning: [modernize-use-trailing-return-type]

  114 | bool GatewayConnectionManager::IsLinked()
      | ~~~~                           ^         
      | auto                                      -> bool

src/GatewayConnectionManager.cpp:119:49: warning: [modernize-use-trailing-return-type]

  119 | AccountLinkResultCode GatewayConnectionManager::Link(std::string_view linkCode)
      | ~~~~~~~~~~~~~~~~~~~~~                           ^                              
      | auto                                                                            -> AccountLinkResultCode

src/GatewayConnectionManager.cpp:136:24: warning: [cppcoreguidelines-avoid-magic-numbers]

404 is a magic number; consider replacing it with a named constant
```
  136 |   if (response.code == 404) {
      |                        ^
```
src/GatewayConnectionManager.cpp:150:24: warning: [cppcoreguidelines-avoid-magic-numbers]

200 is a magic number; consider replacing it with a named constant
```
  150 |   if (response.code != 200) {
      |                        ^
```

src/GatewayConnectionManager.cpp:177:32: warning: [modernize-use-trailing-return-type]

  177 | bool GatewayConnectionManager::SendMessageTXT(std::string_view data)
      | ~~~~                           ^                                    
      | auto                                                                 -> bool

src/GatewayConnectionManager.cpp:187:32: warning: [modernize-use-trailing-return-type]

  187 | bool GatewayConnectionManager::SendMessageBIN(tcb::span<const uint8_t> data)
      | ~~~~                           ^                                            
      | auto                                                                         -> bool

src/GatewayConnectionManager.cpp:207:6: warning: [modernize-use-trailing-return-type]

  207 | bool FetchHubInfo(std::string authToken)
      | ~~~~ ^                                  
      | auto                                     -> bool

src/GatewayConnectionManager.cpp:220:24: warning: [cppcoreguidelines-avoid-magic-numbers]

401 is a magic number; consider replacing it with a named constant
```
  220 |   if (response.code == 401) {
      |                        ^
```
src/GatewayConnectionManager.cpp:234:24: warning: [cppcoreguidelines-avoid-magic-numbers]

200 is a magic number; consider replacing it with a named constant
```
  234 |   if (response.code != 200) {
      |                        ^
```
src/GatewayConnectionManager.cpp:251:6: warning: [modernize-use-trailing-return-type]

use a trailing return type for this function
```
  251 | bool StartConnectingToLCG()
      | ~~~~ ^                     
      | auto                        -> bool
```
src/GatewayConnectionManager.cpp:284:24: warning: [cppcoreguidelines-avoid-magic-numbers]

401 is a magic number; consider replacing it with a named constant
```
  284 |   if (response.code == 401) {
      |                        ^
```
src/GatewayConnectionManager.cpp:298:24: warning: [cppcoreguidelines-avoid-magic-numbers]

200 is a magic number; consider replacing it with a named constant
```
  298 |   if (response.code != 200) {
      |                        ^
```
src/message_handlers/websocket/gateway/Ping.cpp:15:3: warning: [readability-qualified-auto]

'auto msg' can be declared as 'const auto *msg'
```
   15 |   auto msg = root->payload_as_Ping();
      |   ^~~~
      |   const auto *
```

Have any feedback or feature suggestions? Share it here.

implementation

7d7abd1

github-project-automation Bot added this to Roadmap May 25, 2026

github-project-automation Bot moved this to Todo in Roadmap May 25, 2026

LucHeart requested a review from hhvrc May 25, 2026 15:24

chatgpt-codex-connector Bot reviewed May 25, 2026

View reviewed changes

Comment thread src/GatewayClient.cpp

fix: timeout not resetting on connect

2882d0f

hhvrc previously approved these changes May 25, 2026

View reviewed changes

chore: add changeset

962b9ab

LucHeart dismissed hhvrc’s stale review via 962b9ab May 25, 2026 15:44

hhvrc approved these changes May 25, 2026

View reviewed changes

LucHeart merged commit a3433fa into develop May 25, 2026
38 checks passed

LucHeart deleted the feature/keepalive-timeout-reconnect branch May 25, 2026 16:01

github-project-automation Bot moved this from Todo to Done in Roadmap May 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep Alive timeout with auto reconnect#469

Keep Alive timeout with auto reconnect#469
LucHeart merged 3 commits into
developfrom
feature/keepalive-timeout-reconnect

LucHeart commented May 25, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented May 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

LucHeart commented May 25, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions Bot commented May 25, 2026

Cpp-Linter Report ⚠️

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants