using ksz8081 only from ESP-IDF 4.4 onwards #5918
Conversation
The previous assertion only considerate the existance of ESP-IDF 4.3, but with the ESP-IDF 4.3.1 release this assertion would generate errors. Now only includes from ESP-IDF 4.4 onwards.
|
|
libraries/Ethernet/src/ETH.cpp
Outdated
| @@ -282,7 +282,7 @@ bool ETHClass::begin(uint8_t phy_addr, int power, int mdc, int mdio, eth_phy_typ | |||
| break; | |||
| #endif | |||
| case ETH_PHY_KSZ8081: | |||
| #if ESP_IDF_VERSION > ESP_IDF_VERSION_VAL(4,3,0) | |||
| #if ESP_IDF_VERSION_MAJOR >= 4 && ESP_IDF_VERSION_MINOR >= 4 | |||
There was a problem hiding this comment.
this will fail for 5.2.0 for example. Use ESP_IDF_VERSION > ESP_IDF_VERSION_VAL(4,4,0) instead. In any case, current Arduino does not support anything below 4.4 (and now has a cmake check added to ensure that the version of IDF is correct) which makes this change unnecessary. Has the API for 4.3 changed?
There was a problem hiding this comment.
No, 4.3 hasn't changed but they released 4.3.1, which fails the previous assertion. If it's ensured by cmake check, this assertion can be deleted, but as it's don't make any significant improvement it can be like that. Arduino 2.0.0 don't make this assertion, should I make this change and make a new PR?
There was a problem hiding this comment.
If updating to v4.4+ it should be ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4,4,0) (note the = being added)
There was a problem hiding this comment.
Updated the commit as instructed
Summary
The previous assertion only considerate the existence of ESP-IDF 4.3, but with the ESP-IDF 4.3.1 release this assertion would generate errors. Now only includes from ESP-IDF 4.4 onwards.
Impact
Now don't generate esp_eth_phy_new_ksz8081 not included error