-
Notifications
You must be signed in to change notification settings - Fork 23
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
Fix esp-idf and IPv6 support by using plain UDP socket, drop Syslog library #17
base: main
Are you sure you want to change the base?
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 |
---|---|---|
|
@@ -2,14 +2,14 @@ | |
|
||
#include "esphome/core/log.h" | ||
#include "esphome/core/application.h" | ||
#include "esphome/core/version.h" | ||
|
||
#ifdef USE_LOGGER | ||
#include "esphome/components/logger/logger.h" | ||
#endif | ||
/* | ||
#include "esphome/core/helpers.h" | ||
#include "esphome/core/defines.h" | ||
#include "esphome/core/version.h" | ||
*/ | ||
|
||
namespace esphome { | ||
|
@@ -23,12 +23,51 @@ static const uint8_t esphome_to_syslog_log_levels[] = {0, 3, 4, 6, 5, 7, 7, 7}; | |
|
||
SyslogComponent::SyslogComponent() { | ||
this->settings_.client_id = App.get_name(); | ||
// Get the WifiUDP client here instead of getting it in setup() to make sure we always have a client when calling log() | ||
// Calling log() without the device connected should not be an issue since there is a wifi connected check and WifiUDP fails "silently" and doesn't generate an exception anyways | ||
this->udp_ = new WiFiUDP(); | ||
} | ||
|
||
void SyslogComponent::setup() { | ||
|
||
/* | ||
* Older versions of socket::set_sockaddr() return bogus results | ||
* if trying to log to a Legacy IP address when IPv6 is enabled. | ||
* Fixed by https://github.com/esphome/esphome/pull/7196 | ||
*/ | ||
this->server_socklen = 0; | ||
if (ESPHOME_VERSION_CODE >= VERSION_CODE(2024, 8, 0)) { | ||
this->server_socklen = socket::set_sockaddr((struct sockaddr *)&this->server, sizeof(this->server), | ||
this->settings_.address, this->settings_.port); | ||
} | ||
#if USE_NETWORK_IPV6 | ||
else if (this->settings_.address.find(':') != std::string::npos) { | ||
auto *server6 = reinterpret_cast<sockaddr_in6 *>(&this->server); | ||
memset(server6, 0, sizeof(*server6)); | ||
server6->sin6_family = AF_INET6; | ||
server6->sin6_port = htons(this->settings_.port); | ||
|
||
ip6_addr_t ip6; | ||
inet6_aton(this->settings_.address.c_str(), &ip6); | ||
memcpy(server6->sin6_addr.un.u32_addr, ip6.addr, sizeof(ip6.addr)); | ||
this->server_socklen = sizeof(*server6); | ||
} | ||
#endif /* USE_NETWORK_IPV6 */ | ||
else { | ||
auto *server4 = reinterpret_cast<sockaddr_in *>(&this->server); | ||
memset(server4, 0, sizeof(*server4)); | ||
server4->sin_family = AF_INET; | ||
server4->sin_addr.s_addr = inet_addr(this->settings_.address.c_str()); | ||
server4->sin_port = htons(this->settings_.port); | ||
this->server_socklen = sizeof(*server4); | ||
} | ||
if (!this->server_socklen) { | ||
ESP_LOGW(TAG, "Failed to parse server IP address '%s'", this->settings_.address.c_str()); | ||
return; | ||
} | ||
this->socket_ = socket::socket(this->server.ss_family, SOCK_DGRAM, IPPROTO_UDP); | ||
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.
Maybe there is a better way but this works for me. (Also tested with esp-idf.) |
||
if (!this->socket_) { | ||
ESP_LOGW(TAG, "Failed to create UDP socket"); | ||
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. Same here, loge, mark failed. |
||
return; | ||
} | ||
|
||
this->log(ESPHOME_LOG_LEVEL_INFO , "syslog", "Syslog started"); | ||
ESP_LOGI(TAG, "Started"); | ||
|
||
|
@@ -53,21 +92,16 @@ void SyslogComponent::loop() { | |
void SyslogComponent::log(uint8_t level, const std::string &tag, const std::string &payload) { | ||
level = level > 7 ? 7 : level; | ||
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. Do something like:
|
||
|
||
// Simple check to make sure that there is connectivity, if not, log the issue and return | ||
if(WiFi.status() != WL_CONNECTED) { | ||
ESP_LOGW(TAG, "Tried to send \"%s\"@\"%s\" with level %d but Wifi isn't connected yet", tag.c_str(), payload.c_str(), level); | ||
if (!this->socket_) { | ||
ESP_LOGW(TAG, "Tried to send \"%s\"@\"%s\" with level %d but socket isn't connected", tag.c_str(), payload.c_str(), level); | ||
return; | ||
} | ||
|
||
Syslog syslog( | ||
*this->udp_, | ||
this->settings_.address.c_str(), | ||
this->settings_.port, | ||
this->settings_.client_id.c_str(), | ||
tag.c_str(), | ||
LOG_KERN | ||
); | ||
if(!syslog.log(esphome_to_syslog_log_levels[level], payload.c_str())) { | ||
int pri = esphome_to_syslog_log_levels[level]; | ||
std::string buf = str_sprintf("<%d>1 - %s %s - - - \xEF\xBB\xBF%s", | ||
pri, this->settings_.client_id.c_str(), | ||
tag.c_str(), payload.c_str()); | ||
if (this->socket_->sendto(buf.c_str(), buf.length(), 0, (struct sockaddr *)&this->server, this->server_socklen) < 0) { | ||
ESP_LOGW(TAG, "Tried to send \"%s\"@\"%s\" with level %d but but failed for an unknown reason", tag.c_str(), payload.c_str(), level); | ||
} | ||
} | ||
|
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.
IMO i would log error, mark component failed and then return from setup.