SD driver: a dummy byte before the ready test; say why a write failed (#21)

The card "refused" a write about once in 2,000 multi-block writes: three
1.7 MB uploads in ten. Measured with a driver that records where it gives
up: every time, all blocks were accepted, and the status check after Stop
Tran came back as 0xFF or 0x1F. The driver tests for ready with the first
byte after selecting the card, which reads 0xFF before the card has
signalled busy, so CMD13 went out mid-programming. A dummy byte first, as
in ChaN's reference driver, and one after Stop Tran.

30 uploads in a row since, each read back by SHA-256, ten with the radio
listening: no fault. 10 MHz made no difference; the card stays at 20 MHz.

`info` shows the driver's write faults; `put` prints the step and the
card's answer when one happens. ADR 0007.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EhqxQ49eCju4CzKYNjZzwT

lib/SD/src/sd_diskio.cpp

@@ -1,3 +1,8 @@
+// roro9stack's copy of the SD-over-SPI driver from Arduino-ESP32 3.3.12 (libraries/SD/src/
+// sd_diskio.cpp), linked instead of the framework's: defining every function the SD class needs
+// keeps the library's file out of the link. Changes are marked "roro:". Why (issue #21): the
+// original gives up on a write without saying why, and never resends a block the card rejects.
+//
 // Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -28,10 +33,25 @@ extern "C" {
 #endif
 //#include "esp_vfs.h"
 #include "esp_vfs_fat.h"
+
 char CRC7(const char *data, int length);
 unsigned short CRC16(const char *data, int length);
 }
 
