[ethernet] add support for enc28j60 in SPI mode

[cocus]

What does this implement/fix?

Adds support for the ENC28J60 SPI to ethernet chip. It's registered as another type of the ethernet component. It uses the sources from the ENC28J60 example provided on esp-idf v4.4.

Types of changes

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

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#4180

Test Environment

• [X] ESP32
• [ ] ESP32 IDF
• [ ] ESP8266
• [ ] RP2040
• [ ] BK72xx
• [ ] RTL87xx

Example entry for config.yaml:

ethernet:
  type: ENC28J60
  clk_pin: GPIO14
  mosi_pin: GPIO13
  miso_pin: GPIO12
  cs_pin: GPIO15
  interrupt_pin: GPIO4
  clock_speed: 20000000

Checklist:

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

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

• [X] Documentation added/updated in esphome-docs.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 53.95%. Comparing base (4d8b5ed) to head (3ef120c). Report is 1202 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #7330      +/-   ##
==========================================
+ Coverage   53.70%   53.95%   +0.24%     
==========================================
  Files          50       50              
  Lines        9408     9665     +257     
  Branches     1654     1706      +52     
==========================================
+ Hits         5053     5215     +162     
- Misses       4056     4126      +70     
- Partials      299      324      +25     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[cocus]

I'm thinking of adding two changes to this:

1. add a new define for the W5500 so I don't use #ifndef or #else when SPI is selected (future proofing the code).
2. guard the entire esp_eth_enc28j60.c source around #ifdef USE_ETHERNET_ENC28J60 instead of just USE_ESP32.

Just want to mention that this is pretty stable on my crappy esp32 wroom board, with flying wires to the enc28j60 board. Both half-duplex and full-duplex seem to work fine, although there doesn't seem to be that much of a speed gain by forcing full duplex.

[bmork]

I was looking at this to see if we could do something smarter for SPI devices, in the interest of getting https://github.com/esphome/esphome/pull/6861 merged without having to do yet another refactor to resolve conflicts.

Specifically I was wondering if you couldn't also use https://github.com/esphome/esphome/pull/6861/commits/0342d5edd70dfda4fd6493f5c789cdc1e5a206d8 to avoid ifdeffing the SPI framing. And I believe you could in esp-idf v4.4 where the enc28j60 example still existed.

But I had a hard time finding the code in esp-idf HEAD. Due to https://github.com/espressif/esp-idf/commit/feedad8467a31bdc993f8e4fc133766e72e5893c

Based on that I wonder if this really has any future? What will happen when we move to esp-idf v5.4 or higher? Maybe the example driver should be copied from esp-idf instead, if we are to support enc28j60?

[cocus]

I was looking at this to see if we could do something smarter for SPI devices, in the interest of getting #6861 merged without having to do yet another refactor to resolve conflicts.

Specifically I was wondering if you couldn't also use 0342d5e to avoid ifdeffing the SPI framing. And I believe you could in esp-idf v4.4 where the enc28j60 example still existed.

I think that's fair. I don't recall if setting these properties to 0 worked or not; I just copied the example code (which was different from the W5500). This PR got really old and no one reviewed it, sadly.

But I had a hard time finding the code in esp-idf HEAD. Due to espressif/esp-idf@feedad8

Based on that I wonder if this really has any future? What will happen when we move to esp-idf v5.4 or higher? Maybe the example driver should be copied from esp-idf instead, if we are to support enc28j60?

Yeah, I didn't know that because this PR was older than that removal. Probably we could copy the removed sources. I understand the issues and drawbacks of this enc28j60, but I had one lying around and probably some other people might have and find this useful.

What do you suggest? I was thinking that if you help me out to get this PR reviewed, I can rebase it with the current master, and move the driver to its own c file (taking the removed driver from the idf repo).

I didn't update this anymore because there wasn't any interest at that time :(

[bmork]

What do you suggest? I was thinking that if you help me out to get this PR reviewed

I'd love to do that if I only knew how... I believe there is a bit of reviewer overload for ethernet components, and probably other subsystems without a subsystem maintainer. Resulting in merge conflicts and discussions like this, which only make the problem worse.

My DM9051 implementation has been mostly ready since June 4th. It was reviewed with a couple of minor changes requested on Aug 7th. They were fixed the same day. Then https://github.com/esphome/esphome/pull/7020 was merged and I fixed up the conflicts. Then nothing.

Not sure where to go from there. I probably messed up the process by rebasing and force-pushing. My first time here and these policies differ between projects. Would have been easier to do without rebasing if there weren't so many PRs in the queue. Tends to cause conflicts all over the place. Could be merged with conflict resolution of course. But then there is an eternal mess, while the rebasing only cause a mess during review. Can't win both.

I seriously consider dropping the ball on https://github.com/esphome/esphome/pull/6861 to at least take some unnecessary load away from both me and reviewers. Unfortunate since the feature has been requested (by others than me) for more than a year: https://github.com/esphome/feature-requests/issues/2427 . But that's the only sensible action I have left AFAICS.

[functionpointer]

Force pushing isn't a problem. In fact, sometimes maintainers request it. As you said yourself, it is a consequence of long review times

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!

[bdraco]

Espressif has since published an idf component that could be used to avoid vendoring the whole code here

https://components.espressif.com/components/espressif/enc28j60/versions/1.0.1/readme