-
-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release v0.6 #175
Release v0.6 #175
Conversation
### TL;DR Added support for an alternate action when using HomeKey authentication, triggered by a physical button. ### What changed? - Added configuration options for an alternate action GPIO pin and timing - Introduced a button-activated alternate action mode with LED feedback - Added MQTT publishing for alternate action events - Created new web UI fields for configuring alternate action settings - Implemented GPIO control for alternate action output ### How to test? 1. Configure alternate action settings in the web UI: - Set GPIO pin for init button - Configure LED feedback pin (optional) - Set timeout values - Define output GPIO pin and state 2. Press and hold the configured button 3. Authenticate with HomeKey while button is held 4. Verify: - LED feedback (if configured) - GPIO output triggers - MQTT message publishes ### Why make this change? Enables users to trigger an alternative action (like opening a secondary door or gate) when authenticating with HomeKey, providing more flexibility in access control scenarios. This is particularly useful in situations where a single HomeKey reader needs to control multiple outputs based on user intent.
* Improved config provisioning through tighter integration with the WebUI * Slightly revised visual design
After struggling for about 1 hour trying to make PlatformIO use ESP-IDF 5.3.2 with Arduino-ESP32 3.1.0 as component with endless errors i had finally gave up. Looks like there might be some business-disagreements going on between PlatformIO and Espressif (and others) and newer versions of arduino-esp32 and esp-idf are not properly supported or not at all by PIO. Hopefully things get better but right now in order to move forward, PIO will no longer be used in this project. I've much later found out there is a fork of PIO called [pioarduino](https://github.com/pioarduino) exactly because of this whole debacle and esphome has switched to it but i already have things setup so it will stay this way.
# Ethernet support and device naming improvements - Standardized device naming scheme using BT MAC address - Added ethernet configuration UI with board presets - Added ethernet backend configuration and support - Added ethernet event handling and network callbacks - Improved logging consistency across codebase - Implemented HomeSpan connection callback API - Added UI button to start the AP to help reconfiguring WiFi The ethernet configuration supports both RMII and SPI based ethernet chips with preset configurations for common development boards. ## Release Notes: Ethernet Configuration Update - **New Features** - Added Ethernet configuration support with multiple chip type options - Introduced new configuration tab for Ethernet settings - Implemented dynamic Ethernet configuration management - **Technical Enhancements** - Updated web interface to support Ethernet configuration - Updated configuration defaults for various system components
WalkthroughThis update spans configuration, CI/CD, front-end, and core source files. Compilation flags in the Clangd configuration, environment sourcing, and various build settings have been modified. GitHub Actions workflows now support multiple ESP targets along with a new LittleFS job. Several form inputs and HTML structures have been reorganized for clarity. Additionally, legacy files such as the Makefile, PlatformIO configurations, and test documentation have been removed, while new submodules and custom components have been added to extend functionality in the main firmware and service layers. Changes
Sequence Diagram(s)sequenceDiagram
participant Trigger
participant Workflow
participant Job1 as esp32
participant Job2 as esp32c3
participant Job3 as esp32s3
participant Job4 as littlefs
Trigger->>Workflow: Trigger (push/pull_request/dispatch)
Workflow->>Job1: Initialize esp32 job
Job1->>Job1: Checkout, cache, build firmware
Workflow->>Job2: Initialize esp32c3 job
Job2->>Job2: Checkout, cache, build firmware
Workflow->>Job3: Initialize esp32s3 job
Job3->>Job3: Checkout, cache, build firmware
Workflow->>Job4: Initialize littlefs job
Job4->>Job4: Checkout data, create LittleFS image
sequenceDiagram
participant Main
participant AltTask as Alt Action Task
participant NFC as NFC Retry Task
participant MQTT as MQTT Handler
Main->>AltTask: Create alternate action task
Main->>NFC: Spawn NFC reconnection task
Main->>MQTT: Initialize MQTT client and set LWT
MQTT-->>Main: Acknowledge configuration
Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🔭 Outside diff range comments (2)
main/main.cpp (2)
408-453
:⚠️ Potential issueGPIO task expansions.
Adds digitalWrites for lock states and referencesgpioActionPin
. Validate that each path handles concurrency correctly (especially if multiple sources queue updates). Also confirm that changing lock states from multiple tasks (MQTT, override, etc.) does not cause race conditions.
1493-1657
:⚠️ Potential issue
nfc_thread_entry
expansions.
Handles partial reinit logic ifwriteRegister
fails. Re-creating tasks in a loop can be risky if triggered often. Also check for memory fragmentation on repeated retries.
🧹 Nitpick comments (14)
main/main.cpp (11)
45-45
: Keep array usage maintainable.
pixelTypeMap
is a straightforward mapping array. Consider using stronger type-safety (e.g., enum + mapping function) if the set of pixel types grows or the code referencing them becomes more complex.
169-197
: MQTT config extension for alternative action.
IntroducinghkAltActionTopic
in the MQTT config broadens customization. Ensure your new topic usage is documented, and confirm all references in the code correctly publish/subscribe to this new key.
297-297
: Additional log in constructor is helpful.
Logging “Configuring PhysicalLockBattery” clarifies initialization but keep logs concise in constructors to avoid excessive overhead.
530-536
: NFC GPIO handling – Alt action interplay.
This new case (2) triggers the alt-action pin ifhkAltActionActive
is true. Confirm that toggling the pin state for an extended delay doesn’t unexpectedly block other time-sensitive tasks.
628-628
: Decoded data debug log.
Again, careful with the potential exposure of raw NFC data in logs. Ensure that redaction or environment-based logging levels are used in production.
679-691
: Pairing callback – new issuer logic.
Retrieving the LTPK hash ensures uniqueness. Make sure the 8-byte truncated hash is stable if ephemeral changes to the LTPK are possible. Also consider verifying the end-user is aware that multiple controllers might share partial collisions.
762-764
: Issuer printing.
Again, watch for security concerns if these logs might expose keys in production.
933-933
: Commented-out debug line.
Leaving a commented call toESP_LOGD
can clutter code. Remove if it’s no longer needed.
992-1016
: Directory listing logs.
These logs are helpful for debugging file operations. Consider more robust error handling if LittleFS usage escalates.
1775-1780
: LittleFS mounting error log.
Consider adding a fallback or user instruction if the FS fails to mount, especially if config depends on it.
1823-1823
: Issuer data logging.
Reiterating the caution that even partial public keys can be sensitive. Confirm you have the recommended logging policy for production.main/include/config.h (1)
97-102
: Safe default GPIO configuration.The new GPIO pins for alternative actions are properly initialized to 255 (disabled state), preventing accidental GPIO activation. The timeout values are consistent with other similar configurations in the file.
Consider adding an enum or const values for these magic numbers:
+// GPIO pin states +#define GPIO_PIN_DISABLED 255 + -#define GPIO_HK_ALT_ACTION_INIT_PIN 255 +#define GPIO_HK_ALT_ACTION_INIT_PIN GPIO_PIN_DISABLED -#define GPIO_HK_ALT_ACTION_INIT_LED_PIN 255 +#define GPIO_HK_ALT_ACTION_INIT_LED_PIN GPIO_PIN_DISABLED -#define GPIO_HK_ALT_ACTION_PIN 255 +#define GPIO_HK_ALT_ACTION_PIN GPIO_PIN_DISABLEDcomponents/HomeSpan/CMakeLists.txt (1)
1-1
: File Globbing for SourcesUsing
FILE(GLOB_RECURSE ...)
simplifies source inclusion for the component. However, keep in mind that changes (such as newly added source files) may not automatically trigger a CMake reconfiguration. Consider adding a note in your documentation or CMake instructions to remind developers to re-run CMake if source files are added or removed.data/assets/misc.css (1)
193-211
: Responsive Adjustments for Mid-Sized Screens
The media query for max-width: 1090px adapts flex container behavior (e.g.,.flex-sm
,.around-sm
,.flex-center-sm
, and.flex-col-sm
) to ensure that layouts remain balanced on intermediate screen sizes.There is a duplicate specification for
gap: inherit
in the.flex-sm
block; review if it can be consolidated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
data/assets/favicon.ico
is excluded by!**/*.ico
📒 Files selected for processing (35)
.clangd
(1 hunks).envrc
(1 hunks).github/workflows/esp32.yml
(1 hunks).gitignore
(1 hunks).gitmodules
(1 hunks).nvim.lua
(1 hunks).vscode/extensions.json
(0 hunks).vscode/settings.json
(1 hunks).zed/settings.json
(1 hunks)CMakeLists.txt
(1 hunks)Makefile
(0 hunks)components/AsyncTCP
(1 hunks)components/ESPAsyncWebServer
(1 hunks)components/HK-HomeKit-Lib
(1 hunks)components/HomeSpan/CMakeLists.txt
(1 hunks)components/HomeSpan/upstream
(1 hunks)components/PN532
(1 hunks)data/assets/misc.css
(5 hunks)data/index.html
(5 hunks)data/routes/actions.html
(2 hunks)data/routes/hkinfo.html
(1 hunks)data/routes/info.html
(0 hunks)data/routes/misc.html
(2 hunks)data/routes/mqtt.html
(3 hunks)fs.py
(0 hunks)main/CMakeLists.txt
(1 hunks)main/Kconfig.projbuild
(1 hunks)main/idf_component.yml
(1 hunks)main/include/NFC_SERV_CHARS.h
(1 hunks)main/include/config.h
(2 hunks)main/main.cpp
(37 hunks)platformio.ini
(0 hunks)sdkconfig.defaults
(1 hunks)src/CMakeLists.txt
(0 hunks)test/README
(0 hunks)
💤 Files with no reviewable changes (7)
- .vscode/extensions.json
- data/routes/info.html
- test/README
- src/CMakeLists.txt
- platformio.ini
- fs.py
- Makefile
✅ Files skipped from review due to trivial changes (8)
- components/ESPAsyncWebServer
- components/HomeSpan/upstream
- components/PN532
- .envrc
- components/HK-HomeKit-Lib
- data/routes/hkinfo.html
- components/AsyncTCP
- .gitmodules
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/esp32.yml
[error] 151-151: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: esp32c3
- GitHub Check: esp32s3
- GitHub Check: esp32
🔇 Additional comments (125)
main/main.cpp (27)
1-24
: Includes look consistent and necessary.
All newly added includes (sodium, memory, logging, etc.) appear valid for their respective functionalities. Ensure no extraneous headers are introduced and that each one remains necessary as the code evolves.
37-39
: Validate task handles’ usage and cleanup.
The newly introduced task handles (alt_action_task_handle
,nfc_reconnect_task
,nfc_poll_task
) must be carefully managed. Ensure each task is either deleted or suspended properly to avoid memory leaks or undefined behavior in long-running environments.
60-61
: Confirm the switch to BT MAC.
You switched from reading the Wi-Fi MAC toESP_MAC_BT
. Verify that this aligns with your ID requirements across the codebase (e.g., logs, provisioning, or user references).
66-71
: Structure definition seems fine.
eth_chip_desc_t
neatly encapsulates the Ethernet chip info. Ensure theNLOHMANN_DEFINE_TYPE_INTRUSIVE_ONLY_SERIALIZE
macro usage matches your serialization/deserialization needs if you later consume this struct from external data.
73-107
: Comprehensive board presets structure.
Theeth_board_presets_t
struct covers both EMAC and SPI-based presets. Double-check that the conditionally compiled sections do not break the build on unsupported boards. Also confirm that the JSON serialization aligns with any front-end or CLI usage.
109-151
: Namespace approach for Ethernet configs looks good.
Grouping supported chips and board presets ineth_config_ns
keeps the code organized. Validate that dynamic access tosupportedChips
andboardPresets
is always done with proper checks to avoid out-of-range errors (e.g., reading from an emptystd::map
).
241-271
: Extended fields inmisc_config_t
.
You’ve added new alt-action pins, Ethernet config, and other flags. Carefully verify that each field is validated at runtime to prevent invalid GPIO usage or conflicting pin assignments.
283-283
: Shared pointer usage for Pixel is sensible.
Storingpixel
as astd::shared_ptr
allows flexible lifetime management. Just confirm that any tasks referencingpixel
will gracefully handle cases where it might be reset or replaced.
311-311
: LockManagement constructor log is fine.
Similar to the battery-service constructor, ensure initialization logs remain meaningful without overloading the console.
320-349
: AccessoryInformation extension for NFC
The new fields (e.g. custom hardware finish, manufacturer “rednblkx”) look consistent. Confirm that referencing the BT MAC for theSerialNumber
remains correct if a user expects the Wi-Fi MAC or a combined ID.
371-394
:alt_action_task
loop logic.
This background task monitors a pin for a high signal and toggleshkAltActionActive
. Confirm thatvTaskDelay
intervals and the toggling of any LED are correct for short button presses or noise. Consider hardware debouncing if spurious triggers occur.
[verification]
618-620
: Informative debug logs.
These logs help check provisioning details. Just be mindful not to leak sensitive data ifreader_gid
orreader_id
are considered private.
660-671
:getHashIdentifier
function.
Generates an 8-byte hashed ID. Implementation usingmbedtls_sha256
is straightforward. Double-check that partial usage of the hash (first 8 bytes) is sufficient for preventing collisions in your domain.
704-717
: Flow logs are straightforward.
New logs for flow types “FAST”, “STANDARD”, and “ATTESTATION” are fine for debugging. Just confirm final numbering for each flow type matches your enumeration.
726-741
: Log level expansions.
Outputs for “ERROR”, “WARNING”, etc. help in user debugging. Ensure consistent usage ofLOG
macros across tasks to prevent partial coverage.
852-855
: Use of BT MAC when connected to MQTT.
You’re consistently usingesp_read_mac(mac, ESP_MAC_BT)
. Confirm the broker or integrators expect the same ID in every scenario (Wi-Fi vs. Ethernet).
969-980
: New style MQTT client config.
This is a clearer structure-based approach. Confirm that the set last-will topic and message align with your broader status approach (locking, bridging, etc.).
1024-1024
: Version injection.
Inserting the application version into the served page is a neat approach. Just ensure the server updates if the version changes after an OTA.
1367-1376
: AP starting route.
Exposing a route (/start_config_ap
) to forcibly enter AP mode is convenient for provisioning. Ensure it’s secured behind authentication or an authorized user check.
1419-1430
: WebAuth enabling on multiple routes.
Applying basic auth toroutesHandle
,dataProvision
, etc. is good. Double-check that each route requiring secure access is indeed covered.
1442-1447
:wifiCallback
with MQTT start & web setup.
Invokesmqtt_app_start
only if the broker address is not0.0.0.0
. Good logic. Watch for concurrency if Wi-Fi resets repeatedly.
1659-1681
: EthernetonEvent
callback.
Logs for ETH start, IP acquisition, etc. are thorough. If the device might switch between Wi-Fi and Ethernet, ensure logic in other tasks can handle toggling states.
1765-1773
: Alt action pins initialization.
Ensure correct pull-up/down configurations if hardware demands it. Also confirm input-floating lines don’t cause false triggers.
1818-1819
: Reader ID log expansions.
As with other identifiers, double-check that printing these IDs in plain text is acceptable.
1828-1832
: Host name suffix from BT MAC.
Again, be sure the environment expects the host name derived from BT MAC rather than Wi-Fi or Ethernet MAC.
1862-1873
: Controller & connection callbacks.
You’re addingpairCallback
andwifiCallback
. Good. Just ensure errors in callback logic do not block HomeSpan’s standard flow.
1874-1886
: Task creations for NFC, GPIO, alt action.
Multiple tasks are spun up at startup. Confirm that stack sizes (4096, 8192, 2048) reflect real usage and memory constraints on your device..clangd (1)
2-4
: Compilation database and custom flags updated.
Using a local compilation database (build
) is a clean approach. Removing extraneous flags (e.g.-march=rv32imc_zicsr_zifencei
) and adding-ferror-limit=0
is reasonable, but verify they match your actual toolchain’s requirements.main/Kconfig.projbuild (1)
1-6
: New menu and config for USB-JTAG logging.
ARDUINO_USE_USB_JTAG
is clearly documented with a “bool” type. Confirm the dependency!IDF_TARGET_ESP32
doesn’t inadvertently exclude boards that do support JTAG-based serial logs..nvim.lua (2)
5-9
: Enhanced cross-compilation support.The query-driver configuration now properly supports both Xtensa and RISC-V architectures, which is essential for ESP32 development. This improvement enables better code analysis across different ESP32 variants.
11-12
: Improved code completion capabilities.The addition of
--import-insertions
and--all-scopes-completion
enhances the IDE experience with better code completion and automatic import management.main/include/NFC_SERV_CHARS.h (1)
9-15
: Well-structured service implementations.The LockManagement and NFCAccess services are well-organized:
- Proper inheritance from SpanService
- Clear initialization of required characteristics
- Appropriate service IDs and names
Also applies to: 16-23
sdkconfig.defaults (3)
10-29
: Optimized binary size through selective compilation.Disabling unused Arduino features helps reduce the binary size and memory footprint. The selective compilation flags are well-chosen for a HomeKey implementation.
32-34
: Enhanced Ethernet support.Added support for multiple Ethernet drivers (DM9051, W5500, KSZ8851SNL) provides flexibility in hardware selection.
41-42
:✅ Verification successful
Security and reliability improvements.
- Added HKDF support for key derivation
- MQTT reliability enhancement by skipping publish on disconnection
🌐 Web query:
What is HKDF (HMAC-based Key Derivation Function) and why is it important for security?
💡 Result:
HKDF (HMAC-based Key Derivation Function) is a cryptographic algorithm designed to securely derive cryptographic keys from an initial input keying material (IKM), such as shared secrets or random values[4][7]. It uses HMAC (Hash-based Message Authentication Code) in a two-step process called extract-then-expand[3][4], ensuring keys are both cryptographically strong and application-specific.
How HKDF Works
Extract Phase
Converts potentially non-uniform input keying material (e.g., Diffie-Hellman shared secrets) into a fixed-length pseudorandom key (PRK) using HMAC and an optional salt[4][7]:PRK = HMAC-Hash(salt, IKM)
The salt enhances entropy distribution but can be omitted if unavailable[7].
Expand Phase
Generates output keys of arbitrary length by repeatedly applying HMAC to the PRK with context-specificinfo
:OKM = HMAC-Hash(PRK, T1 || info || 0x01) || HMAC-Hash(PRK, T2 || info || 0x02) || ...
Here,
T
values chain previous HMAC outputs[4][7].Importance for Security
- Entropy Concentration: Safely processes non-uniform inputs (e.g., Diffie-Hellman outputs) into uniform cryptographic keys[3][5].
- Key Separation: Derives multiple independent keys from one secret using unique
info
values per application (e.g., encryption vs authentication keys)[4][8].- Collision Resistance: Built on HMAC’s security properties to prevent key correlation or leakage[1][5].
- Standardization: Widely adopted in protocols like QUIC TLS handshakes[1], exposure notification systems[1], IKEv2[3], and EAP-AKA[3].
Use Cases
- Exposure Notification Systems: Generates Rolling Proximity Identifiers (RPIs) without revealing user identities[1].
- QUIC Protocol: Derives initial TLS handshake secrets from connection IDs[1].
- Hybrid Encryption: Converts asymmetric encryption results into symmetric keys[7].
Limitations
- Not for Low-Entropy Inputs: Avoid using with passwords; instead use Argon2 or PBKDF2[5][6].
- No Built-In Slowdown: Unlike password-based KDFs, HKDF lacks mechanisms to deter brute-force attacks on weak inputs[3][5].
HKDF’s design ensures cryptographic agility while maintaining simplicity and formal security proofs[2][3]. Its standardized approach reduces implementation errors compared to ad-hoc KDFs[5][7].
Citations:
- 1: https://www.nearform.com/insights/getting-to-know-hkdf
- 2: https://eprint.iacr.org/2010/264.pdf
- 3: https://www.rfc-editor.org/rfc/rfc5869.txt
- 4: https://en.wikipedia.org/wiki/HKDF
- 5: https://blog.trailofbits.com/2025/01/28/best-practices-for-key-derivation/
- 6: https://www.comparitech.com/blog/information-security/key-derivation-function-kdf/
- 7: https://datatracker.ietf.org/doc/html/rfc5869
- 8: https://cendyne.dev/posts/2023-01-30-how-to-use-hkdf.html
- 9: https://www.ibm.com/docs/en/sdk-java-technology/8?topic=iip-hmac-based-extract-then-expand-key-derivation-function-hkdf
- 10: https://pkg.go.dev/golang.org/x/crypto/hkdf
Config Validation: HKDF and MQTT Enhancements Verified
The configuration changes in sdkconfig.defaults (lines 41–42) are confirmed to improve security and reliability. Enabling
CONFIG_MBEDTLS_HKDF_C=y
activates HKDF support, which securely derives cryptographic keys through an HMAC-based extract-then-expand process—ensuring robust key derivation even from non-uniform keying material. Additionally, settingCONFIG_MQTT_SKIP_PUBLISH_IF_DISCONNECTED=y
enhances MQTT reliability by preventing publishing attempts when disconnected, thereby reducing potential errors.No further modifications are required.
main/include/config.h (1)
64-64
: Well-documented MQTT topic addition.The new alternative action topic is properly documented with a clear comment explaining its purpose.
main/idf_component.yml (1)
1-5
: Dependency Specification: Clear and InformativeThe dependencies block cleanly defines external libraries with precise version constraints. This improves reproducibility and ensures compatibility during builds. Please verify that these version constraints are aligned with the requirements of your other components.
.gitignore (1)
11-14
: Updated Ignore PatternsThe updated ignore patterns now correctly exclude files and directories like all
sdkconfig*
files,dependencies.lock
,build
, andmanaged_components
. This streamlines your repository by ensuring that generated or transient files are not accidentally committed. Double-check that these patterns do not inadvertently exclude any important configuration files needed for local development.main/CMakeLists.txt (2)
1-3
: Component Registration VerificationThe addition of
idf_component_register
with explicit source, include directory, and dependency requirements is well structured. Ensure that all the listed dependencies (HomeSpan, PN532, HK-HomeKit-Lib, ESPAsyncWebServer, mqtt, libsodium) are correctly integrated in your directory structure so that the build system finds them without issues.
4-4
: LittleFS Partition Image CreationThe command
littlefs_create_partition_image(spiffs ../data FLASH_IN_PROJECT)
is clearly added to manage file system partitions. Please verify that the relative path (../data
) is correct in relation to your build directory and that it points to the intended source of the filesystem image.CMakeLists.txt (3)
1-1
: CMake Version UpdateThe minimum required CMake version has been updated to 3.16. Confirm that your build environments all support this version to avoid any compatibility issues.
3-3
: Project DeclarationThe project is correctly declared as "HomeKey-ESP32". Ensure that any references or configurations depending on the project name are updated accordingly in associated scripts or documentation.
4-8
: Conditional Compilation FlagsThe conditional block checking for non-esp32 targets with USB/JTAG enabled is well implemented. The compile options (
-DARDUINO_USB_CDC_ON_BOOT
and-DARDUINO_USB_MODE
) are appended only under the specified conditions. Please verify that this behavior meets your multi-target build requirements and that all potential target configurations are handled appropriately.components/HomeSpan/CMakeLists.txt (2)
3-5
: Component Registration for HomeSpanThe component registration via
idf_component_register
is clear and integrates multiple include directories and required dependencies (arduino-esp32, libsodium, app_update, nvs_flash). Verify that these dependencies are consistently defined across your project, especially if they are used in multiple places.
6-6
: Suppressing Format WarningsThe compile options that disable specific format warnings
(-Wno-error=format= -Wno-format)
should help avoid build failures due to non-critical format issues. Just ensure that suppressing these warnings will not mask underlying issues in formatting that might be important in the long term..zed/settings.json (1)
1-17
: Zed Editor Configuration Looks Good
The JSON structure clearly defines folder-specific settings for the Zed editor. The"load_direnv": "direct"
property and the LSP configuration—with customclangd
binary arguments containing wildcard paths for both Xtensa and RISC-V toolchains—are set up correctly..vscode/settings.json (2)
2-12
: Enhanced C/C++ Settings and File Associations
Introducing"C_Cpp.intelliSenseEngine": "default"
improves IntelliSense reliability, and the expanded file associations for various C++ headers ensure better language support. Verify that these new associations do not conflict with project-specific header conventions.
83-94
: Robust ESP-IDF and Clangd Setup
The added settings for ESP-IDF—such as"idf.flashType": "UART"
,"idf.port": "/dev/ttyACM0"
, and"idf.openOcdConfigs": ["board/esp32s3-builtin.cfg"]
—coupled with the customized Clangd path and arguments, form a cohesive configuration that should boost development productivity..github/workflows/esp32.yml (6)
8-12
: Optimized Workflow Trigger Settings
The newpaths-ignore
directive (excluding.vscode/**
,.gitignore
,LICENSE
, andREADME.md
) during pull requests is a sensible approach to avoid unnecessary builds.
13-14
: Manual Workflow Triggering Enabled
Adding theworkflow_dispatch
trigger allows manual initiation of workflows, which is beneficial for debugging and on-demand builds.
16-55
: ESP32 Build Job Configuration is Solid
The ESP32 job steps—using recursive submodule checkout, strategic caching (including Docker image caching), and clear artifact archiving—are well structured. Ensure that the cache keys (e.g.,runner.os
concatenated with target info) remain unique and consistent.
55-93
: Consistent ESP32C3 Build Job
The ESP32C3 build job mirrors the ESP32 configuration effectively with appropriate keys and target-specific settings. Just double-check that the naming of artifacts (esp32c3-firmware
andesp32c3-firmware-merged
) meets your deployment expectations.
94-132
: Well-Structured ESP32S3 Build Job
The ESP32S3 build steps are consistent with the other target jobs. Verifying the target specifications and artifact paths is recommended to ensure smooth integration into your build pipeline.
133-151
: New LittleFS Job for Filesystem Image Generation
The LittleFS job, which performs a sparse checkout on thedata
directory, installs the necessary Python package, creates the LittleFS image, and archives it properly, is an excellent addition.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 151-151: no new line character at the end of file
(new-line-at-end-of-file)
data/assets/misc.css (17)
1-5
: Global Styling Definitions Are Clear
The:root
block establishes a consistent background color and image pattern. The styles are clear and well-organized.
19-22
: Refined Heading Style for h5
The newh5
styling—with a specific RGB color and the use of!important
—improves readability. Ensure that this override does not unintentionally affect other h5 elements globally.
66-70
: Effective.tabs-list
Layout
The flex layout withjustify-content: space-around
andoverflow: auto
manages tab alignment and scrolling well.
72-74
: Consistent Input Field Sizing
Setting a max-width oninput[type="number"]
ensures that these inputs remain visually balanced with other elements.
76-78
: Proper Margin for.tabs-container
Adding a bottom margin creates adequate spacing between the tab controls and subsequent content.
80-86
: Uniform Padding viadiv[class$="-selected-body"]
Replacing the old padding withpadding-inline
andpadding-block
ensures more consistent spacing around the selected body content.
88-95
: Enhanced Selected Tab Visuals
The style updates—including the new background gradient, border-radius, box-shadow, and bold font—clearly differentiate active tabs.
97-103
: Clickable Tab Button Styling is Appropriate
.tab-btn
now has definitions for margin, dynamic width (fit-content
), and proper cursor indication which are well suited for interactive elements.
109-113
: Useful Flex Layout with.flex-col-lg
The introduced.flex-col-lg
ensures consistent column alignment with a standardized gap, aiding in responsive design layout.
115-119
: Clear Row Layout with.flex-row-lg
Utilizing.flex-row-lg
for row arrangements, with appropriate gap configuration, supports a clean design.
121-124
: Balanced.input-group
Styling
The.input-group
class facilitates a neat horizontal alignment of form labels and inputs.
126-142
: Thorough Mobile Responsive Adjustments
The media query for screens with a max-width of 600px adjusts elements like#top-bar
,#top-btns
, and various flex containers effectively to accommodate smaller displays.
165-173
: Subtle Hover and Active Effects for Buttons
The hover and active state modifications for buttons (and specifically#restart-btn
and.selected-btn
) enhance interactivity while maintaining the overall design language.
213-217
: Large Screen Centering with.flex-center-lg
The minimal styling for.flex-center-lg
effectively ensures centered alignment on larger displays.
219-228
: Improved.card-content
Styling
Replacing the old.fill-container
with.card-content
provides a modernized look—using borders, border-radius, padding, and a subtle box-shadow—to better present card-like elements.
230-233
: Select Element Refinement
The style update forselect
elements, including max-width and text alignment, should result in a more polished appearance across browsers.
235-269
: Consistent Styling for Buttons, Loader, and Animations
The remaining style rules—including those for buttons (with precise padding and border settings), the loader animation, and keyframes—are clear and contribute to a consistent theming across the application.data/routes/mqtt.html (11)
3-3
: Improved Form Identification and Semantic Markup
Changing the form’s id toconfig-form
and adding the attributedata-type="mqtt"
enhances clarity and ensures that form handling logic can more easily differentiate between multiple forms if present.
12-14
: MQTT Broker Field Updated
The broker address input now uses the camelCase namemqttBroker
, which aligns with modern naming conventions and improves consistency across form fields.
16-17
: MQTT Port Field Naming Consistency
Renaming the port field tomqttPort
while maintaining proper validation properties (e.g.,min="0"
,max="65535"
) ensures robust input handling.
20-23
: Standardized MQTT Client ID Field
The update tomqttClientId
for the client ID field contributes to a cohesive naming strategy across the MQTT configuration form.
25-26
: LWT Topic Field Naming Updated
Switching the LWT Topic field’s name tolwtTopic
further reinforces the use of camelCase across input names. Ensure that this change is mirrored in any backend processing.
31-37
: Credential Fields Now Use CamelCase
The renaming of the username and password fields tomqttUsername
andmqttPassword
streamlines form data consistency and minimizes confusion during data handling.
39-45
: Refined HASS MQTT Discovery Section
Employing an.input-group
to align the HASS MQTT Discovery label and select field enhances visual coherence. Confirm that the processing logic takes the new field names into account.
59-93
: MQTT Topics Section Now Follows CamelCase Standards
Field names such ashkTopic
,lockStateTopic
,lockStateCmd
,lockCStateCmd
,lockTStateCmd
, andbtrLvlCmdTopic
maintain a consistent naming convention. Ensure corresponding backend mappings are updated to reflect these new identifiers.
120-126
: Custom Lock Actions Naming Revision
The custom lock actions use names with an exclamation mark (e.g.,customLockActions!UNLOCK
andcustomLockActions!LOCK
). While these names are now distinct, please verify that the form-processing backend accepts special characters like “!” in input names.
130-156
: Custom Lock States Naming Revised
Similarly, naming custom lock state fields (e.g.,customLockStates!C_UNLOCKING
,customLockStates!C_LOCKING
, etc.) with an exclamation mark is unconventional and might pose issues during parsing. Confirm that your server-side logic or frontend framework properly handles such characters.
163-168
: Submission Controls are Well Configured
The submit and reset buttons are configured consistently with previous styling guidelines, which should offer a reliable user experience during form submissions.data/routes/actions.html (13)
1-1
: Form Element Update: Updated Form ID and Data Attribute
The form element now usesid="config-form"
and includes the attributedata-type="actions"
. This change standardizes the form naming across routes for easier selection and processing.
5-8
: Section Header and Container Structuring Enhancement
The card container and section headings (lines 5–8) have been restructured and now include exclamation marks in the descriptive text. This improves visual emphasis and provides clearer guidance to the user.
19-24
: Neopixel Input Field Naming Conventions
The input field for the GPIO pin (e.g.,<input ... name="nfcNeopixelPin" ...>
) is now renamed using camelCase. This update ensures consistency with the new naming standard applied across the codebase.
25-34
: Neopixel Pixel Type Selection Updated
The select element for pixel type now uses the nameneoPixelType
, which aligns with the revised naming conventions. This change simplifies integration with the underlying configuration logic.
38-45
: Timeout Fields Renamed for Clarity
The timeout input fields have been renamed toneopixelSuccessTime
andneopixelFailTime
for successful and failed authentications respectively. This renaming improves clarity and makes the code self-documenting.
54-64
: Auth Success Color Input Structuring
The color inputs for authentication success now use structured IDs (e.g.,neopixelSuccessColor!0!1
). This enhances the handling of multi-channel color values in a systematic way.
69-82
: Auth Failure Color Input Structuring
Similarly, the color inputs for authentication failure are updated to structured identifiers (e.g.,neopixelFailureColor!0!1
) to maintain consistency in handling color arrays.
87-104
: Simple GPIO Auth Success Section Update
The input fields for the "Auth Success" event are now updated with new names:nfcSuccessPin
,nfcSuccessTime
, andnfcSuccessHL
. These changes align with the new camelCase conventions and improve consistency for form data processing.
111-131
: Simple GPIO Auth Failure Section Update
The "Auth Failure" section now uses updated identifiers—nfcFailPin
,nfcFailTime
, andnfcFailHL
—which align with the standardized naming conventions. Ensure that all related processing logic is updated accordingly.
133-154
: Alternate Action Configuration Updated
The alternate action section has been revised with new names such ashkAltActionPin
,hkAltActionTimeout
, andhkAltActionGpioState
. This enhances consistency and readability across similar configuration sections.
158-168
: HomeKit Triggers Section Enhancement
In the HomeKit triggers card, updated text (with added exclamation marks) and input field names (e.g.,gpioActionPin
,gpioActionLockState
,gpioActionUnlockState
,hkGpioControlledState
,gpioActionMomentaryEnabled
, andgpioActionMomentaryTimeout
) improve clarity and adhere to the new naming scheme.
216-242
: Dummy Action Configuration for HomeKit
The dummy configuration tab reuses momentary state fields. While this re-use may be intentional (to mirror the "Always Lock/Unlock" option), please verify that the duplicate usage of fields likegpioActionMomentaryEnabled
andgpioActionMomentaryTimeout
does not lead to data conflicts.
247-250
: Form Submission Buttons and Closure Validation
The submit and reset buttons are positioned at the end of the form. Ensure that their event handling integrates with the updated centralized JavaScript logic inindex.html
.data/index.html (20)
8-9
: Metadata Update: Favicon and Title
The favicon link has been updated to use a WEBP image (assets/favicon.webp
), and the document title has been changed to "HK-ESP32". Both changes enhance performance and branding consistency.
11-12
: Initialization of Environment Variables
The script now declares aconst dev = false;
and initializes an emptyethConfig
object. These variables lay the groundwork for handling development flags and Ethernet configuration data uniformly.
13-24
: Popstate Event Listener for Navigation Management
The new popstate listener (lines 13–24) updates the displayed component based on the URL hash. This centralizes browser navigation handling and ensures that the UI state tracks history changes properly.
25-38
: DOMContentLoaded Listener for Initial Setup and Ethernet Check
The DOMContentLoaded event listener fetches Ethernet configuration and initializes the UI accordingly. The conditional logic for updating the WiFi signal or displaying the Ethernet status is clear and well-integrated.
53-57
: switchTab Function Minor Adjustment
The switchTab function now resets the previously selected tab’s class before adding the new selection. This small update will help avoid stale UI states when switching tabs.
60-67
: addComponent Function Signature and Cleanup Enhancements
The signature ofaddComponent
has been updated toasync function addComponent(button, fromHistory)
, ensuring the first parameter is the button element. The function now removes any existing submit event listener from the form before reassigning it, which prevents duplicate bindings.
83-90
: HTML Fragment Caching Mechanism Implementation
The code now checkssessionStorage
for a cached HTML fragment (using the keyhtmlCache-${name}
) before fetching it. This approach minimizes network requests on subsequent loads and is well encapsulated in thedev
flag condition.
90-98
: Dynamic Component Configuration Loading
After caching, the function fetches component-specific configuration data and updates the component’s inputs accordingly (e.g., updating issuer and endpoint details for"hkinfo"
). This dynamic injection supports a flexible and modular UI.
129-129
: Ethernet Preset Initialization for Misc Component
For the"misc"
component, the function callshandleEthPreset
to initialize Ethernet settings. This automatic update ensures that the form reflects the current hardware configuration.
192-195
: Rebinding Form Submission Handler
After appending the new component, the code rebinds the form’s submit event tohandleForm
. This update ensures that form submissions trigger the appropriate handler with the updated form structure.
196-199
: Browser History Management and Title Update
When a component is loaded (and not via history), the function pushes a new state to history and updates the document title to include the component name. This change improves navigation clarity and browser history tracking.
201-207
: New Function: start_ap for Access Point Activation
The new asynchronous functionstart_ap()
confirms with the user before issuing a fetch request to/start_config_ap
. This update adds an important utility for starting the configuration AP with appropriate user confirmation.
229-233
: Utility Function: isNumeric
The newly addedisNumeric
function reliably checks whether a given input is numeric. Its implementation is straightforward and enhances form value validation downstream.
234-276
: Form Submission Handler: handleForm
ThehandleForm
function prevents the default submission, converts form data (including specially formatted keys using delimiters like "!") into a JSON object, and submits it via a POST request. The meticulous conversion—especially for numeric values and array/object structures—demonstrates a robust approach to handling complex form data.
277-289
: Ethernet Preset Handler: handleEthPreset
ThehandleEthPreset
function enables or disables Ethernet configuration inputs depending on the selected preset value. This dynamic update improves the user experience by preventing invalid input when using a preset, and it properly cascades changes to chip configuration withhandleEthChipChange
.
290-322
: Ethernet Chip Change Handler: handleEthChipChange
This function adjusts the displayed input fields based on whether the Ethernet chip supports EMAC. It smartly splits the configuration between RMII and SPI modes and disables irrelevant fields. The clear delineation improves overall form robustness and prevents user error.
328-343
: Top-Bar Button Enhancements (Start AP)
The Start AP button (lines 328–343) now features enhanced styling with touch feedback. The use of inline styles for highlighting on touch events improves the responsiveness of the button across devices.
351-357
: Top-Bar Button Enhancements (Reset HomeKit)
The Reset HomeKit pairings button is updated with consistent styling and touch feedback. These visual improvements provide a more cohesive user experience among the top-bar control buttons.
358-364
: Top-Bar Button Enhancements (Reset WiFi)
Similarly, the Reset WiFi credentials button now benefits from updated styling and touch event adjustments, ensuring consistency with other action buttons on the top bar.
369-373
: Component Selector Button Updates
The buttons used to select components (e.g., "HK Info", "MQTT", "Actions", "Misc") are updated with padding and cursor styling. This ensures a uniform look and clear affordance for interactive elements in the main component area.data/routes/misc.html (10)
3-3
: Form Identification and Data Attribute Update
The form now usesid="config-form"
and has the attributedata-type="misc"
, aligning it with the updated naming conventions across other routes. This change simplifies global form selection and processing.
5-7
: General Settings Container and Layout Enhancement
The card content container for general settings has been adjusted (including inline styling such asmax-width: 26rem;
). Additionally, grouping elements inside flex containers improves the layout and readability of the form elements.
8-26
: General Settings Input Updates
Input fields for general settings (e.g.,deviceName
,setupCode
,lockAlwaysLock
,lockAlwaysUnlock
,proxBatEnabled
, andbtrLowStatusThreshold
) now follow camelCase naming conventions. The labels and placeholder values have been updated to enhance clarity and consistency.
53-70
: Alternate Action Initiator Button Configuration
Within the custom tabs for the HomeKit section, the fieldset for the alternate action initiator button now uses updated input names and IDs (hkAltActionInitPin
,hkAltActionInitTimeout
,hkAltActionInitLedPin
). The refined layout inside a flex column improves user clarity.
72-107
: HomeKey Card Finish Configuration Update
The "HomeKey Card Finish" section now employs radio buttons with corresponding labels that update the background image of a preview element (hkfinish
). This dynamic visual feedback greatly enhances usability and demonstrates an interactive configuration option.
108-130
: PN532 Configuration with Structured Pin Naming
The PN532 section employs input fields with names that include the "!" delimiter (e.g.,nfcGpioPins!0
,nfcGpioPins!1
, etc.). This structured naming allows for easier processing of array-like data and maintains consistency with similar patterns in other forms.
132-148
: HomeSpan Configuration Inputs Update
The HomeSpan section has been streamlined with updated input fields (otaPasswd
,controlPin
,hsStatusPin
). The removal of extraneous styling and the focus on essential inputs improve overall clarity.
149-237
: Ethernet Configuration Overhaul
The Ethernet configuration section introduces a dropdown for status (ethernetEnabled
), board presets (ethActivePreset
), and Ethernet chip selection (ethPhyType
), along with dedicated containers for SPI and RMII configurations. The inputs are initially disabled and are enabled conditionally via JavaScript. This modular structure should reduce user errors and simplify maintenance.
239-268
: WebUI Configuration Section Refinement
The WebUI card content is updated with inputs for authentication (webAuthEnabled
,webUsername
,webPassword
) using styling that fits the content. This section now clearly distinguishes between the main configuration and the authentication controls.
272-280
: Submission Buttons Styling and Layout
The Submit and Reset buttons at the bottom of the form have updated padding and cursor styles, ensuring that the form controls are both visually consistent and user-friendly.
|
||
KeyFlow hkFlow = KeyFlow::kFlowFAST; | ||
bool hkAltActionActive = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Check concurrency impacts of hkAltActionActive
.
A global boolean might become a concurrency bottleneck if accessed by multiple tasks. Consider using atomic operations or a mutex if multiple tasks read/write this flag.
} else if (!strcmp(p->name().c_str(), "cstate-locked")) { | ||
espConfig::mqttData.customLockStates["C_LOCKED"] = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "cstate-unlocked")) { | ||
espConfig::mqttData.customLockStates["C_UNLOCKED"] = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "cstate-jammed")) { | ||
espConfig::mqttData.customLockStates["C_JAMMED"] = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "cstate-unknown")) { | ||
espConfig::mqttData.customLockStates["C_UNKNOWN"] = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "nfc-tags-ignore-mqtt")) { | ||
espConfig::mqttData.nfcTagNoPublish = p->value().toInt(); | ||
if (espConfig::mqttData.nfcTagNoPublish) { | ||
std::string rfidTopic; | ||
rfidTopic.append("homeassistant/tag/").append(espConfig::mqttData.mqttClientId).append("/rfid/config"); | ||
esp_mqtt_client_publish(client, rfidTopic.c_str(), "", 0, 0, false); | ||
if (inputData.contains("unique_identifier")) { | ||
serializedData["unique_identifier"] = red_log::bufToHexString(readerData.reader_id.data(), readerData.reader_id.size(), true); | ||
} | ||
if (inputData.contains("issuers")) { | ||
serializedData["issuers"] = json::array(); | ||
for (auto it = inputData.at("issuers").begin(); it != inputData.at("issuers").end(); ++it) | ||
{ | ||
json issuer; | ||
if (it.value().contains("issuerId")) { | ||
std::vector<uint8_t> id = it.value().at("issuerId").get<std::vector<uint8_t>>(); | ||
issuer["issuerId"] = red_log::bufToHexString(id.data(), id.size(), true); | ||
} | ||
if (it.value().contains("endpoints") && it.value().at("endpoints").size() > 0) { | ||
issuer["endpoints"] = json::array(); | ||
for (auto it2 = it.value().at("endpoints").begin(); it2 != it.value().at("endpoints").end(); ++it2) { | ||
json endpoint; | ||
if (it2.value().contains("endpointId")) { | ||
std::vector<uint8_t> id = it2.value().at("endpointId").get<std::vector<uint8_t>>(); | ||
endpoint["endpointId"] = red_log::bufToHexString(id.data(), id.size(), true); | ||
} | ||
issuer["endpoints"].push_back(endpoint); | ||
} | ||
} | ||
serializedData["issuers"].push_back(issuer); | ||
} | ||
} | ||
} else if (!strcmp(p->name().c_str(), "mqtt-btrprox-cmd-topic")) { | ||
espConfig::mqttData.btrLvlCmdTopic = p->value().c_str(); | ||
} else { | ||
req->send(400); | ||
return; | ||
} | ||
if (!serializedData.empty()) { | ||
req->send(200, "application/json", serializedData.dump().c_str()); | ||
} else { | ||
req->send(500); | ||
} | ||
} else req->send(500); | ||
}); | ||
AsyncCallbackWebHandler* ethSuppportConfig = new AsyncCallbackWebHandler(); | ||
webServer.addHandler(ethSuppportConfig); | ||
ethSuppportConfig->setUri("/eth_get_config"); | ||
ethSuppportConfig->setMethod(HTTP_GET); | ||
ethSuppportConfig->onRequest([](AsyncWebServerRequest *req) { | ||
json eth_config; | ||
eth_config["supportedChips"] = json::array(); | ||
for (auto &&v : eth_config_ns::supportedChips) { | ||
eth_config.at("supportedChips").push_back(v.second); | ||
} | ||
eth_config["boardPresets"] = eth_config_ns::boardPresets; | ||
eth_config["ethEnabled"] = espConfig::miscConfig.ethernetEnabled; | ||
req->send(200, "application/json", eth_config.dump().c_str()); | ||
}); | ||
AsyncCallbackWebHandler* dataClear = new AsyncCallbackWebHandler(); | ||
webServer.addHandler(dataClear); | ||
dataClear->setUri("/config/clear"); | ||
dataClear->setMethod(HTTP_POST); | ||
dataClear->onRequest([](AsyncWebServerRequest* req) { | ||
if (req->hasParam("type")) { | ||
AsyncWebParameter* data = req->getParam(0); | ||
std::array<std::string, 3> pages = { "mqtt", "actions", "misc" }; | ||
if (std::equal(data->value().begin(), data->value().end(), pages[0].begin(), pages[0].end())) { | ||
LOG(D, "MQTT CONFIG SEL"); | ||
nvs_erase_key(savedData, "MQTTDATA"); | ||
espConfig::mqttData = {}; | ||
req->send(200, "text/plain", "200 Success"); | ||
} else if (std::equal(data->value().begin(), data->value().end(), pages[1].begin(), pages[1].end())) { | ||
LOG(D, "ACTIONS CONFIG SEL"); | ||
nvs_erase_key(savedData, "MISCDATA"); | ||
espConfig::miscConfig = {}; | ||
req->send(200, "text/plain", "200 Success"); | ||
} else if (std::equal(data->value().begin(), data->value().end(), pages[2].begin(), pages[2].end())) { | ||
LOG(D, "MISC CONFIG SEL"); | ||
nvs_erase_key(savedData, "MISCDATA"); | ||
espConfig::miscConfig = {}; | ||
req->send(200, "text/plain", "200 Success"); | ||
} else { | ||
req->send(400); | ||
return; | ||
} | ||
} else { | ||
req->send(400); | ||
return; | ||
} | ||
}); | ||
AsyncCallbackWebHandler* dataLoad = new AsyncCallbackWebHandler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Web server configuration – GET routes.
Defines new endpoints for retrieving or clearing device config, HK data, or board presets. Validate user input carefully, especially if these routes are accessible outside a trusted LAN.
rebootMsg = "Pixel Type was changed, reboot needed! Rebooting..."; | ||
} | ||
} else if (it.key() == std::string("gpioActionPin")) { | ||
if (espConfig::miscConfig.gpioActionPin == 255 && it.value() != 255 ) { | ||
LOG(D, "ENABLING HomeKit Trigger - Simple GPIO"); | ||
pinMode(it.value(), OUTPUT); | ||
if(gpio_lock_task_handle == nullptr){ | ||
xTaskCreate(gpio_task, "gpio_task", 4096, NULL, 2, &gpio_lock_task_handle); | ||
} | ||
if(espConfig::miscConfig.hkDumbSwitchMode){ | ||
serializedData->at("hkDumbSwitchMode") = false; | ||
} | ||
} else if (espConfig::miscConfig.gpioActionPin != 255 && it.value() == 255) { | ||
LOG(D, "DISABLING HomeKit Trigger - Simple GPIO"); | ||
if( gpio_lock_task_handle != nullptr){ | ||
gpioLockAction status{ .source = gpioLockAction::OTHER, .action = 2 }; | ||
xQueueSend(gpio_lock_handle, &status, 0); | ||
gpio_lock_task_handle = nullptr; | ||
} | ||
gpio_reset_pin(gpio_num_t(espConfig::miscConfig.gpioActionPin)); | ||
} | ||
} else if(p->value().toInt() != 255) { | ||
pinMode(p->value().toInt(), OUTPUT); | ||
} else if (it.key() == std::string("hkDumbSwitchMode") && gpio_lock_task_handle == nullptr) { | ||
xTaskCreate(gpio_task, "gpio_task", 4096, NULL, 2, &gpio_lock_task_handle); | ||
} | ||
espConfig::miscConfig.nfcFailPin = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "nfc-s-hl")) { | ||
espConfig::miscConfig.nfcSuccessHL = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "nfc-f-hl")) { | ||
espConfig::miscConfig.nfcFailHL = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "nfc-s-time")) { | ||
espConfig::miscConfig.nfcSuccessTime = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "nfc-f-time")) { | ||
espConfig::miscConfig.nfcFailTime = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "gpio-a-pin")) { | ||
if (!GPIO_IS_VALID_GPIO(p->value().toInt()) && !GPIO_IS_VALID_OUTPUT_GPIO(p->value().toInt()) && p->value().toInt() != 255) { | ||
std::string msg = p->value().c_str(); | ||
msg.append(" is not a valid GPIO Pin"); | ||
request->send(200, "text/plain", msg.c_str()); | ||
return; | ||
configData.at(it.key()) = it.value(); | ||
} | ||
std::vector<uint8_t> vectorData = json::to_msgpack(configData); | ||
esp_err_t set_nvs = nvs_set_blob(savedData, selConfig == 0 ? "MQTTDATA" : "MISCDATA", vectorData.data(), vectorData.size()); | ||
esp_err_t commit_nvs = nvs_commit(savedData); | ||
LOG(D, "SET_STATUS: %s", esp_err_to_name(set_nvs)); | ||
LOG(D, "COMMIT_STATUS: %s", esp_err_to_name(commit_nvs)); | ||
if (set_nvs == ESP_OK && commit_nvs == ESP_OK) { | ||
LOG(I, "Config successfully saved to NVS"); | ||
if (selConfig == 0) { | ||
configData.get_to<espConfig::mqttConfig_t>(espConfig::mqttData); | ||
} else { | ||
configData.get_to<espConfig::misc_config_t>(espConfig::miscConfig); | ||
} | ||
if (espConfig::miscConfig.gpioActionPin == 255 && p->value().toInt() != 255 && gpio_lock_task_handle == nullptr) { | ||
pinMode(p->value().toInt(), OUTPUT); | ||
xTaskCreate(gpio_task, "gpio_task", 4096, NULL, 2, &gpio_lock_task_handle); | ||
} else if (espConfig::miscConfig.gpioActionPin != 255 && p->value().toInt() == 255 && gpio_lock_task_handle != nullptr) { | ||
gpioLockAction status{ .source = gpioLockAction::OTHER, .action = 2 }; | ||
xQueueSend(gpio_lock_handle, &status, 0); | ||
gpio_lock_task_handle = nullptr; | ||
} else { | ||
LOG(E, "Something went wrong, could not save to NVS"); | ||
} | ||
if (selConfig == 0 || selConfig == 2) { | ||
req->send(200, "text/plain", "Saved! Restarting..."); | ||
vTaskDelay(1000 / portTICK_PERIOD_MS); | ||
ESP.restart(); | ||
} else { | ||
if(rebootNeeded){ | ||
req->send(200, "text/plain", rebootMsg.c_str()); | ||
} else { | ||
req->send(200, "text/plain", "Saved and applied!"); | ||
} | ||
espConfig::miscConfig.gpioActionPin = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "gpio-a-lock")) { | ||
espConfig::miscConfig.gpioActionLockState = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "homekey-gpio-state")) { | ||
espConfig::miscConfig.hkGpioControlledState = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "gpio-a-unlock")) { | ||
espConfig::miscConfig.gpioActionUnlockState = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "gpio-a-momentary")) { | ||
espConfig::miscConfig.gpioActionMomentaryEnabled = p->value().toInt(); | ||
} else if (!strcmp(p->name().c_str(), "gpio-a-mo-timeout")) { | ||
espConfig::miscConfig.gpioActionMomentaryTimeout = p->value().toInt(); | ||
} | ||
} | ||
json json_misc_config = espConfig::miscConfig; | ||
std::vector<uint8_t> misc_buf = nlohmann::json::to_msgpack(json_misc_config); | ||
esp_err_t set_nvs = nvs_set_blob(savedData, "MISCDATA", misc_buf.data(), misc_buf.size()); | ||
esp_err_t commit_nvs = nvs_commit(savedData); | ||
LOG(V, "SET_STATUS: %s", esp_err_to_name(set_nvs)); | ||
LOG(V, "COMMIT_STATUS: %s", esp_err_to_name(commit_nvs)); | ||
|
||
request->send(200, "text/plain", "Configuration applied!"); | ||
}); | ||
webServer.addHandler(actionsConfigHandle); | ||
}); | ||
auto rebootDeviceHandle = new AsyncCallbackWebHandler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Web server configuration – POST to save config.
Complex logic merges user inputs into device config with runtime checks. Confirm that each break condition properly returns an HTTP response. Also confirm concurrency handling if multiple config writes occur simultaneously.
void nfc_retry(void* arg) { | ||
ESP_LOGI(TAG, "Starting reconnecting PN532"); | ||
while (1) { | ||
nfc->begin(); | ||
uint32_t versiondata = nfc->getFirmwareVersion(); | ||
if (!versiondata) { | ||
ESP_LOGE("NFC_SETUP", "Error establishing PN532 connection"); | ||
} else { | ||
unsigned int model = (versiondata >> 24) & 0xFF; | ||
ESP_LOGI("NFC_SETUP", "Found chip PN5%x", model); | ||
int maj = (versiondata >> 16) & 0xFF; | ||
int min = (versiondata >> 8) & 0xFF; | ||
ESP_LOGI("NFC_SETUP", "Firmware ver. %d.%d", maj, min); | ||
nfc->SAMConfig(); | ||
nfc->setRFField(0x02, 0x01); | ||
nfc->setPassiveActivationRetries(0); | ||
ESP_LOGI("NFC_SETUP", "Waiting for an ISO14443A card"); | ||
vTaskResume(nfc_poll_task); | ||
vTaskDelete(NULL); | ||
return; | ||
} | ||
nfc->stop(); | ||
vTaskDelay(50 / portTICK_PERIOD_MS); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
nfc_retry
function.
Continuous while loop tries to re-init PN532, then reactivates the poll task. If repeated errors occur, ensure you don’t spin forever or starve other tasks.
require("lspconfig").clangd.setup({ | ||
capabilities = require("cmp_nvim_lsp").default_capabilities(vim.lsp.protocol.make_client_capabilities()), | ||
cmd = { | ||
os.getenv("HOME") .. "/esp-clang/bin/clangd", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the clangd binary path.
The path has been moved from "Downloads/esp-clang" to "esp-clang", which is a more standard location. Ensure this path matches your development environment setup.
🏁 Script executed:
#!/bin/bash
# Check if the clangd binary exists in the new location
test -f "$HOME/esp-clang/bin/clangd" && echo "clangd binary found" || echo "clangd binary not found"
Length of output: 102
Action Required: Update the clangd binary path
- The configuration in
.nvim.lua
now points to:which was intended to standardize the installation location.os.getenv("HOME") .. "/esp-clang/bin/clangd",
- However, the verification script confirmed that the clangd binary isn't found at this new location.
- Please verify whether the clangd binary is installed in the expected location. If not, update the configuration to either reflect the correct path or ensure that clangd is installed at
$HOME/esp-clang/bin/clangd
. - The additional query-driver and clangd option updates appear fine and support ESP32 development.
main/include/NFC_SERV_CHARS.h
Outdated
CUSTOM_CHAR(HardwareFinish, 264, PR, TLV_ENC, NULL_TLV, NULL_TLV, NULL_TLV, true) | ||
CUSTOM_CHAR(NFCAccessControlPoint, 264, PR+PW+WR, TLV_ENC, NULL_TLV, NULL_TLV, NULL_TLV, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix ID conflict between custom characters.
HardwareFinish and NFCAccessControlPoint both use ID 264, which could cause conflicts. Each characteristic must have a unique identifier.
Apply this diff to resolve the conflict:
-CUSTOM_CHAR(HardwareFinish, 264, PR, TLV_ENC, NULL_TLV, NULL_TLV, NULL_TLV, true)
+CUSTOM_CHAR(HardwareFinish, 266, PR, TLV_ENC, NULL_TLV, NULL_TLV, NULL_TLV, true)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
CUSTOM_CHAR(HardwareFinish, 264, PR, TLV_ENC, NULL_TLV, NULL_TLV, NULL_TLV, true) | |
CUSTOM_CHAR(NFCAccessControlPoint, 264, PR+PW+WR, TLV_ENC, NULL_TLV, NULL_TLV, NULL_TLV, true) | |
CUSTOM_CHAR(HardwareFinish, 266, PR, TLV_ENC, NULL_TLV, NULL_TLV, NULL_TLV, true) | |
CUSTOM_CHAR(NFCAccessControlPoint, 264, PR+PW+WR, TLV_ENC, NULL_TLV, NULL_TLV, NULL_TLV, true) |
name: esp32s3-app-firmware | ||
path: .pio/build/s3/firmware.bin | ||
name: littlefs-binary | ||
path: littlefs.bin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing Newline at File End
A new line character is missing at the end of the file. Adding it will align with YAML lint requirements and prevent potential issues from other tooling.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 151-151: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
main/include/NFC_SERV_CHARS.h (1)
24-24
: Add newline at end of file.Consider adding a newline at the end of the file to follow standard coding practices.
} +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
main/include/NFC_SERV_CHARS.h
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: esp32s3
- GitHub Check: esp32
- GitHub Check: esp32c3
🔇 Additional comments (3)
main/include/NFC_SERV_CHARS.h (3)
1-5
: LGTM! Previous ID conflict has been resolved.The custom character definitions look good. The ID conflict between HardwareFinish and NFCAccessControlPoint that was previously flagged has been resolved by changing HardwareFinish's ID to 26C.
9-15
: LGTM! Service structure is well-defined.The LockManagement service is properly initialized with a unique ID and includes the necessary characteristics.
16-23
: LGTM! Service structure is well-defined.The NFCAccess service is properly initialized with a unique ID and includes all the necessary characteristics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/esp32.yml (1)
151-151
:⚠️ Potential issueMissing Newline at File End
Static analysis tools (YAMLlint 1.35.1) report a missing newline at the end of this file. Adding a newline will improve compatibility with YAML parsers and prevent potential issues with tooling.Please apply the following diff:
@@ - path: littlefs.bin \ No newline at end of file + path: littlefs.bin +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 151-151: no new line character at the end of file
(new-line-at-end-of-file)
🧹 Nitpick comments (2)
.github/workflows/esp32.yml (2)
7-13
: Workflow Trigger Configuration Updated
The workflow now supports manual triggering throughworkflow_dispatch
and refines thepull_request
event withpaths-ignore
for files like.vscode/**
,.gitignore
,LICENSE
, andREADME.md
. Make sure these ignore patterns align with your intended CI scope.
16-24
: ESP32 Job – Checkout & Caching Enhancements
Theesp32
job now includes a recursive checkout for submodules (submodules: 'recursive'
) and extends caching paths to include~/.ccache
,build
,managed_components
, andsdkconfig
for improved build performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/esp32.yml
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/esp32.yml
[error] 151-151: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (12)
.github/workflows/esp32.yml (12)
33-36
: ESP32 Job – Docker Cache Configuration
Caching for Docker images is configured usingScribeMD/docker-cache@0.5.0
with a key based on the OS and esp-idf version. Verify that the key naming conventions meet your organizational standards.
37-44
: ESP32 Build Step Configuration
The build command now leveragesidf.py --ccache build && idf.py merge-bin
with extra Docker arguments ensuring the ccache directory is correctly mounted. Double-check that merging binaries immediately after building meets your firmware deployment requirements.
45-54
: ESP32 Artifact Archiving Consistency
Artifact upload steps for both firmware (build/HomeKey-ESP32.bin
) and the merged binary (build/merged-binary.bin
) are consistently defined. Confirm that these file paths match the output of the build process.
55-64
: ESP32C3 Job – Checkout & Caching Configuration
Theesp32c3
job mirrors theesp32
job’s recursive submodule checkout and caching directories. Consistency across jobs is maintained, which is great for reproducibility.
72-73
: ESP32C3 Docker Cache Step
The Docker image caching configuration for the ESP32C3 job uses a similar pattern to the ESP32 job. Ensure that the cache key (${{ runner.os }}-esp32c3-build
) is unique and does not conflict with other jobs.
76-83
: ESP32C3 Build Step Verification
The build step for ESP32C3 utilizes the same command structure (idf.py --ccache build && idf.py merge-bin
) and specifiestarget: esp32c3
. Verify that the toolchain supports the ESP32C3 target without issues.
84-93
: ESP32C3 Artifact Archiving
Artifact upload steps for the ESP32C3 firmware and merged binary are defined with appropriate names and paths. Confirm that the resulting artifacts are as expected from the build process.
94-103
: ESP32S3 Job – Checkout & Caching Consistency
Theesp32s3
job maintains consistency with the other ESP jobs, including recursive checkout and caching of directories. This uniformity helps in maintaining the build environment.
112-114
: ESP32S3 Docker Image Caching
Docker cache configuration for the ESP32S3 job follows the established pattern. Ensure that the key (docker-${{ runner.os }}-espidf
) remains unique per workflow run when used across multiple jobs.
115-122
: ESP32S3 Build Step Review
The build command for ESP32S3 is consistent with earlier targets, usingidf.py --ccache build && idf.py merge-bin
and specifyingtarget: esp32s3
. It is advisable to test that the merged binary is generated correctly for this target.
123-132
: ESP32S3 Artifact Archiving Verification
Artifact steps for the ESP32S3 firmware and merged binary follow the established naming conventions. Double-check that the produced artifacts (build/HomeKey-ESP32.bin
andbuild/merged-binary.bin
) are correctly generated.
133-151
: LittleFS Job Integration
The newlittlefs
job introduces a sparse-checkout for thedata
directory, sets up Python (version 3.11), installs the LittleFS tool, and creates the LittleFS image usinglittlefs-python
. Please verify that the provided parameters (--fs-size=0x20000 --name-max=64 --block-size=4096
) match your filesystem requirements and that the artifact (littlefs.bin
) is generated and archived as expected.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 151-151: no new line character at the end of file
(new-line-at-end-of-file)
#164
#165
#167
#163
Summary by CodeRabbit
New Features
Style
Chores