Skip to content
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

Remove EMAC related code for devices that don't have one #2333

Merged
merged 1 commit into from
Oct 6, 2024

Conversation

LennartF22
Copy link
Contributor

It does not make sense to be able to configure Ethernet via the internal EMAC for targets other than the "original" ESP32, as they don't have one! Devices from the ESP32-C or ESP32-S series only support Ethernet via an external MAC, like the W5500. Therefore, the user should not be able to configure it and neither should see it in the pin configuration on the "Device-Manager" page of the OpenDTU web interface.

Furthermore, the usage of ETH_PHY_LAN8720 in the PinMappingClass caused a compilation error when using Arduino core v3.0, because with v3.0 the definition only exists for targets where internal EMAC support is configured (see ETH.h).

@tbnobody tbnobody merged commit eaa2f07 into tbnobody:master Oct 6, 2024
8 of 9 checks passed
@schlimmchen
Copy link
Contributor

@tbnobody
Copy link
Owner

tbnobody commented Oct 6, 2024

Haven't tested it yet, but what happens if pins are defined via define and the pin mapping contains a empty or non-exsiting eth section?

@schlimmchen
Copy link
Contributor

You will then (still) see the category "eth" in the web UI if and only if the firmware was compiled with CONFIG_ETH_USE_ESP32_EMAC, as also the #defines for the respective pins are guarded by this #define.

Or in general: A category will appear in the device manager web UI if it is included in curPin at the api/device/config endpoint, regardless of whether or not it is in the pin mapping. Sections in the pin mapping are masked if the firmware says "I don't know this category of pins", which I think makes sense in general.

@LennartF22
Copy link
Contributor Author

LennartF22 commented Oct 6, 2024

Haven't tested it yet, but what happens if pins are defined via define and the pin mapping contains a empty or non-exsiting eth section?

The defines from the platformio.ini are ignored, since they are never used due to my patch. However, when Ethernet pins are defined in the pin_mapping.json, they will still be shown in the web interface, because without the patch from @schlimmchen, the web interface will happily show anything configured in the pin_mapping.json.

@schlimmchen
Copy link
Contributor

because - as far as I know - the web interface will happily show anything configured in the pin_mapping.json.

That's exactly what the one-liner in hoylabs@d6d5f32 prevents. The categories (and only the categories, not the individual pins) are filtered by the known/supported categories of the firmware.

@LennartF22 LennartF22 deleted the eth-config-disable branch October 6, 2024 20:33
@LennartF22
Copy link
Contributor Author

That's exactly what the one-liner in helgeerbe@d6d5f32 prevents. The categories (and only the categories, not the individual pins) are filtered by the known/supported categories of the firmware.

Yes, sorry, I only noticed that @tbnobody‘s question was directed towards your change after writing my comment, but I already updated it ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants