Fixed reading blocks when sector has more than 4 blocks (#52)

- Separate functions for reading trailer and regular blocks - Function for reading regular blocks depends on trailer block - Added unit tests for most of mifare functions
abobija · Sep 21, 2024 · c46a4da · c46a4da
1 parent 85361dc
commit c46a4da
Show file tree

Hide file tree

Showing 6 changed files with 255 additions and 116 deletions.
diff --git a/examples/memory_dump/main/memory_dump.c b/examples/memory_dump/main/memory_dump.c
@@ -88,17 +88,17 @@ static void dump_block(rc522_mifare_sector_block_t *block, uint8_t sector_index)
     // Value (if it's value block)
     else if (block->type == RC522_MIFARE_BLOCK_VALUE) {
         DUMP("  (val=%ld (0x%04l" RC522_X "), adr=0x%02" RC522_X ")",
-            block->value_data.value,
-            block->value_data.value,
-            block->value_data.addr);
+            block->value_info.value,
+            block->value_info.value,
+            block->value_info.addr);
     }
 
     // Errors and warnings
-    if (block->trailer_data.err == RC522_ERR_MIFARE_ACCESS_BITS_INTEGRITY_VIOLATION) {
+    if (block->type == RC522_MIFARE_BLOCK_TRAILER && block->error == RC522_ERR_MIFARE_ACCESS_BITS_INTEGRITY_VIOLATION) {
         DUMP("  ABITS_ERR");
     }
 
-    if (block->value_data.err == RC522_ERR_MIFARE_VALUE_BLOCK_INTEGRITY_VIOLATION) {
+    if (block->type == RC522_MIFARE_BLOCK_VALUE && block->error == RC522_ERR_MIFARE_VALUE_BLOCK_INTEGRITY_VIOLATION) {
         DUMP("  VAL_ERR");
     }
 
@@ -126,11 +126,19 @@ static esp_err_t dump_memory(rc522_handle_t rc522, rc522_picc_t *picc)
         ESP_RETURN_ON_ERROR(rc522_mifare_get_sector_desc(sector_index, &sector), TAG, "");
         ESP_RETURN_ON_ERROR(rc522_mifare_auth_sector(rc522, picc, &sector, &key), TAG, "");
 
-        uint8_t block_offset = sector.number_of_blocks - 1;
+        rc522_mifare_sector_block_t trailer;
+        ESP_RETURN_ON_ERROR(rc522_mifare_read_sector_trailer_block(rc522, picc, &sector, &trailer), TAG, "");
+
+        dump_block(&trailer, sector_index);
+
+        // Start from the highest (non-trailer) block
+        uint8_t block_offset = sector.number_of_blocks - 2;
 
         do {
             rc522_mifare_sector_block_t block;
-            ESP_RETURN_ON_ERROR(rc522_mifare_read_sector_block(rc522, picc, &sector, block_offset, &block), TAG, "");
+            ESP_RETURN_ON_ERROR(rc522_mifare_read_sector_block(rc522, picc, &sector, &trailer, block_offset, &block),
+                TAG,
+                "");
 
             dump_block(&block, sector_index);
         }

diff --git a/idf_component.yml b/idf_component.yml
@@ -1,4 +1,4 @@
-version: "3.0.0"
+version: "3.1.0"
 description: "MFRC522 driver"
 url: "https://github.com/abobija/esp-idf-rc522"
 repository: "https://github.com/abobija/esp-idf-rc522.git"

diff --git a/include/picc/rc522_mifare.h b/include/picc/rc522_mifare.h
@@ -23,6 +23,13 @@ typedef struct
     uint8_t number_of_sectors;
 } rc522_mifare_desc_t;
 
+typedef struct
+{
+    uint8_t index;            // Zero-based index of Sector
+    uint8_t number_of_blocks; // Total number of blocks inside of Sector
+    uint8_t block_0_address;  // Zero-based index of the first Block inside of MIFARE memory
+} rc522_mifare_sector_desc_t;
+
 typedef enum
 {
     RC522_MIFARE_KEY_A = 0,
@@ -46,40 +53,34 @@ typedef enum
 
 typedef struct
 {
-    uint8_t group; // 3: Trailer, 2: Block 2, 1: Block 1, 0: Block 0
     uint8_t c1 :1;
     uint8_t c2 :1;
     uint8_t c3 :1;
 } rc522_mifare_access_bits_t;
 
 typedef struct
 {
-    uint8_t index;            // Zero-based index of Sector
-    uint8_t number_of_blocks; // Total number of blocks inside of Sector
-    uint8_t block_0_address;  // Zero-based index of the first Block inside of MIFARE memory
-} rc522_mifare_sector_desc_t;
-
-typedef struct
-{
-    uint8_t access_bit_groups[4]; // [3] = Trailer, [2] = Block 2, [1] = Block 1, [0] = Block 0
-    esp_err_t err;                // Parsing error
-} rc522_mifare_sector_trailer_data_t;
+    rc522_mifare_access_bits_t access_bits[4]; // [3] = Trailer, [2] = Block 2, [1] = Block 1, [0] = Block 0
+} rc522_mifare_sector_trailer_info_t;
 
 typedef struct
 {
     int32_t value; // Value stored in the block
     uint8_t addr;  // Value block address (this is not the memory address)
-    esp_err_t err; // Parsing error
-} rc522_mifare_value_block_data_t;
+} rc522_mifare_sector_value_block_info_t;
 
 typedef struct
 {
-    uint8_t address; // Zero-based index of Block inside of MIFARE memory
-    uint8_t bytes[RC522_MIFARE_BLOCK_SIZE];
+    uint8_t address;
     rc522_mifare_block_type_t type;
-    rc522_mifare_sector_trailer_data_t trailer_data; // Valid only if type == RC522_MIFARE_BLOCK_TRAILER
+    uint8_t bytes[RC522_MIFARE_BLOCK_SIZE];
+    union
+    {
+        rc522_mifare_sector_trailer_info_t trailer_info;   // Valid only if type == RC522_MIFARE_BLOCK_TRAILER
+        rc522_mifare_sector_value_block_info_t value_info; // Valid only if type == RC522_MIFARE_BLOCK_VALUE
+    };
     rc522_mifare_access_bits_t access_bits;
-    rc522_mifare_value_block_data_t value_data; // Valid only if type == RC522_MIFARE_BLOCK_VALUE
+    esp_err_t error; // e.g. acces bits or value block integrity violation
 } rc522_mifare_sector_block_t;
 
 // {{ MIFARE_Specific_Functions
