Fix esp-idf and IPv6 support by using plain UDP socket, drop Syslog library #17
Conversation
…ibrary It turns out to be fairly simple to just send the UDP packets directly, and the UDP socket handling can then be a whole lot more generic.
Using a Legacy IP syslog server in an IPv6-enabled build fails due to a socket::set_sockaddr() bug. Work around it in older versions of esphome.
|
Thanks for this PR, it works great with esp-idf! However with arduino I get the following error: I'm using esphome 2024.9.0 with the following config: Any chance of making the code compatible with both arduino and esp-idf? |
|
Hm, which platform is that on? I thought I'd tested it with Arduino for ESP32. I think there are some platforms where UDP isn't available directly in LwIP and we have to use the Arduino UDP as before, as I mentioned above. I'll have a look at making the change conditional. |
|
Oh, just realized that this was on a esp8266 device (not esp32). But there are still plenty of those out there :D |
| this->server_socklen = sizeof(*server4); | ||
| } | ||
| if (!this->server_socklen) { | ||
| ESP_LOGW(TAG, "Failed to parse server IP address '%s'", this->settings_.address.c_str()); |
There was a problem hiding this comment.
IMO i would log error, mark component failed and then return from setup.
| } | ||
| this->socket_ = socket::socket(this->server.ss_family, SOCK_DGRAM, IPPROTO_UDP); | ||
| if (!this->socket_) { | ||
| ESP_LOGW(TAG, "Failed to create UDP socket"); |
| @@ -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.
Do something like:
if (this->is_failed()) {
// maybe log with ESP_LOGV or ESP_LOGVV to reduce spam since component is marked failed
return;
}
| 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.
IPPROTO_UDP is not defined on arduino for esp8266.
I managed to fix this by adding
#ifndef IPPROTO_UDP
#include <lwip/ip.h>
#define IPPROTO_UDP IP_PROTO_UDP
#endif
Maybe there is a better way but this works for me. (Also tested with esp-idf.)
It turns out to be fairly simple to just send the UDP packets directly, and the UDP socket handling can then be a whole lot more generic.