Reorganize settings to allow for multiple named JSON endpoints#1
Reorganize settings to allow for multiple named JSON endpoints#1DTTerastar merged 3 commits intomainfrom
Conversation
WalkthroughThe changes reorganize the configuration settings management by segmenting parameters into distinct endpoints for WiFi, MQTT, network, and API configurations. New variables are introduced, and existing MQTT settings are updated with revised key names. Documentation and comments have been revised to reflect the new endpoints while maintaining legacy compatibility. Moreover, the internal data handling is refactored to use parallel vectors, and a new method, Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer Code
participant HWS as HeadlessWiFiSettings
Dev->>HWS: markEndpoint("mqtt")
HWS->>HWS: findOrCreateEndpoint("mqtt")
HWS-->>Dev: Endpoint "mqtt" registered
sequenceDiagram
participant C as Client
participant WS as Web Server
participant HWS as HeadlessWiFiSettings
C->>WS: GET /settings/mqtt
WS->>HWS: findOrCreateEndpoint("mqtt")
HWS-->>WS: Return MQTT parameters
WS->>C: Send JSON response
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/HeadlessWiFiSettings.cpp (4)
165-168: Consider storing endpoints in a map instead of parallel vectors.
Current parallel vectors (endpointNames and endpointParams) can introduce index synchronization complexities. A map or unordered_map keyed by endpoint name could improve maintainability and prevent potential out-of-range errors.
170-177: Clarify the lazy initialization in params().
Because params() also creates the "main" endpoint if none exist, it might be surprising that a simple "getter" function modifies state. Renaming or documenting this side-effect could enhance code clarity.
179-191: Possible performance improvement when searching for endpoints.
Repeatedly scanning endpointNames works for small sets but may become inefficient with many endpoints. Using a map or storing a lookup cache for existing endpoints could optimize lookups if scalability is a concern.
396-458: Duplicate endpoint-finding logic for “extra” endpoint.
The code to search for "extra" duplicates similar logic from findOrCreateEndpoint. Refactoring to a common helper method could reduce repetition and simplify maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/Advanced/Advanced.ino(3 hunks)examples/Basic/Basic.ino(1 hunks)implementation-plan.md(1 hunks)src/HeadlessWiFiSettings.cpp(3 hunks)src/HeadlessWiFiSettings.h(1 hunks)
🧰 Additional context used
🪛 LanguageTool
implementation-plan.md
[uncategorized] ~60-~60: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...markExtra()` to create "extra" endpoint on demand 5. Update parameter storage logic to us...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.17.2)
implementation-plan.md
10-10: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
11-11: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
31-31: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
32-32: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
33-33: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
35-35: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
36-36: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
37-37: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
42-42: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
43-43: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
45-45: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
46-46: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
🔇 Additional comments (11)
src/HeadlessWiFiSettings.cpp (1)
296-298: No issues found in markEndpoint().
The logic to switch endpoints by name is straightforward and readable.examples/Basic/Basic.ino (1)
8-40: Code organization is clear and consistent with the new endpoints.
Using markEndpoint for “network” and “api,” and markExtra for “extra,” cleanly demonstrates how to separate settings into relevant categories. No issues found with the usage.src/HeadlessWiFiSettings.h (1)
17-17: markEndpoint declaration looks good.
The new method appropriately exposes endpoint selection for external usage.examples/Advanced/Advanced.ino (4)
20-23: LGTM! Well-organized WiFi settings.The WiFi settings are logically organized under the main endpoint, with proper use of
pstringfor secure password storage.
24-30: LGTM! Well-structured MQTT settings.The MQTT settings are properly organized under a dedicated endpoint with appropriate validation and secure password storage.
39-44: LGTM! Good implementation of debug settings.The debug settings maintain backward compatibility while adding a well-structured log level dropdown.
86-114: LGTM! Comprehensive API documentation.The curl examples are well-documented, covering both legacy and new endpoint formats with clear examples for each setting category.
implementation-plan.md (4)
13-28: LGTM! Memory-efficient design with clear implementation.The parallel vector design is a good choice for managing multiple endpoints while maintaining backward compatibility.
29-38: LGTM! Well-designed API changes.The new
markEndpointmethod and updatedmarkExtramethod provide a clean API while maintaining backward compatibility.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
31-31: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
32-32: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
33-33: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
35-35: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
36-36: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
37-37: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
39-47: Consider adding error handling for invalid endpoint names.While the endpoint structure is good, consider documenting how invalid endpoint names will be handled.
Would you like me to generate example error handling code for the HTTP endpoint handler?
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
42-42: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
43-43: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
45-45: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
46-46: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
70-94: LGTM! Clear examples and memory considerations.The examples effectively demonstrate both legacy and new usage patterns, with well-documented memory impact considerations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/HeadlessWiFiSettings.cpp (1)
396-458: Consider reducing code duplication in legacy endpoint handlers.The endpoint lookup logic is duplicated between GET and POST handlers. Consider extracting it into a helper function.
+ // Helper function to find extra endpoint + auto findExtraEndpoint = []() -> std::pair<bool, size_t> { + for (size_t i = 0; i < endpointNames.size(); i++) { + if (endpointNames[i] == "extra") { + return {true, i}; + } + } + return {false, 0}; + }; // Legacy /extras endpoint redirects to /settings/extra http.on("/extras", HTTP_GET, [this](AsyncWebServerRequest *request) { - // Find extra endpoint - size_t extraIndex = 0; - bool found = false; - for (size_t i = 0; i < endpointNames.size(); i++) { - if (endpointNames[i] == "extra") { - extraIndex = i; - found = true; - break; - } - } + auto [found, extraIndex] = findExtraEndpoint();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/Advanced/Advanced.ino(3 hunks)src/HeadlessWiFiSettings.cpp(4 hunks)
🔇 Additional comments (8)
examples/Advanced/Advanced.ino (5)
20-22: LGTM! Well-organized WiFi settings.Good use of
pstring()for the password field to ensure sensitive data is handled securely.
24-29: LGTM! Well-structured MQTT configuration.Good organization of MQTT settings under a dedicated endpoint with clean parameter naming.
31-37: Add IP address validation for network settings.While the organization is good, consider adding validation for IP addresses to prevent invalid configurations.
39-44: LGTM! Well-implemented debug settings.Good use of dropdown for log levels, providing a better user experience with predefined options.
86-124: LGTM! Excellent documentation of API endpoints.Comprehensive examples that clearly demonstrate how to interact with each endpoint.
src/HeadlessWiFiSettings.cpp (3)
165-191: LGTM! Clean implementation of endpoint management.Good design using parallel vectors for endpoint names and parameters, with efficient endpoint lookup and creation.
308-350: Fix off-by-one error in endpoint name extraction.The substring(9) will include the leading slash, preventing endpoint matching.
460-508: LGTM! Robust WiFi scanning implementation.Excellent handling of duplicate networks and memory management. The implementation correctly:
- Keeps the highest RSSI value for duplicate networks
- Skips hidden networks
- Cleans up scan results with WiFi.scanDelete()
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/HeadlessWiFiSettings.cpp (1)
359-400: Fix off-by-one error in endpoint name extraction.The substring calculation for endpoint name extraction is incorrect.
Apply this diff to fix the off-by-one error:
- endpointName = path.substring(6); // Remove "/wifi/" + endpointName = path.substring(6); // Correctly remove "/wifi/"
🧹 Nitpick comments (3)
examples/Basic/Basic.ino (1)
12-16: Consider using IP address validation for network settings.While the organization is good, consider using
ipv4type for IP address validation.Apply this diff to add validation:
- String ip = HeadlessWiFiSettings.string("ip", "192.168.1.100"); + String ip = HeadlessWiFiSettings.ipv4("ip", "192.168.1.100");src/HeadlessWiFiSettings.cpp (2)
165-191: Consider adding bounds checking for endpoint management.The endpoint management implementation could benefit from additional error handling:
- Check for vector capacity before pushing new endpoints
- Add error handling for index out of bounds
Apply this diff to add error handling:
uint8_t findOrCreateEndpoint(const String& name) { // Look for existing endpoint for (size_t i = 0; i < endpointNames.size(); i++) { if (endpointNames[i] == name) { return i; } } + // Check capacity before creating new endpoint + if (endpointNames.size() >= UINT8_MAX) { + return 0; // Return main endpoint if capacity exceeded + } // Create new endpoint endpointNames.push_back(name); endpointParams.push_back({}); return endpointNames.size() - 1; }
309-356: Consider optimizing memory usage in WiFi scan.The WiFi scan implementation could benefit from memory optimization:
- Reserve vector capacity based on
numNetworks- Clear WiFi scan results earlier
Apply this diff to optimize memory usage:
int numNetworks = WiFi.scanNetworks(); +if (numNetworks < 0) { + request->send(500, "text/plain", "WiFi scan failed"); + return; +} AsyncResponseStream *response = request->beginResponseStream("application/json"); response->print("{\"networks\":{"); bool needsComma = false; struct Network { String ssid; int rssi; }; std::vector<Network> networks; +networks.reserve(numNetworks); // Reserve capacity // First pass: collect all networks with their RSSI values for (int i = 0; i < numNetworks; i++) { String ssid = WiFi.SSID(i); if (ssid.isEmpty()) continue; // Skip hidden networks int rssi = WiFi.RSSI(i); bool found = false; // Update existing network if we've seen it before for (auto& network : networks) { if (network.ssid == ssid) { if (rssi > network.rssi) { network.rssi = rssi; // Keep the highest RSSI value } found = true; break; } } // Add new network if we haven't seen it if (!found) { networks.push_back({ssid, rssi}); } } +WiFi.scanDelete(); // Clear scan results earlier // Second pass: output the networks with their highest RSSI values for (const auto& network : networks) { if (needsComma) response->print(","); response->printf("\"%s\":%d", json_encode(network.ssid).c_str(), network.rssi); needsComma = true; } response->print("}}"); request->send(response); -WiFi.scanDelete();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/Advanced/Advanced.ino(3 hunks)examples/Basic/Basic.ino(1 hunks)src/HeadlessWiFiSettings.cpp(4 hunks)
🔇 Additional comments (9)
examples/Basic/Basic.ino (3)
8-10: LGTM! Clear and well-structured WiFi settings.The WiFi settings are properly organized in the main endpoint with appropriate use of
stringandpstringfor sensitive data.
23-25: LGTM! Well-structured extra settings.The extra settings are properly organized with appropriate use of
integerfor the refresh interval.
18-21: Consider adding URL validation for the API endpoint.Consider adding validation to ensure the API endpoint is a valid URL.
✅ Verification successful
🌐 Web query:
What is the best way to validate URLs in Arduino/ESP32?💡 Result:
The best approaches for validating URLs in ESP32/Arduino projects involve structured parsing techniques and leveraging existing libraries to ensure correctness and security: ## Core Validation Methods **1. PathVariableHandlers Library** For RESTful API endpoints requiring dynamic parameters (e.g., `/sensor/:sensorId`): - Use `TokenIterator` to split URLs into components[2] - Create `UrlTokenBindings` to match template patterns against actual URLs ```cpp // Example: Extract sensorId from /sensor/123 TokenIterator templateUrlIterator("sensor/:sensorId", '/'); TokenIterator urlIterator("sensor/123", '/'); UrlTokenBindings bindings(templateUrlIterator, urlIterator); if(bindings.hasBinding("sensorId")) { String sensor = bindings.get("sensorId"); // Returns "123" } ``` **2. Web Server Request Handling** When using ESP32's built-in web server: - Verify complete URL matches using exact string comparisons instead of `indexOf()`[6] ```cpp // Improved approach vs partial string matching if (header.startsWith("GET /26/on HTTP/1.1")) { // Handle GPIO 26 ON } ``` **3. HTTPS Certificate Validation** For secure connections: ```cpp WiFiClientSecure client; client.setCACert(root_ca); // From [7] if(client.connect(server, 443)) { // Proceed only if certificate validates } ``` ## Best Practices - **Use POST for state changes** to avoid accidental parameter retention in URLs[3] - **Implement parameter whitelisting** for allowed values/ranges - **Sanitize inputs** when converting URL parameters to other data types: ```cpp int sensorId = atoi(bindings.get("sensorId").c_str()); if(sensorId < 0 || sensorId > 100) { // Reject invalid IDs } ``` ## Recommended Libraries 1. `PathVariableHandlers` ([Techedd tutorial][2]) For REST-style parameter extraction 2. `AsyncTCP` + `ESPAsyncWebServer` ([Random Nerd Tutorial][6]) Built-in URI pattern matching capabilities 3. `WiFiClientSecure` ([DFRobot][4],[7]) For SSL/TLS certificate validation For complex applications, combine these methods with proper error handling (HTTP 400 responses for invalid URLs) and consider using websockets[3] or FetchAPI[10] to minimize manual URL manipulation risks. Citations: - 1: https://randomnerdtutorials.com/esp32-http-get-post-arduino/ - 2: https://techtutorialsx.com/2021/08/07/esp32-parsing-url-variables/ - 3: https://forum.arduino.cc/t/esp32-webserver-resetting-the-client-url/1184350 - 4: https://www.dfrobot.com/blog-854.html - 5: https://www.dfrobot.com/blog-1212.html - 6: https://randomnerdtutorials.com/esp32-web-server-arduino-ide/ - 7: https://randomnerdtutorials.com/esp32-https-requests/ - 8: https://github.com/fhessel/esp32_https_server/blob/master/examples/Parameter-Validation/Parameter-Validation.ino - 9: https://github.com/espressif/arduino-esp32/issues/8050 - 10: https://forum.arduino.cc/t/esp32-https-request-with-api-token/1194885
Suggestion: Add URL validation for the API endpoint
The current code in file examples/Basic/Basic.ino (lines 18–21) sets the API endpoint directly without validating that it is formed correctly as a URL. Based on best practices for Arduino/ESP32 projects—as highlighted in the web query results—it’s recommended to incorporate structured URL validation (e.g., using available parsing libraries or regex checks) before using the endpoint. This would help ensure that the endpoint conforms to proper URL standards (e.g., begins with "http://" or "https://", has a valid domain, etc.) and improve overall robustness and security.
examples/Advanced/Advanced.ino (5)
20-22: LGTM! Clear and well-structured WiFi settings.The WiFi settings are properly organized in the main endpoint with appropriate use of
stringandpstringfor sensitive data.
31-38: Consider using IP address validation for network settings.While the organization is good, consider using
ipv4type for IP address, gateway, and netmask validation.
39-44: LGTM! Well-structured debug settings.The debug settings are properly organized with appropriate use of
checkbox,floating, anddropdownfor the respective settings.
86-126: LGTM! Comprehensive API documentation.The example curl commands are well-documented and cover all endpoints with clear examples of both GET and POST operations.
24-29: Consider adding host validation for MQTT settings.Consider adding validation to ensure the MQTT host is a valid hostname or IP address.
✅ Verification successful
🌐 Web query:
What is the best way to validate hostnames in Arduino/ESP32?💡 Result:
To validate hostnames in Arduino/ESP32 environments effectively: ### **Hostname Format Rules** - **Allowed characters**: Letters (`a-z`, `A-Z`), numbers (`0-9`), and hyphens (`-`)[2][4][7]. - **Length**: Up to 63 characters (DNS standard)[4][7]. - **No spaces or special characters** (e.g., underscores or symbols)[7]. - **Cannot start/end with a hyphen**[7]. ### **Validation Implementation** Use a function like this to enforce rules programmatically: ```cpp bool isValidHostname(const char* hostname) { if (strlen(hostname) > 63 || strlen(hostname) == 0) return false; if (!isalnum(hostname[0]) || !isalnum(hostname[strlen(hostname)-1])) return false; for (int i=0; i<strlen(hostname); i++) { if (!isalnum(hostname[i]) && hostname[i] != '-') return false; } return true; } ``` ### **Usage Best Practices** 1. Validate **before** setting: ```cpp const char* customHostname = "esp32-device01"; if (isValidHostname(customHostname)) { WiFi.setHostname(customHostname); } ``` 2. Call `WiFi.setHostname()` **before** `WiFi.begin()`[4][7]. 3. For mDNS compatibility: - Use `.local` suffix when resolving (e.g., `esp32Host.local`)[1]. - Add periodic `MDNS.update()` in `loop()` for sustained functionality[1]. ### **Common Pitfalls** - Routers may ignore invalid hostnames silently[3][7]. - Case sensitivity varies across network devices[7]. - Hostnames like `ESP32_Test` fail due to underscores[7]. ### **Example Workflow** ```cpp #include <WiFi.h> void setup() { const char* hostname = "esp32-sensor"; if (!isValidHostname(hostname)) { Serial.println("Invalid hostname!"); return; } WiFi.config(INADDR_NONE, INADDR_NONE, INADDR_NONE, INADDR_NONE); WiFi.setHostname(hostname); // Must be before WiFi.begin() WiFi.begin("SSID", "PASSWORD"); } ``` This approach ensures compatibility with routers and mDNS services while adhering to DNS standards[1][4][7]. Citations: - 1: https://techtutorialsx.com/2021/10/29/esp32-mdns-host-name-resolution/ - 2: https://lededitpro.com/set-a-custom-hostname-for-esp32-using-arduino-ide/ - 3: https://forum.arduino.cc/t/cant-ping-esp32-cam-hostname/1209698 - 4: https://deepbluembedded.com/esp32-hostname-change-arduino/ - 5: https://github.com/espressif/arduino-esp32/issues/5246 - 6: https://forum.arduino.cc/t/resolving-host-names/317186 - 7: https://randomnerdtutorials.com/esp32-set-custom-hostname-arduino/ - 8: https://randomnerdtutorials.com/esp32-useful-wi-fi-functions-arduino/ - 9: https://forum.arduino.cc/t/best-way-to-check-if-host-is-up/695224 - 10: http://esp32.io/viewtopic.php?t=31705
Add hostname validation for MQTT host.
The web query confirms that validating hostnames in Arduino/ESP32 should enforce DNS standards (e.g., allowed characters, length restrictions, no leading or trailing hyphens). Consider implementing a function like the one below to validate the MQTT host before using it:
bool isValidHostname(const char* hostname) { size_t len = strlen(hostname); if (len == 0 || len > 63) return false; if (!isalnum(hostname[0]) || !isalnum(hostname[len - 1])) return false; for (size_t i = 0; i < len; i++) { if (!isalnum(hostname[i]) && hostname[i] != '-') return false; } return true; }
- Use this function to validate the value returned by
HeadlessWiFiSettings.string("host", "mqtt.example.org").- If the hostname is invalid, handle the error appropriately (e.g., revert to a default setting or alert the user).
This improvement would help ensure that only well-formed hostnames are used for MQTT settings.
src/HeadlessWiFiSettings.cpp (1)
403-444: Fix off-by-one error in endpoint name extraction.The substring calculation for endpoint name extraction is incorrect.
1 participant
Summary by CodeRabbit