Arduino and LoRa. Not all bytes are received

I am working with Arduino and Lora. Arduino A has a camera, radio and SD card. Arduino B has the same setup minus the camera.

The objective is to send a picture from A to B. The following code is supposed to read 32 bytes from SD card A (which it does wonderfully thanks to mmixLinus ), and send via LoRa to Arduino B, which saves those 32 bytes to its own SD card. The file will be sent 32 bytes at a time, until all its 53k arrive.

After some troubleshooting, and thinking that I was losing packets I decided to count the number sent vs the one received. They match.

I also sent to console each packet's byte count. They match.

However the resulting file saved in the receiving SD card is smaller than the original on the sending SD card. It is unusable.

Something I have noticed is that I was expecting each packet to be 32 bytes, except the last one that could be smaller. However, even when the bytes sent and received match, i am unsure why my code would not take the full 32 bytes per file read.

These are my main questions.

Can you help me spot what am I doing wrong?
What would be an ideally simple way to implement error checking, making sure that every packet sent is checked and validated when received, and letting know the sender that it can send the next package?

This is the sender's code.

oFile = SD.open("ring.jpg", FILE_READ); //open source file for read.
long cByteTotal = oFile.size(); //get total bytes
Serial.print("Original size: ");
Serial.println(cByteTotal);
while (cByteTotal > 0) {
  int length = cByteTotal < 32 ? cByteTotal : 32;
  if (length < 32) {
    Serial.println("Sending last packet."); //info
  }
  oFile.read(cArray, length); //read 32 bytes from file
  cArray[length] = '\0'; // terminate the char array
  sendMessage(String(cArray)); //send the file via Lora after casting to string
  delay(250); // just in case to give time to receiver to process
  cByteTotal -= length;
}
oFile.close();
Serial.println("Done Sending.");

void sendMessage(String cArray) {
  LoRa.beginPacket();
  LoRa.write(cArray.length());
  LoRa.write(cArray);
  LoRa.endPacket();
}

This is the receiver's code.

void loop() {
  onReceive(LoRa.parsePacket());
}

void onReceive(int packetSize) {
  if (packetSize == 0) {
    return;
  }
  packageTotal++;
  byte incomingLength = LoRa.read();
  String incoming = "";
  while (LoRa.available()) {
    incoming += (char)LoRa.read();
  }
  if (incomingLength != incoming.length()) {
    // check length for error
    Serial.println("LENGHT [ERROR]"); // this does not get triggered, which is good.
    return; // skip rest of function
  } else {
    cFile = SD.open("wimage.jpg", FILE_WRITE);
    cFile.print(incoming);
    cFile.close();
  }
}

I am using the following Lora library

Thank you for your help!

Solution

The main error you are making here is that you are treating the file contents as a String, which you should definitely not. A binary file can have 0x00, the String terminator, which would definitely ruin your lunch.

What you need to do first is treat the 32-byte chunks as they should be, as binary blobs, and use write(const uint8_t *buffer, size_t size);. Also, you are using LoRa.write(cArray.length());, which doesn't do much of anything: it doesn't tell the LoRa library how many bytes you are sending, and the receiver knows anyway how many bytes were received. Your function should look like this:

void sendMessage(String cArray) {
  LoRa.beginPacket();
  LoRa.write(cArray, cArray.length());
  LoRa.endPacket();
}

Next up, transmission per se. You are transmitting in a while loop, with a very small delay, way too small for at least 2 reasons, and without checking whether the receiver has indeed received chunk #x, and received it properly. This will cause so many issues...

You already have a loop, the aptly-named loop(), so you don't need another. Instead, inside loop, read 32 bytes from the card, send them in a packet that includes chunk number, data, and basic CRC. For example, add every byte in the packet, and use this, as a 16-bit number, as the last 2 bytes of the packet. You'd have a structure of 2 [counter] + 32 [data] + 2 [CRC].

Then switch to "wait for confirmation" mode. Until you hear back from your receiver, sending you 4 bytes, 2 for the index, 2 for the CRC, you don't do anything. If the CRC sent back is wrong, you don't increment the counter. If the CRC is right, increment. Then you switch to sending mode. Rinse and repeat.

On the receiver, you are in standby mode. When you receive a packet, you calculate the CRC and compare – if it's incorrect, you send that back, and go back to receive mode. If it's correct, you save the 32 bytes in the properly position inside a buffer. And send back index+CRC. And at the end you save.

Oh and, it's recEIve, not recieve...