feat: better account linking errors by LucHeart · Pull Request #484 · OpenShock/Firmware

LucHeart · 2026-06-28T14:58:30Z

stage-review · 2026-06-28T14:58:44Z

Ready to review this PR? Stage has broken it down into 4 individual chapters for you:

	Title
1	Define granular account linking result codes
2	Implement detailed error reporting in connection manager
3	Expose new error codes to the captive portal
4	Add frontend error messages and documentation

_{Chapters generated by Stage for commit 2f96b7f on Jun 28, 2026 6:18pm UTC.}

github-actions · 2026-06-28T14:58:53Z

Change file check

✅ OK — 1 valid change file(s) added in this PR.

github-actions · 2026-06-28T15:00:11Z

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

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

src/captiveportal/CaptivePortalInstance.cpp

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

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

avoid 'pragma once' directive; use include guards instead
```
    1 | #pragma once
      | ^
```
include/AccountLinkResultCode.h:3:10: error: [clang-diagnostic-error]

'cstdint' file not found
```
    3 | #include <cstdint>
      |          ^~~~~~~~~
```
include/AccountLinkResultCode.h:6:14: warning: [performance-enum-size]

enum 'AccountLinkResultCode' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size
```
    6 |   enum class AccountLinkResultCode : uint8_t {
      |              ^
```
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]

use a trailing return type for this function

   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]

use a trailing return type for this function

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

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

300'000 is a magic number; consider replacing it with a named constant

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

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

use a trailing return type for this function

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

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

20'000 is a magic number; consider replacing it with a named constant

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

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

use a trailing return type for this function

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

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

use a trailing return type for this function

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

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

use a trailing return type for this function

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

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

use a trailing return type for this function

  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:161:24: warning: [cppcoreguidelines-avoid-magic-numbers]

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

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

use a trailing return type for this function

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

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

use a trailing return type for this function

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

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

use a trailing return type for this function

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

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

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

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

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

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

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

src/captiveportal/CaptivePortalInstance.cpp:40:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   40 | static const char* const JSON_ERR_INTERNAL        = "{\"error\":\"InternalError\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"InternalError"})"

src/captiveportal/CaptivePortalInstance.cpp:41:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   41 | static const char* const JSON_ERR_MISSING_PARAM   = "{\"error\":\"MissingParam\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"MissingParam"})"

src/captiveportal/CaptivePortalInstance.cpp:42:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   42 | static const char* const JSON_ERR_INVALID_PIN     = "{\"error\":\"InvalidPin\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"InvalidPin"})"

src/captiveportal/CaptivePortalInstance.cpp:43:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   43 | static const char* const JSON_ERR_MISSING_SSID    = "{\"error\":\"MissingSsid\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"MissingSsid"})"

src/captiveportal/CaptivePortalInstance.cpp:44:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   44 | static const char* const JSON_ERR_INVALID_SSID    = "{\"error\":\"InvalidSsid\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"InvalidSsid"})"

src/captiveportal/CaptivePortalInstance.cpp:45:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   45 | static const char* const JSON_ERR_PASSWORD_SHORT  = "{\"error\":\"PasswordTooShort\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"PasswordTooShort"})"

src/captiveportal/CaptivePortalInstance.cpp:46:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   46 | static const char* const JSON_ERR_PASSWORD_LONG   = "{\"error\":\"PasswordTooLong\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"PasswordTooLong"})"

src/captiveportal/CaptivePortalInstance.cpp:47:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   47 | static const char* const JSON_ERR_CODE_REQUIRED   = "{\"error\":\"CodeRequired\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"CodeRequired"})"

src/captiveportal/CaptivePortalInstance.cpp:48:53: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   48 | static const char* const JSON_ERR_INVALID_CHANNEL = "{\"error\":\"InvalidChannel\"}";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     R"({"error":"InvalidChannel"})"

src/captiveportal/CaptivePortalInstance.cpp:49:54: warning: [modernize-raw-string-literal]

escaped string literal can be written as a raw string literal

   49 | static const char* const JSON_ERR_RATE_LIMITED     = "{\"error\":\"RateLimited\"}";
      |                                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                      R"({"error":"RateLimited"})"

src/captiveportal/CaptivePortalInstance.cpp:51:32: warning: [modernize-use-trailing-return-type]

use a trailing return type for this function

   51 | static OpenShock::RateLimiter& getAccountLinkRateLimiter()
      |        ~~~~~~~~~~~~~~~~~~~~~~~ ^                          
      |        auto                                                -> OpenShock::RateLimiter&

src/captiveportal/CaptivePortalInstance.cpp:53:34: warning: [readability-identifier-length]

variable name 'rl' is too short, expected at least 3 characters
```
   53 |   static OpenShock::RateLimiter* rl = nullptr;
      |                                  ^
```
src/captiveportal/CaptivePortalInstance.cpp:55:5: warning: [cppcoreguidelines-owning-memory]

assigning newly created 'gsl::owner<>' to non-owner 'OpenShock::RateLimiter *'
```
   55 |     rl = new OpenShock::RateLimiter();
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
src/captiveportal/CaptivePortalInstance.cpp:56:18: warning: [cppcoreguidelines-avoid-magic-numbers]

60'000 is a magic number; consider replacing it with a named constant
```
   56 |     rl->addLimit(60'000, 5);   // 5 attempts per minute
      |                  ^
```
src/captiveportal/CaptivePortalInstance.cpp:56:26: warning: [cppcoreguidelines-avoid-magic-numbers]

5 is a magic number; consider replacing it with a named constant
```
   56 |     rl->addLimit(60'000, 5);   // 5 attempts per minute
      |                          ^
```
src/captiveportal/CaptivePortalInstance.cpp:57:18: warning: [cppcoreguidelines-avoid-magic-numbers]

300'000 is a magic number; consider replacing it with a named constant
```
   57 |     rl->addLimit(300'000, 10); // 10 attempts per 5 minutes
      |                  ^
```
src/captiveportal/CaptivePortalInstance.cpp:57:27: warning: [cppcoreguidelines-avoid-magic-numbers]

10 is a magic number; consider replacing it with a named constant
```
   57 |     rl->addLimit(300'000, 10); // 10 attempts per 5 minutes
      |                           ^
```

src/captiveportal/CaptivePortalInstance.cpp:64:31: warning: [modernize-use-trailing-return-type]

use a trailing return type for this function

   64 | static const esp_partition_t* getStaticPartition()
      |        ~~~~~~~~~~~~~~~~~~~~~~ ^                   
      |        auto                                        -> const esp_partition_t*

src/captiveportal/CaptivePortalInstance.cpp:79:20: warning: [modernize-use-trailing-return-type]

use a trailing return type for this function

   79 | static const char* getPartitionHash()
      |        ~~~~~~~~~~~ ^                 
      |        auto                           -> const char*

src/captiveportal/CaptivePortalInstance.cpp:86:10: warning: [cppcoreguidelines-avoid-c-arrays]

do not declare C-style arrays, use 'std::array' instead
```
   86 |   static char hash[65];
      |          ^
```
src/captiveportal/CaptivePortalInstance.cpp:86:20: warning: [cppcoreguidelines-avoid-magic-numbers]

65 is a magic number; consider replacing it with a named constant
```
   86 |   static char hash[65];
      |                    ^
```
src/captiveportal/CaptivePortalInstance.cpp:91:10: warning: [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead
```
   91 |   return hash;
      |          ^
```
src/captiveportal/CaptivePortalInstance.cpp:98:5: warning: [readability-redundant-member-init]

initializer for member 'm_fileSystem' is redundant
```
   98 |   , m_fileSystem()
      |     ^~~~~~~~~~~~~~
```

src/captiveportal/CaptivePortalInstance.cpp:108:8: warning: [cppcoreguidelines-init-variables]

variable 'dnsStarted' is not initialized

  108 |   bool dnsStarted = m_dnsServer.start(DNS_PORT, "*", WiFi.softAPIP());
      |        ^                                      
      |                                                = false

src/captiveportal/CaptivePortalInstance.cpp:125:47: warning: [cppcoreguidelines-avoid-magic-numbers]

10U is a magic number; consider replacing it with a named constant
```
  125 |     if (!m_fileSystem.begin(false, "/static", 10U, "static0")) {
      |                                               ^
```

src/captiveportal/CaptivePortalInstance.cpp:519:105: warning: [cppcoreguidelines-avoid-magic-numbers]

8192 is a magic number; consider replacing it with a named constant

  519 |     if (TaskUtils::TaskCreateExpensive(Util::FnProxy<&CaptivePortal::CaptivePortalInstance::task>, TAG, 8192, this, 1, &m_taskHandle) != pdPASS) {  // PROFILED: 4-6KB stack usage
      |                                                                                                         ^

src/captiveportal/CaptivePortalInstance.cpp:538:44: warning: [readability-convert-member-functions-to-static]

method 'task' can be made static
```
  538 | void CaptivePortal::CaptivePortalInstance::task()
      |                                            ^
```

src/captiveportal/CaptivePortalInstance.cpp:548:44: warning: [readability-convert-member-functions-to-static]

method 'handleWebSocketClientConnected' can be made static

  548 | void CaptivePortal::CaptivePortalInstance::handleWebSocketClientConnected(uint8_t socketId)
      |                                            ^
      | static

src/captiveportal/CaptivePortalInstance.cpp:566:44: warning: [readability-convert-member-functions-to-static]

method 'handleWebSocketClientDisconnected' can be made static

  566 | void CaptivePortal::CaptivePortalInstance::handleWebSocketClientDisconnected(uint8_t socketId)
      |                                            ^
      | static

src/captiveportal/CaptivePortalInstance.cpp:571:44: warning: [readability-convert-member-functions-to-static]

method 'handleWebSocketEvent' can be made static

  571 | void CaptivePortal::CaptivePortalInstance::handleWebSocketEvent(uint8_t socketId, WebSocketMessageType type, tcb::span<const uint8_t> payload)
      |                                            ^
      | static

src/captiveportal/CaptivePortalInstance.cpp:574:5: warning: [bugprone-branch-clone]

switch has 2 consecutive identical branches

  574 |     case WebSocketMessageType::Connected:
      |     ^
src/captiveportal/CaptivePortalInstance.cpp:579:12: note: last of these clones ends here
  579 |       break;
      |            ^

Have any feedback or feature suggestions? Share it here.

add better account linking errors

2a34bf3

github-project-automation Bot moved this to Todo in Roadmap Jun 28, 2026

github-project-automation Bot added this to Roadmap Jun 28, 2026

LucHeart changed the title ~~add better account linking errors~~ feat: better account linking errors Jun 28, 2026

LucHeart requested a review from hhvrc June 28, 2026 14:58

LucHeart added 2 commits June 28, 2026 20:05

add changes file

2389747

drop pr field

2f96b7f

hhvrc approved these changes Jun 28, 2026

View reviewed changes

hhvrc merged commit 818085d into develop Jun 28, 2026
38 checks passed

github-project-automation Bot moved this from Todo to Done in Roadmap Jun 28, 2026

hhvrc deleted the feature/better-linking-errors branch June 28, 2026 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: better account linking errors#484

feat: better account linking errors#484
hhvrc merged 3 commits into
developfrom
feature/better-linking-errors

LucHeart commented Jun 28, 2026 •

edited by stage-review Bot

Loading

Uh oh!

stage-review Bot commented Jun 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

LucHeart commented Jun 28, 2026 • edited by stage-review Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stage-review Bot commented Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change file check

Uh oh!

github-actions Bot commented Jun 28, 2026

Cpp-Linter Report ⚠️

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LucHeart commented Jun 28, 2026 •

edited by stage-review Bot

Loading

stage-review Bot commented Jun 28, 2026 •

edited

Loading

github-actions Bot commented Jun 28, 2026 •

edited

Loading