Fix for FW hashe not being calculated correctly on NRF52

Device.h

@@ -141,6 +141,38 @@ void device_save_firmware_hash() {
 }
 
 #if MCU_VARIANT == MCU_NRF52
+#define FW_LENGTH_LEN 4
+
+#if MCU_VARIANT == MCU_NRF52
+    void set_fw_length(uint8_t* length_array) {
+        fw_length_file.open(FW_LENGTH_FILE, FILE_O_WRITE);
+
+        fw_length_file.seek(0);
+
+        fw_length_file.write(length_array, FW_LENGTH_LEN);
+
+        fw_length_file.close();
+
+        hard_reset();
+    }
+
+    unsigned long get_fw_length() {
+        fw_length_file.open(FW_LENGTH_FILE, FILE_O_READ);
+
+        // If file doesn't exist yet
+        if (!fw_length_file) {
+            return 0;
+        }
+
+        uint8_t length_array[4];
+
+        fw_length_file.read(length_array, 4);
+
+        unsigned long length = (length_array[0] << 24) | (length_array[1] << 16) | (length_array[2] << 8) | length_array[3];
+        return length;
+    }
+#endif
+
 void calculate_region_hash(unsigned long long start, unsigned long long end, uint8_t* return_hash) {
     // this function calculates the hash digest of a region of memory,
     // currently it is only designed to work for the application region
@@ -158,10 +190,10 @@ void calculate_region_hash(unsigned long long start, unsigned long long end, uin
     int end_count = 0;
     unsigned long length = 0;
 
-    while (start < end - 1 ) {
+    while (start < end) {
         const void* src = (const void*)start;
         if (start + CHUNK_SIZE >= end) {
-            size = (end - 1) - start;
+            size = end - start;
         }
         else {
             size = CHUNK_SIZE;
@@ -169,74 +201,14 @@ void calculate_region_hash(unsigned long long start, unsigned long long end, uin
 
         memcpy(chunk, src, CHUNK_SIZE);
 
-        // check if we've reached the end of the program
-        // if we're checking the application region
         if (application) {
             for (int i = 0; i < CHUNK_SIZE; i++) {
-                if (chunk[i] == 0xFF) {
+                // do nothing
-                    bool matched = true;
-                    end_count = 1;
-                    // check if rest of chunk is FFs as well, only if FF is not
-                    // at the end of chunk
-                    if (i < CHUNK_SIZE - 1) {
-                        for (int x = 0; x < CHUNK_SIZE - i; x++) {
-                            if (chunk[i+x] != 0xFF) {
-                                matched = false;
-                                break;
-                            }
-                            end_count++;
-                        }
-                    }
-
-                    if (matched) {
-                        while (end_count < END_SECTION_SIZE) {
-                            // check if bytes in next chunk up to total
-                            // required are also FFs
-                            for (int x = 1; x <= ceil(END_SECTION_SIZE / CHUNK_SIZE); x++) {
-                                const void* src_next = (const void*)start + CHUNK_SIZE*x;
-                                if ((END_SECTION_SIZE - end_count) > CHUNK_SIZE) {
-                                    size = CHUNK_SIZE;
-                                } else {
-                                    size = END_SECTION_SIZE - end_count;
-                                }
-                                memcpy(chunk_next, src_next, size);
-                                for (int y = 0; y < size; y++) {
-                                    if (chunk_next[y] != 0xFF) {
-                                        matched = false;
-                                        break;
-                                    }
-                                    end_count++;
-                                }
-
-                                if (!matched) {
-                                    break;
-                                }
-                            }
-                            if (!matched) {
-                                break;
-                            }
-                        }
-
-                        if (matched) {
-                            finish = true;
-                            size = i;
-                            break;
-                        }
-                    }
-                }
             }
-        }
-
-        if (finish) {
             hash.update(chunk, size);
             length += size;
-            break;
+            start += CHUNK_SIZE;
-        } else {
-            hash.update(chunk, size);
         }
-
-        start += CHUNK_SIZE;
-        length += CHUNK_SIZE;
     }
     hash.end(return_hash);
 }
@@ -257,7 +229,7 @@ void device_validate_partitions() {
   esp_partition_get_sha256(esp_ota_get_running_partition(), dev_firmware_hash);
   #elif MCU_VARIANT == MCU_NRF52
   // todo, add bootloader, partition table, or softdevice?
-  calculate_region_hash(APPLICATION_START, USER_DATA_START, dev_firmware_hash);
+  calculate_region_hash(APPLICATION_START, APPLICATION_START+get_fw_length(), dev_firmware_hash);
   #endif
   #if VALIDATE_FIRMWARE
     for (uint8_t i = 0; i < DEV_HASH_LEN; i++) {

Framing.h

@@ -73,6 +73,7 @@
   #define ROM_UNLOCK_BYTE 0xF8
   #define CMD_RESET       0x55
   #define CMD_RESET_BYTE  0xF8
+  #define CMD_FW_LENGTH   0x65
 
   #define CMD_INTERFACES  0x64
 

Makefile

@@ -202,6 +202,11 @@ upload-rak4631:
 	arduino-cli upload -p /dev/ttyACM0 --fqbn rakwireless:nrf52:WisCoreRAK4631Board
 	unzip -o build/rakwireless.nrf52.WisCoreRAK4631Board/RNode_Firmware_CE.ino.zip -d build/rakwireless.nrf52.WisCoreRAK4631Board
 	rnodeconf /dev/ttyACM0 --firmware-hash $$(sha256sum ./build/rakwireless.nrf52.WisCoreRAK4631Board/RNode_Firmware_CE.ino.bin | grep -o '^\S*')
+	@echo
+	@echo This target currently uses a custom version of rnodeconf to set the firmware length on the device.
+	@echo This will be removed once the feature has been included upstream, or another solution has been found.
+	@echo
+	python3 rnodeconf.py /dev/ttyACM0 --set-firmware-length $$(ls -l ./build/rakwireless.nrf52.WisCoreRAK4631Board/RNode_Firmware_CE.ino.bin | awk '{print $$5}')
 
 
 

RNode_Firmware_CE.ino

@@ -1028,6 +1028,21 @@ void serialCallback(uint8_t sbyte) {
         }
 
       #endif
+    } else if (command == CMD_FW_LENGTH) {
+        if (sbyte == FESC) {
+              ESCAPE = true;
+          } else {
+              if (ESCAPE) {
+                  if (sbyte == TFEND) sbyte = FEND;
+                  if (sbyte == TFESC) sbyte = FESC;
+                  ESCAPE = false;
+              }
+              if (frame_len < CMD_L) cmdbuf[frame_len++] = sbyte;
+          }
+
+          if (frame_len == FW_LENGTH_LEN) {
+            set_fw_length(cmdbuf);
+          }
     }
   }
 }

Utilities.h

@@ -27,9 +27,11 @@
     #include <InternalFileSystem.h>
     using namespace Adafruit_LittleFS_Namespace;
     #define EEPROM_FILE "eeprom"
+    #define FW_LENGTH_FILE "fw_length"
     bool file_exists = false;
     int written_bytes = 4;
-    File file(InternalFS);
+    File eeprom_file(InternalFS);
+    File fw_length_file(InternalFS);
 #endif
 #include <stddef.h>
 
@@ -1159,21 +1161,21 @@ void promisc_disable() {
     bool eeprom_begin() {
         InternalFS.begin();
 
-        file.open(EEPROM_FILE, FILE_O_READ);
+        eeprom_file.open(EEPROM_FILE, FILE_O_READ);
 
         // if file doesn't exist
-        if (!file) {
+        if (!eeprom_file) {
-            if (file.open(EEPROM_FILE, FILE_O_WRITE)) {
+            if (eeprom_file.open(EEPROM_FILE, FILE_O_WRITE)) {
                 // initialise the file with empty content
                 uint8_t empty_content[EEPROM_SIZE] = {0};
-                file.write(empty_content, EEPROM_SIZE);
+                eeprom_file.write(empty_content, EEPROM_SIZE);
                 return true;
             } else {
                 return false;
             }
         } else {
-            file.close();
+            eeprom_file.close();
-            file.open(EEPROM_FILE, FILE_O_WRITE);
+            eeprom_file.open(EEPROM_FILE, FILE_O_WRITE);
             return true;
         }
     }
@@ -1181,8 +1183,8 @@ void promisc_disable() {
     uint8_t eeprom_read(uint32_t mapped_addr) {
         uint8_t byte;
         void* byte_ptr = &byte;
-        file.seek(mapped_addr);
+        eeprom_file.seek(mapped_addr);
-        file.read(byte_ptr, 1);
+        eeprom_file.read(byte_ptr, 1);
         return byte;
     }
 #endif
@@ -1243,8 +1245,8 @@ void kiss_dump_eeprom() {
 #if !HAS_EEPROM && MCU_VARIANT == MCU_NRF52
 void eeprom_flush() {
     // sync file contents to flash
-    file.close();
+    eeprom_file.close();
-    file.open(EEPROM_FILE, FILE_O_WRITE);
+    eeprom_file.open(EEPROM_FILE, FILE_O_WRITE);
     written_bytes = 0;
 }
 #endif
@@ -1260,11 +1262,11 @@ void eeprom_update(int mapped_addr, uint8_t byte) {
         // each time is really slow, but this is also suboptimal
         uint8_t read_byte;
         void* read_byte_ptr = &read_byte;
-        file.seek(mapped_addr);
+        eeprom_file.seek(mapped_addr);
-        file.read(read_byte_ptr, 1);
+        eeprom_file.read(read_byte_ptr, 1);
-        file.seek(mapped_addr);
+        eeprom_file.seek(mapped_addr);
         if (read_byte != byte) {
-            file.write(byte);
+            eeprom_file.write(byte);
         }
         written_bytes++;
         
@@ -1274,8 +1276,8 @@ void eeprom_update(int mapped_addr, uint8_t byte) {
         }
 
         if (written_bytes >= 4) {
-            file.close();
+            eeprom_file.close();
-            file.open(EEPROM_FILE, FILE_O_WRITE);
+            eeprom_file.open(EEPROM_FILE, FILE_O_WRITE);
             written_bytes = 0;
         }
 	#endif