JAndrassy / ArduinoOTA

Arduino library to upload sketch over network to Arduino board with WiFi or Ethernet libraries
GNU Lesser General Public License v2.1
450 stars 91 forks source link

Uncaught out of memory error? #132

Open liamcollins opened 2 years ago

liamcollins commented 2 years ago

I'm uploading to a SAMD board (Automation Direct P1AM-100) using a very slight adeptation of the example that pulls from a web address.

Smaller binary files work just fine. My final sketch/binary is 126632 bytes (48% of available mem) and InternalStorage.open(length) seems to run just fine but then the following loop crashes:

  while (length > 0) {
    if (!client.readBytes(&b, 1)) { // reading a byte with timeout
      Serial.println(F("Breaking"));
      break;
    }
    InternalStorage.write(b);
    length--;
    Serial.println(length);
  }

I see length come down to the same number every time (133 at the current sketch size) and then nothing after that.

Any thoughts about what might be going on here? I'd prefer if this wasn't the upper limit of the size of my sketch.

JAndrassy commented 2 years ago

what networking library?

liamcollins commented 2 years ago

ArduinoHttpClient

liamcollins commented 2 years ago

Here's my whole sketch:

/*
  This example downloads sketch update over network.
  It doesn't start the OTA upload sever of the ArduinoOTA library,
  it only uses the InternalStorage object of the library
  to store and apply the downloaded binary file.
  To create the bin file for update of a SAMD board (except of M0),
  use in Arduino IDE command "Export compiled binary".
  To create a bin file for AVR boards see the instructions in README.MD.
  To try this example, you should have a web server where you put
  the binary update.
  Modify the constants below to match your configuration.
  Created for ArduinoOTA library in February 2020
  by Juraj Andrassy
*/

#include <ArduinoOTA.h> // only for InternalStorage
#include <Ethernet.h>
#include <ArduinoHttpClient.h>

const short VERSION = 1;

byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };

#ifdef ARDUINO_SAM_ZERO // M0
#define Serial SerialUSB
#endif

void handleSketchDownload() {

  const char* SERVER = "<public-s3-bucket-here>"; // must be string for HttpClient
  const unsigned short SERVER_PORT = 80;
  const char* PATH = "/update-v8.bin";
  const unsigned long CHECK_INTERVAL = 5000;

  static unsigned long previousMillis;

  unsigned long currentMillis = millis();

  if (currentMillis - previousMillis < CHECK_INTERVAL)
    return;
  previousMillis = currentMillis;

  EthernetClient transport;
  HttpClient client(transport, SERVER, SERVER_PORT);

  char buff[32];
  snprintf(buff, sizeof(buff), PATH, VERSION + 1);

  Serial.print("Check for update file ");
  Serial.println(buff);

  client.get(buff);

  int statusCode = client.responseStatusCode();
  Serial.print("Update status code: ");
  Serial.println(statusCode);
  if (statusCode != 200) {
    client.stop();
    return;
  }

  long length = client.contentLength();
  if (length == HttpClient::kNoContentLengthHeader) {
    client.stop();
    Serial.println("Server didn't provide Content-length header. Can't continue with update.");
    return;
  }
  Serial.print("Server returned update file of size ");
  Serial.print(length);
  Serial.println(" bytes");

  if (!InternalStorage.open(length)) {
    client.stop();
    Serial.println("There is not enough space to store the update. Can't continue with update.");
    return;
  }
  byte b;
  while (length > 0) {
    if (!client.readBytes(&b, 1)) { // reading a byte with timeout
      Serial.println(F("Breaking"));
      break;
    }
    InternalStorage.write(b);
    length--;
    Serial.println(length);
  }
  Serial.println(F("Closing storage"));
  InternalStorage.close();
  Serial.println(F("Stopping client"));
  client.stop();

  if (length > 0) {
    Serial.print("Timeout downloading update file at ");
    Serial.print(length);
    Serial.println(" bytes. Can't continue with update.");
    return;
  }

  Serial.println("Sketch update apply and reset.");
  Serial.flush();
  InternalStorage.apply(); // this doesn't return
}

void setup() {

  Serial.begin(115200);
  while (!Serial);

  Serial.print("Sketch version ");
  Serial.println(VERSION);

  Serial.println("Initialize Ethernet with DHCP:");
  if (Ethernet.begin(mac) == 0) {
    Serial.println("Failed to configure Ethernet using DHCP");
    while (true);
  }
  Serial.println("Ethernet connected");
}

void loop() {
  // check for updates
  handleSketchDownload();

  // add your normal loop code below ...
  Serial.println("version 2");
  delay(3000);
}
JAndrassy commented 2 years ago

and it stops after 133 bytes?

liamcollins commented 2 years ago

No it stops after ~127k bytes, with 133 bytes left to go.

JAndrassy commented 2 years ago

the Serial.println(length); can cause problems. it slows down the download significantly

liamcollins commented 2 years ago

Yeah but it was broken before that, I just added that to help diagnose what was going on.

JAndrassy commented 2 years ago

I guess it breaks even if you comment out all InternalStorage lines?

liamcollins commented 2 years ago

if I comment out InternalStorage.write(b); it runs without error (although obviously then there is no code binary file to apply at the end).

liamcollins commented 2 years ago

correct. Seems to fail right around 127k bytes, which is just under half the available memory.

JAndrassy commented 2 years ago

InternalStorage.write writes directly to flash memory. client.read reads directly from the Wiznet chip over SPI so there is no way to run out of memory.

liamcollins commented 2 years ago

hmm, weird. Any other thoughts about what might be happening then?

JAndrassy commented 2 years ago

I suspect some timeout. I would try to use a buffer to read more bytes from client at once. It will make reading over SPI more efficient and the whole process faster.