Read Firmware from SD card and flash to P1 or Photon

Hi,

I am trying to write a script that will allow me to update firmware on a p1 or photon from a .bin file on an SD card. I recently asked a related question here: Using OpenLog to update Particle Firmware, but have since made progress and have a new question.

I can’t use SDFat because we only have access to UART on the current version of our board, so I’m looking trying to create a solution that uses Serial1 to communicate directly with OpenLog.

Below is a gist of what I currently have. It almost works, but the firmware flash is garbled for reasons that I don’t quite understand.

The reason that this is particularly confusing to me is that it appears to be related to when the bytes are instantiated. I reproduced the same bug in a different code base by instantiating binary in the loop instead of before it. In the following gist, one set of code is a working example of code that can be used to flash tinker code onto a photon, and the other fails, simply because const uint8_t tinker[] is defined in flashTinker(), which lives in loop(). Why would this cause an issue?

Thanks in advance for the help!

What exactly does that mean?
If you produce a short code snippet that shows what exactly you mean is usually easier for us to understand than trying to find that in a plethora of code we don't have any context to.

Local variables exist only locally and are not available to functions outside of that scope and especially important for big structs and arrays local variables are stored on the stack which is considerably smaller than the available RAM.

With UART you need to be aware that you are dealing with a 64 (-1) byte buffer to transfer your data. If you fill that buffer too quickly you may start overwriting data before there was any chance for the client to read it.

@ScruffR thanks for your response, and yes, sorry for adding such long code snippets, I just didn’t want to leave out anything that could be relevant!

This (see in context) is what I mean by defined in the loop, and referenced in it, like so:

void setup() { ... }

void loop() {
  flashTinker()
}

void flashTinker() {
  const uint8_t tinker[] = { 0x00, 0x00, 0x0a, 0x08,...}
 ...
// Reference to `tinker` in this function
}

In the working (successfully flashing) example, it is defined before and outside the loop, but is referenced in it, like so:

const uint8_t tinker[] = { 0x00, 0x00, 0x0a, 0x08,...}

void setup() { ... }

void loop() {
  flashTinker()
}

void flashTinker() { 
 ...
// Reference to `tinker` in this function
}

This is interesting, and seems like potentially our issue. The chunk size is currently being set to 512 (64 byte), for the update, but perhaps we are buffering too quickly. How would I prevent this? A short delay? delay(1);, or something like that?

The answer to the local vs. global definition of the byte array is two-fold.
The main part I have already answered in my previous post

And the other point is that global const variables don't consume any of your precious RAM but locals will.

Gotcha. I wasn't connecting the dots there. Thanks for spelling it out! Given that understanding, how do we define a byte array locally without consuming too much RAM? Can we define it globally with an oversized buffer (not as const) then fill it locally?

Yes you can, but you want it properly sized - not oversized :wink:

@ScruffR So how would be properly size it before we know its size? Or do we size it as we fill the byte array?

uint8_t tinker_empty[????]; // What number to put here if we don't know the size of the byte array?

void setup() {
}

void loop() {
  flashTinker();
}
void flashTinker() {
  tinker_empty = {...}; // Or is this where we size it?
  const uint8_t tinker[sizeof(tinker_empty)] = tinker_empty;

``

You do know the size.

Your buffer must be able to take chunk_size bytes since Spark_Save_Firmware_Chunk() expects you to pass in - but IIRC I’ve already elaborated on that in one of the previous threads about that topic.

You cannot store a complete binary in RAM, so you need to read one chunk from your external storage into RAM then write tha chunk to flash and then move on to the next chunk all the way through.

@ScruffR Thanks for your patience here. I’m a bit over my head and you’re being really helpful. There are still some things I’m confused about, either because I’m just not grasping it or because I’m not asking the right questions.

I understand that I am need to buffer the bytes in chunks to Spark_Save_Firmware_Chunk(), but I’m not clear how to do this without creating a local variable that uses RAM.

If the ultimate goal is to read binary of an SD card and flash the firmware, the options appear to be:
A - read binary entirely, store it, and pass to Spark_Save_Firmware_Chunk() in chunks
B - read binary in chunks and pass to Spark_Save_Firmware_Chunk() in chunks OR

I am currently trying option A, but it sounds like getting snagged by the “store it” step. Are you saying that option A is untenable and I need to try option B, or is there another option that I’m not seeing?

Then don't, just go with a gloabl one. 512 byte off of the 3K stack your entire system has to work with would chew a big chunk of that that's better used otherwise.

Option A is out of question since you are probably looking at 40K which you don't have to play with in a considerable project.

So yes, I am

But that's not an unsolvable feat.

@ScruffR

Ok, so if going with a global variable is the only solution, could something like the example (below) work? If so, my question then is how do we define the buffer size of file_bytes? In the example below I provided a buffer size of 4 so that it could compile, but in the actual project, we won't be able to know the buffer size when it is defined, because the size is equal to the size of the firmware that is being flashed, which we won't have access to.

p.s. thanks again for talking this through with me! I'm going to keep scouring the particle ecosystem for examples of how firmware flashing is done. @rickkas7, you used SDFat in your sdfirmwareflash solution. Have you tried doing this just with Serial1?

// Small byte array for example purposes.
// In our project this would come from an SD Card
const uint8_t sd_firmware[] = { 0x00, 0x00, 0x0a, 0x08 }; 

// In this simplified example the buffer size is 4 because that is the size of`sd_firmware`
// In our project, we wouldn't know this, so we wouldn't be able to know the size of the buffer ahead of time
// How do I get the buffer size here?
char file_bytes[4]; 

void readFirmware();

void setup() { 
  Serial.begin(9600); 
}

void loop() {
  readFirmware("sd_firmware");
}

void readFirmware(char *fileName) {
  int file_size = getFileSize(fileName);
  for(int _stop = 0 ; _stop < file_size ; _stop++) {
    int spot = 0;
    while(1) {
      file_bytes[spot++] = Serial1.read();
      if(spot > (file_size - 2)) break;
    }
    file_bytes[spot] = '\0';
    _stop = 0;

    delay(1);
  }
  
  uint8_t file_bytes_ = *file_bytes;
// I'm not using it in this example, but this is where I would pass the bytes to a function that would break the bytes into chunks
// and send them to Spark_Save_Firmware_Chunk
//
// updateFirmwareFromFile(&file_bytes_);
}

int getFileSize(char *fileName) {
  // In our project, we would be able to determine this value
  return sizeof(sd_firmware);
}

You get the file size of your file system, but that hasn’t got anything to do with the file_bytes buffer. Its size is - as already said - set by the requirements of the receiving function (e.g. Spark_Save_Firmware_Chunk()).
So you do something like this

  • get file size (e.g. 2000byte)
  • read 512 byte chunk into buffer and send to Spark_Save_Firmware_Chunk()
  • remaining bytes to read from file (2000 - 512 = 1488)
  • read 512 byte chunk into buffer and send to Spark_Save_Firmware_Chunk()
  • remaining bytes to read from file (1488 - 512 = 976)
  • read 512 byte chunk into buffer and send to Spark_Save_Firmware_Chunk()
  • remaining bytes to read from file (976 - 512 = 464)
  • read 464 byte chunk into buffer and send to Spark_Save_Firmware_Chunk() with shortend chunk size parameter

What you’re describing is what (I believe) I’m doing in this script (relevant lines highlighted), but the firmware transfer is failing.

Is there something basic that I’m missing in that code example?

Not quite.
Your while() only stops when all but two file_size bytes are read when it should stop when spot >= sizeof(file_bytes).
And don’t at a final '\0' outside of the array. For one that will corrupt data not belonging to your array and you are not dealing with strings but binary data. Binary data needn’t and cannot be terminated that way since '\0' may be anywhere inside the byte stream too.

In what way is it failing?
When you are receiving your data via Serial1 how do you tell the device sending the data to stop sending while you are flashing the data away (which is not shown in your code)?

Huh. Ok, I'll try these things and see if it changes anything.

This is why I was posting the simplified tinker code example, because the failed tinker
It appears to fail to return a result on line 91, after many chunks:

int result = Spark_Prepare_For_Firmware_Update(file, 0, NULL);

Here is successful output:

0000000643 [hal.wlan] INFO: Using external antenna
0000000668 [hal.wlan] INFO: Joining CBCI-29C9-2.4
0000000668 [hal.wlan] TRACE: Free RAM connect: 55848
0000003859 [hal.wlan] INFO: Bringing WiFi interface up with DHCP
starting flash size=3952
chunk_size=512 file_address=0x80c0000
offset=0 chunk_address=0x80c0000 chunk_size=512 result=0
offset=512 chunk_address=0x80c0200 chunk_size=512 result=0
offset=1024 chunk_address=0x80c0400 chunk_size=512 result=0
offset=1536 chunk_address=0x80c0600 chunk_size=512 result=0
...
offset=261120 chunk_address=0x80ffc00 chunk_size=512 result=0
offset=261632 chunk_address=0x80ffe00 chunk_size=512 result=0
Serial connection closed.  Attempting to reconnect...

And here it is failing. You'll noticed that the final Spark_Prepare_For_Firmware_Update function call never returns 0 AND it ends sending chunks early.

Serial monitor opened successfully:
0000000642 [hal.wlan] INFO: Using external antenna
0000000667 [hal.wlan] INFO: Joining CBCI-29C9-2.4
0000000667 [hal.wlan] TRACE: Free RAM connect: 55848
0000007672 [hal.wlan] ERROR: wiced_join_ap_specific(), result: 1024
0000007672 [hal.wlan] INFO: Joining CBCI-29C9-2.4
0000007673 [hal.wlan] TRACE: Free RAM connect: 55848
0000010870 [hal.wlan] INFO: Bringing WiFi interface up with DHCP
starting flash size=3952
chunk_size=512 file_address=0x80c0000
offset=0 chunk_address=0x80c0000 chunk_size=512 result=0
offset=512 chunk_address=0x80c0200 chunk_size=512 result=0
offset=1024 chunk_address=0x80c0400 chunk_size=512 result=0
offset=1536 chunk_address=0x80c0600 chunk_size=512 result=0
...
offset=121344 chunk_address=0x80dda00 chunk_size=512 result=0
offset=121856 chunk_address=0x80ddc00 chunk_size=512Serial connection closed.  Attempting to reconnect...

One think that isn't clear to me is why there are so many chunks, there should really only be 8. Maybe this is related to this question:

You should add some debug output inside your loop to check why it is not breaking out as it should after the 8th iteration.
In particular you should watch this line which obviously is not working correctly

while(offset < file.file_length) {

So adapting your log output like this should give some extra clue

    Serial.printlnf("chunk_address=0x%x chunk_size=%d (%d < %d == %s)"
    , file.chunk_address, file.chunk_size
    , offset, file.file_length, (offset < file.file_length) ? "continue" : "break");

BTW, your log output doesn't seem to fit your code.
You have a bunch of these lines

offset=0 chunk_address=0x80c0000 chunk_size=512
offset=512 chunk_address=0x80c0200 chunk_size=512
offset=1024 chunk_address=0x80c0400 chunk_size=512

But your posted code seems to only print chunk_address and chunk_size but not offset. Where does the offset come from?
If you post a log you need to post the exact code that produces that log otherwise it's utterly useless.

@ScruffR thanks for your insights. Based on your past two comments, I've made some progress, but I'm still seeing the following errors for the code in this gist:

...
starting flash size=3832
chunk_size=512 file_address=0x80c0000
chunk_address=0x80c0000 chunk_size=512 (0 < 3832 == continue)
chunk_address=0x80c0200 chunk_size=512 (512 < 3832 == continue)
chunk_address=0x80c0400 chunk_size=512 (1024 < 3832 == continue)
chunk_address=0x80c0600 chunk_size=512 (1536 < 3832 == continue)
chunk_address=0x80c0800 chunk_size=512 (2048 < 3832 == continue)
chunk_address=0x80c0a00 chunk_size=512 (2560 < 3832 == continue)
chunk_address=0x80c0c00 chunk_size=512 (3072 < 3832 == continue)
chunk_address=0x80c0e00 chunk_size=248 (3584 < 3832 == continue)
finish failed 1

Also, I was previously seeing a WARN: OTA module not applied warning, but

...
chunk_address=0x80c0e00 chunk_size=248 (3584 < 3832 == continue)
0000017055 [hal] WARN: OTA module not applied
finish failed 1

The file is 3832 bytes, and it appears that every chunk was sent. There aren't any great errors to go off of, so I'm still a bit miffed. I'm going to apply the same changes to the simpler tinker flashing code to see if that simplified codebase can shed any light on this. In the meantime, what do you think a good step would be to take? I'm still unclear what to about your previous post:

@ScruffR One oddity is that file.file_length is assigned the value of fileSize

file.file_length = fileSize; 

Because fileSize is 3832, file.file_length should return 3832, but instead it routinely returns 262144. I’m guessing this has some impact on whether or not the file is flashed properly.

Oddly enough, 262144 = 512^2, so perhaps it is being manipulated by the chunk size? I’m going to dig into this more. Here is the code for that function:

void updateFirmwareFromFile(uint8_t* file_bytes) {
  delay(5000);

  FileTransfer::Descriptor file;

  Serial.printlnf("starting flash size=%d", fileSize);

  file.file_length = fileSize;
  file.file_address = 0; // Automatically set to HAL_OTA_FlashAddress if store is FIRMWARE
  file.chunk_address = 0;
  file.chunk_size = 0; // use default
  file.store = FileTransfer::Store::FIRMWARE;

  int result = Spark_Prepare_For_Firmware_Update(file, 0, NULL);
  if (result != 0) {
    Serial.printlnf("prepare failed %d", result);
    return;
  }

  Serial.printlnf("chunk_size=%d file_address=0x%x fileSize=%d", file.chunk_size, file.file_address, fileSize);
  if (file.chunk_size == 0) {
    file.chunk_size = 512;
  }

  // Note that Spark_Prepare_For_Firmware_Update sets file.file_address so it's not really zero here
  // even though it's what we initialize it to above!
  file.chunk_address = file.file_address;

  size_t offset = 0;

  while(offset < fileSize) {
    if (file.chunk_size > (fileSize - offset)) {
      file.chunk_size = (fileSize - offset);
    }

    Serial.printlnf("chunk_address=0x%x chunk_size=%d (%d < %d == %s)", file.chunk_address, file.chunk_size, offset, file.file_length, (offset < file.file_length) ? "continue" : "break");
    result = Spark_Save_Firmware_Chunk(file, &file_bytes[offset], NULL);

    if (result != 0) {
      Serial.printlnf("save chunk failed %d", result);
      return;
    }
    file.chunk_address += file.chunk_size;
    offset += file.chunk_size;
  }

  result = Spark_Finish_Firmware_Update(file, 0x1, NULL);
  if (result != 0) {
    Serial.printlnf("finish failed %d", result);
    return;
  }

  Serial.printlnf("update complete");

}

And here is the output:

starting flash size=3832
chunk_size=512 file_address=0x80c0000 fileSize=3832
chunk_address=0x80c0000 chunk_size=512 (0 < 262144 == continue)
chunk_address=0x80c0200 chunk_size=512 (512 < 262144 == continue)
chunk_address=0x80c0400 chunk_size=512 (1024 < 262144 == continue)
chunk_address=0x80c0600 chunk_size=512 (1536 < 262144 == continue)
chunk_address=0x80c0800 chunk_size=512 (2048 < 262144 == continue)
chunk_address=0x80c0a00 chunk_size=512 (2560 < 262144 == continue)
chunk_address=0x80c0c00 chunk_size=512 (3072 < 262144 == continue)
chunk_address=0x80c0e00 chunk_size=248 (3584 < 262144 == continue)
0000020648 [hal] WARN: OTA module not applied
finish failed 1

@gemfarmer Were you ever able to get this working? I’m getting the same error in my own code.