8000
Skip to content

Support W5500 SPI-Ethernet polling mode if framework is supported - #7503

Merged
jesserockz merged 25 commits into
esphome:devfrom
slakichi:w5500-polling-mode
Oct 29, 2024
Merged

Support W5500 SPI-Ethernet polling mode if framework is supported#7503
jesserockz merged 25 commits into
esphome:devfrom
slakichi:w5500-polling-mode

Conversation

@slakichi
@slakichi slakichi commented Sep 27, 2024
Copy link
Copy Markdown
Contributor

What does this implement/fix?

W5500 can work without interrupt(IRQ) pin, but crash at init if configure without interrupt_pin.
This patch adds polling mode to SPI-based ethernet if ESP-IDF framework is supported.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): fixes esphome/issues#6268

Pull request in esphome-docs with documentation (if applicable): esphome/esphome.io#4389

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

esphome:
  name: m5atoms3-lite
  friendly_name: "M5Stack M5AtomS3 Lite"
  platformio_options:
    board_build.f_cpu: 80000000L
    board_build.f_flush: 80000000L

esp32:
  board: esp32-s3-devkitc-1
  flash_size: 8MB
  variant: esp32s3
  framework:
    type: esp-idf
# recommended version disables polling mode (interrupt_pin is mandatory; polling_interval is invalid)
#    version: recommended
    version: "5.3.1"
    platform_version: "6.9.0"

network:
  enable_ipv6: false

ethernet:
  type: W5500
  # W5500(SPI) mandatory pins
  # ATOM: CLK=22, CS=19, MISO=23, MOSI=33
  clk_pin: GPIO5
  mosi_pin: GPIO8
  miso_pin: GPIO7
  cs_pin: GPIO6
  # if ESP-IDF >= 5.3 or 5.2.1+ or 5.1.4+ / arduino >= 3.0.0, interrupt_pin is optional; oterwise mandatory
  #interrupt_pin: GPIO4
  # [New] if interrupt_pin is omitted, set polling interval; minimum is 1ms, default is 10ms.
  # available only if ESP-IDF >= 5.3 or 5.2.1+ or 5.1.4+ / arduino >= 3.0.0 (* pio still not supports arduino-esp32 3.0.x)
  polling_interval: 10ms
  #reset_pin:
  clock_speed: 10MHz
  manual_ip:
    static_ip: 192.168.xxx.xxx
    gateway: 192.168.xxx.xxx
    subnet: 255.255.255.0
    dns1: 192.168.xxx.xxx

api:
  encryption:
    key: !secret home_assistant_api_password
  reboot_timeout: 300s

ota:
  platform: esphome
  password: !secret ota_password

esp32_ble_tracker:
  scan_parameters:
    interval: 1100ms
    window: 1100ms
    active: true

bluetooth_proxy:
  active: true

sensor:
  - platform: uptime
    type: timestamp
    entity_category: diagnostic
  - platform: internal_temperature
    entity_category: diagnostic
    name: "Internal Temperture"

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@codecov-commenter
codecov-commenter commented Sep 27, 2024
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.98%. Comparing base (4d8b5ed) to head (58694e6).
Report is 1537 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #7503      +/-   ##
==========================================
+ Coverage   53.70%   53.98%   +0.27%     
==========================================
  Files          50       50              
  Lines        9408     9765     +357     
  Branches     1654     1344     -310     
==========================================
+ Hits         5053     5272     +219     
- Misses       4056     4167     +111     
- Partials      299      326      +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cortices
Copy link
Copy Markdown

This change is a requirement for correct functioning on a few devices I have had issues using, both for the same reason:

  • M5Stack POECam-W
  • Norvi ENET AE06

Both using W5500 with no INTn pin connected

@ijc
ijc commented Sep 28, 2024
Copy link
Copy Markdown

This works for me on an m5stack lite with the Atom POE. Previously I was seeing symptoms similar to esphome/issues#6217. My crash was in a gpio setup function.

@ptrkp
ptrkp commented Oct 6, 2024
Copy link
Copy Markdown

This works for me on an m5stack lite with the Atom POE. Previously I was seeing symptoms similar to esphome/issues#6217. My crash was in a gpio setup function.

What for config did you use?

@ijc
ijc commented Oct 6, 2024
Copy link
Copy Markdown

At commit cc2f39a I used

esphome:
  name: study-bluetooth-proxy
  friendly_name: Study Bluetooth Proxy

esp32:
  board: m5stack-atom
  framework:
    type: esp-idf
    version: "5.3.1" 
    platform_version: "6.9.0" 

ethernet:
  type: W5500
  clk_pin: GPIO22
  mosi_pin: GPIO33
  miso_pin: GPIO23
  cs_pin: GPIO19
  clock_speed: 20MHz
  polling_interval: 10ms
  manual_ip:
    static_ip: ...
    gateway: ...
    subnet: ...
    dns1: ...

