Fix read-flash which didn't work with some lengths (#804)

* wip

* finalize + hil test

hil test

* changelog entry

* fix

Author: playfulFence

.github/workflows/hil.yml

@@ -132,3 +132,6 @@ jobs:
 
       - name: write-bin test
         run: timeout 20 bash espflash/tests/scripts/write-bin.sh
+
+      - name: read-flash test
+        run: timeout 20 bash espflash/tests/scripts/read-flash.sh

CHANGELOG.md

@@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix `-s` argument collision (#731)
 - `address` and `size` in `erase-region` have to be multiples of 4096 (#771)
 - Fixed typos in error variant names (#782)
+- Fix `read-flash` which didn't work with some lengths (#804)
 
 ### Removed
 

espflash/src/connection/mod.rs

@@ -347,12 +347,35 @@ impl Connection {
         result
     }
 
+    /// Read the response from a serial port
+    pub fn read_flash_response(&mut self) -> Result<Option<CommandResponse>, Error> {
+        let mut response = Vec::new();
+
+        self.decoder.decode(&mut self.serial, &mut response)?;
+
+        if response.is_empty() {
+            return Ok(None);
+        }
+        let value = CommandResponseValue::Vector(response.clone());
+
+        let header = CommandResponse {
+            resp: 1_u8,
+            return_op: CommandType::ReadFlash as u8,
+            return_length: response.len() as u16,
+            value,
+            error: 0_u8,
+            status: 0_u8,
+        };
+
+        Ok(Some(header))
+    }
+
     /// Read the response from a serial port
     pub fn read_response(&mut self) -> Result<Option<CommandResponse>, Error> {
         match self.read(10)? {
             None => Ok(None),
             Some(response) => {
-                // Here is what esptool does: https://github.com/espressif/esptool/blob/master/esptool/loader.py#L458
+                // Here is what esptool does: https://github.com/espressif/esptool/blob/81b2eaee261aed0d3d754e32c57959d6b235bfed/esptool/loader.py#L518
                 // from esptool: things are a bit weird here, bear with us
 
                 // We rely on the known and expected response sizes which should be fine for now

espflash/src/error.rs

@@ -87,7 +87,7 @@ pub enum Error {
     #[diagnostic(code(espflash::read_flash::incorrect_digest_length))]
     IncorrectDigestLength(usize),
 
-    #[error("Incorrect response from the sutb/ROM loader")]
+    #[error("Incorrect response from the stub/ROM loader")]
     #[diagnostic(code(espflash::read_flash::incorrect_response))]
     IncorrectResponse,
 

espflash/src/flasher/mod.rs

@@ -1242,7 +1242,7 @@ impl Flasher {
             })?;
 
         while data.len() < size as usize {
-            let response = self.connection.read_response()?;
+            let response = self.connection.read_flash_response()?;
             let chunk: Vec<u8> = if let Some(response) = response {
                 response.value.try_into()?
             } else {
@@ -1262,7 +1262,7 @@ impl Flasher {
             return Err(Error::ReadMoreThanExpected);
         }
 
-        let response = self.connection.read_response()?;
+        let response = self.connection.read_flash_response()?;
         let digest: Vec<u8> = if let Some(response) = response {
             response.value.try_into()?
         } else {

espflash/tests/scripts/read-flash.sh

@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+
+KNOWN_PATTERN=$'\x01\xa0\x02\xB3\x04\xC4\x08\xD5\x10\xE6\x20\xF7\x40\x88\x50\x99'  
+KNOWN_PATTERN+=$'\x60\xAA\x70\xBB\x80\xCC\x90\xDD\xA0\xEE\xB0\xFF\xC0\x11\xD0\x22'  
+KNOWN_PATTERN+=$'\xE0\x33\xF0\x44\x05\x55\x15\x66\x25\x77\x35\x88\x45\x99\x55\xAA'  
+KNOWN_PATTERN+=$'\x65\xBB\x75\xCC\x85\xDD\x95\xEE\xA5\xFF\xB5\x00\xC5\x11\xD5\x22'  
+KNOWN_PATTERN+=$'\xE5\x33\xF5\x44\x06\x55\x16\x66\x26\x77\x36\x88\x46\x99\x56\xAA'  
+KNOWN_PATTERN+=$'\x66\xBB\x76\xCC\x86\xDD\x96\xEE\xA6\xFF\xB6\x00\xC6\x11\xD6\x22'
+
+echo -ne "$KNOWN_PATTERN" > pattern.bin
+result=$(espflash write-bin 0x0 pattern.bin 2>&1)
+echo "$result"
+if [[ ! $result =~ "Binary successfully written to flash!" ]]; then
+    echo "Failed to write binary to flash"
+    exit 1
+fi
+
+lengths=(2 5 10 26 44 86)
+
+for len in "${lengths[@]}"; do
+    echo "Testing read-flash with length: $len"
+
+    result=$(espflash read-flash 0 "$len" flash_content.bin 2>&1)
+    echo "$result"
+    if [[ ! $result =~ "Flash content successfully read and written to" ]]; then
+        echo "Failed to read $len bytes from flash"
+        exit 1
+    fi
+
+    EXPECTED=$(echo -ne "$KNOWN_PATTERN" | head -c "$len")
+
+    if ! cmp -s <(echo -ne "$EXPECTED") flash_content.bin; then
+        echo "Verification failed: content does not match expected for length"
+        exit 1
+    fi
+done
+
+echo "All read-flash tests passed!"