Skip to content

using ksz8081 only from ESP-IDF 4.4 onwards #5918

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 14, 2021
Merged

using ksz8081 only from ESP-IDF 4.4 onwards #5918

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b95a14a
using ksz8081 only from ESP-IDF 4.4 onwards
vitorrussi Nov 22, 2021
4477694
Changes made as instructed
vitorrussi Nov 22, 2021
18e60bf
Fix more or equal check
me-no-dev Dec 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libraries/Ethernet/src/ETH.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

@atanisoft atanisoft Nov 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If updating to v4.4+ it should be ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4,4,0) (note the = being added)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the commit as instructed

eth_phy = esp_eth_phy_new_ksz8081(&phy_config);
#else
log_e("unsupported ethernet type 'ETH_PHY_KSZ8081'");
Expand Down