# Enable logging
logger:

# Enable Home Assistant API
api:
  encryption:
    key: "..."

ota:
  - platform: esphome
    password: "..."

esp32_ble_tracker:
  scan_parameters:
    # We currently use the defaults to ensure Bluetooth
    # can co-exist with WiFi In the future we may be able to
    # enable the built-in coexistence logic in ESP-IDF
    active: true

bluetooth_proxy:
  active: true

button:
  - platform: safe_mode
    id: button_safe_mode
    name: Safe Mode Boot

  - platform: factory_reset
    id: factory_reset_btn
    name: Factory reset

@ijc ijc left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM and I've been running it for a while. I left some comments/suggestions but I think nothing which really affects the functionality

@jesserockz @JeroenVanOort would it be possible to get this on the radar for a future release?

Comment thread esphome/components/ethernet/__init__.py Outdated
Comment thread esphome/components/ethernet/__init__.py Outdated
Comment thread esphome/components/ethernet/__init__.py
Comment thread esphome/components/ethernet/ethernet_component.cpp Outdated
Comment thread esphome/components/ethernet/__init__.py Outdated
@slakichi
Copy link
Copy Markdown
Contributor Author

Moved the polling_interval and interrupt_pin setting checks to _validate(). Compared to the previous code, there are the following differences:

  1. For older (unsupported) frameworks, prepend "In this version of the framework, ..." to error messages when polling_interval is set or interrupt_pin is not set.
  2. If both polling_interval and interrupt_pin are in the config, polling_interval is no longer silently ignored, but an error is raised.
  3. No more warnings in list-components test.

@ijc ijc left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I've built and uploaded this to my device.

BTW, I have merged the 2024.10.0 release tag into my checkout of your branch so actually not quite testing exactly what is here, but I think it makes no practical impact.

Comment thread esphome/components/ethernet/__init__.py Outdated
Comment thread esphome/components/ethernet/__init__.py Outdated
Comment thread esphome/components/ethernet/__init__.py Outdated
Comment thread esphome/components/ethernet/__init__.py Outdated
Comment thread esphome/components/ethernet/__init__.py
Comment thread esphome/components/ethernet/ethernet_component.cpp Outdated
Comment thread esphome/components/ethernet/ethernet_component.cpp Outdated
Comment thread esphome/components/ethernet/ethernet_component.cpp Outdated
Comment thread esphome/components/ethernet/ethernet_component.cpp
Comment thread esphome/components/ethernet/ethernet_component.h
Comment thread esphome/components/ethernet/ethernet_component.h
@esphome
esphome Bot commented Oct 24, 2024
Copy link
Copy Markdown
Contributor

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@esphome
esphome Bot marked this pull request as draft October 24, 2024 01:57
@slakichi
slakichi commented Oct 25, 2024
Copy link
Copy Markdown
Contributor Author
  • Check review comments and apply to files
  • Build firmware with latest code and run for a few days (started at 2024-10-24 21:13 +0900)
  • Make a PR for docs

@jesserockz
Copy link
Copy Markdown
Member

@ijc
ijc commented Oct 25, 2024
Copy link
Copy Markdown

FYI I've uploaded a new build from bc71786 to my system and it seems to be working well.

@slakichi
slakichi marked this pull request as ready for review October 27, 2024 00:59
@esphome
esphome Bot requested a review from jesserockz October 27, 2024 00:59
@slakichi
Copy link
Copy Markdown
Contributor Author

I built W5500+bluetooth_proxy with commit 162e521, and I run for a few days and no issue occ 67E6 ured.
I got warning about there are some uninitialized members in setup, and there may cause something unexpected -- but ignored because there are out of scope for this PR. (will be fixed by small bugfix PR)

src/esphome/components/ethernet/ethernet_component.cpp: In member function 'virtual void esphome::ethernet::EthernetComponent::setup()':
src/esphome/components/ethernet/ethernet_component.cpp:66:3: warning: missing initializer for member 'spi_bus_config_t::isr_cpu_id' [-Wmissing-field-initializers]
src/esphome/components/ethernet/ethernet_component.cpp:107:3: warning: missing initializer for member 'spi_device_interface_config_t::clock_source' [-Wmissing-field-initializers]

@ijc
ijc commented Oct 27, 2024
Copy link
Copy Markdown

I got warning about there are some uninitialized members in setup

FWIW I saw these too.

I think they are due to the non-default version of esp-idf and/or platform I/O and not directly related to this PR (other than it requiring the non-default version).

@jesserockz
jesserockz merged commit 302ba28 into esphome:dev Oct 29, 2024
@slakichi
slakichi deleted the w5500-polling-mode branch October 29, 2024 11:44
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

W5500 SPI-based Ethernet without interrupt pin causes crash at init

6 participants

0