+// roro: why the last write failed.
+#include "sd_fault.h"
+static roro::SdFault s_fault;
+static bool sdFault(roro::SdFault::Step step, uint8_t token = 0, uint32_t resp = 0) {
+  s_fault.step = step;
+  s_fault.token = token;
+  s_fault.resp = resp;
+  s_fault.count++;
+  return false;
+}
+namespace roro {
+SdFault sdLastFault() { return s_fault; }
+}  // namespace roro
+
 typedef enum {
   GO_IDLE_STATE = 0,
   SEND_OP_COND = 1,
@@ -128,6 +148,11 @@ void sdDeselectCard(uint8_t pdrv) {
 bool sdSelectCard(uint8_t pdrv) {
   ardu_sdcard_t *card = s_cards[pdrv];
   digitalWrite(card->ssPin, LOW);
+  // roro: one dummy byte before asking whether the card is ready, as ChaN's reference driver does.
+  // A card that has just been given a write takes about a byte of clock to signal busy; without
+  // this, that first byte reads 0xFF, "ready", and the next command (the status check after a
+  // write) goes out while the card is still programming and comes back as garbage (#21).
+  card->spi->transfer(0xFF);
   bool s = sdWait(pdrv, 500);
   if (!s) {
     log_e("Select Failed");
@@ -308,9 +333,10 @@ bool sdReadSectors(uint8_t pdrv, char *buffer, unsigned long long sector, int co
 }
 
 bool sdWriteSector(uint8_t pdrv, const char *buffer, unsigned long long sector) {
+  using roro::SdFault;
   for (int f = 0; f < 3; f++) {
     if (!sdSelectCard(pdrv)) {
-      return false;
+      return sdFault(SdFault::Select);  // roro: say why
     }
     if (!sdCommand(pdrv, WRITE_BLOCK_SINGLE, (s_cards[pdrv]->type == CARD_SDHC) ? sector : sector << 9, NULL)) {
       char token = sdWriteBytes(pdrv, buffer, 0xFE);
@@ -319,12 +345,13 @@ bool sdWriteSector(uint8_t pdrv, const char *buffer, unsigned long long sector)
       if (token == 0x0A) {
         continue;
       } else if (token == 0x0C) {
-        return false;
+        return sdFault(SdFault::DataToken, token);
       }
 
       unsigned int resp;
-      if (sdTransaction(pdrv, SEND_STATUS, 0, &resp) || resp) {
-        return false;
+      char status = sdTransaction(pdrv, SEND_STATUS, 0, &resp);
+      if (status || resp) {
+        return token != 0x05 ? sdFault(SdFault::DataToken, token, resp) : sdFault(SdFault::Status, status, resp);
       }
       return true;
     } else {
@@ -332,25 +359,29 @@ bool sdWriteSector(uint8_t pdrv, const char *buffer, unsigned long long sector)
     }
   }
   sdDeselectCard(pdrv);
-  return false;
+  return sdFault(SdFault::Command);
 }
 
 bool sdWriteSectors(uint8_t pdrv, const char *buffer, unsigned long long sector, int count) {
+  using roro::SdFault;
   char token;
   const char *currentBuffer = buffer;
   unsigned long long currentSector = sector;
   int currentCount = count;
   ardu_sdcard_t *card = s_cards[pdrv];
+  SdFault::Step why = SdFault::Command;  // roro: what stopped it, for the last return
+  uint8_t whyToken = 0;
 
   for (int f = 0; f < 3;) {
     if (card->type != CARD_MMC) {
-      if (sdTransaction(pdrv, SET_WR_BLK_ERASE_COUNT, currentCount, NULL)) {
-        return false;
+      char refused = sdTransaction(pdrv, SET_WR_BLK_ERASE_COUNT, currentCount, NULL);
+      if (refused) {
+        return sdFault(SdFault::EraseCount, refused);
       }
     }
 
     if (!sdSelectCard(pdrv)) {
-      return false;
+      return sdFault(SdFault::Select);
     }
 
     if (!sdCommand(pdrv, WRITE_BLOCK_MULTIPLE, (card->type == CARD_SDHC) ? currentSector : currentSector << 9, NULL)) {
@@ -365,20 +396,25 @@ bool sdWriteSectors(uint8_t pdrv, const char *buffer, unsigned long long sector,
       } while (--currentCount);
 
       if (!sdWait(pdrv, 500)) {
+        why = SdFault::BusyAfter;
         break;
       }
 
       if (currentCount == 0) {
         sdStop(pdrv);
+        card->spi->transfer(0xFF);  // roro: the byte the card takes to go busy after Stop Tran (#21)
         sdDeselectCard(pdrv);
 
         unsigned int resp;
-        if (sdTransaction(pdrv, SEND_STATUS, 0, &resp) || resp) {
-          return false;
+        char status = sdTransaction(pdrv, SEND_STATUS, 0, &resp);
+        if (status || resp) {
+          return sdFault(SdFault::Status, status, resp);
         }
         return true;
       } else {
         if (sdCommand(pdrv, STOP_TRANSMISSION, 0, NULL)) {
+          why = SdFault::StopCommand;
+          whyToken = token;
           break;
         }
 
@@ -402,6 +438,8 @@ bool sdWriteSectors(uint8_t pdrv, const char *buffer, unsigned long long sector,
           currentCount = count - writtenBlocks;
           continue;
         } else {
+          why = SdFault::DataToken;
+          whyToken = token;
           break;
         }
       }
@@ -410,7 +448,7 @@ bool sdWriteSectors(uint8_t pdrv, const char *buffer, unsigned long long sector,
     }
   }
   sdDeselectCard(pdrv);
-  return false;
+  return sdFault(why, whyToken);
 }
 
 unsigned long sdGetSectorsCount(uint8_t pdrv) {

lib/SD/src/sd_fault.h

@@ -0,0 +1,29 @@
+#pragma once
+
+#include <cstdint>
+
+namespace roro {
+
+// Why the SD driver last gave up on a write (see sd_diskio.cpp, issue #21). The framework's driver
+// fails without saying why; ours records it.
+struct SdFault {
+    enum Step : uint8_t {
+        None,
+        EraseCount,   // ACMD23 before a multi-block write was refused
+        Select,       // the card stayed busy for 500 ms
+        Command,      // the write command itself was refused
+        DataToken,    // the card's answer to a data block: 0x0B CRC error, 0x0D write error
+        BusyAfter,    // still busy 500 ms after the last block
+        Status,       // CMD13 after the write reported an error (resp)
+        StopCommand,  // CMD12 after a rejected block was refused
+    };
+    Step step = None;
+    uint8_t token = 0;   // the driver's or the card's answer at that step
+    uint32_t resp = 0;   // CMD13's status bits, for Status
+    uint32_t count = 0;  // failed writes since boot
+    uint32_t retried = 0;  // blocks resent after a CRC error, since boot
+};
+
+SdFault sdLastFault();
+
+}  // namespace roro