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

Disable Ethernet library if CONFIG_ETH_ENABLED not defined in sdkconfig.h #8595

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

arkhipenko
Copy link

Description of Change

When used in Platform IO in a combined arduino, espidf platform setting, and disabling ETH component, the ETH library generates a lot of error messages

Tests scenarios

Running previously running firmware that did not use Ethernet, but does use WiFi and BLE
Tested on esp32 pico d4 with Arduino 2.0.9, IDF 4.4.4

@CLAassistant
Copy link

CLAassistant commented Sep 1, 2023

CLA assistant check
All committers have signed the CLA.

@me-no-dev me-no-dev added this to the 2.0.13 milestone Sep 12, 2023
@me-no-dev me-no-dev changed the base branch from master to release/v2.x September 13, 2023 07:41
@@ -18,6 +18,8 @@
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/

#ifdef CONFIG_ETH_ENABLED
Copy link
Member

Choose a reason for hiding this comment

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

you are checking too early if CONFIG_ETH_ENABLED is defined. At this point in the header, nothing is yet included. You need to at least #include "sdkconfig.h" for this to work

@@ -21,6 +21,8 @@
#ifndef _ETH_H_
#define _ETH_H_

#ifdef CONFIG_ETH_ENABLED
Copy link
Member

Choose a reason for hiding this comment

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

same comment as above

@me-no-dev me-no-dev merged commit fff900d into espressif:release/v2.x Sep 13, 2023
35 checks passed
Jason2866 added a commit to Jason2866/arduino-esp32 that referenced this pull request Oct 1, 2023
Jason2866 added a commit to tasmota/arduino-esp32 that referenced this pull request Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants