Closed elcojacobs closed 7 months ago
Test build is failing because no stubs for esp_eth exist in mock-idf. I can add some empty functions, but not sure if that's the right approach.
The mocks here are mostly just empty stubs. On my current platform, we can run our entire app as a cross compiled executable, including network, GPIO mocking, etc. Would be nice to have this on ESP32 too.
I found this repo, but haven't tried it yet. https://github.com/mireq/esp32-simulator
Smooth can be cross compiled and network was functional when running on Linux prior to this PR so that needs to work. IO is currently just mocked.
I'll try to find some time tomorrow or at least the next few days to look at this PR.
I have considered clang format, but found uncrustify worked better, at least it did a few years ago.
I added eth
stubs in mock-idf.
Compile now fails on acces_point test, because it still expects to have some static functions on the Wifi class, that I have moved to the NetworkInterface class.
It does make sense to have only one Wifi instance. In theory, you can have multiple Ethernet instances on the SPI bus. But we can choose not to implement this use case.
An option is a global wifi getter and construct it on first use. It can then be removed from App members.
Wifi& get_wifi(){
static Wifi* wifi = new Wifi();
return *wifi;
}
Global singleton added for Wifi, tests passing :smile:
Had a quick look, overall it looks good. I have a few things I want to think about/look closer at in an actual code-editor.
It's a bit outside the coding-part, but I feel like a small documentation section about the h/w side could be of use for the wired ethernet. Perhaps some links to resources that gets people started?
I would just refer to the esp-idf docs, because the options I added to kconfig are copy-paste from their examples. The only change I made is to set the default to phy address 0. That gotcha took me a while to figure out before I got it to work. I am also using the framework functions to initialize everything, nothing custom about that. Just some C++ around the api calls.
For reference, I am using the internal MAC with a LAN8742 phy. It is cheaper and easier to get than the 8720. I let the ESP generate the 50Mhz clock for the phy on pin gpio0, and use the default pins. I use the WROOM-32D. If I remember correctly, the variants with external PSRAM don't have ethernet. Ethernet needs quite a lot of dedicated pins.
CONFIG_SMOOTH_USE_INTERNAL_ETHERNET=y
# CONFIG_SMOOTH_USE_DM9051 is not set
# CONFIG_SMOOTH_ETH_PHY_IP101 is not set
# CONFIG_SMOOTH_ETH_PHY_RTL8201 is not set
# CONFIG_SMOOTH_ETH_PHY_LAN8720 is not set
CONFIG_SMOOTH_ETH_PHY_LAN8742=y
# CONFIG_SMOOTH_ETH_PHY_DP83848 is not set
CONFIG_SMOOTH_ETH_MDC_GPIO=23
CONFIG_SMOOTH_ETH_MDIO_GPIO=18
CONFIG_SMOOTH_ETH_PHY_RST_GPIO=5
CONFIG_SMOOTH_ETH_PHY_ADDR=0
No need in duplicating the docs here: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/hw-reference/esp32/get-started-ethernet-kit.html#gpio-allocation https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_eth.html
I would add those links and leave it at that.
Thanks for the feedback.
After playing with smooth for a bit, I actually think it might be a bit too opinionated for my goals. It uses a lot of events by default and is quite verbose, with ease of use over performance. Many of the logs generated by smooth are duplications of the logs already emitted by esp-idf.
I have been playing with asio, and I think it will be a better fit for my application. I prefer to stay a bit closer to esp-idf where possible, and use the framework provided implementations for mdns for example.
I'll still make the changes to this PR and will look at smooth for inspiration here and there, but I think it will not combine well with asio. asio is not even available when smooth is included, I am not sure why.
@elcojacobs thanks for the implementation of the ethernet part! It will help on my current project a lot! please finish your current PR if possible, if you need/want help: just give me a ping ;)
I don't have time to work on this in the coming days. Feel free to pull my branch and work on it or even finish it.
After playing with smooth for a bit, I actually think it might be a bit too opinionated for my goals. It uses a lot of events by default and is quite verbose, with ease of use over performance.
That's is a correct observation; it's was one of my my goal with it.
Many of the logs generated by smooth are duplications of the logs already emitted by esp-idf.
Oh, anything in particular you have in mind here?
I have been playing with asio, and I think it will be a better fit for my application. I prefer to stay a bit closer to esp-idf where possible, and use the framework provided implementations for mdns for example.
Then you should ofc use it! The right tool for the job etc. :)
I'll still make the changes to this PR and will look at smooth for inspiration here and there, but I think it will not combine well with asio. asio is not even available when smooth is included, I am not sure why.
Apart from a possible conflict with the network layer I don't see a problem using asio together with smooth. I've not disabled asio in Smooth afaik.
First of all, your framework looks well designed so I think this could be my first of many PR's.
I added support for wired Ethernet, which required a refactor of the WiFi class too. Please let me know your thoughts on the changes.
I tested this on custom hardware with the internal MAC and a LAN8742 phy. The LAN8742 is very similar to the 8720, but not included yet in esp-idf. Will later send a PR for that to esp-idf.
Changes
Result of test app and idf.py monitor below. You can see that the ESP gets 2 ip addresses. Connections are accepted on both IPs, tested with:
Formatting For my own projects, I use clang-format. Integrates very nicely with vscode to format on save. I ran uncrustify for this PR, because my clang-format rules are different.
I use these settings and the
xaver.clang-format
vscode plugin: https://github.com/BrewBlox/brewblox-firmware/blob/develop/.clang-formatHave you tried/considered clang-format instead of uncrustify?