@@ -123,10 +124,17 @@ esp_err_t rc522_mifare_get_desc(rc522_picc_t *picc, rc522_mifare_desc_t *out_mif
 esp_err_t rc522_mifare_get_sector_desc(uint8_t sector_index, rc522_mifare_sector_desc_t *out_sector_desc);
 
 /**
- * @brief Read and parse MIFARE sector block
+ * @brief Read and parse MIFARE sector trailer block
+ */
+esp_err_t rc522_mifare_read_sector_trailer_block(rc522_handle_t rc522, rc522_picc_t *picc,
+    rc522_mifare_sector_desc_t *sector_desc, rc522_mifare_sector_block_t *out_trailer);
+
+/**
+ * @brief Read and parse MIFARE sector (non-trailer) block
  */
 esp_err_t rc522_mifare_read_sector_block(rc522_handle_t rc522, rc522_picc_t *picc,
-    rc522_mifare_sector_desc_t *sector_desc, uint8_t block_offset, rc522_mifare_sector_block_t *out_block);
+    rc522_mifare_sector_desc_t *sector_desc, rc522_mifare_sector_block_t *trailer, uint8_t block_offset,
+    rc522_mifare_sector_block_t *out_block);
 
 // }} MIFARE_Utility_Functions
 

diff --git a/private_include/rc522_types_private.h b/private_include/rc522_types_private.h
@@ -62,9 +62,10 @@ struct rc522
     }                                                                                                                  \
     while (0)
 
-#define RC522_RETURN_ON_FALSE(a, err_code) ESP_RETURN_ON_FALSE(a, err_code, TAG, "")
-#define RC522_CHECK_AND_RETURN(a, ret_val) ESP_RETURN_ON_FALSE(!(a), ret_val, TAG, #a)
-#define RC522_CHECK(a)                     RC522_CHECK_AND_RETURN(a, ESP_ERR_INVALID_ARG)
+#define RC522_RETURN_ON_FALSE(a, err_code)   ESP_RETURN_ON_FALSE(a, err_code, TAG, "")
+#define RC522_CHECK_WITH_MESSAGE(a, message) ESP_RETURN_ON_FALSE(!(a), ESP_ERR_INVALID_ARG, TAG, message)
+#define RC522_CHECK_AND_RETURN(a, ret_val)   ESP_RETURN_ON_FALSE(!(a), ret_val, TAG, #a)
+#define RC522_CHECK(a)                       ESP_RETURN_ON_FALSE(!(a), ESP_ERR_INVALID_ARG, TAG, #a)
 
 #define RC522_RETURN_ON_ERROR_SILENTLY(x)                                                                              \
     do {                                                                                                               \