Fix build errors for SPI ethernet #3178
Conversation
WalkthroughThe updates introduce a new configuration option, Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant GPIO_ISR_Service
participant SPI_Bus
participant Ethernet_Driver
App->>GPIO_ISR_Service: Install ISR service
App->>SPI_Bus: Initialize SPI bus (using ESP32_ETHERNET_SPI_HOST, SPI_DMA_CH_AUTO)
App->>Ethernet_Driver: Configure with ESP32_ETHERNET_SPI_HOST
Ethernet_Driver-->>App: Initialization complete
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
targets/ESP32/_IDF/sdkconfig.default_ble_rev3.esp32(1 hunks)targets/ESP32/_IDF/sdkconfig.default_nopsram_rev3.esp32(1 hunks)targets/ESP32/_IDF/sdkconfig.default_rev3.esp32(1 hunks)targets/ESP32/_Network/NF_ESP32_Ethernet.cpp(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: nf-interpreter (Check_Build_Options)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
🔇 Additional comments (5)
targets/ESP32/_IDF/sdkconfig.default_ble_rev3.esp32 (1)
71-71: LGTM! Loopback interface correctly disabled.This change properly disables the LWIP loopback interface, which should resolve the issue where the network interface IP address was displaying as 127.0.0.1 instead of the actual IP address.
targets/ESP32/_IDF/sdkconfig.default_nopsram_rev3.esp32 (1)
50-50: LGTM! Consistent loopback configuration across build variants.This change maintains consistency with other ESP32 configuration files by disabling the LWIP loopback interface, ensuring uniform network behavior across different build configurations.
targets/ESP32/_IDF/sdkconfig.default_rev3.esp32 (1)
59-59: LGTM! Completes consistent loopback configuration.This change completes the uniform application of loopback interface disabling across all ESP32 rev3 configuration variants, ensuring consistent network behavior.
targets/ESP32/_Network/NF_ESP32_Ethernet.cpp (2)
160-160: LGTM! Unified SPI host configuration.The consistent replacement of
CONFIG_EXAMPLE_ETH_SPI_HOSTwithESP32_ETHERNET_SPI_HOSTacross all Ethernet modules (DM9051, W5500, ENJ28J60) improves code maintainability and ensures consistent SPI host usage.Also applies to: 178-178, 195-195
146-146:✅ Verification successful
LGTM! Improved SPI configuration with automatic DMA selection.
The changes to use
ESP32_ETHERNET_SPI_HOSTmacro andSPI_DMA_CH_AUTOare excellent improvements. The automatic DMA channel selection prevents conflicts and the macro usage provides better consistency.Verify that the
ESP32_ETHERNET_SPI_HOSTmacro is properly defined:
🏁 Script executed:
#!/bin/bash # Description: Verify ESP32_ETHERNET_SPI_HOST macro definition # Search for ESP32_ETHERNET_SPI_HOST definition echo "=== ESP32_ETHERNET_SPI_HOST macro definition ===" rg -A 3 -B 3 "#define.*ESP32_ETHERNET_SPI_HOST" # Check if it's used consistently echo "=== All usages of ESP32_ETHERNET_SPI_HOST ===" rg "ESP32_ETHERNET_SPI_HOST"Length of output: 1651
Confirmed ESP32 Ethernet SPI host macro definition and usage
Verified that
ESP32_ETHERNET_SPI_HOSTis defined in
- targets/ESP32/_Network/esp32_ethernet_options.h.in
#define ESP32_ETHERNET_SPI_HOST SPI2_HOSTand is consistently used in
- targets/ESP32/_Network/NF_ESP32_Ethernet.cpp
All SPI initializations now leverage the macro with
SPI_DMA_CH_AUTOas intended.
josesimoes
added
the
Platform: ESP32
Description
This corrects a build error with an incorrect SPI host name.
Also changes the DM channels to auto instead of hard coding to 1
The ESP32 rev3 configs updated to disable the NetIF loopback as this was causing the IP address in network interface to be 127.0.0.1 instead of actual IP.
Motivation and Context
Reported on discord
https://discord.com/channels/478725473862549535/1371829744793817260
How Has This Been Tested?
Tested locally using gpio pins from discord
Types of changes
Checklist
Summary by CodeRabbit
Chores
Refactor