-
Notifications
You must be signed in to change notification settings - Fork 24
fixed NetworkInterface.h include for ESPHOME #78
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ | |
#ifdef ARDUINO | ||
#include <esp32-hal.h> | ||
#include <esp32-hal-log.h> | ||
#if (ESP_IDF_VERSION_MAJOR >= 5) | ||
#if (ESP_IDF_VERSION_MAJOR >= 5) && !defined(ARDUINO) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is part of Arduino so your comparison is wrong. All ESP32 Arduino based on IDF5 has this file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah you are right, is the header file really needed? I use ESPHome and compiling fails because of the lib, even if i only add the lib without anything that uses it. Your lib is used in many ESPHome components like web server or wifi ap. With my fix above it compiles just fine, so this is why i wonder if this file is needed.
yaml file:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can not say why it is failing for you, but you should look into that reason instead of removing the requirement here. All versions of Arduino that are 3+ have this file and should be includable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not failing only for me but for multiple people that use ESPHome see issue in my repo below. I will open an issue in esphome and further investigate, with my suggested fix it does build. |
||
#include <NetworkInterface.h> | ||
#endif | ||
#else | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional logic is becoming complex. Consider adding a comment explaining why NetworkInterface.h should not be included in Arduino environments with ESP-IDF 5.x to help future maintainers understand this specific exclusion.
Copilot uses AI. Check for mistakes.