From bcc8088dada945556433f047e37830b023f8cc11 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Sun, 25 Oct 2026 19:57:50 +0100 Subject: [PATCH 01/12] feat(kconfig): move build-time tunables into per-component Kconfig fragments Task stacks and priorities, timeouts, retry intervals and queue and buffer sizes were literals spread over the sources. They are now Kconfig options, each in a Kconfig.openshock next to the component that reads it. main/Kconfig.projbuild holds the single OpenShock menu and sources every fragment with a glob, so all CONFIG_OPENSHOCK_* symbols exist in every build. That covers cross-component uses and the host tests, which source the same root file. The old components/core/Kconfig.projbuild is gone, and its domain and hostname options moved to config's fragment. The code uses the CONFIG_OPENSHOCK_* symbols directly where it needs them, so the setting that drives a line is visible at that line. Named constants remain only where a setting changes unit: the download limit (MB to bytes), the portal's grace and auto-close times (s to us) and the gateway send timeout (ms to ticks). WebSocket.h's MaxGatewayMessageSize and MaxLocalMessageSize are gone. Kconfig ints are int literals, so format specifiers that printed the old size_t / int64_t copies are now %d. Comments that explained a value's choice moved into the help of its Kconfig option. Every default is the value the code had, so a default build is unchanged. Compile-time checks keep related options consistent: - the E-Stop task stays above the RF task's priority - the scan and retry minimums stay at or below their maximums - the portal keeps a socket per client plus one - the E-Stop debounce fits its 16-sample history The E-Stop poll interval is configurable (5 to 20 ms). The 65 ms debounce stays fixed in code, and the sample count follows from the interval: 13 at the default 5 ms. The hold-to-clear and rearm times, the RF protocol timings, task core affinity and on-flash formats stay in code. The log level gets a Kconfig choice that defaults to following the build mode, so CI keeps one sdkconfig.h across channels and keeps sharing compiled ESP-IDF objects. Picking a level overrides it. The portal's station limit and websocket client count are now one option. The unused OPENSHOCK_URI_BUFFER_SIZE is removed. --- .changes/kconfig-tunables.md | 7 ++ components/captive_portal/Kconfig.openshock | 72 ++++++++++++ .../captiveportal/CaptivePortalInstance.cpp | 50 ++++----- .../src/captiveportal/CaptivePortalInstance.h | 4 +- .../src/captiveportal/Manager.cpp | 12 +- components/common/include/OpenShock.h | 3 +- components/config/Kconfig.openshock | 29 +++++ components/core/Kconfig.openshock | 90 +++++++++++++++ components/core/Kconfig.projbuild | 49 -------- .../core/include/message_handlers/WebSocket.h | 5 - components/core/src/GatewayClient.cpp | 24 ++-- .../core/src/GatewayConnectionManager.cpp | 10 +- .../message_handlers/websocket/Gateway.cpp | 4 +- .../src/message_handlers/websocket/Local.cpp | 4 +- components/core/src/wifi/WiFiManager.cpp | 8 +- components/core/src/wifi/WiFiScanManager.cpp | 10 +- components/device_control/Kconfig.openshock | 106 ++++++++++++++++++ .../main/test_estop_state_machine.cpp | 2 +- .../device_control/src/CommandHandler.cpp | 29 ++--- .../device_control/src/EStopManager.cpp | 14 +-- .../src/estop/EStopStateMachine.h | 9 +- .../src/radio/RFTransmitter.cpp | 15 +-- components/dns_server/Kconfig.openshock | 20 ++++ components/dns_server/src/DNSServer.cpp | 6 +- components/http/Kconfig.openshock | 37 ++++++ components/http/src/HTTPRequestManager.cpp | 28 ++--- components/logging/Kconfig.openshock | 43 +++++++ components/logging/include/Logging.h | 7 ++ components/ota/Kconfig.openshock | 12 ++ components/ota/src/OtaUpdateManager.cpp | 2 +- components/serial/Kconfig.openshock | 20 ++++ components/serial/src/Serial.cpp | 13 +-- components/serial_console/Kconfig.openshock | 18 +++ .../serial_console/src/SerialInputHandler.cpp | 13 +-- main/Kconfig.projbuild | 24 ++++ test/host_support/CMakeLists.txt | 2 +- test/host_support/Kconfig.projbuild | 8 +- 37 files changed, 596 insertions(+), 213 deletions(-) create mode 100644 .changes/kconfig-tunables.md create mode 100644 components/captive_portal/Kconfig.openshock create mode 100644 components/config/Kconfig.openshock create mode 100644 components/core/Kconfig.openshock delete mode 100644 components/core/Kconfig.projbuild create mode 100644 components/device_control/Kconfig.openshock create mode 100644 components/dns_server/Kconfig.openshock create mode 100644 components/http/Kconfig.openshock create mode 100644 components/logging/Kconfig.openshock create mode 100644 components/ota/Kconfig.openshock create mode 100644 components/serial/Kconfig.openshock create mode 100644 components/serial_console/Kconfig.openshock create mode 100644 main/Kconfig.projbuild diff --git a/.changes/kconfig-tunables.md b/.changes/kconfig-tunables.md new file mode 100644 index 00000000..6e34ca2e --- /dev/null +++ b/.changes/kconfig-tunables.md @@ -0,0 +1,7 @@ +--- +kind: changed +--- +Move build-time settings into Kconfig + +- Task stacks and priorities, timeouts, retry intervals, queue and buffer sizes are now set in `menuconfig` under OpenShock, each next to the component that uses it +- The log level can be overridden in `menuconfig`; by default it still follows the build mode diff --git a/components/captive_portal/Kconfig.openshock b/components/captive_portal/Kconfig.openshock new file mode 100644 index 00000000..e8cc6dbd --- /dev/null +++ b/components/captive_portal/Kconfig.openshock @@ -0,0 +1,72 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "Captive portal" + + config OPENSHOCK_PORTAL_MAX_CLIENTS + int "Clients (SoftAP stations and websockets)" + range 1 8 + default 4 + help + The SoftAP's station limit and the number of websocket clients the portal + serves. Must stay below the HTTP server's socket count. + + config OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS + int "HTTP server sockets" + range 2 13 + default 8 + help + Browsers open several connections per page load; with only 4 the LRU purge + evicted the websocket (server push, so it looks idle). Also sizes the broadcast + fd list. httpd needs 3 lwIP sockets besides these (checked against + LWIP_MAX_SOCKETS). + + config OPENSHOCK_PORTAL_HTTPD_STACK_SIZE + int "HTTP server task stack (bytes)" + range 4096 32768 + default 8192 + + config OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S + int "HTTP socket send/receive timeout (s)" + range 1 60 + default 5 + + config OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE + int "Largest local websocket message (bytes)" + range 1024 32768 + default 4096 + + config OPENSHOCK_PORTAL_WS_MAX_QUEUED_SENDS + int "Queued websocket sends" + range 4 256 + default 32 + help + Outgoing websocket messages waiting for httpd. Bounds the memory a burst of + broadcasts can hold while no client drains them; past it, new ones are dropped. + + config OPENSHOCK_PORTAL_STARTUP_GRACE_S + int "Startup grace period (s)" + range 0 600 + default 30 + help + The portal stays up this long after boot, whatever the connection state. + + config OPENSHOCK_PORTAL_AUTO_CLOSE_S + int "Auto-close delay (s)" + range 10 3600 + default 300 + help + Once the hub is online and no client is connected, the portal closes after this long. + + config OPENSHOCK_PORTAL_MANAGER_TASK_STACK_SIZE + int "Portal manager task stack (bytes)" + range 4096 16384 + default 6144 + + config OPENSHOCK_PORTAL_ACCOUNT_LINK_TASK_STACK_SIZE + int "Account link task stack (bytes)" + range 4096 16384 + default 8192 + help + Runs the HTTPS request that exchanges a link code for a token. + +endmenu diff --git a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index b369b79b..80e4c6a6 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -35,15 +35,8 @@ const char* const TAG = "CaptivePortalInstance"; #include const uint16_t HTTP_PORT = 80; -// Browsers open several connections per page load; with only 4 slots the LRU purge evicted the (server-push, therefore -// "idle" looking) WebSocket. Needs CONFIG_LWIP_MAX_SOCKETS >= this + 3 (+ DNS/HTTP client). Also sizes the broadcast -// fd list, which must hold every open socket. -const size_t HTTP_MAX_OPEN_SOCKETS = 8; -static_assert(CONFIG_LWIP_MAX_SOCKETS >= HTTP_MAX_OPEN_SOCKETS + 3, "httpd needs 3 lwIP sockets besides its client sockets"); - -// Largest inbound WebSocket message we'll accept (local FlatBuffer commands are tiny). -// Nothing larger is accepted by the local message handler, so don't buffer more than that. -static constexpr size_t MAX_WS_MSG = OpenShock::MessageHandlers::WebSocket::MaxLocalMessageSize; +static_assert(CONFIG_LWIP_MAX_SOCKETS >= CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS + 3, "httpd needs 3 lwIP sockets besides its client sockets"); +static_assert(CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS > CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS, "every websocket client needs a socket, plus one for plain requests"); // HTTP status lines (esp_http_server needs the full "code reason" string, and stores // the pointer rather than copying, so these must have static storage duration). @@ -367,7 +360,7 @@ static esp_err_t apiAccountLink(httpd_req_t* req) auto* job = new AccountLinkJob {std::move(instance), asyncReq, std::move(*code)}; // TLS needs a large stack - if (TaskUtils::TaskCreateExpensive(accountLinkTask, "AccountLink", 8192, job, 1, nullptr) != pdPASS) { + if (TaskUtils::TaskCreateExpensive(accountLinkTask, "AccountLink", CONFIG_OPENSHOCK_PORTAL_ACCOUNT_LINK_TASK_STACK_SIZE, job, 1, nullptr) != pdPASS) { OS_LOGE(TAG, "Failed to create account link task"); delete job; esp_err_t err = sendResp(asyncReq, S500, HTTP::ContentType::JSON, JSON_ERR_INTERNAL); @@ -651,9 +644,6 @@ namespace { #error "CONFIG_HTTPD_QUEUE_WORK_BLOCKING must be disabled" #endif - // Bounds the memory a burst of broadcasts can hold while no client drains them - constexpr size_t kMaxQueuedSends = 32; - void sendToWsClient(httpd_handle_t hd, int fd, httpd_ws_frame_t& frame) { if (httpd_ws_send_frame_async(hd, fd, &frame) != ESP_OK) { @@ -672,7 +662,7 @@ namespace { int CaptivePortal::CaptivePortalInstance::idForFd(int fd) { ScopedLock lock__(&m_clientsMutex); - for (uint8_t i = 0; i < MAX_WS_CLIENTS; ++i) { + for (uint8_t i = 0; i < CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS; ++i) { if (m_clients[i].used && m_clients[i].fd == fd) { return i; } @@ -683,7 +673,7 @@ int CaptivePortal::CaptivePortalInstance::idForFd(int fd) int CaptivePortal::CaptivePortalInstance::fdForId(uint8_t socketId) { ScopedLock lock__(&m_clientsMutex); - if (socketId < MAX_WS_CLIENTS && m_clients[socketId].used) { + if (socketId < CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS && m_clients[socketId].used) { return m_clients[socketId].fd; } return -1; @@ -694,13 +684,13 @@ uint8_t CaptivePortal::CaptivePortalInstance::onWsOpen(int fd) ScopedLock lock__(&m_clientsMutex); // Idempotent: an fd already tracked keeps its id. - for (uint8_t i = 0; i < MAX_WS_CLIENTS; ++i) { + for (uint8_t i = 0; i < CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS; ++i) { if (m_clients[i].used && m_clients[i].fd == fd) { return i; } } - for (uint8_t i = 0; i < MAX_WS_CLIENTS; ++i) { + for (uint8_t i = 0; i < CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS; ++i) { if (!m_clients[i].used) { m_clients[i].used = true; m_clients[i].fd = fd; @@ -719,7 +709,7 @@ void CaptivePortal::CaptivePortalInstance::onWsClose(int fd) int id = -1; { ScopedLock lock__(&m_clientsMutex); - for (uint8_t i = 0; i < MAX_WS_CLIENTS; ++i) { + for (uint8_t i = 0; i < CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS; ++i) { if (m_clients[i].used && m_clients[i].fd == fd) { m_clients[i].used = false; m_clients[i].reasmActive = false; @@ -783,7 +773,7 @@ esp_err_t CaptivePortal::CaptivePortalInstance::wsHandler(httpd_req_t* req) return ESP_OK; } - if (frame.len > MAX_WS_MSG) { + if (frame.len > CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE) { // Nothing larger is accepted by the local message handler, so don't buffer it OS_LOGE(TAG, "WebSocket frame too large (%u bytes), dropping", static_cast(frame.len)); return ESP_FAIL; } @@ -845,9 +835,9 @@ void CaptivePortal::CaptivePortalInstance::onWsFrame(int fd, httpd_ws_type_t opc OS_LOGW(TAG, "WebSocket client %u sent a continuation frame without a message start, dropping it", socketId); break; } - // MAX_WS_MSG only bounds single frames; also bound the reassembled message - if (m_clients[idx].reasm.size() + payload.size() > MAX_WS_MSG) { - OS_LOGE(TAG, "WebSocket client %u fragmented message exceeds %u bytes, closing", socketId, static_cast(MAX_WS_MSG)); + // The frame check only bounds single frames; also bound the reassembled message + if (m_clients[idx].reasm.size() + payload.size() > CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE) { + OS_LOGE(TAG, "WebSocket client %u fragmented message exceeds %d bytes, closing", socketId, CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE); m_clients[idx].reasmActive = false; m_clients[idx].reasm.clear(); m_clients[idx].reasm.shrink_to_fit(); @@ -935,7 +925,7 @@ bool CaptivePortal::CaptivePortalInstance::queueSend(int fd, bool broadcast, boo { ScopedLock lock__(&m_sendMutex); - if (m_sendQueue.size() >= kMaxQueuedSends) { + if (m_sendQueue.size() >= CONFIG_OPENSHOCK_PORTAL_WS_MAX_QUEUED_SENDS) { OS_LOGW(TAG, "WebSocket send queue full, dropping message"); return false; } @@ -1003,8 +993,8 @@ void CaptivePortal::CaptivePortalInstance::drainSends() continue; } - size_t count = HTTP_MAX_OPEN_SOCKETS; - int fds[HTTP_MAX_OPEN_SOCKETS]; + size_t count = CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS; + int fds[CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS]; if (httpd_get_client_list(m_server, &count, fds) == ESP_OK) { for (size_t i = 0; i < count; ++i) { if (httpd_ws_get_fd_info(m_server, fds[i]) == HTTPD_WS_CLIENT_WEBSOCKET) { @@ -1059,7 +1049,7 @@ bool CaptivePortal::CaptivePortalInstance::broadcastMessageBIN(std::span s_alwaysEnabled = false; @@ -190,7 +190,7 @@ static bool captiveportal_start() apConfig.ap.ssid_len = strlen(reinterpret_cast(apConfig.ap.ssid)); apConfig.ap.channel = 1; apConfig.ap.authmode = WIFI_AUTH_OPEN; - apConfig.ap.max_connection = 4; + apConfig.ap.max_connection = CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS; apConfig.ap.beacon_interval = 100; esp_err_t err = esp_wifi_set_mode(WIFI_MODE_APSTA); @@ -312,12 +312,12 @@ static void captiveportal_tick(ManagerState& state) return; } - // Auto-close: no clients connected, WiFi + gateway are up, 5 minutes elapsed + // Auto-close: no clients connected, WiFi + gateway are up, AUTO_CLOSE_DELAY_US elapsed if (running && !state.alwaysEnabled && connected && !hasClients) { if (state.autoCloseAt == 0) { state.autoCloseAt = now + AUTO_CLOSE_DELAY_US; } else if (now >= state.autoCloseAt) { - OS_LOGI(TAG, "Auto-closing captive portal AP (no clients for 5 minutes)"); + OS_LOGI(TAG, "Auto-closing captive portal AP (no clients for %d s)", CONFIG_OPENSHOCK_PORTAL_AUTO_CLOSE_S); captiveportal_stop(state); return; } @@ -378,7 +378,7 @@ bool CaptivePortal::Init() return false; } - if (TaskUtils::TaskCreateExpensive(captiveportal_managertask, "CaptivePortalManager", 6144, initialState, 1, &s_managerTask) != pdPASS) { + if (TaskUtils::TaskCreateExpensive(captiveportal_managertask, "CaptivePortalManager", CONFIG_OPENSHOCK_PORTAL_MANAGER_TASK_STACK_SIZE, initialState, 1, &s_managerTask) != pdPASS) { OS_LOGE(TAG, "Failed to create captive portal manager task"); delete initialState; return false; diff --git a/components/common/include/OpenShock.h b/components/common/include/OpenShock.h index baf66047..1c7f5cfc 100644 --- a/components/common/include/OpenShock.h +++ b/components/common/include/OpenShock.h @@ -9,7 +9,8 @@ TypeName& operator=(TypeName&&) = delete // Config comes from two places: Kconfig (CONFIG_OPENSHOCK_*, via sdkconfig.h - -// domains, hostname, AP prefix, buffer sizes; see components/core/Kconfig.projbuild) +// domains, hostname, task stacks and priorities, timeouts, queue and buffer sizes; +// see main/Kconfig.projbuild and components/*/Kconfig.openshock) // and the generated board header (OPENSHOCK_FW_CHIP, build mode, log level, and the // board the build was configured for; see scripts/gen_env_header.py). // diff --git a/components/config/Kconfig.openshock b/components/config/Kconfig.openshock new file mode 100644 index 00000000..a54f4930 --- /dev/null +++ b/components/config/Kconfig.openshock @@ -0,0 +1,29 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "Defaults" + + config OPENSHOCK_API_DOMAIN + string "OpenShock API domain" + default "api.openshock.app" + help + Default backend API domain the hub connects to. Overridable at runtime. + + config OPENSHOCK_FW_CDN_DOMAIN + string "Firmware CDN domain" + default "firmware.openshock.org" + help + Default CDN domain used for OTA firmware/version lookups. Overridable at runtime. + + config OPENSHOCK_FW_HOSTNAME + string "Default hostname" + default "OpenShock" + help + Default network hostname; also the base of the default user agent. + + config OPENSHOCK_FW_AP_PREFIX + string "SoftAP SSID prefix" + default "OpenShock-" + help + Prefix for the captive-portal SoftAP SSID (the device MAC is appended). + +endmenu diff --git a/components/core/Kconfig.openshock b/components/core/Kconfig.openshock new file mode 100644 index 00000000..d23853f8 --- /dev/null +++ b/components/core/Kconfig.openshock @@ -0,0 +1,90 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "WiFi" + + config OPENSHOCK_WIFI_TASK_STACK_SIZE + int "WiFi manager task stack (bytes)" + range 2048 16384 + default 4096 + + config OPENSHOCK_WIFI_TASK_PRIORITY + int "WiFi manager task priority" + range 1 24 + default 5 + + config OPENSHOCK_WIFI_SCAN_MIN_MS_PER_CHANNEL + int "Scan time per channel, minimum (ms)" + range 10 1000 + default 60 + + config OPENSHOCK_WIFI_SCAN_MAX_MS_PER_CHANNEL + int "Scan time per channel, maximum (ms)" + range 10 1500 + default 120 + help + Must be at least the minimum. esp_wifi walks every channel itself, so this is + the main knob for scan duration. Every scan takes the radio off the captive + portal AP's channel for this long per channel, and portal clients lose frames + meanwhile: keep it near the ESP-IDF default (120 ms). + + config OPENSHOCK_WIFI_AUTOSCAN_INTERVAL_MS + int "Auto-scan interval when nothing to connect to (ms)" + range 10000 3600000 + default 120000 + + config OPENSHOCK_WIFI_CONNECT_BACKOFF_MAX_MS + int "Connect retry backoff cap (ms)" + range 2000 600000 + default 60000 + help + A network that failed to connect is retried after 2 s, 4 s, 8 s, ... up to this. + +endmenu + +menu "Gateway" + + config OPENSHOCK_GATEWAY_TASK_STACK_SIZE + int "Websocket client task stack (bytes)" + range 4096 16384 + default 6144 + + config OPENSHOCK_GATEWAY_MAX_MESSAGE_SIZE + int "Largest gateway message (bytes)" + range 1024 32768 + default 4096 + + config OPENSHOCK_GATEWAY_PING_TIMEOUT_MS + int "Ping timeout (ms)" + range 10000 600000 + default 90000 + help + Reconnect when the gateway sent no ping for this long. + + config OPENSHOCK_GATEWAY_SEND_TIMEOUT_MS + int "Send timeout (ms)" + range 1000 60000 + default 10000 + + config OPENSHOCK_GATEWAY_AUTH_RETRY_MS + int "Retry after the backend rejected the token (ms)" + range 10000 3600000 + default 300000 + + config OPENSHOCK_GATEWAY_CONNECT_RETRY_MS + int "Retry between gateway connection attempts (ms)" + range 1000 600000 + default 20000 + + config OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MIN_MS + int "Hub info fetch retry, first (ms)" + range 1000 600000 + default 5000 + + config OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MAX_MS + int "Hub info fetch retry, cap (ms)" + range 1000 3600000 + default 300000 + help + The retry after a failed hub info fetch doubles from the first value up to this. + +endmenu diff --git a/components/core/Kconfig.projbuild b/components/core/Kconfig.projbuild deleted file mode 100644 index 965136fa..00000000 --- a/components/core/Kconfig.projbuild +++ /dev/null @@ -1,49 +0,0 @@ -menu "OpenShock Board" - - # The board's GPIO assignments deliberately do NOT live here. - # - # A Kconfig symbol ends up in sdkconfig.h, which most of ESP-IDF includes, so two - # boards that differed only by an LED pin produced a different sdkconfig.h and - # therefore shared no compiled framework objects at all. The pins are plain - # generated macros in openshock_board.h instead: - # - # OPENSHOCK_RF_TX_GPIO OPENSHOCK_ESTOP_PIN OPENSHOCK_LED_GPIO - # OPENSHOCK_LED_WS2812B OPENSHOCK_LED_SWAP_RG_CHANNELS - # - # They are set per board as bare `OPENSHOCK_*=` lines in boards/.defaults, - # read by scripts/gen_env_header.py, and stripped from the sdkconfig fragment by - # scripts/build.py. Only the settings below - identical across every board - remain - # Kconfig, because they cost nothing in shared objects. - - config OPENSHOCK_API_DOMAIN - string "OpenShock API domain" - default "api.openshock.app" - help - Default backend API domain the hub connects to. Overridable at runtime. - - config OPENSHOCK_FW_CDN_DOMAIN - string "Firmware CDN domain" - default "firmware.openshock.org" - help - Default CDN domain used for OTA firmware/version lookups. Overridable at runtime. - - config OPENSHOCK_FW_HOSTNAME - string "Default hostname" - default "OpenShock" - help - Default network hostname; also the base of the default user agent. - - config OPENSHOCK_FW_AP_PREFIX - string "SoftAP SSID prefix" - default "OpenShock-" - help - Prefix for the captive-portal SoftAP SSID (the device MAC is appended). - - config OPENSHOCK_URI_BUFFER_SIZE - int "HTTP URI buffer size (bytes)" - default 256 - help - Stack buffer size for composed request URIs. Increase if a configured - domain plus path can exceed this. - -endmenu diff --git a/components/core/include/message_handlers/WebSocket.h b/components/core/include/message_handlers/WebSocket.h index 22547865..73135908 100644 --- a/components/core/include/message_handlers/WebSocket.h +++ b/components/core/include/message_handlers/WebSocket.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -9,10 +8,6 @@ namespace OpenShock { } namespace OpenShock::MessageHandlers::WebSocket { - // Largest message the handlers accept; transports should not buffer more than this. - inline constexpr std::size_t MaxGatewayMessageSize = 4096; // TODO: Profile this - inline constexpr std::size_t MaxLocalMessageSize = 4096; // TODO: Profile this - /// @brief Handles a message received on `client`, which replies go back through. void HandleGatewayBinary(GatewayClient& client, std::span data); void HandleLocalBinary(uint8_t socketId, std::span data); diff --git a/components/core/src/GatewayClient.cpp b/components/core/src/GatewayClient.cpp index 79b2230c..030f8d00 100644 --- a/components/core/src/GatewayClient.cpp +++ b/components/core/src/GatewayClient.cpp @@ -21,16 +21,8 @@ const char* const TAG = "GatewayClient"; using namespace OpenShock; -const int64_t GATEWAY_PING_TIMEOUT = 90'000; - // Covers both the handle lock wait and the send itself -const TickType_t GATEWAY_SEND_TIMEOUT = pdMS_TO_TICKS(10'000); - -// Message handlers (config reads, flash writes, FlatBuffers verification) run on the websocket task. -const int GATEWAY_TASK_STACK_SIZE = 6 * 1024; - -// Upper bound for a reassembled message: the gateway message handler limit. -const std::size_t GATEWAY_MAX_MESSAGE_SIZE = OpenShock::MessageHandlers::WebSocket::MaxGatewayMessageSize; +const TickType_t GATEWAY_SEND_TIMEOUT = pdMS_TO_TICKS(CONFIG_OPENSHOCK_GATEWAY_SEND_TIMEOUT_MS); // WebSocket opcodes (RFC 6455) as delivered by esp_websocket_client event data. static constexpr int WS_OP_TEXT = 0x01; @@ -129,9 +121,9 @@ void GatewayClient::connect(const std::string& host, uint16_t port, const std::s config.transport = WEBSOCKET_TRANSPORT_OVER_SSL; config.user_agent = OpenShock::Constants::UserAgent(); config.headers = m_headers.c_str(); - config.disable_auto_reconnect = true; // GatewayConnectionManager owns reconnection - config.crt_bundle_attach = esp_crt_bundle_attach; // verify server against the compiled-in CA bundle - config.task_stack = GATEWAY_TASK_STACK_SIZE; + config.disable_auto_reconnect = true; // GatewayConnectionManager owns reconnection + config.crt_bundle_attach = esp_crt_bundle_attach; // verify server against the compiled-in CA bundle + config.task_stack = CONFIG_OPENSHOCK_GATEWAY_TASK_STACK_SIZE; // Message handlers (config reads, flash writes, FlatBuffers verification) run on it // TLS work stays on core 0 with the WiFi/lwIP tasks, away from the E-Stop and RF tasks on core 1 config.task_core_id_set = true; config.task_core_id = 0; @@ -228,8 +220,8 @@ bool GatewayClient::loop() } // Timestamp is seeded on connect, so a connection that never sees a ping also times out. - if ((OpenShock::millis() - m_lastPingTimestamp.load()) > GATEWAY_PING_TIMEOUT) { - OS_LOGW(TAG, "No ping received from gateway for %lld ms, forcing reconnect", GATEWAY_PING_TIMEOUT); + if ((OpenShock::millis() - m_lastPingTimestamp.load()) > CONFIG_OPENSHOCK_GATEWAY_PING_TIMEOUT_MS) { + OS_LOGW(TAG, "No ping received from gateway for %d ms, forcing reconnect", CONFIG_OPENSHOCK_GATEWAY_PING_TIMEOUT_MS); // The link is presumed dead, so skip the close handshake; destroy stops the client task and frees the handle. _destroyHandle(); _setState(GatewayClientState::Disconnected); @@ -354,8 +346,8 @@ void GatewayClient::_handleData(const esp_websocket_event_data_t* data) } if (!m_binReasmDiscard) { - if (m_binReasm.size() + data->data_len > GATEWAY_MAX_MESSAGE_SIZE) { - OS_LOGE(TAG, "Fragmented message exceeds %zu bytes, dropping it", GATEWAY_MAX_MESSAGE_SIZE); + if (m_binReasm.size() + data->data_len > CONFIG_OPENSHOCK_GATEWAY_MAX_MESSAGE_SIZE) { // The gateway message handler limit + OS_LOGE(TAG, "Fragmented message exceeds %d bytes, dropping it", CONFIG_OPENSHOCK_GATEWAY_MAX_MESSAGE_SIZE); m_binReasm.clear(); m_binReasm.shrink_to_fit(); m_binReasmDiscard = true; diff --git a/components/core/src/GatewayConnectionManager.cpp b/components/core/src/GatewayConnectionManager.cpp index 13ef31a4..78c5f66c 100644 --- a/components/core/src/GatewayConnectionManager.cpp +++ b/components/core/src/GatewayConnectionManager.cpp @@ -37,9 +37,13 @@ const uint8_t LINK_CODE_LENGTH = 6; static std::atomic s_flags = 0; // Retry timing, owned by the main task (Update()). Other tasks ask for a reset through s_retryResetRequested. -static OpenShock::Backoff s_authBackoff(300'000, 300'000); // After the backend rejects the token: 5 minutes -static OpenShock::Backoff s_connectBackoff(20'000, 20'000); // Between LCG connection attempts: 20 seconds -static OpenShock::Backoff s_hubInfoBackoff(5'000, 300'000); // After a failed hub info fetch: 5 s doubling up to 5 min +// After the backend rejects the token +static OpenShock::Backoff s_authBackoff(CONFIG_OPENSHOCK_GATEWAY_AUTH_RETRY_MS, CONFIG_OPENSHOCK_GATEWAY_AUTH_RETRY_MS); +// Between LCG connection attempts +static OpenShock::Backoff s_connectBackoff(CONFIG_OPENSHOCK_GATEWAY_CONNECT_RETRY_MS, CONFIG_OPENSHOCK_GATEWAY_CONNECT_RETRY_MS); +// After a failed hub info fetch: doubling from the first value up to the cap +static OpenShock::Backoff s_hubInfoBackoff(CONFIG_OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MIN_MS, CONFIG_OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MAX_MS); +static_assert(CONFIG_OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MIN_MS <= CONFIG_OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MAX_MS, "hub info retry above its cap"); static std::atomic s_retryResetRequested = false; static OpenShock::SimpleMutex s_clientMutex; static std::shared_ptr s_wsClient = nullptr; diff --git a/components/core/src/message_handlers/websocket/Gateway.cpp b/components/core/src/message_handlers/websocket/Gateway.cpp index ede3c687..a788259b 100644 --- a/components/core/src/message_handlers/websocket/Gateway.cpp +++ b/components/core/src/message_handlers/websocket/Gateway.cpp @@ -39,9 +39,9 @@ static std::array s_serverHandlers = []() void MessageHandlers::WebSocket::HandleGatewayBinary(GatewayClient& client, std::span data) { - auto msg = Serialization::GetVerifiedRoot(data, MaxGatewayMessageSize); + auto msg = Serialization::GetVerifiedRoot(data, CONFIG_OPENSHOCK_GATEWAY_MAX_MESSAGE_SIZE); if (msg == nullptr) { - OS_LOGE(TAG, "Invalid message (%zu bytes, max %zu)", data.size(), MaxGatewayMessageSize); + OS_LOGE(TAG, "Invalid message (%zu bytes, max %d)", data.size(), CONFIG_OPENSHOCK_GATEWAY_MAX_MESSAGE_SIZE); return; } diff --git a/components/core/src/message_handlers/websocket/Local.cpp b/components/core/src/message_handlers/websocket/Local.cpp index 73967f93..38e10071 100644 --- a/components/core/src/message_handlers/websocket/Local.cpp +++ b/components/core/src/message_handlers/websocket/Local.cpp @@ -33,9 +33,9 @@ static std::array s_localHandlers = []() { void MessageHandlers::WebSocket::HandleLocalBinary(uint8_t socketId, std::span data) { - auto msg = Serialization::GetVerifiedRoot(data, MaxLocalMessageSize); + auto msg = Serialization::GetVerifiedRoot(data, CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE); if (msg == nullptr) { - OS_LOGE(TAG, "Invalid message (%zu bytes, max %zu)", data.size(), MaxLocalMessageSize); + OS_LOGE(TAG, "Invalid message (%zu bytes, max %d)", data.size(), CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE); return; } diff --git a/components/core/src/wifi/WiFiManager.cpp b/components/core/src/wifi/WiFiManager.cpp index 99748849..0bb4dee8 100644 --- a/components/core/src/wifi/WiFiManager.cpp +++ b/components/core/src/wifi/WiFiManager.cpp @@ -143,12 +143,12 @@ static bool isConnectRateLimited(const WiFiNetwork& net) return false; } - // Back off from the first failure: 2 s, 4 s, 8 s, ... capped at 60 s. Every attempt is an all-channel scan that + // Back off from the first failure: 2 s, 4 s, 8 s, ... capped at CONFIG_OPENSHOCK_WIFI_CONNECT_BACKOFF_MAX_MS. Every attempt is an all-channel scan that // takes the radio off the captive portal AP's channel, so retrying every second starves portal clients. int64_t now = OpenShock::millis(); int64_t diff = now - net.lastConnectAttempt; uint16_t shift = std::min(net.connectAttempts, 5); - int64_t backoff = std::min(1000LL << shift, 60'000); + int64_t backoff = std::min(1000LL << shift, CONFIG_OPENSHOCK_WIFI_CONNECT_BACKOFF_MAX_MS); return diff < backoff; } @@ -560,7 +560,7 @@ static void wifimanagerUpdateTask(void*) if (s_wifiState.load(std::memory_order_relaxed) == WiFiState::Disconnected && !WiFiScanManager::IsScanning()) { if (!tryConnect()) { int64_t now = OpenShock::millis(); - if (lastScanRequest == 0 || now - lastScanRequest > 120'000) { // Auto-scan at boot and then every 2 mins + if (lastScanRequest == 0 || now - lastScanRequest > CONFIG_OPENSHOCK_WIFI_AUTOSCAN_INTERVAL_MS) { // Auto-scan at boot and then periodically lastScanRequest = now; OS_LOGV(TAG, "No networks to connect to, starting scan..."); @@ -640,7 +640,7 @@ bool WiFiManager::Init() esp_netif_set_hostname(s_staNetif, hostname->c_str()); TaskHandle_t updateTask = nullptr; - if (TaskUtils::TaskCreateUniversal(wifimanagerUpdateTask, TAG, 4096, nullptr, 5, &updateTask, 1) != pdPASS) { // TODO: Re-profile stack usage + if (TaskUtils::TaskCreateUniversal(wifimanagerUpdateTask, TAG, CONFIG_OPENSHOCK_WIFI_TASK_STACK_SIZE, nullptr, CONFIG_OPENSHOCK_WIFI_TASK_PRIORITY, &updateTask, 1) != pdPASS) { // TODO: Re-profile stack usage OS_LOGE(TAG, "Failed to create WiFiManager update task"); return false; } diff --git a/components/core/src/wifi/WiFiScanManager.cpp b/components/core/src/wifi/WiFiScanManager.cpp index 5de0ed88..e31f729c 100644 --- a/components/core/src/wifi/WiFiScanManager.cpp +++ b/components/core/src/wifi/WiFiScanManager.cpp @@ -14,11 +14,7 @@ const char* const TAG = "WiFiScanManager"; #include #include -// Per-channel active-scan dwell time. esp_wifi walks every channel itself, so this is the main knob for scan -// duration. While scanning, the radio is off the AP's channel, so captive-portal clients lose frames for each dwell: -// keep it close to the IDF default (120 ms) rather than 300 ms. -const uint32_t OPENSHOCK_WIFI_SCAN_MIN_MS_PER_CHANNEL = 60; -const uint32_t OPENSHOCK_WIFI_SCAN_MAX_MS_PER_CHANNEL = 120; +static_assert(CONFIG_OPENSHOCK_WIFI_SCAN_MIN_MS_PER_CHANNEL <= CONFIG_OPENSHOCK_WIFI_SCAN_MAX_MS_PER_CHANNEL, "scan time minimum above its maximum"); // Idle -> Scanning (StartScan) -> Idle (results handled, failure, STA stop). // Scanning -> Stopping (AbortScan) -> Idle (the stopped scan's SCAN_DONE, or kStopTimeout without one). @@ -210,8 +206,8 @@ bool WiFiScanManager::StartScan() wifi_scan_config_t config = {}; config.show_hidden = true; config.scan_type = WIFI_SCAN_TYPE_ACTIVE; - config.scan_time.active.min = OPENSHOCK_WIFI_SCAN_MIN_MS_PER_CHANNEL; - config.scan_time.active.max = OPENSHOCK_WIFI_SCAN_MAX_MS_PER_CHANNEL; + config.scan_time.active.min = CONFIG_OPENSHOCK_WIFI_SCAN_MIN_MS_PER_CHANNEL; + config.scan_time.active.max = CONFIG_OPENSHOCK_WIFI_SCAN_MAX_MS_PER_CHANNEL; config.home_chan_dwell_time = WIFI_SCAN_HOME_CHANNEL_DWELL_DEFAULT_TIME; // 0 (zero-init) is not the default // async: WIFI_EVENT_SCAN_DONE fans out results diff --git a/components/device_control/Kconfig.openshock b/components/device_control/Kconfig.openshock new file mode 100644 index 00000000..7c443d9a --- /dev/null +++ b/components/device_control/Kconfig.openshock @@ -0,0 +1,106 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "Radio" + + config OPENSHOCK_RF_TASK_STACK_SIZE + int "RF transmitter task stack (bytes)" + range 2048 16384 + default 4096 + help + Profiled at about 1.4 KB. + + config OPENSHOCK_RF_TASK_PRIORITY + int "RF transmitter task priority" + range 1 24 + default 9 + help + Above the network tasks (5): it blocks on its queue or on RMT almost all the time, + and frames (and the terminators after an E-Stop) shouldn't wait behind TLS work. + Must stay below the E-Stop task's priority, so a press preempts a transmission + (checked at compile time). + + config OPENSHOCK_RF_QUEUE_SIZE + int "RF command queue length" + range 8 256 + default 64 + help + Commands waiting for the transmitter task. A full queue drops the command. + + config OPENSHOCK_RF_TERMINATOR_DURATION_MS + int "Stop terminator duration (ms)" + range 100 2000 + default 300 + help + How long a finished command keeps sending its zero-intensity terminator, so + the shocker stops even if it missed the last frames. + +endmenu + +menu "Keep-alive" + + config OPENSHOCK_KEEPALIVE_INTERVAL_MS + int "Keep-alive interval (ms)" + range 10000 600000 + default 60000 + help + A shocker that saw no command for this long gets a short zero-intensity + vibrate, so it doesn't go to sleep and miss the next command. + + config OPENSHOCK_KEEPALIVE_DURATION_MS + int "Keep-alive command duration (ms)" + range 100 2000 + default 300 + + config OPENSHOCK_KEEPALIVE_MAX_SHOCKERS + int "Shockers kept alive" + range 1 64 + default 16 + help + Shocker IDs come from the network, so the set is capped: past this, the least + recently used shocker is dropped from it. + + config OPENSHOCK_KEEPALIVE_QUEUE_SIZE + int "Keep-alive queue length" + range 4 128 + default 32 + + config OPENSHOCK_KEEPALIVE_TASK_STACK_SIZE + int "Keep-alive task stack (bytes)" + range 2048 16384 + default 4096 + help + Profiled at about 1.5 KB. + + config OPENSHOCK_KEEPALIVE_TASK_PRIORITY + int "Keep-alive task priority" + range 1 24 + default 1 + +endmenu + +menu "Emergency stop" + + config OPENSHOCK_ESTOP_POLL_INTERVAL_MS + int "Button poll interval (ms)" + range 5 20 + default 5 + help + How often the E-Stop task samples the button. The debounce time is fixed in + code (65 ms); the number of samples it takes follows from this interval. + At most 16 samples fit the debounce history, hence the 5 ms floor. + + config OPENSHOCK_ESTOP_TASK_STACK_SIZE + int "E-Stop task stack (bytes)" + range 2048 16384 + default 4096 + + config OPENSHOCK_ESTOP_TASK_PRIORITY + int "E-Stop task priority" + range 2 24 + default 10 + help + Above the network tasks (5) and the RF task: it is blocked almost all the time, + and a sample that waits behind TLS work delays the E-Stop by that long. Must stay + above the RF transmitter task's priority (checked at compile time). + +endmenu diff --git a/components/device_control/host_test/main/test_estop_state_machine.cpp b/components/device_control/host_test/main/test_estop_state_machine.cpp index 0bf15efc..d468f90e 100644 --- a/components/device_control/host_test/main/test_estop_state_machine.cpp +++ b/components/device_control/host_test/main/test_estop_state_machine.cpp @@ -14,7 +14,7 @@ namespace { const int kPressed = 0; // Input is pulled up; the button pulls it low const int kReleased = 1; - const int64_t kSampleMs = 5; // EStopManager samples at 200 Hz + const int64_t kSampleMs = CONFIG_OPENSHOCK_ESTOP_POLL_INTERVAL_MS; // EStopManager's poll interval struct Rig { EStopStateMachine machine; diff --git a/components/device_control/src/CommandHandler.cpp b/components/device_control/src/CommandHandler.cpp index b68b4461..a625be2f 100644 --- a/components/device_control/src/CommandHandler.cpp +++ b/components/device_control/src/CommandHandler.cpp @@ -24,15 +24,10 @@ const char* const TAG = "CommandHandler"; #include #include -const int64_t KEEP_ALIVE_INTERVAL = 60'000; -const uint16_t KEEP_ALIVE_DURATION = 300; -// Shocker IDs come from the network, so the set is capped; past it the least recently used shocker is dropped -const size_t KEEP_ALIVE_MAX_SHOCKERS = 16; - static uint32_t calculateEepyTime(int64_t timeToKeepAlive) { int64_t now = OpenShock::millis(); - return static_cast(std::clamp(timeToKeepAlive - now, 0, KEEP_ALIVE_INTERVAL)); + return static_cast(std::clamp(timeToKeepAlive - now, 0, CONFIG_OPENSHOCK_KEEPALIVE_INTERVAL_MS)); } struct KnownShocker { @@ -96,7 +91,7 @@ using namespace OpenShock; static void commandhandler_keepalivetask() { - int64_t timeToKeepAlive = KEEP_ALIVE_INTERVAL; + int64_t timeToKeepAlive = CONFIG_OPENSHOCK_KEEPALIVE_INTERVAL_MS; // Map of (model, shocker ID) to its last activity; IDs are only unique per model std::unordered_map activityMap; @@ -113,14 +108,14 @@ static void commandhandler_keepalivetask() } uint32_t key = (static_cast(cmd.model) << 16) | cmd.shockerId; - if (activityMap.size() >= KEEP_ALIVE_MAX_SHOCKERS && !activityMap.contains(key)) { + if (activityMap.size() >= CONFIG_OPENSHOCK_KEEPALIVE_MAX_SHOCKERS && !activityMap.contains(key)) { auto oldest = std::min_element(activityMap.begin(), activityMap.end(), [](const auto& a, const auto& b) { return a.second.lastActivityTimestamp < b.second.lastActivityTimestamp; }); - OS_LOGW(TAG, "Keeping more than %zu shockers alive, dropping shocker %u", KEEP_ALIVE_MAX_SHOCKERS, oldest->second.shockerId); + OS_LOGW(TAG, "Keeping more than %d shockers alive, dropping shocker %u", CONFIG_OPENSHOCK_KEEPALIVE_MAX_SHOCKERS, oldest->second.shockerId); activityMap.erase(oldest); } activityMap[key] = cmd; - eepyTime = calculateEepyTime(std::min(timeToKeepAlive, cmd.lastActivityTimestamp + KEEP_ALIVE_INTERVAL)); + eepyTime = calculateEepyTime(std::min(timeToKeepAlive, cmd.lastActivityTimestamp + CONFIG_OPENSHOCK_KEEPALIVE_INTERVAL_MS)); } // Update the time to now @@ -130,18 +125,18 @@ static void commandhandler_keepalivetask() // nothing. Shockers seen before the E-Stop are forgotten; only commands accepted after it count again. if (EStopManager::IsEStopped()) { activityMap.clear(); - timeToKeepAlive = now + KEEP_ALIVE_INTERVAL; + timeToKeepAlive = now + CONFIG_OPENSHOCK_KEEPALIVE_INTERVAL_MS; continue; } // Keep track of the minimum activity time, so we know when to wake up - timeToKeepAlive = now + KEEP_ALIVE_INTERVAL; + timeToKeepAlive = now + CONFIG_OPENSHOCK_KEEPALIVE_INTERVAL_MS; // For every entry that has a keep-alive time less than now, send a keep-alive for (auto it = activityMap.begin(); it != activityMap.end(); ++it) { auto& cmdRef = it->second; - if (cmdRef.lastActivityTimestamp + KEEP_ALIVE_INTERVAL < now) { + if (cmdRef.lastActivityTimestamp + CONFIG_OPENSHOCK_KEEPALIVE_INTERVAL_MS < now) { OS_LOGV(TAG, "Sending keep-alive for shocker %u", cmdRef.shockerId); auto transmitter = GetTransmitter(); @@ -150,14 +145,14 @@ static void commandhandler_keepalivetask() break; } - if (!transmitter->SendCommand(cmdRef.model, cmdRef.shockerId, ShockerCommandType::Vibrate, 0, KEEP_ALIVE_DURATION, false)) { + if (!transmitter->SendCommand(cmdRef.model, cmdRef.shockerId, ShockerCommandType::Vibrate, 0, CONFIG_OPENSHOCK_KEEPALIVE_DURATION_MS, false)) { OS_LOGW(TAG, "Failed to send keep-alive for shocker %u", cmdRef.shockerId); } cmdRef.lastActivityTimestamp = now; } - timeToKeepAlive = std::min(timeToKeepAlive, cmdRef.lastActivityTimestamp + KEEP_ALIVE_INTERVAL); + timeToKeepAlive = std::min(timeToKeepAlive, cmdRef.lastActivityTimestamp + CONFIG_OPENSHOCK_KEEPALIVE_INTERVAL_MS); } } } @@ -176,13 +171,13 @@ static bool internalSetKeepAliveEnabled(bool enabled) if (enabled) { OS_LOGV(TAG, "Enabling keep-alive task"); - s_keepAliveQueue = xQueueCreate(32, sizeof(KnownShocker)); + s_keepAliveQueue = xQueueCreate(CONFIG_OPENSHOCK_KEEPALIVE_QUEUE_SIZE, sizeof(KnownShocker)); if (s_keepAliveQueue == nullptr) { OS_LOGE(TAG, "Failed to create keep-alive task"); return false; } - if (!s_keepAliveTask.startExpensive("KeepAliveTask", 4096, 1, commandhandler_keepalivetask)) { // PROFILED: 1.5KB stack usage + if (!s_keepAliveTask.startExpensive("KeepAliveTask", CONFIG_OPENSHOCK_KEEPALIVE_TASK_STACK_SIZE, CONFIG_OPENSHOCK_KEEPALIVE_TASK_PRIORITY, commandhandler_keepalivetask)) { OS_LOGE(TAG, "Failed to create keep-alive task"); vQueueDelete(s_keepAliveQueue); diff --git a/components/device_control/src/EStopManager.cpp b/components/device_control/src/EStopManager.cpp index 4cb66e67..2a2b45e3 100644 --- a/components/device_control/src/EStopManager.cpp +++ b/components/device_control/src/EStopManager.cpp @@ -23,13 +23,6 @@ const char* const TAG = "EStopManager"; using namespace OpenShock; -const uint32_t k_estopUpdateRate = 5; // 200 Hz (debounce/hold timings live in EStopStateMachine) - -const uint32_t k_estopTaskStackSize = 4096; // TODO: profile and tune -// Above the network tasks (5) and the RF task: it is blocked almost all the time, and a sample that waits behind TLS -// work delays the E-Stop by that long. -const UBaseType_t k_estopTaskPriority = 10; - static OpenShock::SimpleMutex s_estopMutex = {}; // Guarded via Mutex static ManagedTask s_estopTask(TAG, "EStop task"); @@ -86,8 +79,9 @@ static void estopmgr_managerTask(gpio_num_t estopPin) TickType_t lastWake = xTaskGetTickCount(); while (!s_estopTask.stopRequested()) { watchdog.feed(); - // Fixed rate: the debounce and hold timings assume samples k_estopUpdateRate apart, whatever the loop body costs - xTaskDelayUntil(&lastWake, pdMS_TO_TICKS(k_estopUpdateRate)); + // Fixed rate: the debounce and hold timings (EStopStateMachine) assume samples this far apart, whatever the loop + // body costs + xTaskDelayUntil(&lastWake, pdMS_TO_TICKS(CONFIG_OPENSHOCK_ESTOP_POLL_INTERVAL_MS)); // Get current time int64_t now = OpenShock::millis(); @@ -189,7 +183,7 @@ static bool estopmgr_taskStart() // The pin is captured by value, so the task keeps sampling the pin it was started with even if s_estopPin changes. gpio_num_t pin = s_estopPin; - if (!s_estopTask.start(TAG, k_estopTaskStackSize, k_estopTaskPriority, 1, [pin] { estopmgr_managerTask(pin); })) { + if (!s_estopTask.start(TAG, CONFIG_OPENSHOCK_ESTOP_TASK_STACK_SIZE, CONFIG_OPENSHOCK_ESTOP_TASK_PRIORITY, 1, [pin] { estopmgr_managerTask(pin); })) { OS_LOGE(TAG, "Failed to create EStop event handler task"); return false; } diff --git a/components/device_control/src/estop/EStopStateMachine.h b/components/device_control/src/estop/EStopStateMachine.h index bc8ae9dc..a9007635 100644 --- a/components/device_control/src/estop/EStopStateMachine.h +++ b/components/device_control/src/estop/EStopStateMachine.h @@ -2,6 +2,8 @@ #include "estop/EStopState.h" +#include + #include namespace OpenShock { @@ -11,8 +13,11 @@ namespace OpenShock { class EStopStateMachine { public: static constexpr int64_t kHoldToClearTime = 5000; - static constexpr uint32_t kCheckCount = 13; // 65 ms at 200 Hz - static constexpr uint16_t kCheckMask = 0xFFFF >> ((sizeof(uint16_t) * 8) - kCheckCount); // Mask to check only last kCheckCount bits within history + // The poll interval is a build setting; the debounce time is not, so the sample count follows from it + static constexpr uint32_t kDebounceTimeMs = 65; + static constexpr uint32_t kCheckCount = (kDebounceTimeMs + CONFIG_OPENSHOCK_ESTOP_POLL_INTERVAL_MS - 1) / CONFIG_OPENSHOCK_ESTOP_POLL_INTERVAL_MS; // 13 at 5 ms + static_assert(kCheckCount >= 1 && kCheckCount <= 16, "the debounce history holds 16 samples"); + static constexpr uint16_t kCheckMask = 0xFFFF >> ((sizeof(uint16_t) * 8) - kCheckCount); // Mask to check only last kCheckCount bits within history // Grace period after deactivation (prevents immediate re-trigger on release bounce/EMI) static constexpr int64_t kRearmGraceTime = 250; // tune as needed diff --git a/components/device_control/src/radio/RFTransmitter.cpp b/components/device_control/src/radio/RFTransmitter.cpp index b7b33697..eb128f13 100644 --- a/components/device_control/src/radio/RFTransmitter.cpp +++ b/components/device_control/src/radio/RFTransmitter.cpp @@ -20,13 +20,8 @@ const char* const TAG = "RFTransmitter"; #include #include -const UBaseType_t kQueueSize = 64; -// Above the network tasks (5): it blocks on its queue or on RMT almost all the time, and frames (and the terminators -// after an E-Stop) shouldn't wait behind TLS work. Below the E-Stop sampler (10). Never spins: see transmitSymbols. -const BaseType_t kTaskPriority = 9; -const uint32_t kTaskStackSize = 4096; // PROFILED: 1.4KB stack usage -const int64_t kTerminatorDurationMs = 300; -const int32_t kRmtTimeoutMs = 100; +static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY < CONFIG_OPENSHOCK_ESTOP_TASK_PRIORITY, "an E-Stop press must preempt a transmission"); +const int32_t kRmtTimeoutMs = 100; // After this many transmit failures in a row the RMT channel is presumed broken and the pending sequences are dropped const uint32_t kMaxConsecutiveFailures = 20; const uint32_t kMaxFailureBackoffMs = 100; @@ -93,7 +88,7 @@ RFTransmitter::RFTransmitter(gpio_num_t gpioPin) return; } - m_queueHandle = xQueueCreate(kQueueSize, sizeof(Command)); + m_queueHandle = xQueueCreate(CONFIG_OPENSHOCK_RF_QUEUE_SIZE, sizeof(Command)); if (m_queueHandle == nullptr) { OS_LOGE(TAG, "[pin-%hhi] Failed to create queue", m_txPin); destroy(); @@ -103,7 +98,7 @@ RFTransmitter::RFTransmitter(gpio_num_t gpioPin) char name[32]; snprintf(name, sizeof(name), "RFTransmitter-%u", m_txPin); - if (!m_task.startExpensive(name, kTaskStackSize, kTaskPriority, [this] { TransmitTask(); })) { + if (!m_task.startExpensive(name, CONFIG_OPENSHOCK_RF_TASK_STACK_SIZE, CONFIG_OPENSHOCK_RF_TASK_PRIORITY, [this] { TransmitTask(); })) { OS_LOGE(TAG, "[pin-%hhi] Failed to create task", m_txPin); destroy(); return; @@ -304,7 +299,7 @@ static void writeSequences(rmt_channel_handle_t channel, rmt_encoder_handle_t en // Remove command once it has sent out its termination sequence for long enough. A sequence that never got a // terminator out (a slow round skipped past the whole window, or a failed fill zeroed its end) sends one first. - if (timeToLive <= -kTerminatorDurationMs && seq->terminatorSent()) { + if (timeToLive <= -CONFIG_OPENSHOCK_RF_TERMINATOR_DURATION_MS && seq->terminatorSent()) { seq = sequences.erase(seq); continue; } diff --git a/components/dns_server/Kconfig.openshock b/components/dns_server/Kconfig.openshock new file mode 100644 index 00000000..69efb609 --- /dev/null +++ b/components/dns_server/Kconfig.openshock @@ -0,0 +1,20 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "Captive portal DNS server" + + config OPENSHOCK_DNS_TASK_STACK_SIZE + int "DNS server task stack (bytes)" + range 2048 8192 + default 3072 + + config OPENSHOCK_DNS_TASK_PRIORITY + int "DNS server task priority" + range 1 24 + default 4 + help + Above the other application tasks (1): a phone joining the AP sends a burst of + DNS queries, and the small UDP receive mailbox drops them if this task is starved, + which delays captive-portal detection by seconds. Keep it below httpd (5) and the + E-Stop task. + +endmenu diff --git a/components/dns_server/src/DNSServer.cpp b/components/dns_server/src/DNSServer.cpp index 6c0142a6..6ecbdddc 100644 --- a/components/dns_server/src/DNSServer.cpp +++ b/components/dns_server/src/DNSServer.cpp @@ -77,11 +77,7 @@ bool DNSServer::start(const char* responseIpv4, uint16_t port) return false; } - // Above the other application tasks (priority 1): a phone joining the AP sends a burst of DNS queries, and the - // small UDP receive mailbox drops them if this task is starved, which delays captive-portal detection by seconds. - // Still below httpd and the E-Stop task (5). - constexpr UBaseType_t kTaskPriority = 4; - if (!m_task.startExpensive("DNSServer", 3072, kTaskPriority, [this] { task(); })) { + if (!m_task.startExpensive("DNSServer", CONFIG_OPENSHOCK_DNS_TASK_STACK_SIZE, CONFIG_OPENSHOCK_DNS_TASK_PRIORITY, [this] { task(); })) { OS_LOGE(TAG, "Failed to create DNS task"); close(m_socket); m_socket = -1; diff --git a/components/http/Kconfig.openshock b/components/http/Kconfig.openshock new file mode 100644 index 00000000..d468cb4e --- /dev/null +++ b/components/http/Kconfig.openshock @@ -0,0 +1,37 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "HTTP client" + + config OPENSHOCK_HTTP_BUFFER_SIZE + int "Download buffer (bytes)" + range 512 32768 + default 4096 + + config OPENSHOCK_HTTP_MAX_STRING_RESPONSE_SIZE + int "Largest response read into memory (bytes)" + range 4096 262144 + default 65536 + help + JSON and text responses are read whole; larger ones are rejected. + + config OPENSHOCK_HTTP_DOWNLOAD_SIZE_LIMIT_MB + int "Largest streamed download (MB)" + range 1 2000 + default 200 + + config OPENSHOCK_HTTP_RETRY_AFTER_MIN_S + int "Shortest Retry-After honoured (s)" + range 1 600 + default 2 + help + Also used when the header is missing or unparseable. Above zero, so a "0" can't + turn into a tight retry loop. + + config OPENSHOCK_HTTP_RETRY_AFTER_MAX_S + int "Longest Retry-After honoured (s)" + range 1 86400 + default 600 + help + So one bad header can't keep the hub offline for long. + +endmenu diff --git a/components/http/src/HTTPRequestManager.cpp b/components/http/src/HTTPRequestManager.cpp index 8785d7b0..2d70d5dc 100644 --- a/components/http/src/HTTPRequestManager.cpp +++ b/components/http/src/HTTPRequestManager.cpp @@ -28,14 +28,13 @@ const char* const TAG = "HTTPRequestManager"; using namespace std::string_view_literals; -const std::size_t HTTP_BUFFER_SIZE = 4096LLU; -const int HTTP_DOWNLOAD_SIZE_LIMIT = 200 * 1024 * 1024; // 200 MB - static OpenShock::SimpleMutex s_rateLimitsMutex = {}; static std::unordered_map> s_rateLimits = {}; using namespace OpenShock; +const int64_t HTTP_DOWNLOAD_SIZE_LIMIT = CONFIG_OPENSHOCK_HTTP_DOWNLOAD_SIZE_LIMIT_MB * 1024LL * 1024LL; + // Rate limits are kept per host, so the API host gets its own limits independent of CDN/firmware hosts. static std::string_view getHostFromURL(std::string_view url) { @@ -252,14 +251,12 @@ HTTP::Response if (responseCode == 429) { // Too Many Requests // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After // Only delay-seconds (whole seconds, RFC 9110) is parsed; a missing header, an HTTP-date or anything else - // unparseable gets the minimum. At least 2 seconds, so a "0" can't turn into a tight retry loop; at most 10 - // minutes, so one bad header can't keep us offline for long. - static constexpr uint32_t kMinRetryAfterSeconds = 2; - static constexpr uint32_t kMaxRetryAfterSeconds = 600; + // unparseable gets the minimum. + static_assert(CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MIN_S <= CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MAX_S, "Retry-After minimum above its maximum"); - uint32_t retryAfter = OpenShock::Convert::ToUint32(m_retryAfter).value_or(kMinRetryAfterSeconds); + uint32_t retryAfter = OpenShock::Convert::ToUint32(m_retryAfter).value_or(CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MIN_S); - rateLimiter->blockFor(static_cast(std::clamp(retryAfter, kMinRetryAfterSeconds, kMaxRetryAfterSeconds)) * 1000); + rateLimiter->blockFor(static_cast(std::clamp(retryAfter, CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MIN_S, CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MAX_S)) * 1000); return fail(RequestResult::RateLimited, responseCode, 0); // body not drained } @@ -293,7 +290,7 @@ HTTP::Response return {RequestResult::Success, responseCode, 0}; } - uint8_t* buffer = static_cast(malloc(HTTP_BUFFER_SIZE)); + uint8_t* buffer = static_cast(malloc(CONFIG_OPENSHOCK_HTTP_BUFFER_SIZE)); if (buffer == nullptr) { OS_LOGE(TAG, "Failed to allocate HTTP buffer"); return fail(RequestResult::InternalError, responseCode, 0); @@ -313,7 +310,7 @@ HTTP::Response } esp_http_client_set_timeout_ms(m_handle, static_cast(remaining)); - read = esp_http_client_read(m_handle, reinterpret_cast(buffer), static_cast(HTTP_BUFFER_SIZE)); + read = esp_http_client_read(m_handle, reinterpret_cast(buffer), static_cast(CONFIG_OPENSHOCK_HTTP_BUFFER_SIZE)); if (read == -ESP_ERR_HTTP_EAGAIN) { OS_LOGW(TAG, "Request timed out"); // The remaining budget ran out inside the read result = HTTP::RequestResult::TimedOut; @@ -353,13 +350,10 @@ HTTP::Response HTTP::Response HTTP::Client::GetString(std::string_view url, const std::map& headers, std::span acceptedCodes, uint32_t timeoutMs) { - // String responses are small JSON documents; refuse anything that would only exhaust the heap. - static constexpr std::size_t kMaxStringResponseSize = 64 * 1024; - std::string result; auto allocator = [&result](std::size_t contentLength) { - if (contentLength > kMaxStringResponseSize) { + if (contentLength > CONFIG_OPENSHOCK_HTTP_MAX_STRING_RESPONSE_SIZE) { OS_LOGE(TAG, "Response too large (%zu bytes)", contentLength); return false; } @@ -367,8 +361,8 @@ HTTP::Response HTTP::Client::GetString(std::string_view url, const return true; }; auto writer = [&result](std::size_t offset, const uint8_t* data, std::size_t len) { - if (result.size() + len > kMaxStringResponseSize) { - OS_LOGE(TAG, "Response exceeds %zu bytes", kMaxStringResponseSize); + if (result.size() + len > CONFIG_OPENSHOCK_HTTP_MAX_STRING_RESPONSE_SIZE) { + OS_LOGE(TAG, "Response exceeds %d bytes", CONFIG_OPENSHOCK_HTTP_MAX_STRING_RESPONSE_SIZE); return false; } result.append(reinterpret_cast(data), len); diff --git a/components/logging/Kconfig.openshock b/components/logging/Kconfig.openshock new file mode 100644 index 00000000..7ee1884c --- /dev/null +++ b/components/logging/Kconfig.openshock @@ -0,0 +1,43 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "Logging" + + choice OPENSHOCK_LOG_LEVEL_CHOICE + prompt "Log level" + default OPENSHOCK_LOG_LEVEL_FROM_BUILD_MODE + help + Compile-time level of the OS_LOG* macros; anything below it is compiled out. + + By default it follows the build mode (scripts/gen_env_header.py: verbose for + local and develop builds, debug for beta, info for stable). That value lives + in openshock_board.h, not sdkconfig.h, so CI builds of every channel keep one + sdkconfig.h and share their compiled ESP-IDF objects. Pick a level here to + override it for a local build. + + config OPENSHOCK_LOG_LEVEL_FROM_BUILD_MODE + bool "From the build mode" + config OPENSHOCK_LOG_LEVEL_SET_NONE + bool "None" + config OPENSHOCK_LOG_LEVEL_SET_ERROR + bool "Error" + config OPENSHOCK_LOG_LEVEL_SET_WARN + bool "Warning" + config OPENSHOCK_LOG_LEVEL_SET_INFO + bool "Info" + config OPENSHOCK_LOG_LEVEL_SET_DEBUG + bool "Debug" + config OPENSHOCK_LOG_LEVEL_SET_VERBOSE + bool "Verbose" + endchoice + + config OPENSHOCK_LOG_LEVEL_OVERRIDE + int + default -1 if OPENSHOCK_LOG_LEVEL_FROM_BUILD_MODE + default 0 if OPENSHOCK_LOG_LEVEL_SET_NONE + default 1 if OPENSHOCK_LOG_LEVEL_SET_ERROR + default 2 if OPENSHOCK_LOG_LEVEL_SET_WARN + default 3 if OPENSHOCK_LOG_LEVEL_SET_INFO + default 4 if OPENSHOCK_LOG_LEVEL_SET_DEBUG + default 5 if OPENSHOCK_LOG_LEVEL_SET_VERBOSE + +endmenu diff --git a/components/logging/include/Logging.h b/components/logging/include/Logging.h index c7c1291f..2659c7eb 100644 --- a/components/logging/include/Logging.h +++ b/components/logging/include/Logging.h @@ -1,6 +1,7 @@ #pragma once #include "openshock_board.h" // OPENSHOCK_LOG_LEVEL, which the OS_LOG* macros below are gated on +#include "sdkconfig.h" #include "Temporal.h" #include @@ -14,6 +15,12 @@ #error "OPENSHOCK_LOG_LEVEL is not defined (openshock_board.h)" #endif +// A level picked in menuconfig wins over the build mode's (CONFIG_OPENSHOCK_LOG_LEVEL_CHOICE) +#if defined(CONFIG_OPENSHOCK_LOG_LEVEL_OVERRIDE) && CONFIG_OPENSHOCK_LOG_LEVEL_OVERRIDE >= 0 +#undef OPENSHOCK_LOG_LEVEL +#define OPENSHOCK_LOG_LEVEL CONFIG_OPENSHOCK_LOG_LEVEL_OVERRIDE +#endif + // openshock_log_printf forwards to vprintf (the IDF console: UART0 + any secondary // USB-Serial-JTAG/CDC output). Defined in Logging.cpp so the logging path carries // no hardware include of its own. The OS_PANIC* macros below, however, pull in diff --git a/components/ota/Kconfig.openshock b/components/ota/Kconfig.openshock new file mode 100644 index 00000000..36ab8bd9 --- /dev/null +++ b/components/ota/Kconfig.openshock @@ -0,0 +1,12 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "OTA updates" + + config OPENSHOCK_OTA_TASK_STACK_SIZE + int "OTA task stack (bytes)" + range 8192 32768 + default 16384 + help + Profiled at about 6.2 KB. + +endmenu diff --git a/components/ota/src/OtaUpdateManager.cpp b/components/ota/src/OtaUpdateManager.cpp index fe02e3a0..4255a248 100644 --- a/components/ota/src/OtaUpdateManager.cpp +++ b/components/ota/src/OtaUpdateManager.cpp @@ -656,7 +656,7 @@ bool OtaUpdateManager::Init() } // Start OTA update task. - if (TaskUtils::TaskCreateExpensive(otaum_updatetask, "OTA Update", 16'384, nullptr, 1, &_taskHandle) != pdPASS) { // PROFILED: 6.2KB stack usage + if (TaskUtils::TaskCreateExpensive(otaum_updatetask, "OTA Update", CONFIG_OPENSHOCK_OTA_TASK_STACK_SIZE, nullptr, 1, &_taskHandle) != pdPASS) { OS_LOGE(TAG, "Failed to create OTA update task"); return false; } diff --git a/components/serial/Kconfig.openshock b/components/serial/Kconfig.openshock new file mode 100644 index 00000000..e0cef344 --- /dev/null +++ b/components/serial/Kconfig.openshock @@ -0,0 +1,20 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "Serial driver" + + config OPENSHOCK_SERIAL_RX_BUFFER_SIZE + int "Console driver receive buffer (bytes)" + range 128 4096 + default 256 + + config OPENSHOCK_SERIAL_UART_TX_BUFFER_SIZE + int "UART console transmit buffer (bytes)" + range 256 16384 + default 2048 + + config OPENSHOCK_SERIAL_USJ_TX_BUFFER_SIZE + int "USB-Serial-JTAG console transmit buffer (bytes)" + range 256 16384 + default 1024 + +endmenu diff --git a/components/serial/src/Serial.cpp b/components/serial/src/Serial.cpp index 52d76579..810229a8 100644 --- a/components/serial/src/Serial.cpp +++ b/components/serial/src/Serial.cpp @@ -32,12 +32,6 @@ using namespace OpenShock; -static constexpr int k_consoleBufferSize = 256; - -#if defined(OS_CONSOLE_USJ) -static constexpr int k_usjTxBufferSize = 1024; -#endif - #if defined(OS_CONSOLE_USJ) || defined(OS_CONSOLE_CDC) // How long a USB console write may wait for the host to drain it. Once one wait runs out the host is presumed absent // (a headless hub on USB power) and later writes don't wait at all until one goes through again, so a log line costs @@ -77,7 +71,7 @@ bool Serial::Init() #if defined(OS_CONSOLE_USJ) // A larger TX ring absorbs bursts while the host reads; RX stays small (console commands) - usb_serial_jtag_driver_config_t cfg = {.tx_buffer_size = k_usjTxBufferSize, .rx_buffer_size = k_consoleBufferSize}; + usb_serial_jtag_driver_config_t cfg = {.tx_buffer_size = CONFIG_OPENSHOCK_SERIAL_USJ_TX_BUFFER_SIZE, .rx_buffer_size = CONFIG_OPENSHOCK_SERIAL_RX_BUFFER_SIZE}; esp_err_t err = usb_serial_jtag_driver_install(&cfg); if (err != ESP_OK && err != ESP_ERR_INVALID_STATE) { @@ -88,8 +82,7 @@ bool Serial::Init() // With a TX ring buffer, uart_write_bytes returns once the bytes are queued instead of waiting ~9 ms per 100-byte // line at 115200 baud, so logging no longer stalls the caller (event loop, httpd, ...). OS_PANIC waits 5 s before // restarting, which lets the buffer drain; only OS_PANIC_INSTANT can lose its last line. - static constexpr int k_uartTxBufferSize = 2048; - esp_err_t err = uart_driver_install(static_cast(CONFIG_ESP_CONSOLE_UART_NUM), k_consoleBufferSize, k_uartTxBufferSize, 0, nullptr, 0); + esp_err_t err = uart_driver_install(static_cast(CONFIG_ESP_CONSOLE_UART_NUM), CONFIG_OPENSHOCK_SERIAL_RX_BUFFER_SIZE, CONFIG_OPENSHOCK_SERIAL_UART_TX_BUFFER_SIZE, 0, nullptr, 0); if (err != ESP_OK && err != ESP_ERR_INVALID_STATE) { return false; } @@ -142,7 +135,7 @@ static int driverWrite(const uint8_t* data, std::size_t len) std::size_t sent = 0; while (sent < len) { TickType_t elapsed = xTaskGetTickCount() - started; // Unsigned: wrap-safe - std::size_t chunk = std::min(len - sent, static_cast(k_usjTxBufferSize)); + std::size_t chunk = std::min(len - sent, static_cast(CONFIG_OPENSHOCK_SERIAL_USJ_TX_BUFFER_SIZE)); int n = usb_serial_jtag_write_bytes(data + sent, chunk, elapsed < timeout ? timeout - elapsed : 0); if (n <= 0) { s_usbHostStalled.store(true, std::memory_order_relaxed); diff --git a/components/serial_console/Kconfig.openshock b/components/serial_console/Kconfig.openshock new file mode 100644 index 00000000..14aa9ce3 --- /dev/null +++ b/components/serial_console/Kconfig.openshock @@ -0,0 +1,18 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "Serial console" + + config OPENSHOCK_SERIAL_RX_TASK_STACK_SIZE + int "Serial input task stack (bytes)" + range 4096 32768 + default 10000 + + config OPENSHOCK_SERIAL_LINE_MAX_SIZE + int "Longest input line (bytes)" + range 256 16384 + default 4096 + help + A longer line is discarded. rawconfig and jsonconfig take the whole config on + one line, so this bounds the config the console can set. + +endmenu diff --git a/components/serial_console/src/SerialInputHandler.cpp b/components/serial_console/src/SerialInputHandler.cpp index 4efbf6d3..359af61e 100644 --- a/components/serial_console/src/SerialInputHandler.cpp +++ b/components/serial_console/src/SerialInputHandler.cpp @@ -55,8 +55,7 @@ using namespace std::string_view_literals; using namespace OpenShock; -const int64_t PASTE_INTERVAL_THRESHOLD_MS = 20; -const std::size_t SERIAL_BUFFER_MAX_CAPACITY = 4096; +const int64_t PASTE_INTERVAL_THRESHOLD_MS = 20; static bool s_echoEnabled = true; static std::vector s_commandGroups; @@ -305,7 +304,7 @@ class SerialBuffer { constexpr std::size_t capacity() const noexcept { return m_capacity; } constexpr bool empty() const noexcept { return m_size == 0; } - // Set when a line outgrew SERIAL_BUFFER_MAX_CAPACITY; the rest of that line is discarded and it is never executed. + // Set when a line outgrew CONFIG_OPENSHOCK_SERIAL_LINE_MAX_SIZE; the rest of that line is discarded and it is never executed. constexpr bool overflowed() const noexcept { return m_overflowed; } constexpr void clear() noexcept @@ -318,8 +317,8 @@ class SerialBuffer { { size = (size + 31) & ~31; // Align to 32 bytes - if (size > SERIAL_BUFFER_MAX_CAPACITY) { - OS_LOGE(TAG, "Line exceeds %zu bytes, discarding it", SERIAL_BUFFER_MAX_CAPACITY); + if (size > CONFIG_OPENSHOCK_SERIAL_LINE_MAX_SIZE) { + OS_LOGE(TAG, "Line exceeds %d bytes, discarding it", CONFIG_OPENSHOCK_SERIAL_LINE_MAX_SIZE); m_size = 0; m_overflowed = true; return; @@ -631,7 +630,7 @@ static void serialRxTask(void*) switch (tryReadSerialLine(buffer)) { case SerialReadResult::LineEnd: if (buffer.overflowed()) { - SERPR_ERROR("Line too long (max %zu bytes), ignored", SERIAL_BUFFER_MAX_CAPACITY); + SERPR_ERROR("Line too long (max %d bytes), ignored", CONFIG_OPENSHOCK_SERIAL_LINE_MAX_SIZE); } else { processSerialLine(buffer); } @@ -689,7 +688,7 @@ bool SerialInputHandler::Init() } s_echoEnabled = *echoEnabled; - if (TaskUtils::TaskCreateExpensive(serialRxTask, "SerialRX", 10'000, nullptr, 1, nullptr) != pdPASS) { // TODO: Profile stack size + if (TaskUtils::TaskCreateExpensive(serialRxTask, "SerialRX", CONFIG_OPENSHOCK_SERIAL_RX_TASK_STACK_SIZE, nullptr, 1, nullptr) != pdPASS) { // TODO: Profile stack size OS_LOGE(TAG, "Failed to create serial RX task"); return false; } diff --git a/main/Kconfig.projbuild b/main/Kconfig.projbuild new file mode 100644 index 00000000..3b2b3229 --- /dev/null +++ b/main/Kconfig.projbuild @@ -0,0 +1,24 @@ +menu "OpenShock" + +# The board's GPIO assignments deliberately do NOT live here. +# +# A Kconfig symbol ends up in sdkconfig.h, which most of ESP-IDF includes, so two +# boards that differed only by an LED pin produced a different sdkconfig.h and +# therefore shared no compiled framework objects at all. The pins are stamped into +# the image per board instead (BoardDescriptor.h): bare `OPENSHOCK_*=` lines in +# boards/.defaults, stripped from the sdkconfig fragment by scripts/build.py. +# +# Everything in it is identical across every board and build channel, so it costs +# nothing in shared objects. Keep it that way: a setting that has to differ per +# board or per channel doesn't belong here (see OPENSHOCK_LOG_LEVEL_CHOICE). +# +# Fixed in code on purpose: the RF protocol timings, the E-Stop debounce, hold and +# rearm times (safety behaviour), task core affinity, and on-flash formats. +# +# Each component keeps its own settings in components//Kconfig.openshock. They are +# all sourced here, also for components a build leaves out, so every CONFIG_OPENSHOCK_* +# symbol exists in every build (the host tests build only their own sources). + +orsource "../components/*/Kconfig.openshock" + +endmenu diff --git a/test/host_support/CMakeLists.txt b/test/host_support/CMakeLists.txt index c3e29bef..d40e64ad 100644 --- a/test/host_support/CMakeLists.txt +++ b/test/host_support/CMakeLists.txt @@ -14,7 +14,7 @@ # not, so this component compiles it, together with # common/src/BoardDescriptor.cpp, and every suite has # Board::Get() (after calling Board::Init() in its main). -# Kconfig.projbuild - sources core's Kconfig.projbuild, so the suites see the +# Kconfig.projbuild - sources main/Kconfig.projbuild, so the suites see the # real CONFIG_OPENSHOCK_* defaults through sdkconfig.h. # # Everything else a suite needs comes from the real OpenShock headers, ESP-IDF's diff --git a/test/host_support/Kconfig.projbuild b/test/host_support/Kconfig.projbuild index 25b0bb0b..016ea7e0 100644 --- a/test/host_support/Kconfig.projbuild +++ b/test/host_support/Kconfig.projbuild @@ -1,4 +1,4 @@ -# The CONFIG_OPENSHOCK_* settings (domains, hostname, AP prefix, buffer sizes) the -# firmware reads through sdkconfig.h. Sourced from core rather than copied, so the -# host tests always build against the same defaults as the firmware. -rsource "../../components/core/Kconfig.projbuild" +# The CONFIG_OPENSHOCK_* settings the firmware reads through sdkconfig.h. Sourced from +# the firmware's own menu (which pulls in every component's Kconfig.openshock) rather +# than copied, so the host tests always build against the same defaults as the firmware. +rsource "../../main/Kconfig.projbuild" From ef05c4760f2be3c7b4f0636d056690233667b9b4 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Sun, 25 Oct 2026 20:25:36 +0100 Subject: [PATCH 02/12] fix(kconfig): address review findings on the tunables - Add the Release Note section the change-file check requires - Describe the portal startup grace as a delay before opening, and log the configured value - Keep lwIP sockets free for the DNS server and outbound connections (httpd sockets now 2..10) - Let the WiFi connect backoff reach its configured cap instead of stopping at 32 s - Check at compile time that RF priority is above the WiFi manager and DNS tasks, and DNS below E-Stop - Accept a serial line exactly at the configured limit by checking before alignment - Raise the console RX buffer minimum above the 128-byte UART FIFO - Enforce the download size limit on chunked responses --- .changes/kconfig-tunables.md | 3 +++ components/captive_portal/Kconfig.openshock | 8 +++++--- .../src/captiveportal/CaptivePortalInstance.cpp | 3 ++- .../captive_portal/src/captiveportal/Manager.cpp | 2 +- components/core/src/wifi/WiFiManager.cpp | 2 +- components/device_control/Kconfig.openshock | 10 +++++----- components/device_control/src/radio/RFTransmitter.cpp | 2 ++ components/dns_server/Kconfig.openshock | 2 +- components/dns_server/src/DNSServer.cpp | 2 ++ components/http/src/HTTPRequestManager.cpp | 7 +++++++ components/serial/Kconfig.openshock | 4 +++- components/serial_console/src/SerialInputHandler.cpp | 5 +++-- 12 files changed, 35 insertions(+), 15 deletions(-) diff --git a/.changes/kconfig-tunables.md b/.changes/kconfig-tunables.md index 6e34ca2e..5116f7ed 100644 --- a/.changes/kconfig-tunables.md +++ b/.changes/kconfig-tunables.md @@ -3,5 +3,8 @@ kind: changed --- Move build-time settings into Kconfig +## Release Note +Build-time settings can now be changed in menuconfig + - Task stacks and priorities, timeouts, retry intervals, queue and buffer sizes are now set in `menuconfig` under OpenShock, each next to the component that uses it - The log level can be overridden in `menuconfig`; by default it still follows the build mode diff --git a/components/captive_portal/Kconfig.openshock b/components/captive_portal/Kconfig.openshock index e8cc6dbd..fae77852 100644 --- a/components/captive_portal/Kconfig.openshock +++ b/components/captive_portal/Kconfig.openshock @@ -12,12 +12,13 @@ menu "Captive portal" config OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS int "HTTP server sockets" - range 2 13 + range 2 10 default 8 help Browsers open several connections per page load; with only 4 the LRU purge evicted the websocket (server push, so it looks idle). Also sizes the broadcast - fd list. httpd needs 3 lwIP sockets besides these (checked against + fd list. httpd needs 3 lwIP sockets besides these, and the DNS server, the + gateway connection and an outbound HTTP request need one each (checked against LWIP_MAX_SOCKETS). config OPENSHOCK_PORTAL_HTTPD_STACK_SIZE @@ -48,7 +49,8 @@ menu "Captive portal" range 0 600 default 30 help - The portal stays up this long after boot, whatever the connection state. + On a fully configured hub, how long after boot the portal waits for the gateway + connection before it opens. It stays closed if the hub connects in time. config OPENSHOCK_PORTAL_AUTO_CLOSE_S int "Auto-close delay (s)" diff --git a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index 80e4c6a6..954baf34 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -35,7 +35,8 @@ const char* const TAG = "CaptivePortalInstance"; #include const uint16_t HTTP_PORT = 80; -static_assert(CONFIG_LWIP_MAX_SOCKETS >= CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS + 3, "httpd needs 3 lwIP sockets besides its client sockets"); +// httpd needs 3 lwIP sockets besides its client sockets; the DNS server, the gateway connection and an outbound HTTP request need one each +static_assert(CONFIG_LWIP_MAX_SOCKETS >= CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS + 3 + 3, "httpd client sockets would leave no lwIP sockets for the DNS server and outbound connections"); static_assert(CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS > CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS, "every websocket client needs a socket, plus one for plain requests"); // HTTP status lines (esp_http_server needs the full "code reason" string, and stores diff --git a/components/captive_portal/src/captiveportal/Manager.cpp b/components/captive_portal/src/captiveportal/Manager.cpp index ed832d2a..933fc712 100644 --- a/components/captive_portal/src/captiveportal/Manager.cpp +++ b/components/captive_portal/src/captiveportal/Manager.cpp @@ -368,7 +368,7 @@ bool CaptivePortal::Init() // If device is already fully configured, set a startup grace period before opening portal if (isDeviceFullyConfigured()) { initialState->startupGraceEnd = esp_timer_get_time() + STARTUP_GRACE_PERIOD_US; - OS_LOGI(TAG, "Device fully configured, startup grace period of 30s before opening portal"); + OS_LOGI(TAG, "Device fully configured, startup grace period of %ds before opening portal", CONFIG_OPENSHOCK_PORTAL_STARTUP_GRACE_S); } s_forceCloseDone = xSemaphoreCreateBinary(); diff --git a/components/core/src/wifi/WiFiManager.cpp b/components/core/src/wifi/WiFiManager.cpp index 0bb4dee8..1dca2277 100644 --- a/components/core/src/wifi/WiFiManager.cpp +++ b/components/core/src/wifi/WiFiManager.cpp @@ -147,7 +147,7 @@ static bool isConnectRateLimited(const WiFiNetwork& net) // takes the radio off the captive portal AP's channel, so retrying every second starves portal clients. int64_t now = OpenShock::millis(); int64_t diff = now - net.lastConnectAttempt; - uint16_t shift = std::min(net.connectAttempts, 5); + uint16_t shift = std::min(net.connectAttempts, 20); // 1000 << 20 is past the largest cap int64_t backoff = std::min(1000LL << shift, CONFIG_OPENSHOCK_WIFI_CONNECT_BACKOFF_MAX_MS); return diff < backoff; diff --git a/components/device_control/Kconfig.openshock b/components/device_control/Kconfig.openshock index 7c443d9a..772bb4dc 100644 --- a/components/device_control/Kconfig.openshock +++ b/components/device_control/Kconfig.openshock @@ -11,13 +11,13 @@ menu "Radio" config OPENSHOCK_RF_TASK_PRIORITY int "RF transmitter task priority" - range 1 24 + range 2 24 default 9 help Above the network tasks (5): it blocks on its queue or on RMT almost all the time, and frames (and the terminators after an E-Stop) shouldn't wait behind TLS work. - Must stay below the E-Stop task's priority, so a press preempts a transmission - (checked at compile time). + Must stay above the WiFi manager and DNS server tasks, and below the E-Stop task's + priority so a press preempts a transmission (checked at compile time). config OPENSHOCK_RF_QUEUE_SIZE int "RF command queue length" @@ -96,11 +96,11 @@ menu "Emergency stop" config OPENSHOCK_ESTOP_TASK_PRIORITY int "E-Stop task priority" - range 2 24 + range 3 24 default 10 help Above the network tasks (5) and the RF task: it is blocked almost all the time, and a sample that waits behind TLS work delays the E-Stop by that long. Must stay - above the RF transmitter task's priority (checked at compile time). + above the RF transmitter and DNS server tasks' priorities (checked at compile time). endmenu diff --git a/components/device_control/src/radio/RFTransmitter.cpp b/components/device_control/src/radio/RFTransmitter.cpp index eb128f13..6faa8728 100644 --- a/components/device_control/src/radio/RFTransmitter.cpp +++ b/components/device_control/src/radio/RFTransmitter.cpp @@ -21,6 +21,8 @@ const char* const TAG = "RFTransmitter"; #include static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY < CONFIG_OPENSHOCK_ESTOP_TASK_PRIORITY, "an E-Stop press must preempt a transmission"); +static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY > CONFIG_OPENSHOCK_WIFI_TASK_PRIORITY, "frames and E-Stop terminators must not wait behind the WiFi manager"); +static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY > CONFIG_OPENSHOCK_DNS_TASK_PRIORITY, "frames and E-Stop terminators must not wait behind the DNS server"); const int32_t kRmtTimeoutMs = 100; // After this many transmit failures in a row the RMT channel is presumed broken and the pending sequences are dropped const uint32_t kMaxConsecutiveFailures = 20; diff --git a/components/dns_server/Kconfig.openshock b/components/dns_server/Kconfig.openshock index 69efb609..3a0bcac6 100644 --- a/components/dns_server/Kconfig.openshock +++ b/components/dns_server/Kconfig.openshock @@ -15,6 +15,6 @@ menu "Captive portal DNS server" Above the other application tasks (1): a phone joining the AP sends a burst of DNS queries, and the small UDP receive mailbox drops them if this task is starved, which delays captive-portal detection by seconds. Keep it below httpd (5) and the - E-Stop task. + E-Stop task (checked at compile time). endmenu diff --git a/components/dns_server/src/DNSServer.cpp b/components/dns_server/src/DNSServer.cpp index 6ecbdddc..149b5de9 100644 --- a/components/dns_server/src/DNSServer.cpp +++ b/components/dns_server/src/DNSServer.cpp @@ -15,6 +15,8 @@ const char* const TAG = "DNSServer"; #include #include +static_assert(CONFIG_OPENSHOCK_DNS_TASK_PRIORITY < CONFIG_OPENSHOCK_ESTOP_TASK_PRIORITY, "a DNS query burst must not delay E-Stop sampling"); + using namespace OpenShock; DNSServer::DNSServer() diff --git a/components/http/src/HTTPRequestManager.cpp b/components/http/src/HTTPRequestManager.cpp index 2d70d5dc..4b872f6e 100644 --- a/components/http/src/HTTPRequestManager.cpp +++ b/components/http/src/HTTPRequestManager.cpp @@ -320,6 +320,13 @@ HTTP::Response break; } + // Content-Length was checked against the limit up front, a chunked body is only bounded here + if (static_cast(totalWritten) + read > HTTP_DOWNLOAD_SIZE_LIMIT) { + OS_LOGE(TAG, "Response body exceeds %d MB", CONFIG_OPENSHOCK_HTTP_DOWNLOAD_SIZE_LIMIT_MB); + result = HTTP::RequestResult::RequestFailed; + break; + } + if (!downloadCallback(totalWritten, buffer, static_cast(read))) { OS_LOGW(TAG, "Request cancelled by callback"); result = HTTP::RequestResult::Cancelled; diff --git a/components/serial/Kconfig.openshock b/components/serial/Kconfig.openshock index e0cef344..34830ad5 100644 --- a/components/serial/Kconfig.openshock +++ b/components/serial/Kconfig.openshock @@ -4,8 +4,10 @@ menu "Serial driver" config OPENSHOCK_SERIAL_RX_BUFFER_SIZE int "Console driver receive buffer (bytes)" - range 128 4096 + range 256 4096 default 256 + help + The UART driver needs more than its 128-byte hardware FIFO. config OPENSHOCK_SERIAL_UART_TX_BUFFER_SIZE int "UART console transmit buffer (bytes)" diff --git a/components/serial_console/src/SerialInputHandler.cpp b/components/serial_console/src/SerialInputHandler.cpp index 359af61e..49ead882 100644 --- a/components/serial_console/src/SerialInputHandler.cpp +++ b/components/serial_console/src/SerialInputHandler.cpp @@ -315,14 +315,15 @@ class SerialBuffer { inline void reserve(std::size_t size) { - size = (size + 31) & ~31; // Align to 32 bytes - if (size > CONFIG_OPENSHOCK_SERIAL_LINE_MAX_SIZE) { OS_LOGE(TAG, "Line exceeds %d bytes, discarding it", CONFIG_OPENSHOCK_SERIAL_LINE_MAX_SIZE); m_size = 0; m_overflowed = true; return; } + + size = (size + 31) & ~31; // Align to 32 bytes + if (size <= m_capacity) { return; } From b835639c5c71dfab275385214a644e985121e3b5 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:14:16 +0000 Subject: [PATCH 03/12] fix(serial): enforce the line length limit even when buffer capacity is sufficient --- components/serial_console/src/SerialInputHandler.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/serial_console/src/SerialInputHandler.cpp b/components/serial_console/src/SerialInputHandler.cpp index 49ead882..446055a2 100644 --- a/components/serial_console/src/SerialInputHandler.cpp +++ b/components/serial_console/src/SerialInputHandler.cpp @@ -345,6 +345,13 @@ class SerialBuffer { return; } + if (m_size + len > CONFIG_OPENSHOCK_SERIAL_LINE_MAX_SIZE) { + OS_LOGE(TAG, "Line exceeds %d bytes, discarding it", CONFIG_OPENSHOCK_SERIAL_LINE_MAX_SIZE); + m_size = 0; + m_overflowed = true; + return; + } + if (m_size + len > m_capacity) { reserve(m_size + len); if (m_overflowed) { From 6c2d6286227343ba0cc8bd1f2ad798c9c61f84bf Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 27 Oct 2026 04:58:47 +0100 Subject: [PATCH 04/12] feat(kconfig): make the portal AP address and venue info URL configurable The captive portal's SoftAP address (4.3.2.1) and the RFC 8908 venue-info-url (https://openshock.org) were literals. They are now OPENSHOCK_PORTAL_AP_IP, in the captive portal's fragment, and OPENSHOCK_PORTAL_VENUE_INFO_URL, in a new rfc8908 fragment. Defaults are unchanged. An empty venue URL leaves the field out of the captive API response. The AP address is checked at compile time by IPv4Utils::IsDottedDecimal, a new constexpr helper in common with host tests. It rejects leading zeros, since lwIP reads those as octal. The serial console's domain and hostname commands now show CONFIG_OPENSHOCK_API_DOMAIN and CONFIG_OPENSHOCK_FW_HOSTNAME as their example values, replacing the stale api.shocklink.net and a literal "OpenShock". --- .changes/kconfig-tunables.md | 1 + components/captive_portal/Kconfig.openshock | 8 +++++ .../src/captiveportal/Manager.cpp | 13 ++++---- .../common/host_test/main/CMakeLists.txt | 1 + .../common/host_test/main/test_ipv4utils.cpp | 32 +++++++++++++++++++ components/common/include/util/IPv4Utils.h | 26 +++++++++++++++ components/rfc8908/Kconfig.openshock | 12 +++++++ components/rfc8908/src/RFC8908Handler.cpp | 4 ++- .../src/command_handlers/domain.cpp | 2 +- .../src/command_handlers/hostname.cpp | 2 +- 10 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 components/common/host_test/main/test_ipv4utils.cpp create mode 100644 components/common/include/util/IPv4Utils.h create mode 100644 components/rfc8908/Kconfig.openshock diff --git a/.changes/kconfig-tunables.md b/.changes/kconfig-tunables.md index 5116f7ed..6b9abe70 100644 --- a/.changes/kconfig-tunables.md +++ b/.changes/kconfig-tunables.md @@ -7,4 +7,5 @@ Move build-time settings into Kconfig Build-time settings can now be changed in menuconfig - Task stacks and priorities, timeouts, retry intervals, queue and buffer sizes are now set in `menuconfig` under OpenShock, each next to the component that uses it +- The default API and firmware CDN domains, hostname, SoftAP SSID prefix, captive portal IP address and venue info URL are set there too - The log level can be overridden in `menuconfig`; by default it still follows the build mode diff --git a/components/captive_portal/Kconfig.openshock b/components/captive_portal/Kconfig.openshock index fae77852..1df60aea 100644 --- a/components/captive_portal/Kconfig.openshock +++ b/components/captive_portal/Kconfig.openshock @@ -2,6 +2,14 @@ menu "Captive portal" + config OPENSHOCK_PORTAL_AP_IP + string "SoftAP IPv4 address" + default "4.3.2.1" + help + The fixed address the portal's SoftAP, DHCP server, DNS responder and captive + portal API serve from, on a /24 network. Must be a dotted-decimal IPv4 address + (checked at compile time). + config OPENSHOCK_PORTAL_MAX_CLIENTS int "Clients (SoftAP stations and websockets)" range 1 8 diff --git a/components/captive_portal/src/captiveportal/Manager.cpp b/components/captive_portal/src/captiveportal/Manager.cpp index 933fc712..5b6f100e 100644 --- a/components/captive_portal/src/captiveportal/Manager.cpp +++ b/components/captive_portal/src/captiveportal/Manager.cpp @@ -22,6 +22,7 @@ const char* const TAG = "CaptivePortal"; #include #include "SimpleMutex.h" +#include "util/IPv4Utils.h" #include "util/RetiredList.h" #include "util/TaskUtils.h" @@ -72,9 +73,7 @@ static std::atomic s_instanceLive = false; static RetiredList s_retiredInstances; static esp_netif_t* s_apNetif = nullptr; -// The captive portal AP always serves from this fixed address; the DNS server and -// RFC8908 handler reference it too (see CaptivePortal::ApIPv4String()). -static const char* const CAPTIVE_PORTAL_AP_IP = "4.3.2.1"; +static_assert(IPv4Utils::IsDottedDecimal(CONFIG_OPENSHOCK_PORTAL_AP_IP), "CONFIG_OPENSHOCK_PORTAL_AP_IP must be a dotted-decimal IPv4 address"); static void sendCommand(uint32_t command) { @@ -161,8 +160,8 @@ static bool captiveportal_start() } esp_netif_ip_info_t ipInfo = {}; - esp_netif_str_to_ip4(CAPTIVE_PORTAL_AP_IP, &ipInfo.ip); - esp_netif_str_to_ip4(CAPTIVE_PORTAL_AP_IP, &ipInfo.gw); + esp_netif_str_to_ip4(CONFIG_OPENSHOCK_PORTAL_AP_IP, &ipInfo.ip); + esp_netif_str_to_ip4(CONFIG_OPENSHOCK_PORTAL_AP_IP, &ipInfo.gw); esp_netif_str_to_ip4("255.255.255.0", &ipInfo.netmask); esp_netif_dhcps_stop(s_apNetif); @@ -172,7 +171,7 @@ static bool captiveportal_start() // can open the portal without waiting for their connectivity probes to be redirected. esp_netif keeps the // pointer, so the string must outlive the netif. static char captivePortalUri[64]; - snprintf(captivePortalUri, sizeof(captivePortalUri), "http://%s/captive-portal/api", CAPTIVE_PORTAL_AP_IP); // RFC8908Handler's API path + snprintf(captivePortalUri, sizeof(captivePortalUri), "http://%s/captive-portal/api", CONFIG_OPENSHOCK_PORTAL_AP_IP); // RFC8908Handler's API path esp_err_t uriErr = esp_netif_dhcps_option(s_apNetif, ESP_NETIF_OP_SET, ESP_NETIF_CAPTIVEPORTAL_URI, captivePortalUri, strlen(captivePortalUri)); if (uriErr != ESP_OK) { OS_LOGW(TAG, "Failed to set DHCP captive portal URI: %s", esp_err_to_name(uriErr)); @@ -457,7 +456,7 @@ bool CaptivePortal::IsRunning() const char* CaptivePortal::ApIPv4String() { - return CAPTIVE_PORTAL_AP_IP; + return CONFIG_OPENSHOCK_PORTAL_AP_IP; } bool CaptivePortal::SendMessageTXT(uint8_t socketId, std::string_view data) diff --git a/components/common/host_test/main/CMakeLists.txt b/components/common/host_test/main/CMakeLists.txt index c4839282..4dc82d84 100644 --- a/components/common/host_test/main/CMakeLists.txt +++ b/components/common/host_test/main/CMakeLists.txt @@ -9,6 +9,7 @@ idf_component_register( "test_stringutils.cpp" "test_convert.cpp" "test_hexutils.cpp" + "test_ipv4utils.cpp" "test_semver.cpp" "test_checksum.cpp" "test_boarddescriptor.cpp" diff --git a/components/common/host_test/main/test_ipv4utils.cpp b/components/common/host_test/main/test_ipv4utils.cpp new file mode 100644 index 00000000..23db3efb --- /dev/null +++ b/components/common/host_test/main/test_ipv4utils.cpp @@ -0,0 +1,32 @@ +// IPv4Utils: dotted-decimal validation, used to check CONFIG_OPENSHOCK_PORTAL_AP_IP at compile time. +#include "unity.h" + +#include "util/IPv4Utils.h" + +using namespace OpenShock; + +TEST_CASE("IsDottedDecimal accepts valid addresses", "[util][ipv4]") +{ + TEST_ASSERT_TRUE(IPv4Utils::IsDottedDecimal("4.3.2.1")); + TEST_ASSERT_TRUE(IPv4Utils::IsDottedDecimal("0.0.0.0")); + TEST_ASSERT_TRUE(IPv4Utils::IsDottedDecimal("255.255.255.255")); + TEST_ASSERT_TRUE(IPv4Utils::IsDottedDecimal("192.168.10.1")); +} + +TEST_CASE("IsDottedDecimal rejects malformed addresses", "[util][ipv4]") +{ + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("")); + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3")); // too few octets + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4.5")); // too many + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4.")); // trailing dot + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal(".1.2.3.4")); // leading dot + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1..3.4")); // empty octet + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.256")); // out of range + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1000.2.3.4")); // too many digits + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("01.2.3.4")); // leading zero (octal to lwIP) + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4 ")); // trailing junk + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("a.b.c.d")); +} + +static_assert(IPv4Utils::IsDottedDecimal("4.3.2.1")); +static_assert(!IPv4Utils::IsDottedDecimal("01.2.3.4")); diff --git a/components/common/include/util/IPv4Utils.h b/components/common/include/util/IPv4Utils.h new file mode 100644 index 00000000..417a6072 --- /dev/null +++ b/components/common/include/util/IPv4Utils.h @@ -0,0 +1,26 @@ +#pragma once + +#include +#include + +namespace OpenShock::IPv4Utils { + /// @brief Checks that a string is a dotted-decimal IPv4 address: four 0-255 octets. + /// @remark Leading zeros are rejected, since lwIP reads them as octal. Usable in a static_assert. + constexpr bool IsDottedDecimal(std::string_view str) noexcept + { + int octets = 0; + while (true) { + std::size_t digits = 0; + int value = 0; + while (digits < str.size() && str[digits] >= '0' && str[digits] <= '9') { + value = value * 10 + (str[digits] - '0'); + if (++digits > 3) return false; + } + if (digits == 0 || value > 255 || (digits > 1 && str[0] == '0')) return false; + str.remove_prefix(digits); + if (++octets == 4) return str.empty(); + if (str.empty() || str[0] != '.') return false; + str.remove_prefix(1); + } + } +} // namespace OpenShock::IPv4Utils diff --git a/components/rfc8908/Kconfig.openshock b/components/rfc8908/Kconfig.openshock new file mode 100644 index 00000000..f81dfce0 --- /dev/null +++ b/components/rfc8908/Kconfig.openshock @@ -0,0 +1,12 @@ +# Sourced into the OpenShock menu by main/Kconfig.projbuild + +menu "Captive portal detection" + + config OPENSHOCK_PORTAL_VENUE_INFO_URL + string "Venue info URL" + default "https://openshock.org" + help + The venue-info-url the RFC 8908 captive portal API reports; some phones offer + it as a link next to the portal. Leave empty to leave it out. + +endmenu diff --git a/components/rfc8908/src/RFC8908Handler.cpp b/components/rfc8908/src/RFC8908Handler.cpp index 67239df6..c9ae9fd5 100644 --- a/components/rfc8908/src/RFC8908Handler.cpp +++ b/components/rfc8908/src/RFC8908Handler.cpp @@ -49,7 +49,9 @@ static esp_err_t captiveApiHandler(httpd_req_t* req) json_gen_start_object(gen); json_gen_obj_set_bool(gen, "captive", true); JSON::objSetString(gen, "user-portal-url", portalUrl.c_str()); - JSON::objSetString(gen, "venue-info-url", "https://openshock.org"); + if constexpr (sizeof(CONFIG_OPENSHOCK_PORTAL_VENUE_INFO_URL) > 1) { // empty: leave it out + JSON::objSetString(gen, "venue-info-url", CONFIG_OPENSHOCK_PORTAL_VENUE_INFO_URL); + } json_gen_end_object(gen); std::string jsonStr = writer.finish(); diff --git a/components/serial_console/src/command_handlers/domain.cpp b/components/serial_console/src/command_handlers/domain.cpp index aaa16ddd..49d0ae54 100644 --- a/components/serial_console/src/command_handlers/domain.cpp +++ b/components/serial_console/src/command_handlers/domain.cpp @@ -69,7 +69,7 @@ OpenShock::SerialCmds::CommandGroup OpenShock::SerialCmds::CommandHandlers::Doma group.addCommand("Get the backend domain."sv, handleDomainCommand); auto& setCommand = group.addCommand("Set the backend domain."sv, handleDomainCommand); - setCommand.addArgument("domain"sv, "must be a string"sv, "api.shocklink.net"sv); + setCommand.addArgument("domain"sv, "must be a string"sv, std::string_view(CONFIG_OPENSHOCK_API_DOMAIN)); return group; } diff --git a/components/serial_console/src/command_handlers/hostname.cpp b/components/serial_console/src/command_handlers/hostname.cpp index d125a046..d4beaa77 100644 --- a/components/serial_console/src/command_handlers/hostname.cpp +++ b/components/serial_console/src/command_handlers/hostname.cpp @@ -59,7 +59,7 @@ OpenShock::SerialCmds::CommandGroup OpenShock::SerialCmds::CommandHandlers::Host group.addCommand("Get the network hostname."sv, handleHostnameCommand); auto& setCommand = group.addCommand("Set the network hostname."sv, handleHostnameCommand); - setCommand.addArgument("hostname"sv, "letters, digits and hyphens, max 32 characters"sv, "OpenShock"sv); + setCommand.addArgument("hostname"sv, "letters, digits and hyphens, max 32 characters"sv, std::string_view(CONFIG_OPENSHOCK_FW_HOSTNAME)); return group; } From d6d0a17b6dfd4bb8ae8a1c48b56597b34008b5d6 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 27 Oct 2026 05:00:47 +0100 Subject: [PATCH 05/12] style(common): clang-format the IPv4Utils host test --- .../common/host_test/main/test_ipv4utils.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/components/common/host_test/main/test_ipv4utils.cpp b/components/common/host_test/main/test_ipv4utils.cpp index 23db3efb..49cbbac3 100644 --- a/components/common/host_test/main/test_ipv4utils.cpp +++ b/components/common/host_test/main/test_ipv4utils.cpp @@ -16,15 +16,15 @@ TEST_CASE("IsDottedDecimal accepts valid addresses", "[util][ipv4]") TEST_CASE("IsDottedDecimal rejects malformed addresses", "[util][ipv4]") { TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("")); - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3")); // too few octets - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4.5")); // too many - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4.")); // trailing dot - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal(".1.2.3.4")); // leading dot - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1..3.4")); // empty octet - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.256")); // out of range - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1000.2.3.4")); // too many digits - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("01.2.3.4")); // leading zero (octal to lwIP) - TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4 ")); // trailing junk + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3")); // too few octets + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4.5")); // too many + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4.")); // trailing dot + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal(".1.2.3.4")); // leading dot + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1..3.4")); // empty octet + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.256")); // out of range + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1000.2.3.4")); // too many digits + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("01.2.3.4")); // leading zero (octal to lwIP) + TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("1.2.3.4 ")); // trailing junk TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("a.b.c.d")); } From b67399ece70f6a4ee506f960c40d3a04a011ed22 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 29 Oct 2026 21:41:02 +0100 Subject: [PATCH 06/12] refactor(kconfig): use standard component Kconfig files and check option ownership Each component's options move from Kconfig.openshock, sourced by a glob in main/Kconfig.projbuild, to the standard components//Kconfig that ESP-IDF includes for every component in the build. Their menus are named OpenShock ... under Component config. The host tests source the same files. Options more than one component reads move to the lowest component they all depend on. The default domains, hostname and SoftAP prefix, and every task's stack and priority, are in components/common/Kconfig, and the priority order between the tasks is checked in one place (TaskUtils.cpp) instead of in device_control and dns_server. The local message size limit moves from captive_portal to core, whose handler enforces it, as OPENSHOCK_LOCAL_MAX_MESSAGE_SIZE. The min/max pairs are Kconfig ranges now, so menuconfig refuses a maximum below its minimum, and their static_asserts are gone. scripts/check_kconfig_ownership.py fails when a component reads an option of a component it does not require; host-tests runs it first. --- .changes/kconfig-tunables.md | 2 +- .github/workflows/host-tests.yml | 4 + .../{Kconfig.openshock => Kconfig} | 26 +-- .../captiveportal/CaptivePortalInstance.cpp | 6 +- components/common/Kconfig | 169 ++++++++++++++++++ components/common/include/OpenShock.h | 2 +- components/common/src/TaskUtils.cpp | 8 + components/config/Kconfig.openshock | 29 --- .../core/{Kconfig.openshock => Kconfig} | 45 +++-- .../core/src/GatewayConnectionManager.cpp | 1 - .../src/message_handlers/websocket/Local.cpp | 4 +- components/core/src/wifi/WiFiScanManager.cpp | 2 - .../{Kconfig.openshock => Kconfig} | 51 +----- .../src/radio/RFTransmitter.cpp | 3 - components/dns_server/Kconfig.openshock | 20 --- components/dns_server/src/DNSServer.cpp | 2 - .../http/{Kconfig.openshock => Kconfig} | 6 +- components/http/src/HTTPRequestManager.cpp | 2 - .../logging/{Kconfig.openshock => Kconfig} | 4 +- components/ota/Kconfig.openshock | 12 -- .../rfc8908/{Kconfig.openshock => Kconfig} | 4 +- .../serial/{Kconfig.openshock => Kconfig} | 4 +- .../{Kconfig.openshock => Kconfig} | 9 +- docs/DEVELOPING.md | 10 ++ main/Kconfig.projbuild | 24 --- scripts/check_kconfig_ownership.py | 105 +++++++++++ scripts/tests/test_check_kconfig_ownership.py | 58 ++++++ test/host_support/CMakeLists.txt | 2 +- test/host_support/Kconfig.projbuild | 9 +- 29 files changed, 397 insertions(+), 226 deletions(-) rename components/captive_portal/{Kconfig.openshock => Kconfig} (74%) create mode 100644 components/common/Kconfig delete mode 100644 components/config/Kconfig.openshock rename components/core/{Kconfig.openshock => Kconfig} (69%) rename components/device_control/{Kconfig.openshock => Kconfig} (51%) delete mode 100644 components/dns_server/Kconfig.openshock rename components/http/{Kconfig.openshock => Kconfig} (90%) rename components/logging/{Kconfig.openshock => Kconfig} (95%) delete mode 100644 components/ota/Kconfig.openshock rename components/rfc8908/{Kconfig.openshock => Kconfig} (76%) rename components/serial/{Kconfig.openshock => Kconfig} (86%) rename components/serial_console/{Kconfig.openshock => Kconfig} (58%) delete mode 100644 main/Kconfig.projbuild create mode 100644 scripts/check_kconfig_ownership.py create mode 100644 scripts/tests/test_check_kconfig_ownership.py diff --git a/.changes/kconfig-tunables.md b/.changes/kconfig-tunables.md index 6b9abe70..32f9f3d9 100644 --- a/.changes/kconfig-tunables.md +++ b/.changes/kconfig-tunables.md @@ -6,6 +6,6 @@ Move build-time settings into Kconfig ## Release Note Build-time settings can now be changed in menuconfig -- Task stacks and priorities, timeouts, retry intervals, queue and buffer sizes are now set in `menuconfig` under OpenShock, each next to the component that uses it +- Task stacks and priorities, timeouts, retry intervals, queue and buffer sizes are now set in `menuconfig`, in the OpenShock menus under Component config - The default API and firmware CDN domains, hostname, SoftAP SSID prefix, captive portal IP address and venue info URL are set there too - The log level can be overridden in `menuconfig`; by default it still follows the build mode diff --git a/.github/workflows/host-tests.yml b/.github/workflows/host-tests.yml index 4dd54bab..fe4ebfe4 100644 --- a/.github/workflows/host-tests.yml +++ b/.github/workflows/host-tests.yml @@ -39,6 +39,10 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + # Standard library only, so it runs before (and fails faster than) the ESP-IDF setup + - name: Check Kconfig ownership + run: python3 scripts/check_kconfig_ownership.py + # The linux target links libbsd, ESP-IDF generates the CMock mocks with Ruby - name: Install host build dependencies run: sudo apt-get update && sudo apt-get install -y --no-install-recommends libbsd-dev ruby diff --git a/components/captive_portal/Kconfig.openshock b/components/captive_portal/Kconfig similarity index 74% rename from components/captive_portal/Kconfig.openshock rename to components/captive_portal/Kconfig index 1df60aea..475457bc 100644 --- a/components/captive_portal/Kconfig.openshock +++ b/components/captive_portal/Kconfig @@ -1,6 +1,4 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "Captive portal" +menu "OpenShock captive portal" config OPENSHOCK_PORTAL_AP_IP string "SoftAP IPv4 address" @@ -29,21 +27,11 @@ menu "Captive portal" gateway connection and an outbound HTTP request need one each (checked against LWIP_MAX_SOCKETS). - config OPENSHOCK_PORTAL_HTTPD_STACK_SIZE - int "HTTP server task stack (bytes)" - range 4096 32768 - default 8192 - config OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S int "HTTP socket send/receive timeout (s)" range 1 60 default 5 - config OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE - int "Largest local websocket message (bytes)" - range 1024 32768 - default 4096 - config OPENSHOCK_PORTAL_WS_MAX_QUEUED_SENDS int "Queued websocket sends" range 4 256 @@ -67,16 +55,4 @@ menu "Captive portal" help Once the hub is online and no client is connected, the portal closes after this long. - config OPENSHOCK_PORTAL_MANAGER_TASK_STACK_SIZE - int "Portal manager task stack (bytes)" - range 4096 16384 - default 6144 - - config OPENSHOCK_PORTAL_ACCOUNT_LINK_TASK_STACK_SIZE - int "Account link task stack (bytes)" - range 4096 16384 - default 8192 - help - Runs the HTTPS request that exchanges a link code for a token. - endmenu diff --git a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index 954baf34..262005e7 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -774,7 +774,7 @@ esp_err_t CaptivePortal::CaptivePortalInstance::wsHandler(httpd_req_t* req) return ESP_OK; } - if (frame.len > CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE) { // Nothing larger is accepted by the local message handler, so don't buffer it + if (frame.len > CONFIG_OPENSHOCK_LOCAL_MAX_MESSAGE_SIZE) { // Nothing larger is accepted by the local message handler, so don't buffer it OS_LOGE(TAG, "WebSocket frame too large (%u bytes), dropping", static_cast(frame.len)); return ESP_FAIL; } @@ -837,8 +837,8 @@ void CaptivePortal::CaptivePortalInstance::onWsFrame(int fd, httpd_ws_type_t opc break; } // The frame check only bounds single frames; also bound the reassembled message - if (m_clients[idx].reasm.size() + payload.size() > CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE) { - OS_LOGE(TAG, "WebSocket client %u fragmented message exceeds %d bytes, closing", socketId, CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE); + if (m_clients[idx].reasm.size() + payload.size() > CONFIG_OPENSHOCK_LOCAL_MAX_MESSAGE_SIZE) { + OS_LOGE(TAG, "WebSocket client %u fragmented message exceeds %d bytes, closing", socketId, CONFIG_OPENSHOCK_LOCAL_MAX_MESSAGE_SIZE); m_clients[idx].reasmActive = false; m_clients[idx].reasm.clear(); m_clients[idx].reasm.shrink_to_fit(); diff --git a/components/common/Kconfig b/components/common/Kconfig new file mode 100644 index 00000000..5a3e40f0 --- /dev/null +++ b/components/common/Kconfig @@ -0,0 +1,169 @@ +# OpenShock settings that more than one component reads. common sits below every OpenShock +# component, so each of them may use these (scripts/check_kconfig_ownership.py). +# +# The board's GPIO assignments deliberately do NOT live in Kconfig. A Kconfig symbol ends up +# in sdkconfig.h, which most of ESP-IDF includes, so two boards that differed only by an LED +# pin would share no compiled framework objects at all. The pins are stamped into the image +# per board instead (BoardDescriptor.h): bare `OPENSHOCK_*=` lines in boards/.defaults, +# stripped from the sdkconfig fragment by scripts/build.py. Keep every OpenShock option the +# same across boards and build channels (see OPENSHOCK_LOG_LEVEL_CHOICE). +# +# Fixed in code on purpose: the RF protocol timings, the E-Stop debounce, hold and rearm +# times (safety behaviour), task core affinity, and on-flash formats. + +menu "OpenShock defaults" + + config OPENSHOCK_API_DOMAIN + string "OpenShock API domain" + default "api.openshock.app" + help + Default backend API domain the hub connects to. Overridable at runtime. + + config OPENSHOCK_FW_CDN_DOMAIN + string "Firmware CDN domain" + default "firmware.openshock.org" + help + Default CDN domain used for OTA firmware/version lookups. Overridable at runtime. + + config OPENSHOCK_FW_HOSTNAME + string "Default hostname" + default "OpenShock" + help + Default network hostname; also the base of the default user agent. + + config OPENSHOCK_FW_AP_PREFIX + string "SoftAP SSID prefix" + default "OpenShock-" + help + Prefix for the captive-portal SoftAP SSID (the device MAC is appended). + +endmenu + +# Every OpenShock task's stack and priority, in one place so the priority order between them +# can be read and checked together (the static_asserts in src/TaskUtils.cpp). ESP-IDF's own +# network tasks and the portal's HTTP server run at priority 5. +menu "OpenShock tasks" + + menu "E-Stop" + + config OPENSHOCK_ESTOP_TASK_PRIORITY + int "Priority" + range 3 24 + default 10 + help + Above the network tasks (5) and the RF task: it is blocked almost all the time, + and a sample that waits behind TLS work delays the E-Stop by that long. Must stay + above the RF transmitter and DNS server tasks' priorities (checked at compile time). + + config OPENSHOCK_ESTOP_TASK_STACK_SIZE + int "Stack (bytes)" + range 2048 16384 + default 4096 + + endmenu + + menu "RF transmitter" + + config OPENSHOCK_RF_TASK_PRIORITY + int "Priority" + range 2 24 + default 9 + help + Above the network tasks (5): it blocks on its queue or on RMT almost all the time, + and frames (and the terminators after an E-Stop) shouldn't wait behind TLS work. + Must stay above the WiFi manager and DNS server tasks, and below the E-Stop task's + priority so a press preempts a transmission (checked at compile time). + + config OPENSHOCK_RF_TASK_STACK_SIZE + int "Stack (bytes)" + range 2048 16384 + default 4096 + help + Profiled at about 1.4 KB. + + endmenu + + menu "Keep-alive" + + config OPENSHOCK_KEEPALIVE_TASK_PRIORITY + int "Priority" + range 1 24 + default 1 + + config OPENSHOCK_KEEPALIVE_TASK_STACK_SIZE + int "Stack (bytes)" + range 2048 16384 + default 4096 + help + Profiled at about 1.5 KB. + + endmenu + + menu "WiFi manager" + + config OPENSHOCK_WIFI_TASK_PRIORITY + int "Priority" + range 1 24 + default 5 + + config OPENSHOCK_WIFI_TASK_STACK_SIZE + int "Stack (bytes)" + range 2048 16384 + default 4096 + + endmenu + + menu "Captive portal DNS server" + + config OPENSHOCK_DNS_TASK_PRIORITY + int "Priority" + range 1 24 + default 4 + help + Above the other application tasks (1): a phone joining the AP sends a burst of + DNS queries, and the small UDP receive mailbox drops them if this task is starved, + which delays captive-portal detection by seconds. Keep it below httpd (5) and the + E-Stop task (checked at compile time). + + config OPENSHOCK_DNS_TASK_STACK_SIZE + int "Stack (bytes)" + range 2048 8192 + default 3072 + + endmenu + + config OPENSHOCK_GATEWAY_TASK_STACK_SIZE + int "Gateway websocket client stack (bytes)" + range 4096 16384 + default 6144 + + config OPENSHOCK_PORTAL_HTTPD_STACK_SIZE + int "Captive portal HTTP server stack (bytes)" + range 4096 32768 + default 8192 + + config OPENSHOCK_PORTAL_MANAGER_TASK_STACK_SIZE + int "Captive portal manager stack (bytes)" + range 4096 16384 + default 6144 + + config OPENSHOCK_PORTAL_ACCOUNT_LINK_TASK_STACK_SIZE + int "Account link stack (bytes)" + range 4096 16384 + default 8192 + help + Runs the HTTPS request that exchanges a link code for a token. + + config OPENSHOCK_OTA_TASK_STACK_SIZE + int "OTA update stack (bytes)" + range 8192 32768 + default 16384 + help + Profiled at about 6.2 KB. + + config OPENSHOCK_SERIAL_RX_TASK_STACK_SIZE + int "Serial input stack (bytes)" + range 4096 32768 + default 10000 + +endmenu diff --git a/components/common/include/OpenShock.h b/components/common/include/OpenShock.h index 1c7f5cfc..26081cc9 100644 --- a/components/common/include/OpenShock.h +++ b/components/common/include/OpenShock.h @@ -10,7 +10,7 @@ // Config comes from two places: Kconfig (CONFIG_OPENSHOCK_*, via sdkconfig.h - // domains, hostname, task stacks and priorities, timeouts, queue and buffer sizes; -// see main/Kconfig.projbuild and components/*/Kconfig.openshock) +// see components/*/Kconfig, shared ones in components/common/Kconfig) // and the generated board header (OPENSHOCK_FW_CHIP, build mode, log level, and the // board the build was configured for; see scripts/gen_env_header.py). // diff --git a/components/common/src/TaskUtils.cpp b/components/common/src/TaskUtils.cpp index fd8ba56d..1fe86c4b 100644 --- a/components/common/src/TaskUtils.cpp +++ b/components/common/src/TaskUtils.cpp @@ -6,6 +6,14 @@ #include +#include "sdkconfig.h" + +// The priority order between OpenShock's tasks (Kconfig "OpenShock tasks"), checked in one place +static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY < CONFIG_OPENSHOCK_ESTOP_TASK_PRIORITY, "an E-Stop press must preempt a transmission"); +static_assert(CONFIG_OPENSHOCK_DNS_TASK_PRIORITY < CONFIG_OPENSHOCK_ESTOP_TASK_PRIORITY, "a DNS query burst must not delay E-Stop sampling"); +static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY > CONFIG_OPENSHOCK_WIFI_TASK_PRIORITY, "frames and E-Stop terminators must not wait behind the WiFi manager"); +static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY > CONFIG_OPENSHOCK_DNS_TASK_PRIORITY, "frames and E-Stop terminators must not wait behind the DNS server"); + using namespace OpenShock; bool TaskUtils::WatchdogSubscribe() diff --git a/components/config/Kconfig.openshock b/components/config/Kconfig.openshock deleted file mode 100644 index a54f4930..00000000 --- a/components/config/Kconfig.openshock +++ /dev/null @@ -1,29 +0,0 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "Defaults" - - config OPENSHOCK_API_DOMAIN - string "OpenShock API domain" - default "api.openshock.app" - help - Default backend API domain the hub connects to. Overridable at runtime. - - config OPENSHOCK_FW_CDN_DOMAIN - string "Firmware CDN domain" - default "firmware.openshock.org" - help - Default CDN domain used for OTA firmware/version lookups. Overridable at runtime. - - config OPENSHOCK_FW_HOSTNAME - string "Default hostname" - default "OpenShock" - help - Default network hostname; also the base of the default user agent. - - config OPENSHOCK_FW_AP_PREFIX - string "SoftAP SSID prefix" - default "OpenShock-" - help - Prefix for the captive-portal SoftAP SSID (the device MAC is appended). - -endmenu diff --git a/components/core/Kconfig.openshock b/components/core/Kconfig similarity index 69% rename from components/core/Kconfig.openshock rename to components/core/Kconfig index d23853f8..eeb8048f 100644 --- a/components/core/Kconfig.openshock +++ b/components/core/Kconfig @@ -1,16 +1,4 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "WiFi" - - config OPENSHOCK_WIFI_TASK_STACK_SIZE - int "WiFi manager task stack (bytes)" - range 2048 16384 - default 4096 - - config OPENSHOCK_WIFI_TASK_PRIORITY - int "WiFi manager task priority" - range 1 24 - default 5 +menu "OpenShock WiFi" config OPENSHOCK_WIFI_SCAN_MIN_MS_PER_CHANNEL int "Scan time per channel, minimum (ms)" @@ -19,13 +7,13 @@ menu "WiFi" config OPENSHOCK_WIFI_SCAN_MAX_MS_PER_CHANNEL int "Scan time per channel, maximum (ms)" - range 10 1500 + range OPENSHOCK_WIFI_SCAN_MIN_MS_PER_CHANNEL 1500 default 120 help - Must be at least the minimum. esp_wifi walks every channel itself, so this is - the main knob for scan duration. Every scan takes the radio off the captive - portal AP's channel for this long per channel, and portal clients lose frames - meanwhile: keep it near the ESP-IDF default (120 ms). + At least the minimum. esp_wifi walks every channel itself, so this is the main + knob for scan duration. Every scan takes the radio off the captive portal AP's + channel for this long per channel, and portal clients lose frames meanwhile: + keep it near the ESP-IDF default (120 ms). config OPENSHOCK_WIFI_AUTOSCAN_INTERVAL_MS int "Auto-scan interval when nothing to connect to (ms)" @@ -41,12 +29,7 @@ menu "WiFi" endmenu -menu "Gateway" - - config OPENSHOCK_GATEWAY_TASK_STACK_SIZE - int "Websocket client task stack (bytes)" - range 4096 16384 - default 6144 +menu "OpenShock gateway" config OPENSHOCK_GATEWAY_MAX_MESSAGE_SIZE int "Largest gateway message (bytes)" @@ -82,9 +65,21 @@ menu "Gateway" config OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MAX_MS int "Hub info fetch retry, cap (ms)" - range 1000 3600000 + range OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MIN_MS 3600000 default 300000 help The retry after a failed hub info fetch doubles from the first value up to this. endmenu + +menu "OpenShock local messages" + + config OPENSHOCK_LOCAL_MAX_MESSAGE_SIZE + int "Largest local message (bytes)" + range 1024 32768 + default 4096 + help + The largest message the local (captive portal websocket) message handler accepts. + The portal drops a websocket message past this before buffering it. + +endmenu diff --git a/components/core/src/GatewayConnectionManager.cpp b/components/core/src/GatewayConnectionManager.cpp index 78c5f66c..35c1ecd6 100644 --- a/components/core/src/GatewayConnectionManager.cpp +++ b/components/core/src/GatewayConnectionManager.cpp @@ -43,7 +43,6 @@ static OpenShock::Backoff s_authBackoff(CONFIG_OPENSHOCK_GATEWAY_AUTH_RETRY_MS, static OpenShock::Backoff s_connectBackoff(CONFIG_OPENSHOCK_GATEWAY_CONNECT_RETRY_MS, CONFIG_OPENSHOCK_GATEWAY_CONNECT_RETRY_MS); // After a failed hub info fetch: doubling from the first value up to the cap static OpenShock::Backoff s_hubInfoBackoff(CONFIG_OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MIN_MS, CONFIG_OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MAX_MS); -static_assert(CONFIG_OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MIN_MS <= CONFIG_OPENSHOCK_GATEWAY_HUB_INFO_RETRY_MAX_MS, "hub info retry above its cap"); static std::atomic s_retryResetRequested = false; static OpenShock::SimpleMutex s_clientMutex; static std::shared_ptr s_wsClient = nullptr; diff --git a/components/core/src/message_handlers/websocket/Local.cpp b/components/core/src/message_handlers/websocket/Local.cpp index 38e10071..4427cd91 100644 --- a/components/core/src/message_handlers/websocket/Local.cpp +++ b/components/core/src/message_handlers/websocket/Local.cpp @@ -33,9 +33,9 @@ static std::array s_localHandlers = []() { void MessageHandlers::WebSocket::HandleLocalBinary(uint8_t socketId, std::span data) { - auto msg = Serialization::GetVerifiedRoot(data, CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE); + auto msg = Serialization::GetVerifiedRoot(data, CONFIG_OPENSHOCK_LOCAL_MAX_MESSAGE_SIZE); if (msg == nullptr) { - OS_LOGE(TAG, "Invalid message (%zu bytes, max %d)", data.size(), CONFIG_OPENSHOCK_PORTAL_MAX_MESSAGE_SIZE); + OS_LOGE(TAG, "Invalid message (%zu bytes, max %d)", data.size(), CONFIG_OPENSHOCK_LOCAL_MAX_MESSAGE_SIZE); return; } diff --git a/components/core/src/wifi/WiFiScanManager.cpp b/components/core/src/wifi/WiFiScanManager.cpp index e31f729c..1ccc589a 100644 --- a/components/core/src/wifi/WiFiScanManager.cpp +++ b/components/core/src/wifi/WiFiScanManager.cpp @@ -14,8 +14,6 @@ const char* const TAG = "WiFiScanManager"; #include #include -static_assert(CONFIG_OPENSHOCK_WIFI_SCAN_MIN_MS_PER_CHANNEL <= CONFIG_OPENSHOCK_WIFI_SCAN_MAX_MS_PER_CHANNEL, "scan time minimum above its maximum"); - // Idle -> Scanning (StartScan) -> Idle (results handled, failure, STA stop). // Scanning -> Stopping (AbortScan) -> Idle (the stopped scan's SCAN_DONE, or kStopTimeout without one). // Stopping keeps a new scan from starting until the stopped one's SCAN_DONE is out of the way, so that event can't be diff --git a/components/device_control/Kconfig.openshock b/components/device_control/Kconfig similarity index 51% rename from components/device_control/Kconfig.openshock rename to components/device_control/Kconfig index 772bb4dc..7bfe01fa 100644 --- a/components/device_control/Kconfig.openshock +++ b/components/device_control/Kconfig @@ -1,23 +1,4 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "Radio" - - config OPENSHOCK_RF_TASK_STACK_SIZE - int "RF transmitter task stack (bytes)" - range 2048 16384 - default 4096 - help - Profiled at about 1.4 KB. - - config OPENSHOCK_RF_TASK_PRIORITY - int "RF transmitter task priority" - range 2 24 - default 9 - help - Above the network tasks (5): it blocks on its queue or on RMT almost all the time, - and frames (and the terminators after an E-Stop) shouldn't wait behind TLS work. - Must stay above the WiFi manager and DNS server tasks, and below the E-Stop task's - priority so a press preempts a transmission (checked at compile time). +menu "OpenShock radio" config OPENSHOCK_RF_QUEUE_SIZE int "RF command queue length" @@ -36,7 +17,7 @@ menu "Radio" endmenu -menu "Keep-alive" +menu "OpenShock keep-alive" config OPENSHOCK_KEEPALIVE_INTERVAL_MS int "Keep-alive interval (ms)" @@ -64,21 +45,9 @@ menu "Keep-alive" range 4 128 default 32 - config OPENSHOCK_KEEPALIVE_TASK_STACK_SIZE - int "Keep-alive task stack (bytes)" - range 2048 16384 - default 4096 - help - Profiled at about 1.5 KB. - - config OPENSHOCK_KEEPALIVE_TASK_PRIORITY - int "Keep-alive task priority" - range 1 24 - default 1 - endmenu -menu "Emergency stop" +menu "OpenShock emergency stop" config OPENSHOCK_ESTOP_POLL_INTERVAL_MS int "Button poll interval (ms)" @@ -89,18 +58,4 @@ menu "Emergency stop" code (65 ms); the number of samples it takes follows from this interval. At most 16 samples fit the debounce history, hence the 5 ms floor. - config OPENSHOCK_ESTOP_TASK_STACK_SIZE - int "E-Stop task stack (bytes)" - range 2048 16384 - default 4096 - - config OPENSHOCK_ESTOP_TASK_PRIORITY - int "E-Stop task priority" - range 3 24 - default 10 - help - Above the network tasks (5) and the RF task: it is blocked almost all the time, - and a sample that waits behind TLS work delays the E-Stop by that long. Must stay - above the RF transmitter and DNS server tasks' priorities (checked at compile time). - endmenu diff --git a/components/device_control/src/radio/RFTransmitter.cpp b/components/device_control/src/radio/RFTransmitter.cpp index 6faa8728..d3edbee4 100644 --- a/components/device_control/src/radio/RFTransmitter.cpp +++ b/components/device_control/src/radio/RFTransmitter.cpp @@ -20,9 +20,6 @@ const char* const TAG = "RFTransmitter"; #include #include -static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY < CONFIG_OPENSHOCK_ESTOP_TASK_PRIORITY, "an E-Stop press must preempt a transmission"); -static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY > CONFIG_OPENSHOCK_WIFI_TASK_PRIORITY, "frames and E-Stop terminators must not wait behind the WiFi manager"); -static_assert(CONFIG_OPENSHOCK_RF_TASK_PRIORITY > CONFIG_OPENSHOCK_DNS_TASK_PRIORITY, "frames and E-Stop terminators must not wait behind the DNS server"); const int32_t kRmtTimeoutMs = 100; // After this many transmit failures in a row the RMT channel is presumed broken and the pending sequences are dropped const uint32_t kMaxConsecutiveFailures = 20; diff --git a/components/dns_server/Kconfig.openshock b/components/dns_server/Kconfig.openshock deleted file mode 100644 index 3a0bcac6..00000000 --- a/components/dns_server/Kconfig.openshock +++ /dev/null @@ -1,20 +0,0 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "Captive portal DNS server" - - config OPENSHOCK_DNS_TASK_STACK_SIZE - int "DNS server task stack (bytes)" - range 2048 8192 - default 3072 - - config OPENSHOCK_DNS_TASK_PRIORITY - int "DNS server task priority" - range 1 24 - default 4 - help - Above the other application tasks (1): a phone joining the AP sends a burst of - DNS queries, and the small UDP receive mailbox drops them if this task is starved, - which delays captive-portal detection by seconds. Keep it below httpd (5) and the - E-Stop task (checked at compile time). - -endmenu diff --git a/components/dns_server/src/DNSServer.cpp b/components/dns_server/src/DNSServer.cpp index 149b5de9..6ecbdddc 100644 --- a/components/dns_server/src/DNSServer.cpp +++ b/components/dns_server/src/DNSServer.cpp @@ -15,8 +15,6 @@ const char* const TAG = "DNSServer"; #include #include -static_assert(CONFIG_OPENSHOCK_DNS_TASK_PRIORITY < CONFIG_OPENSHOCK_ESTOP_TASK_PRIORITY, "a DNS query burst must not delay E-Stop sampling"); - using namespace OpenShock; DNSServer::DNSServer() diff --git a/components/http/Kconfig.openshock b/components/http/Kconfig similarity index 90% rename from components/http/Kconfig.openshock rename to components/http/Kconfig index d468cb4e..da0ea8d2 100644 --- a/components/http/Kconfig.openshock +++ b/components/http/Kconfig @@ -1,6 +1,4 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "HTTP client" +menu "OpenShock HTTP client" config OPENSHOCK_HTTP_BUFFER_SIZE int "Download buffer (bytes)" @@ -29,7 +27,7 @@ menu "HTTP client" config OPENSHOCK_HTTP_RETRY_AFTER_MAX_S int "Longest Retry-After honoured (s)" - range 1 86400 + range OPENSHOCK_HTTP_RETRY_AFTER_MIN_S 86400 default 600 help So one bad header can't keep the hub offline for long. diff --git a/components/http/src/HTTPRequestManager.cpp b/components/http/src/HTTPRequestManager.cpp index 4b872f6e..a94f49bd 100644 --- a/components/http/src/HTTPRequestManager.cpp +++ b/components/http/src/HTTPRequestManager.cpp @@ -252,8 +252,6 @@ HTTP::Response // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After // Only delay-seconds (whole seconds, RFC 9110) is parsed; a missing header, an HTTP-date or anything else // unparseable gets the minimum. - static_assert(CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MIN_S <= CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MAX_S, "Retry-After minimum above its maximum"); - uint32_t retryAfter = OpenShock::Convert::ToUint32(m_retryAfter).value_or(CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MIN_S); rateLimiter->blockFor(static_cast(std::clamp(retryAfter, CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MIN_S, CONFIG_OPENSHOCK_HTTP_RETRY_AFTER_MAX_S)) * 1000); diff --git a/components/logging/Kconfig.openshock b/components/logging/Kconfig similarity index 95% rename from components/logging/Kconfig.openshock rename to components/logging/Kconfig index 7ee1884c..6c87c372 100644 --- a/components/logging/Kconfig.openshock +++ b/components/logging/Kconfig @@ -1,6 +1,4 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "Logging" +menu "OpenShock logging" choice OPENSHOCK_LOG_LEVEL_CHOICE prompt "Log level" diff --git a/components/ota/Kconfig.openshock b/components/ota/Kconfig.openshock deleted file mode 100644 index 36ab8bd9..00000000 --- a/components/ota/Kconfig.openshock +++ /dev/null @@ -1,12 +0,0 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "OTA updates" - - config OPENSHOCK_OTA_TASK_STACK_SIZE - int "OTA task stack (bytes)" - range 8192 32768 - default 16384 - help - Profiled at about 6.2 KB. - -endmenu diff --git a/components/rfc8908/Kconfig.openshock b/components/rfc8908/Kconfig similarity index 76% rename from components/rfc8908/Kconfig.openshock rename to components/rfc8908/Kconfig index f81dfce0..4c1cee35 100644 --- a/components/rfc8908/Kconfig.openshock +++ b/components/rfc8908/Kconfig @@ -1,6 +1,4 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "Captive portal detection" +menu "OpenShock captive portal detection" config OPENSHOCK_PORTAL_VENUE_INFO_URL string "Venue info URL" diff --git a/components/serial/Kconfig.openshock b/components/serial/Kconfig similarity index 86% rename from components/serial/Kconfig.openshock rename to components/serial/Kconfig index 34830ad5..cf1e405b 100644 --- a/components/serial/Kconfig.openshock +++ b/components/serial/Kconfig @@ -1,6 +1,4 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "Serial driver" +menu "OpenShock serial driver" config OPENSHOCK_SERIAL_RX_BUFFER_SIZE int "Console driver receive buffer (bytes)" diff --git a/components/serial_console/Kconfig.openshock b/components/serial_console/Kconfig similarity index 58% rename from components/serial_console/Kconfig.openshock rename to components/serial_console/Kconfig index 14aa9ce3..8f502d9a 100644 --- a/components/serial_console/Kconfig.openshock +++ b/components/serial_console/Kconfig @@ -1,11 +1,4 @@ -# Sourced into the OpenShock menu by main/Kconfig.projbuild - -menu "Serial console" - - config OPENSHOCK_SERIAL_RX_TASK_STACK_SIZE - int "Serial input task stack (bytes)" - range 4096 32768 - default 10000 +menu "OpenShock serial console" config OPENSHOCK_SERIAL_LINE_MAX_SIZE int "Longest input line (bytes)" diff --git a/docs/DEVELOPING.md b/docs/DEVELOPING.md index d1fb1ef0..de90aa89 100644 --- a/docs/DEVELOPING.md +++ b/docs/DEVELOPING.md @@ -126,6 +126,16 @@ Shared config lives in `sdkconfig.defaults`, including the 4 MB OTA partition ta (`partitions/ota_4mb.csv`) every board uses. `sdkconfig` and `build/` are generated and gitignored. If a `sdkconfig` is stale after a defaults change, delete it or run `fullclean`. +### OpenShock Kconfig options + +Each component defines its own `CONFIG_OPENSHOCK_*` options in `components//Kconfig`; they +show up in `menuconfig` under Component config, in the "OpenShock ..." menus. A component may +read only its own options and those of the components it requires, which +`scripts/check_kconfig_ownership.py` checks in CI. An option more than one component reads goes +in the lowest component they all depend on: the default domains and hostname, and every task's +stack and priority, live in `components/common/Kconfig`, where `src/TaskUtils.cpp` checks the +priority order between the tasks. + ```sh python scripts/stamp_board.py read --image build/OpenShock-Core-V2/firmware.bin ``` diff --git a/main/Kconfig.projbuild b/main/Kconfig.projbuild deleted file mode 100644 index 3b2b3229..00000000 --- a/main/Kconfig.projbuild +++ /dev/null @@ -1,24 +0,0 @@ -menu "OpenShock" - -# The board's GPIO assignments deliberately do NOT live here. -# -# A Kconfig symbol ends up in sdkconfig.h, which most of ESP-IDF includes, so two -# boards that differed only by an LED pin produced a different sdkconfig.h and -# therefore shared no compiled framework objects at all. The pins are stamped into -# the image per board instead (BoardDescriptor.h): bare `OPENSHOCK_*=` lines in -# boards/.defaults, stripped from the sdkconfig fragment by scripts/build.py. -# -# Everything in it is identical across every board and build channel, so it costs -# nothing in shared objects. Keep it that way: a setting that has to differ per -# board or per channel doesn't belong here (see OPENSHOCK_LOG_LEVEL_CHOICE). -# -# Fixed in code on purpose: the RF protocol timings, the E-Stop debounce, hold and -# rearm times (safety behaviour), task core affinity, and on-flash formats. -# -# Each component keeps its own settings in components//Kconfig.openshock. They are -# all sourced here, also for components a build leaves out, so every CONFIG_OPENSHOCK_* -# symbol exists in every build (the host tests build only their own sources). - -orsource "../components/*/Kconfig.openshock" - -endmenu diff --git a/scripts/check_kconfig_ownership.py b/scripts/check_kconfig_ownership.py new file mode 100644 index 00000000..7d776928 --- /dev/null +++ b/scripts/check_kconfig_ownership.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +"""Check that every component reads only the CONFIG_OPENSHOCK_* options it may depend on. + +An option belongs to the component whose ``components//Kconfig`` defines it. A component may read +its own options and those of the components it requires: its REQUIRES and PRIV_REQUIRES, plus whatever +those export through their public REQUIRES. Every option exists in sdkconfig.h of every build, so the +compiler can't catch a component reading another's option behind its back; this does. + +Settings more than one component reads go in the lowest component they all depend on (usually common). +Host tests (``host_test/``) are not checked: they compile sources from several components on purpose. + +Usage: + python scripts/check_kconfig_ownership.py +""" + +import argparse +import re +import sys +from pathlib import Path + +from gen_dep_graph import read_components + +from utils.paths import ROOT + +CONFIG_DEFINITION = re.compile(r'^\s*(?:menuconfig|config|choice)\s+(OPENSHOCK_\w+)', re.MULTILINE) +CONFIG_USE = re.compile(r'\bCONFIG_(OPENSHOCK_\w+)') +SOURCE_SUFFIXES = {'.c', '.cpp', '.h', '.hpp'} + +Violation = tuple[str, Path, str, str | None] # (component, file, option, owner or None if undefined) + + +def read_owners(comp_dir: Path) -> dict[str, str]: + """Option name (without CONFIG_) -> the component whose Kconfig defines it.""" + owners: dict[str, str] = {} + for kconfig in sorted(comp_dir.glob('*/Kconfig')): + for option in CONFIG_DEFINITION.findall(kconfig.read_text(encoding='utf-8')): + owners[option] = kconfig.parent.name + return owners + + +def allowed_owners(name: str, private: dict[str, list[str]], public: dict[str, list[str]]) -> set[str]: + """The component itself, its direct requirements, and everything those export through public REQUIRES.""" + allowed = {name} + pending = list(private.get(name, [])) + while pending: + dep = pending.pop() + if dep not in allowed: + allowed.add(dep) + pending.extend(public.get(dep, [])) + return allowed + + +def source_files(component: Path) -> list[Path]: + return sorted( + path + for path in component.rglob('*') + if path.suffix in SOURCE_SUFFIXES and 'host_test' not in path.relative_to(component).parts + ) + + +def find_violations(comp_dir: Path) -> list[Violation]: + owners = read_owners(comp_dir) + private = read_components(comp_dir, include_priv=True) + public = read_components(comp_dir, include_priv=False) + + violations: list[Violation] = [] + for name in sorted(private): + allowed = allowed_owners(name, private, public) + for path in source_files(comp_dir / name): + for option in sorted(set(CONFIG_USE.findall(path.read_text(encoding='utf-8', errors='replace')))): + owner = owners.get(option) + if owner not in allowed: + violations.append((name, path, option, owner)) + return violations + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument( + '--components-dir', + type=Path, + default=ROOT / 'components', + help='directory holding the component subdirs (default: ./components)', + ) + args = parser.parse_args(argv) + + violations = find_violations(args.components_dir) + for name, path, option, owner in violations: + where = path.relative_to(args.components_dir.parent).as_posix() + if owner is None: + print(f'{where}: CONFIG_{option} is not defined in any components/*/Kconfig', file=sys.stderr) + else: + print(f'{where}: CONFIG_{option} belongs to {owner}, which {name} does not require', file=sys.stderr) + if violations: + print( + f'{len(violations)} Kconfig ownership violation(s): move the option to a component both depend on', + file=sys.stderr, + ) + return 1 + print('Kconfig ownership: every CONFIG_OPENSHOCK_* use is in a component that requires its owner') + return 0 + + +if __name__ == '__main__': + raise SystemExit(main()) diff --git a/scripts/tests/test_check_kconfig_ownership.py b/scripts/tests/test_check_kconfig_ownership.py new file mode 100644 index 00000000..5f600237 --- /dev/null +++ b/scripts/tests/test_check_kconfig_ownership.py @@ -0,0 +1,58 @@ +"""Tests for scripts/check_kconfig_ownership.py: which component may read which CONFIG_OPENSHOCK_* option.""" + +from pathlib import Path + +import check_kconfig_ownership + + +def component(root: Path, name: str, *, requires: str = '', priv: str = '', kconfig: str = '', source: str = ''): + path = root / name + (path / 'src').mkdir(parents=True) + (path / 'CMakeLists.txt').write_text( + f'idf_component_register(SRCS "src/a.cpp" REQUIRES {requires} PRIV_REQUIRES {priv})\n', encoding='utf-8' + ) + if kconfig: + (path / 'Kconfig').write_text(kconfig, encoding='utf-8') + (path / 'src' / 'a.cpp').write_text(source, encoding='utf-8') + + +def violations(root: Path) -> list[tuple[str, str, str | None]]: + return [(name, option, owner) for name, _, option, owner in check_kconfig_ownership.find_violations(root)] + + +def test_own_and_required_options_are_allowed(tmp_path): + component(tmp_path, 'base', kconfig='config OPENSHOCK_BASE\n int "x"\n') + component(tmp_path, 'mid', requires='base', kconfig='config OPENSHOCK_MID\n int "x"\n') + component(tmp_path, 'top', priv='mid', source='CONFIG_OPENSHOCK_MID CONFIG_OPENSHOCK_BASE') + assert violations(tmp_path) == [] + + +def test_a_private_requirement_does_not_export_its_options(tmp_path): + component(tmp_path, 'base', kconfig='config OPENSHOCK_BASE\n int "x"\n') + component(tmp_path, 'mid', priv='base') + component(tmp_path, 'top', requires='mid', source='CONFIG_OPENSHOCK_BASE') + assert violations(tmp_path) == [('top', 'OPENSHOCK_BASE', 'base')] + + +def test_reading_a_component_above_is_a_violation(tmp_path): + component(tmp_path, 'portal', requires='core', kconfig='config OPENSHOCK_PORTAL_SIZE\n int "x"\n') + component(tmp_path, 'core', source='CONFIG_OPENSHOCK_PORTAL_SIZE') + assert violations(tmp_path) == [('core', 'OPENSHOCK_PORTAL_SIZE', 'portal')] + + +def test_an_undefined_option_is_a_violation(tmp_path): + component(tmp_path, 'top', source='CONFIG_OPENSHOCK_NOWHERE') + assert violations(tmp_path) == [('top', 'OPENSHOCK_NOWHERE', None)] + + +def test_choices_count_as_definitions_and_host_tests_are_skipped(tmp_path): + component( + tmp_path, 'log', kconfig='choice OPENSHOCK_LEVEL\n prompt "x"\nendchoice\n', source='CONFIG_OPENSHOCK_LEVEL' + ) + (tmp_path / 'log' / 'host_test').mkdir() + (tmp_path / 'log' / 'host_test' / 't.cpp').write_text('CONFIG_OPENSHOCK_NOWHERE', encoding='utf-8') + assert violations(tmp_path) == [] + + +def test_the_repository_has_no_violations(): + assert check_kconfig_ownership.find_violations(check_kconfig_ownership.ROOT / 'components') == [] diff --git a/test/host_support/CMakeLists.txt b/test/host_support/CMakeLists.txt index d40e64ad..d58dc30c 100644 --- a/test/host_support/CMakeLists.txt +++ b/test/host_support/CMakeLists.txt @@ -14,7 +14,7 @@ # not, so this component compiles it, together with # common/src/BoardDescriptor.cpp, and every suite has # Board::Get() (after calling Board::Init() in its main). -# Kconfig.projbuild - sources main/Kconfig.projbuild, so the suites see the +# Kconfig.projbuild - sources every components/*/Kconfig, so the suites see the # real CONFIG_OPENSHOCK_* defaults through sdkconfig.h. # # Everything else a suite needs comes from the real OpenShock headers, ESP-IDF's diff --git a/test/host_support/Kconfig.projbuild b/test/host_support/Kconfig.projbuild index 016ea7e0..7f4950c0 100644 --- a/test/host_support/Kconfig.projbuild +++ b/test/host_support/Kconfig.projbuild @@ -1,4 +1,5 @@ -# The CONFIG_OPENSHOCK_* settings the firmware reads through sdkconfig.h. Sourced from -# the firmware's own menu (which pulls in every component's Kconfig.openshock) rather -# than copied, so the host tests always build against the same defaults as the firmware. -rsource "../../main/Kconfig.projbuild" +# The CONFIG_OPENSHOCK_* settings the firmware reads through sdkconfig.h. A suite compiles +# component sources straight into its main, so ESP-IDF never sees those components or their +# Kconfig. Sourced from every component's own Kconfig rather than copied, so the host tests +# always build against the same defaults as the firmware. +orsource "../../components/*/Kconfig" From 31fde751b65906722344365e70ef1cb772df2b10 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 29 Oct 2026 21:45:20 +0100 Subject: [PATCH 07/12] test(scripts): leave the repository-wide Kconfig ownership check to host-tests host-tests already runs scripts/check_kconfig_ownership.py over the repository on every component change; the pytest case ran the same check a second time. --- scripts/tests/test_check_kconfig_ownership.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scripts/tests/test_check_kconfig_ownership.py b/scripts/tests/test_check_kconfig_ownership.py index 5f600237..92255cdf 100644 --- a/scripts/tests/test_check_kconfig_ownership.py +++ b/scripts/tests/test_check_kconfig_ownership.py @@ -52,7 +52,3 @@ def test_choices_count_as_definitions_and_host_tests_are_skipped(tmp_path): (tmp_path / 'log' / 'host_test').mkdir() (tmp_path / 'log' / 'host_test' / 't.cpp').write_text('CONFIG_OPENSHOCK_NOWHERE', encoding='utf-8') assert violations(tmp_path) == [] - - -def test_the_repository_has_no_violations(): - assert check_kconfig_ownership.find_violations(check_kconfig_ownership.ROOT / 'components') == [] From 7f7f9c87421a5b275acc87ba390c80709cc275b5 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 29 Oct 2026 22:02:35 +0100 Subject: [PATCH 08/12] fix(kconfig): address review findings on the defaults and the ownership check The portal AP address must be a host on its /24, not the network or broadcast address. The default hostname is checked at build time by the same rule the hostname command applies, which moves to common as HostnameUtils::IsValid so both use one copy. The ownership check no longer treats a choice's name as an option, since ESP-IDF generates no macro for it; Logging.h's comment names the choice without the CONFIG_ prefix. Quoted REQUIRES arguments are unquoted in gen_dep_graph, which the check reads them through. --- .../src/captiveportal/Manager.cpp | 2 +- .../common/host_test/main/test_ipv4utils.cpp | 13 +++++++++- .../common/include/util/HostnameUtils.h | 24 +++++++++++++++++++ components/common/include/util/IPv4Utils.h | 14 +++++++++++ components/config/src/WiFiConfig.cpp | 3 +++ components/logging/include/Logging.h | 2 +- .../src/command_handlers/hostname.cpp | 20 ++-------------- scripts/check_kconfig_ownership.py | 3 ++- scripts/gen_dep_graph.py | 2 +- scripts/tests/test_check_kconfig_ownership.py | 22 ++++++++++++----- scripts/tests/test_gen_dep_graph.py | 5 ++++ 11 files changed, 81 insertions(+), 29 deletions(-) create mode 100644 components/common/include/util/HostnameUtils.h diff --git a/components/captive_portal/src/captiveportal/Manager.cpp b/components/captive_portal/src/captiveportal/Manager.cpp index bc111a51..3aa23e47 100644 --- a/components/captive_portal/src/captiveportal/Manager.cpp +++ b/components/captive_portal/src/captiveportal/Manager.cpp @@ -73,7 +73,7 @@ static std::atomic s_instanceLive = false; static RetiredList s_retiredInstances; static esp_netif_t* s_apNetif = nullptr; -static_assert(IPv4Utils::IsDottedDecimal(CONFIG_OPENSHOCK_PORTAL_AP_IP), "CONFIG_OPENSHOCK_PORTAL_AP_IP must be a dotted-decimal IPv4 address"); +static_assert(IPv4Utils::IsSlash24Host(CONFIG_OPENSHOCK_PORTAL_AP_IP), "CONFIG_OPENSHOCK_PORTAL_AP_IP must be a dotted-decimal IPv4 address with a host octet of 1-254"); static void sendCommand(uint32_t command) { diff --git a/components/common/host_test/main/test_ipv4utils.cpp b/components/common/host_test/main/test_ipv4utils.cpp index 49cbbac3..be88fbf3 100644 --- a/components/common/host_test/main/test_ipv4utils.cpp +++ b/components/common/host_test/main/test_ipv4utils.cpp @@ -1,4 +1,4 @@ -// IPv4Utils: dotted-decimal validation, used to check CONFIG_OPENSHOCK_PORTAL_AP_IP at compile time. +// IPv4Utils: dotted-decimal and /24 host validation, used to check CONFIG_OPENSHOCK_PORTAL_AP_IP at compile time. #include "unity.h" #include "util/IPv4Utils.h" @@ -28,5 +28,16 @@ TEST_CASE("IsDottedDecimal rejects malformed addresses", "[util][ipv4]") TEST_ASSERT_FALSE(IPv4Utils::IsDottedDecimal("a.b.c.d")); } +TEST_CASE("IsSlash24Host rejects the network and broadcast addresses", "[util][ipv4]") +{ + TEST_ASSERT_TRUE(IPv4Utils::IsSlash24Host("4.3.2.1")); + TEST_ASSERT_TRUE(IPv4Utils::IsSlash24Host("192.168.10.254")); + TEST_ASSERT_FALSE(IPv4Utils::IsSlash24Host("0.0.0.0")); + TEST_ASSERT_FALSE(IPv4Utils::IsSlash24Host("192.168.10.0")); + TEST_ASSERT_FALSE(IPv4Utils::IsSlash24Host("192.168.10.255")); + TEST_ASSERT_FALSE(IPv4Utils::IsSlash24Host("192.168.10.01")); // malformed +} + static_assert(IPv4Utils::IsDottedDecimal("4.3.2.1")); +static_assert(IPv4Utils::IsSlash24Host("4.3.2.1")); static_assert(!IPv4Utils::IsDottedDecimal("01.2.3.4")); diff --git a/components/common/include/util/HostnameUtils.h b/components/common/include/util/HostnameUtils.h new file mode 100644 index 00000000..eb300b67 --- /dev/null +++ b/components/common/include/util/HostnameUtils.h @@ -0,0 +1,24 @@ +#pragma once + +#include + +namespace OpenShock::HostnameUtils { + /// @brief Checks that a string is a usable hostname: an RFC 1123 label of 1-32 letters, digits and hyphens + /// (esp_netif's limit), not starting or ending with a hyphen. + /// @remark Usable in a static_assert. + constexpr bool IsValid(std::string_view hostname) noexcept + { + if (hostname.empty() || hostname.size() > 32 || hostname.front() == '-' || hostname.back() == '-') { + return false; + } + + for (char c : hostname) { + bool ok = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-'; + if (!ok) { + return false; + } + } + + return true; + } +} // namespace OpenShock::HostnameUtils diff --git a/components/common/include/util/IPv4Utils.h b/components/common/include/util/IPv4Utils.h index 417a6072..f2e32da3 100644 --- a/components/common/include/util/IPv4Utils.h +++ b/components/common/include/util/IPv4Utils.h @@ -23,4 +23,18 @@ namespace OpenShock::IPv4Utils { str.remove_prefix(1); } } + + /// @brief Checks that a string is a dotted-decimal address usable as a host on its /24 network: the last octet is + /// 1-254, so it is neither the network nor the broadcast address. + /// @remark Usable in a static_assert. + constexpr bool IsSlash24Host(std::string_view str) noexcept + { + if (!IsDottedDecimal(str)) return false; + + int last = 0; + for (char c : str.substr(str.rfind('.') + 1)) { + last = last * 10 + (c - '0'); + } + return last >= 1 && last <= 254; + } } // namespace OpenShock::IPv4Utils diff --git a/components/config/src/WiFiConfig.cpp b/components/config/src/WiFiConfig.cpp index 6a57d177..f0996afb 100644 --- a/components/config/src/WiFiConfig.cpp +++ b/components/config/src/WiFiConfig.cpp @@ -4,9 +4,12 @@ const char* const TAG = "Config::WiFiConfig"; #include "config/internal/utils.h" #include "Logging.h" +#include "util/HostnameUtils.h" using namespace OpenShock::Config; +static_assert(OpenShock::HostnameUtils::IsValid(CONFIG_OPENSHOCK_FW_HOSTNAME), "CONFIG_OPENSHOCK_FW_HOSTNAME must be 1-32 letters, digits or hyphens, not starting or ending with a hyphen"); + WiFiConfig::WiFiConfig() : accessPointSSID(CONFIG_OPENSHOCK_FW_AP_PREFIX) , hostname(CONFIG_OPENSHOCK_FW_HOSTNAME) diff --git a/components/logging/include/Logging.h b/components/logging/include/Logging.h index 2659c7eb..7c6c62f7 100644 --- a/components/logging/include/Logging.h +++ b/components/logging/include/Logging.h @@ -15,7 +15,7 @@ #error "OPENSHOCK_LOG_LEVEL is not defined (openshock_board.h)" #endif -// A level picked in menuconfig wins over the build mode's (CONFIG_OPENSHOCK_LOG_LEVEL_CHOICE) +// A level picked in menuconfig wins over the build mode's (the OPENSHOCK_LOG_LEVEL_CHOICE Kconfig choice) #if defined(CONFIG_OPENSHOCK_LOG_LEVEL_OVERRIDE) && CONFIG_OPENSHOCK_LOG_LEVEL_OVERRIDE >= 0 #undef OPENSHOCK_LOG_LEVEL #define OPENSHOCK_LOG_LEVEL CONFIG_OPENSHOCK_LOG_LEVEL_OVERRIDE diff --git a/components/serial_console/src/command_handlers/hostname.cpp b/components/serial_console/src/command_handlers/hostname.cpp index d4beaa77..4aae8dfb 100644 --- a/components/serial_console/src/command_handlers/hostname.cpp +++ b/components/serial_console/src/command_handlers/hostname.cpp @@ -1,6 +1,7 @@ #include "serial_console/command_handlers/common.h" #include "config/Config.h" +#include "util/HostnameUtils.h" #include @@ -8,23 +9,6 @@ const char* const TAG = "SerialCmds::CommandHandlers::Hostname"; -// RFC 1123 label: 1-32 letters, digits and hyphens (esp_netif limit), not starting or ending with a hyphen. -static bool isValidHostname(std::string_view hostname) -{ - if (hostname.empty() || hostname.size() > 32 || hostname.front() == '-' || hostname.back() == '-') { - return false; - } - - for (char c : hostname) { - bool ok = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-'; - if (!ok) { - return false; - } - } - - return true; -} - static void handleHostnameCommand(std::string_view arg, bool isAutomated) { if (arg.empty()) { @@ -38,7 +22,7 @@ static void handleHostnameCommand(std::string_view arg, bool isAutomated) return; } - if (!isValidHostname(arg)) { + if (!OpenShock::HostnameUtils::IsValid(arg)) { SERPR_ERROR("Invalid hostname: use 1-32 letters, digits or hyphens, not starting or ending with a hyphen"); return; } diff --git a/scripts/check_kconfig_ownership.py b/scripts/check_kconfig_ownership.py index 7d776928..b7c5f19f 100644 --- a/scripts/check_kconfig_ownership.py +++ b/scripts/check_kconfig_ownership.py @@ -22,7 +22,8 @@ from utils.paths import ROOT -CONFIG_DEFINITION = re.compile(r'^\s*(?:menuconfig|config|choice)\s+(OPENSHOCK_\w+)', re.MULTILINE) +# Only config entries become CONFIG_* macros; a choice's own name never does +CONFIG_DEFINITION = re.compile(r'^\s*(?:menuconfig|config)\s+(OPENSHOCK_\w+)', re.MULTILINE) CONFIG_USE = re.compile(r'\bCONFIG_(OPENSHOCK_\w+)') SOURCE_SUFFIXES = {'.c', '.cpp', '.h', '.hpp'} diff --git a/scripts/gen_dep_graph.py b/scripts/gen_dep_graph.py index 74c7fe3d..4021f50e 100644 --- a/scripts/gen_dep_graph.py +++ b/scripts/gen_dep_graph.py @@ -98,7 +98,7 @@ def extract_register_block(text: str) -> str: def parse_requires(cmake_text: str, include_priv: bool) -> list[str]: """Pull the REQUIRES (+ optionally PRIV_REQUIRES) token lists.""" block = extract_register_block(strip_comments(cmake_text)) - tokens = block.split() + tokens = [tok.strip('"') for tok in block.split()] # CMake reads "common" and common alike deps: list[str] = [] active = False diff --git a/scripts/tests/test_check_kconfig_ownership.py b/scripts/tests/test_check_kconfig_ownership.py index 92255cdf..e1d650e8 100644 --- a/scripts/tests/test_check_kconfig_ownership.py +++ b/scripts/tests/test_check_kconfig_ownership.py @@ -45,10 +45,20 @@ def test_an_undefined_option_is_a_violation(tmp_path): assert violations(tmp_path) == [('top', 'OPENSHOCK_NOWHERE', None)] -def test_choices_count_as_definitions_and_host_tests_are_skipped(tmp_path): - component( - tmp_path, 'log', kconfig='choice OPENSHOCK_LEVEL\n prompt "x"\nendchoice\n', source='CONFIG_OPENSHOCK_LEVEL' - ) - (tmp_path / 'log' / 'host_test').mkdir() - (tmp_path / 'log' / 'host_test' / 't.cpp').write_text('CONFIG_OPENSHOCK_NOWHERE', encoding='utf-8') +def test_a_choice_name_is_not_a_macro(tmp_path): + kconfig = 'choice OPENSHOCK_LEVEL\n prompt "x"\n config OPENSHOCK_LEVEL_LOW\n bool "low"\nendchoice\n' + component(tmp_path, 'log', kconfig=kconfig, source='CONFIG_OPENSHOCK_LEVEL_LOW CONFIG_OPENSHOCK_LEVEL') + assert violations(tmp_path) == [('log', 'OPENSHOCK_LEVEL', None)] + + +def test_host_tests_are_skipped(tmp_path): + component(tmp_path, 'top') + (tmp_path / 'top' / 'host_test').mkdir() + (tmp_path / 'top' / 'host_test' / 't.cpp').write_text('CONFIG_OPENSHOCK_NOWHERE', encoding='utf-8') + assert violations(tmp_path) == [] + + +def test_a_quoted_requirement_counts(tmp_path): + component(tmp_path, 'base', kconfig='config OPENSHOCK_BASE\n int "x"\n') + component(tmp_path, 'top', priv='"base"', source='CONFIG_OPENSHOCK_BASE') assert violations(tmp_path) == [] diff --git a/scripts/tests/test_gen_dep_graph.py b/scripts/tests/test_gen_dep_graph.py index ca399212..a877f14d 100644 --- a/scripts/tests/test_gen_dep_graph.py +++ b/scripts/tests/test_gen_dep_graph.py @@ -23,6 +23,11 @@ def test_parse_requires_reads_public_and_private_lists(): assert gen_dep_graph.parse_requires(CMAKE, include_priv=False) == ['fs', '$,y,z>', 'util'] +def test_parse_requires_unquotes_arguments(): + text = 'idf_component_register(SRCS "a.cpp" REQUIRES "common" PRIV_REQUIRES "fs" util)' + assert gen_dep_graph.parse_requires(text, include_priv=True) == ['common', 'fs', 'util'] + + def test_parse_requires_without_a_register_call_is_empty(): assert gen_dep_graph.parse_requires('project(x)\n', include_priv=True) == [] From 1457c9601cebcb0bc8cc547ee0d7705102fc0baf Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 29 Oct 2026 22:03:18 +0100 Subject: [PATCH 09/12] refactor(kconfig): set the portal startup grace and auto-close times in ms Every other OpenShock time option is in milliseconds. The derived microsecond constants follow the kName style. --- components/captive_portal/Kconfig | 16 ++++++++-------- .../captive_portal/src/captiveportal/Manager.cpp | 14 +++++++------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/components/captive_portal/Kconfig b/components/captive_portal/Kconfig index 475457bc..440a2ebf 100644 --- a/components/captive_portal/Kconfig +++ b/components/captive_portal/Kconfig @@ -40,18 +40,18 @@ menu "OpenShock captive portal" Outgoing websocket messages waiting for httpd. Bounds the memory a burst of broadcasts can hold while no client drains them; past it, new ones are dropped. - config OPENSHOCK_PORTAL_STARTUP_GRACE_S - int "Startup grace period (s)" - range 0 600 - default 30 + config OPENSHOCK_PORTAL_STARTUP_GRACE_MS + int "Startup grace period (ms)" + range 0 600000 + default 30000 help On a fully configured hub, how long after boot the portal waits for the gateway connection before it opens. It stays closed if the hub connects in time. - config OPENSHOCK_PORTAL_AUTO_CLOSE_S - int "Auto-close delay (s)" - range 10 3600 - default 300 + config OPENSHOCK_PORTAL_AUTO_CLOSE_MS + int "Auto-close delay (ms)" + range 10000 3600000 + default 300000 help Once the hub is online and no client is connected, the portal closes after this long. diff --git a/components/captive_portal/src/captiveportal/Manager.cpp b/components/captive_portal/src/captiveportal/Manager.cpp index 3aa23e47..155b7966 100644 --- a/components/captive_portal/src/captiveportal/Manager.cpp +++ b/components/captive_portal/src/captiveportal/Manager.cpp @@ -54,8 +54,8 @@ struct ManagerState { int64_t autoCloseAt = 0; // esp_timer time at which an idle portal closes; 0 = not armed }; -static constexpr int64_t STARTUP_GRACE_PERIOD_US = CONFIG_OPENSHOCK_PORTAL_STARTUP_GRACE_S * 1'000'000LL; -static constexpr int64_t AUTO_CLOSE_DELAY_US = CONFIG_OPENSHOCK_PORTAL_AUTO_CLOSE_S * 1'000'000LL; +static constexpr int64_t kStartupGracePeriodUs = CONFIG_OPENSHOCK_PORTAL_STARTUP_GRACE_MS * 1000LL; +static constexpr int64_t kAutoCloseDelayUs = CONFIG_OPENSHOCK_PORTAL_AUTO_CLOSE_MS * 1000LL; // The requested always-enabled setting, and what IsAlwaysEnabled() reports. Written by SetAlwaysEnabled(). static std::atomic s_alwaysEnabled = false; @@ -312,12 +312,12 @@ static void captiveportal_tick(ManagerState& state) return; } - // Auto-close: no clients connected, WiFi + gateway are up, AUTO_CLOSE_DELAY_US elapsed + // Auto-close: no clients connected, WiFi + gateway are up, kAutoCloseDelayUs elapsed if (running && !state.alwaysEnabled && connected && !hasClients) { if (state.autoCloseAt == 0) { - state.autoCloseAt = now + AUTO_CLOSE_DELAY_US; + state.autoCloseAt = now + kAutoCloseDelayUs; } else if (now >= state.autoCloseAt) { - OS_LOGI(TAG, "Auto-closing captive portal AP (no clients for %d s)", CONFIG_OPENSHOCK_PORTAL_AUTO_CLOSE_S); + OS_LOGI(TAG, "Auto-closing captive portal AP (no clients for %d ms)", CONFIG_OPENSHOCK_PORTAL_AUTO_CLOSE_MS); captiveportal_stop(state); return; } @@ -367,8 +367,8 @@ bool CaptivePortal::Init() // If device is already fully configured, set a startup grace period before opening portal if (isDeviceFullyConfigured()) { - initialState->startupGraceEnd = esp_timer_get_time() + STARTUP_GRACE_PERIOD_US; - OS_LOGI(TAG, "Device fully configured, startup grace period of %ds before opening portal", CONFIG_OPENSHOCK_PORTAL_STARTUP_GRACE_S); + initialState->startupGraceEnd = esp_timer_get_time() + kStartupGracePeriodUs; + OS_LOGI(TAG, "Device fully configured, startup grace period of %d ms before opening portal", CONFIG_OPENSHOCK_PORTAL_STARTUP_GRACE_MS); } s_forceCloseDone = xSemaphoreCreateBinary(); From 5bc7522ef592ee662babcba0904e5507948105b7 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 29 Oct 2026 22:06:22 +0100 Subject: [PATCH 10/12] refactor(kconfig): set the portal httpd timeout in ms Every other OpenShock time option is in milliseconds. esp_http_server takes whole seconds, so the timeout is rounded up to them (kHttpdTimeoutS). --- components/captive_portal/Kconfig | 10 ++++++---- .../src/captiveportal/CaptivePortalInstance.cpp | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/components/captive_portal/Kconfig b/components/captive_portal/Kconfig index 440a2ebf..1113fe13 100644 --- a/components/captive_portal/Kconfig +++ b/components/captive_portal/Kconfig @@ -27,10 +27,12 @@ menu "OpenShock captive portal" gateway connection and an outbound HTTP request need one each (checked against LWIP_MAX_SOCKETS). - config OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S - int "HTTP socket send/receive timeout (s)" - range 1 60 - default 5 + config OPENSHOCK_PORTAL_HTTPD_TIMEOUT_MS + int "HTTP socket send/receive timeout (ms)" + range 1000 60000 + default 5000 + help + Rounded up to whole seconds, the unit esp_http_server takes. config OPENSHOCK_PORTAL_WS_MAX_QUEUED_SENDS int "Queued websocket sends" diff --git a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index 262005e7..b0c3bc95 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -35,6 +35,8 @@ const char* const TAG = "CaptivePortalInstance"; #include const uint16_t HTTP_PORT = 80; +// esp_http_server takes its socket timeouts in whole seconds +static constexpr uint16_t kHttpdTimeoutS = (CONFIG_OPENSHOCK_PORTAL_HTTPD_TIMEOUT_MS + 999) / 1000; // httpd needs 3 lwIP sockets besides its client sockets; the DNS server, the gateway connection and an outbound HTTP request need one each static_assert(CONFIG_LWIP_MAX_SOCKETS >= CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS + 3 + 3, "httpd client sockets would leave no lwIP sockets for the DNS server and outbound connections"); static_assert(CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS > CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS, "every websocket client needs a socket, plus one for plain requests"); @@ -1124,8 +1126,8 @@ bool CaptivePortal::CaptivePortalInstance::startHttpServer() config.lru_purge_enable = true; config.stack_size = CONFIG_OPENSHOCK_PORTAL_HTTPD_STACK_SIZE; // Shorter than the default 10 s: one stalled or sleeping client blocks the single httpd task for this long. - config.recv_wait_timeout = CONFIG_OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S; - config.send_wait_timeout = CONFIG_OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S; + config.recv_wait_timeout = kHttpdTimeoutS; + config.send_wait_timeout = kHttpdTimeoutS; config.uri_match_fn = httpd_uri_match_wildcard; // Same core as the WiFi/lwIP tasks, away from the E-Stop and RF tasks on core 1 config.core_id = 0; From d009299bb71e0a803f6765afe6a07af1ea51e3bf Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 29 Oct 2026 22:08:53 +0100 Subject: [PATCH 11/12] Revert "refactor(kconfig): set the portal httpd timeout in ms" This reverts commit 5bc7522ef592ee662babcba0904e5507948105b7. --- components/captive_portal/Kconfig | 10 ++++------ .../src/captiveportal/CaptivePortalInstance.cpp | 6 ++---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/components/captive_portal/Kconfig b/components/captive_portal/Kconfig index 1113fe13..440a2ebf 100644 --- a/components/captive_portal/Kconfig +++ b/components/captive_portal/Kconfig @@ -27,12 +27,10 @@ menu "OpenShock captive portal" gateway connection and an outbound HTTP request need one each (checked against LWIP_MAX_SOCKETS). - config OPENSHOCK_PORTAL_HTTPD_TIMEOUT_MS - int "HTTP socket send/receive timeout (ms)" - range 1000 60000 - default 5000 - help - Rounded up to whole seconds, the unit esp_http_server takes. + config OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S + int "HTTP socket send/receive timeout (s)" + range 1 60 + default 5 config OPENSHOCK_PORTAL_WS_MAX_QUEUED_SENDS int "Queued websocket sends" diff --git a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index b0c3bc95..262005e7 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -35,8 +35,6 @@ const char* const TAG = "CaptivePortalInstance"; #include const uint16_t HTTP_PORT = 80; -// esp_http_server takes its socket timeouts in whole seconds -static constexpr uint16_t kHttpdTimeoutS = (CONFIG_OPENSHOCK_PORTAL_HTTPD_TIMEOUT_MS + 999) / 1000; // httpd needs 3 lwIP sockets besides its client sockets; the DNS server, the gateway connection and an outbound HTTP request need one each static_assert(CONFIG_LWIP_MAX_SOCKETS >= CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS + 3 + 3, "httpd client sockets would leave no lwIP sockets for the DNS server and outbound connections"); static_assert(CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS > CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS, "every websocket client needs a socket, plus one for plain requests"); @@ -1126,8 +1124,8 @@ bool CaptivePortal::CaptivePortalInstance::startHttpServer() config.lru_purge_enable = true; config.stack_size = CONFIG_OPENSHOCK_PORTAL_HTTPD_STACK_SIZE; // Shorter than the default 10 s: one stalled or sleeping client blocks the single httpd task for this long. - config.recv_wait_timeout = kHttpdTimeoutS; - config.send_wait_timeout = kHttpdTimeoutS; + config.recv_wait_timeout = CONFIG_OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S; + config.send_wait_timeout = CONFIG_OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S; config.uri_match_fn = httpd_uri_match_wildcard; // Same core as the WiFi/lwIP tasks, away from the E-Stop and RF tasks on core 1 config.core_id = 0; From 2f305cd95a0c3189632c6f00d3a0040f17b9bbfd Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 29 Oct 2026 22:14:46 +0100 Subject: [PATCH 12/12] Revert "test(scripts): leave the repository-wide Kconfig ownership check to host-tests" This reverts commit 31fde751b65906722344365e70ef1cb772df2b10. --- scripts/tests/test_check_kconfig_ownership.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/tests/test_check_kconfig_ownership.py b/scripts/tests/test_check_kconfig_ownership.py index e1d650e8..a9bf1cb9 100644 --- a/scripts/tests/test_check_kconfig_ownership.py +++ b/scripts/tests/test_check_kconfig_ownership.py @@ -62,3 +62,7 @@ def test_a_quoted_requirement_counts(tmp_path): component(tmp_path, 'base', kconfig='config OPENSHOCK_BASE\n int "x"\n') component(tmp_path, 'top', priv='"base"', source='CONFIG_OPENSHOCK_BASE') assert violations(tmp_path) == [] + + +def test_the_repository_has_no_violations(): + assert check_kconfig_ownership.find_violations(check_kconfig_ownership.ROOT / 'components') == []