elupus / esphome-nibe

Esphome components for nibe heat pumps
MIT License
57 stars 19 forks source link

Use AsyncUDP library #20

Closed ksjoberg closed 1 year ago

ksjoberg commented 1 year ago

in an attempt to make UDP reception more stable.

I've had issues where suddenly the Nibe heatpump goes unavailable in Home Assistant. It seems to be due to the ESP32 stops receiving the UDP datagrams for single register readout.

I stole an older version of the AsyncUDP library from espressif/arduino-esp32, because the latest version depends on an updated Espressif IDF.

I don't know how the PlatformIO LDF could be convinced to finding this library through other means, but for the POC I decided to just bundle it.

Let me know your thoughts.

ksjoberg commented 1 year ago

From what I gather, the LDF should scan the code that has been generated by Esphome to figure out the dependent libraries. But which are the available libraries? I tried new versions of the arduino framework and the platform:

  framework:
    type: arduino
    version: 2.0.5
    platform_version: 5.2.0

which caused WiFi, FS and a few other libraries to bump version to 2.0.0. I expected it would then also find a version of AsyncUDP, but it didn't. And I have no clue why. Do I need to list something somewhere?

elupus commented 1 year ago

You can add dependencies in the .py file that generate too.

ksjoberg commented 1 year ago

I figured out how to reference the AsyncUDP library already in the framework. I've updated the PR to reflect this, the bundled AsyncUDP is now removed. This is working for me, but if it's more reliable remains to be seen.

elupus commented 1 year ago

I pushed a change to your branch (since you opened a pull request on this, i can push to that specific branch on your fork), with changes to allow esp8266 too. Fully untested on both.

I'm a bit afraid of memory leaks. This commit https://github.com/espressif/arduino-esp32/commit/8134a42162f11cb01155038a2465848d6b2b84bc looks to be missing from the version of arduino used for esphome. That looks like a really bad memory leak.

elupus commented 1 year ago

Ah actually nevermind. Those fixes are part of esphome. They have just forgotten to bump the library version in a while.

Let me know how your testing goes. I will need to find a guinea pig for ESP8266 too.

ksjoberg commented 1 year ago

Your changes compiles and runs fine on my ESP32, and it's great that the YAML doesn't need a library reference anymore. Thanks for showing how it's done! It's now running. I'll let it run and see if it can be regarded as reliable. I had issues with the original code occasionally going "Unavailable" but it self-recovered and I regarded it as "normal". When it did not recover anymore, I looked into it deeper. Thanks for being a responsive maintainer!