oarcher / piotech

my custom components for esphome
18 stars 10 forks source link

Fix for incorrect status check from builtin HA http server when a file is removed (http status 200 but body_lenght == -1) #6

Closed kilrah closed 12 months ago

kilrah commented 1 year ago

Not sure why but my ESP32C3 boards would just crash/reboot if the file didn't exist at the provided URL (size returned as -1)

This fixes it, might be a cleaner solution though...

oarcher commented 1 year ago

Thank you for this PR!

Can you confirm that the message "[ota_http:135]: HTTP Request failed; URL: http://example.com/notexists.bin; Error: (404)" is not displayed ?

If so, it's strange that the 404 is not fetched by the code for your device.

kilrah commented 1 year ago

OK it seems to be an interaction with the web server... I'm using HA's built-in server (https://www.home-assistant.io/integrations/http/#hosting-files), after booting HA I get the normal 404, but if I put a file and then remove it it goes to being "there" but with size -1 until HA is restarted.

kilrah commented 1 year ago

In the attached serial log is first a 404, then a "file present", then "file removed", looks like the problem isn't really the -1 sized file, but that the WDT times out while waiting for something

On the web logs it shows up like this:

[14:56:55][D][ota_http:098]: Trying to connect to http://ha-bracelets.local:8123/local/bracelets_firmware/b666.bin
[14:56:55][V][ota_http:112]: http begin successfull.
[14:56:55][VV][ota_http:116]: http client setReuse.
[14:56:55][V][ota_http:121]: md5sum from received data initialized.
[14:56:55][V][ota_http:125]: http headers collected.
[14:56:55][V][ota_http:131]: http GET finished.
[14:56:55][D][ota_http:140]: firmware is -1 bytes length.
[14:56:55][D][ota_http:042]: Using ArduinoESP32OTABackend
[14:56:55][VV][ota_http:153]: Got esp32 stream

missing_file.txt

oarcher commented 1 year ago

I've just changed the error message. Are you able to test my change before I merge ?

kilrah commented 1 year ago

Did not work... I believe the problem is the body_length = client_.getSize();, body_length is of size_t and unless esphome redefines that which doesn't seem to be the case from a quick check it would be unsigned, causing the body_length < 0 comparison to fail.

I've restored the test directly to client_.getSize()...

oarcher commented 1 year ago

You're right, size_t is unsigned. We could switch body_lenght to long, but we should cast it after:

size_t bufsize = std::min(chunk_size, size_t(body_length) - bytes_read);

So using your solution by keeping client_.getSize() is better.

kilrah commented 12 months ago

Was it intentional that this hasn't been merged yet?

oarcher commented 12 months ago

oops, indeed, I forgot to merge!