Skip to content
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

Ota bin receive bug #14

Closed

Conversation

sverreknutsen
Copy link
Contributor

Hello, While using this library to download an ota binary from GitHub instead of the Arduino server in the example I encountered a bug in row 185 of src/Arduino_ESP32_OTA.cpp: _client->read(_ota_header.buf, sizeof(OtaHeader));

In 90% of the tests I did _client->available() returned 0 at the point in time of execution of row 185. The call to this function do not wait for more data to be received, hence the download failed. This did not happen for the Arduino server. I have implemented a solution similar to what is used in lzss_download() which uses the same time out value to fix this.

Also I added a new example downloading bin files form this GitHub repo since I imagine this server would be the go to for most users. Since the bin files does not exist on master yet the example will not work until merged. If you want to test you should be able to use a link to my fork instead:
https://raw.githubusercontent.com/sverreknutsen/Arduino_ESP32_OTA/ota_bin_recieve_bug/examples/LOLIN_32_Blink/LOLIN_32_Blink.ino.ota

I hope this is helpful. Feel free to do what you want with this pull request.

@CLAassistant
Copy link

CLAassistant commented Sep 8, 2023

CLA assistant check
All committers have signed the CLA.

@sverreknutsen sverreknutsen force-pushed the ota_bin_recieve_bug branch 2 times, most recently from 5cafd8d to 57d0af1 Compare September 8, 2023 19:02
@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Memory usage change @ 57d0af1

Board flash % RAM for global variables %
arduino:esp32:nano_nora 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/OTA_Arduino_Server
flash
% examples/OTA_Arduino_Server
RAM for global variables
% examples/OTA_GitHub_Server
flash
% examples/OTA_GitHub_Server
RAM for global variables
% examples/NANO_ESP32_Blink
flash
% examples/NANO_ESP32_Blink
RAM for global variables
%
arduino:esp32:nano_nora N/A N/A N/A N/A N/A N/A N/A N/A 0 0.0 0 0.0
Click for full report CSV
Board,examples/OTA_Arduino_Server<br>flash,%,examples/OTA_Arduino_Server<br>RAM for global variables,%,examples/OTA_GitHub_Server<br>flash,%,examples/OTA_GitHub_Server<br>RAM for global variables,%,examples/NANO_ESP32_Blink<br>flash,%,examples/NANO_ESP32_Blink<br>RAM for global variables,%
arduino:esp32:nano_nora,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,0,0.0,0,0.0

@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Memory usage change @ 1971262

Board flash % RAM for global variables %
arduino:esp32:nano_nora 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
esp32:esp32:esp32 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/OTA_Arduino_Server
flash
% examples/OTA_Arduino_Server
RAM for global variables
% examples/OTA_GitHub_Server
flash
% examples/OTA_GitHub_Server
RAM for global variables
% examples/NANO_ESP32_Blink
flash
% examples/NANO_ESP32_Blink
RAM for global variables
% examples/LOLIN_32_Blink
flash
% examples/LOLIN_32_Blink
RAM for global variables
%
arduino:esp32:nano_nora N/A N/A N/A N/A N/A N/A N/A N/A 0 0.0 0 0.0
esp32:esp32:esp32 N/A N/A N/A N/A N/A N/A N/A N/A 0 0.0 0 0.0
Click for full report CSV
Board,examples/OTA_Arduino_Server<br>flash,%,examples/OTA_Arduino_Server<br>RAM for global variables,%,examples/OTA_GitHub_Server<br>flash,%,examples/OTA_GitHub_Server<br>RAM for global variables,%,examples/NANO_ESP32_Blink<br>flash,%,examples/NANO_ESP32_Blink<br>RAM for global variables,%,examples/LOLIN_32_Blink<br>flash,%,examples/LOLIN_32_Blink<br>RAM for global variables,%
arduino:esp32:nano_nora,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,0,0.0,0,0.0
esp32:esp32:esp32,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,,,,,0,0.0,0,0.0

@pennam
Copy link
Collaborator

pennam commented Sep 11, 2023

Thanks for this PR @sverreknutsen i will test it and merge it as soon as possible!

@pennam
Copy link
Collaborator

pennam commented Oct 2, 2023

merged as #15

@pennam pennam closed this Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants