Add Pin Manager UI for pin status and button state debugging #5122
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughThis PR introduces a new "Pin Info" settings page that displays GPIO pin status, allocation, ownership, and capabilities. It includes UI components, JSON API endpoints for pin serialization, server routing infrastructure, and backend logic for pin data aggregation and touch capability detection. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
…/LOW text, rename Caps to Functions Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…ti Relay support Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…ns and platforms Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@softhack007 did you ever try to implement something like this? Its still a prototype version but most of the functionality is there. copilot is at its wits end it seems though but ouputs and inputs update dynamically, I can see a dot turn green if I pull down a button pin. |
…t in btnPin array Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
|
This would be a nice addition to check pin states and what is going on. relay/output state as well as input buttons and especialy touch buttons (including current touch value) is updated live. Please consider this as an interacive debug tool fo GPIO states. |
|
additional note: this also adds the "touch" capability to buttons in standard button json request used in led config for future use of dropdowns in buttons (not implemented yet in this PR on purpose) |
What is the significance of the green dot? |
it shows live pin state:
as mentioned, extracting correct usermod names is still something that needs to be implemented as I was unable to do it cleanly without a lot of json parsing which felt like not the best way to approach that. in general this dot is drawn if a pin is allocated by the pin-manager as a GPIO (input or output) without special functions so it should work on all usermods as long as they use the pinmanager properly |
|
@copilot following the comments, please implement the pin owners as a lookup function in the firmware. For now, keep the button functions in the UI. Pass the pin owner strings instead of the magic numbers. |
Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
|
@copilot revert the changes to button type lookup: keep button type names in the UI, only keep the look-up for the owner in the firmware. |
Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
DedeHai
left a comment
DedeHai
left a comment
There was a problem hiding this comment.
moved pin owner strings to firmware
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wled00/data/settings_pininfo.htm (1)
16-16: Implicit global variables.
pinsTimerandgpioInfoare declared withoutvarorlet, creating implicit globals which can cause issues in strict mode and make the code harder to maintain.🔎 Proposed fix
- pinsTimer=null, gpioInfo={}; + var pinsTimer=null, gpioInfo={};
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tools/cdata.jswled00/const.hwled00/data/settings.htmwled00/data/settings_pininfo.htmwled00/fcn_declare.hwled00/json.cppwled00/wled_server.cppwled00/xml.cpp
🧰 Additional context used
📓 Path-based instructions (6)
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings.htmwled00/data/settings_pininfo.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings.htmwled00/data/settings_pininfo.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings.htmwled00/data/settings_pininfo.htm
tools/cdata.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tools/cdata.js to convert web assets to embedded C++ headers; do not reimplement this logic elsewhere
Files:
tools/cdata.js
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/const.hwled00/fcn_declare.h
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/wled_server.cppwled00/xml.cppwled00/json.cpp
🧠 Learnings (22)
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/settings*.htm : Name settings pages as settings*.htm within the web UI
Applied to files:
wled00/data/settings.htmtools/cdata.jswled00/data/settings_pininfo.htmwled00/wled_server.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to tools/cdata.js : Use tools/cdata.js to convert web assets to embedded C++ headers; do not reimplement this logic elsewhere
Applied to files:
tools/cdata.js
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
tools/cdata.jswled00/data/settings_pininfo.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
tools/cdata.jswled00/const.hwled00/data/settings_pininfo.htmwled00/wled_server.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/**/*.{htm,html,css,js} : Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Applied to files:
tools/cdata.js
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : Always commit generated html_*.h files along with related source changes
Applied to files:
tools/cdata.js
📚 Learning: 2025-09-14T18:43:59.338Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4932
File: wled00/data/cpal/cpal.htm:14-14
Timestamp: 2025-09-14T18:43:59.338Z
Learning: In WLED, static web pages like cpal.htm are served directly at the root URL level (e.g., "/cpal.htm") via handleStaticContent() from embedded page data, regardless of their physical source folder location in the repository. Relative script references like "common.js" resolve correctly from the served URL path, not the source file path.
Applied to files:
tools/cdata.jswled00/wled_server.cpp
📚 Learning: 2025-07-31T18:21:49.868Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4793
File: wled00/file.cpp:481-513
Timestamp: 2025-07-31T18:21:49.868Z
Learning: In WLED, essential configuration files that require backup have short, controlled names (like "/cfg.json", "/presets.json") that are well within a 32-character buffer limit. The file naming is controlled by developers, making buffer overflow in backup filename construction highly unlikely.
Applied to files:
tools/cdata.js
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
wled00/const.h
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/const.hwled00/data/settings_pininfo.htmwled00/wled_server.cppwled00/xml.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/const.hwled00/data/settings_pininfo.htmwled00/wled_server.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
wled00/fcn_declare.hwled00/json.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.
Applied to files:
wled00/fcn_declare.h
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/index.htm : Main web interface entry file is index.htm; ensure it remains present and functional
Applied to files:
wled00/data/settings_pininfo.htm
📚 Learning: 2025-11-30T15:29:00.726Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4456
File: usermods/deep_sleep/deep_sleep.cpp:224-230
Timestamp: 2025-11-30T15:29:00.726Z
Learning: In WLED, `touchThreshold` is a global variable defined in wled00/wled.h and is accessible to all usermods. It stores the touch sensitivity threshold for touch-capable GPIO pins on ESP32 variants and should not be flagged as undefined when used in usermods.
Applied to files:
wled00/data/settings_pininfo.htmwled00/xml.cpp
📚 Learning: 2025-03-29T01:22:54.617Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Applied to files:
wled00/data/settings_pininfo.htm
📚 Learning: 2025-09-13T13:13:36.092Z
Learnt from: netmindz
Repo: wled/WLED PR: 3777
File: wled00/bus_manager.cpp:982-989
Timestamp: 2025-09-13T13:13:36.092Z
Learning: In the WLED HUB75 implementation, when HUB75 matrix initialization fails after pin allocation, pins are intentionally not deallocated because they're physically connected to the HUB75 panel and cannot be reused for other purposes. This design choice prioritizes simplicity over strict software resource management consistency.
Applied to files:
wled00/data/settings_pininfo.htm
📚 Learning: 2025-09-13T13:13:35.817Z
Learnt from: netmindz
Repo: wled/WLED PR: 3777
File: wled00/bus_manager.cpp:941-947
Timestamp: 2025-09-13T13:13:35.817Z
Learning: For HUB75 matrix panels in WLED, pins are physically dedicated to the matrix hardware and don't need to be deallocated on driver initialization failure since they cannot be reused for other purposes without hardware changes.
Applied to files:
wled00/data/settings_pininfo.htm
📚 Learning: 2025-09-18T03:17:30.107Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), digitalCount is intentionally incremented even for placeholder buses to maintain consistent resource mapping across all bus configurations.
Applied to files:
wled00/data/settings_pininfo.htm
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Applied to files:
wled00/data/settings_pininfo.htmwled00/json.cpp
📚 Learning: 2025-05-26T16:09:34.325Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
Applied to files:
wled00/wled_server.cpp
📚 Learning: 2025-10-10T18:34:06.550Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Applied to files:
wled00/wled_server.cpp
🧬 Code graph analysis (2)
wled00/fcn_declare.h (1)
wled00/json.cpp (2)
serializePins(1093-1219)serializePins(1093-1093)
wled00/json.cpp (2)
wled00/pin_manager.cpp (6)
getPinOwner(270-274)getPinOwner(270-270)isPinAllocated(182-187)isPinAllocated(182-182)isPinOk(206-260)isPinOk(206-206)wled00/button.cpp (2)
isButtonPressed(93-122)isButtonPressed(93-93)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: wled_build / Build Environments (usermods)
🔇 Additional comments (18)
wled00/const.h (1)
462-463: LGTM!The new
SUBPAGE_PINSconstant follows the existing numbering convention, and definingSUBPAGE_LASTas an alias provides a clean upper-bound check for settings page validation throughout the codebase.tools/cdata.js (1)
370-376: LGTM!The new
settings_pininfo.htmentry correctly follows the established pattern for settings pages, using gzip compression and html-minify filter consistent with other settings entries.wled00/fcn_declare.h (1)
163-163: LGTM!The
serializePinsdeclaration is correctly placed alongside other JSON serialization functions and follows the established signature pattern.wled00/data/settings_pininfo.htm (1)
58-83: Overall UI implementation looks good.The fetch/poll mechanism, table rendering, and error handling follow reasonable patterns. The 250ms polling interval provides responsive feedback for debugging touch thresholds and button states as intended by the PR.
wled00/data/settings.htm (1)
45-45: LGTM!The "Pin Info" button is correctly placed after "LED Preferences" as specified in the PR objectives, using the same navigation pattern as other settings buttons.
wled00/wled_server.cpp (3)
688-688: Good use of symbolic constant.Replacing the hardcoded subpage bound with
SUBPAGE_LASTimproves maintainability—new subpages only need to updateSUBPAGE_LASTin const.h. Based on learnings, this aligns with the practice of avoiding magic numbers.
728-728: LGTM!The URL pattern matching for "pins" follows the established convention for other settings subpages.
822-822: LGTM!The switch case correctly maps
SUBPAGE_PINSto the newPAGE_settings_pininfocontent and its length.wled00/xml.cpp (3)
169-184: LGTM!The touch-capable GPIO enumeration correctly handles platform differences—ESP32/S2/S3 have touch support while other variants emit an empty array. Using
digitalPinToTouchChannelis the proper API for detecting touch capability.
193-193: Good use of symbolic constant.Using
SUBPAGE_LASTfor the upper bound check aligns with the changes in wled_server.cpp and improves maintainability.
724-728: LGTM!Reusing
appendGPIOinfofor the pins subpage avoids code duplication and ensures the UI receives consistent GPIO capability data.wled00/json.cpp (7)
1056-1061: LGTM on capability flags.The capability flags are well-documented with clear comments.
PIN_CAP_PWMbeing commented out as unused is intentional since all ESP32 output pins support LEDC PWM.
1063-1091: Good firmware-side owner name resolution.Moving owner name lookup into firmware (via
getPinOwnerName) addresses the reviewer feedback about avoiding magic numbers in the UI. The TODO comment for usermod names acknowledges future improvement without blocking this PR.
1093-1106: LGTM on pin enumeration setup.The ESP8266 special case (17 pins for GPIO0-16 + A0) and the skip logic for unusable pins are correctly implemented.
1115-1133: Correct ADC1-only filtering.Properly excludes ADC2 channels from the ADC capability flag since ADC2 is unusable when WiFi is active—this addresses reviewer feedback about ESP32 platform-specific behavior.
1169-1180: Efficient button lookup.The linear scan through buttons is acceptable given
WLED_MAX_BUTTONSis small (10-32). The early break on match optimizes the common case.
1188-1216: Comprehensive pin state reporting.The logic correctly differentiates between relay outputs, button inputs, and other GPIO owners. Using
isButtonPressed()from button.cpp (as suggested in PR comments) ensures consistent button state detection. Touch raw readings with the V2 right-shift for S2/S3 addresses the platform-specific scaling feedback.
1279-1279: LGTM on JSON API integration.The
pinstarget is cleanly integrated into the existingjson_targetenum andserveJsondispatch, following the established pattern for other endpoints.Also applies to: 1293-1293, 1337-1338
|
Tested working on C3, S3, S3+HUB75, ESP32, ESP8266 although I could not possibly test all combinations of pins, usermods etc. but whatever I did test worked. |
DedeHai
added
enhancement
hardware
Adds a new Pin Manager settings page that displays comprehensive GPIO pin information including capabilities, allocation status, owner, and real-time state for debugging button inputs and relay outputs.
Users are often confused about why certain pins can't be used or don't work as expected. This PR addresses that by providing an interactive debug tool that shows live GPIO states.
Features
Pin Information Display: Shows all available and allocated GPIO pins with their:
Real-time State Monitoring: Live updates (250ms polling) show:
Settings Integration:
/settings/pins/settings/s.js?p=2endpoint for GPIO capabilities + new/json/pinsfor real-time statesTechnical Improvements
getPinOwnerName()function, eliminating hardcoded "magic number" lookups in JavaScript for ownerso) and owner name (n) are passed in JSON for flexibilityt) passed as numeric ID, looked up in UI for human-readable displayappendGPIOinfo()to include touch-capable pins in standard GPIO endpointsAPI
New
/json/pinsendpoint returns per-pin data:{ "pins": [ {"p": 0, "c": 11, "a": true, "o": 133, "n": "Button", "t": 6, "m": 0, "s": 1, "u": 1, "r": 45}, {"p": 16, "c": 3, "a": true, "o": 130, "n": "LED Digital"} ] }p: GPIO numberc: capability flags (Touch, Analog, PWM, Boot, Input Only)a: allocated statuso: owner ID (numeric, for backward compatibility)n: owner name (string from firmware, e.g., "Button", "Relay", "LED Digital")t: button type ID (numeric, displayed in UI as "Push", "Touch", etc.)m: mode (0=input, 1=output) - only for simple GPIOs: state (0=LOW/inactive, 1=HIGH/active) - only for buttons/relaysu: pullup enabled - only for button inputsr: raw touch reading value - only for touch buttonsUse Cases
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.