Support for more than 3 readers

[kanflo]

This PR adds support for more than three readers by allowing a regular driver controlled GPIO as chip select. When using eg. SPI3, the nCS signals are hardware controlled and the ESP32 only supports 3 of them on eg. SPI3.

[gid204]

@kanflo

Hi @gid204 That's strange, I tried your exact config with one single scanner and if worked flawlessly. I guess we should move this discussion to the PR though. I tried setting an incorrect nCS for the reader but did not make it past rc522_pcd_reset in rc522_start, that indicates that your nCS definition is ok.

Some questions come to mind:

1. Are you using one scanner only?
2. What happens if you move RC522_SPI_SCANNER_GPIO_SDA to .spics_io_num and remove .ncs_io_num?
3. Any other logs? I get the following:

W (5792) spi_bus_lock: *** try_acquire_free_dev cs not required
I (5802) gpio: GPIO[8]| InputEn: 0| OutputEn: 1| OpenDrain: 0| Pullup: 0| Pulldown: 0| Intr:0
I (5852) rc522: PCD (fw=v2.0) initialized```

1. Yes, only using one scanner for now.
2. If I do this it works as expected:

#define RC522_SPI_BUS_GPIO_MISO    (25)
#define RC522_SPI_BUS_GPIO_MOSI    (23)
#define RC522_SPI_BUS_GPIO_SCLK    (19)
#define RC522_SPI_SCANNER_GPIO_SDA (22)
#define RC522_SCANNER_GPIO_RST     (-1) // soft-reset

static rc522_spi_config_t driver_config = {
    .host_id = SPI3_HOST,
    .bus_config = &(spi_bus_config_t){
        .miso_io_num = RC522_SPI_BUS_GPIO_MISO,
        .mosi_io_num = RC522_SPI_BUS_GPIO_MOSI,
        .sclk_io_num = RC522_SPI_BUS_GPIO_SCLK,
    },
    .dev_config = {
        .spics_io_num = RC522_SPI_SCANNER_GPIO_SDA,
    },
    .rst_io_num = RC522_SCANNER_GPIO_RST,
    // .ncs_io_num = RC522_SPI_SCANNER_GPIO_SDA
};

I (322) main_task: Started on CPU0
I (332) main_task: Calling app_main()
I (352) rc522: PCD (fw=unknown) initialized
I (352) main_task: Returned from app_main()
I (6782) rc522: 
I (6782) rc522: ╔══════════════╗
I (6782) rc522: ║              ║ Type: MIFARE 1K
I (6782) rc522: ║     RFID     ║ UID:  14 7C 64 6C
I (6792) rc522: ║     CARD     ║ ATQA: 0x0400
I (6802) rc522: ║              ║ SAK:  0x08
I (6802) rc522: ╚══════════════╝
I (6812) rc522: 
I (6812) rc522-read-write-example: Reading data from the block 4
I (6822) rc522-read-write-example: Current data:
72 63 35 32 32 20 69 73 20 64 6F 70 65 00 F4 2D 
I (6832) rc522-read-write-example: Writing data (rc522 is dope) to the block 4:
72 63 35 32 32 20 69 73 20 64 6F 70 65 00 F4 2D 
I (6842) rc522-read-write-example: Write done. Verifying...
I (6852) rc522-read-write-example: New data in the block 4:
72 63 35 32 32 20 69 73 20 64 6F 70 65 00 F4 2D 
I (6852) rc522-read-write-example: Verified.
I (6862) rc522-read-write-example: Read/Write success

3. No other logs after this:

I (262) heap_init: Initializing. RAM available for dynamic allocation:
I (269) heap_init: At 3FFAE6E0 len 00001920 (6 KiB): DRAM
I (275) heap_init: At 3FFB2E78 len 0002D188 (180 KiB): DRAM
I (281) heap_init: At 3FFE0440 len 00003AE0 (14 KiB): D/IRAM
I (287) heap_init: At 3FFE4350 len 0001BCB0 (111 KiB): D/IRAM
I (294) heap_init: At 4008F0E0 len 00010F20 (67 KiB): IRAM
I (301) spi_flash: detected chip: generic
I (304) spi_flash: flash io: dio
W (308) spi_flash: Detected size(4096k) larger than the size in the binary image header(2048k). Using the size in the binary image header.
I (322) main_task: Started on CPU0
I (332) main_task: Calling app_main()
E (372) rc522: rc522_pcd_rw_test(292): FIFO length missmatch after write
E (372) rc522: rc522_start(86): rw test failed
I (372) main_task: Returned from app_main()

I'm not sure I've done anything explicitly wrong as it builds and flashes....but I thought I'd ask, whats the correct way to setup the environment for this PR (whilst it hasn't been merged yet)?

The steps I followed:

1. Cloned your repo and checked out jk-soft-spi-ncs
2. deleted esp-idf-rc522/examples/read_write/main/idf_component.yml
3. added esp-idf-rc522/examples/read_write/components/rc522 and created a symlink to the repo root:

gideon@Gideons-MacBook-Pro-2 rc522 % pwd
/Users/gideon/Projects/esp-idf-rc522/examples/read_write/components/rc522
gideon@Gideons-MacBook-Pro-2 rc522 % ls -la
total 96
drwxr-xr-x  18 gideon  staff    576 16 Jul 14:17 .
drwxr-xr-x  10 gideon  staff    320 16 Jul 13:53 ..
-rw-r--r--   1 gideon  staff   2077 16 Jul 13:53 .clang-format
-rw-r--r--@  1 gideon  staff   6148 16 Jul 15:37 .DS_Store
drwxr-xr-x  12 gideon  staff    384 16 Jul 17:26 .git
drwxr-xr-x   4 gideon  staff    128 16 Jul 13:53 .github
-rw-r--r--   1 gideon  staff     66 16 Jul 13:53 .gitignore
-rw-r--r--   1 gideon  staff    865 16 Jul 13:53 CMakeLists.txt
drwxr-xr-x   3 gideon  staff     96 16 Jul 13:53 docs
drwxr-xr-x   9 gideon  staff    288 16 Jul 15:37 examples
-rw-r--r--   1 gideon  staff    453 16 Jul 14:17 idf_component.yml
drwxr-xr-x  10 gideon  staff    320 16 Jul 14:14 include
drwxr-xr-x   8 gideon  staff    256 16 Jul 14:17 internal
-rw-r--r--   1 gideon  staff    527 16 Jul 13:53 Kconfig
-rw-r--r--   1 gideon  staff  11347 16 Jul 13:53 LICENSE
-rw-r--r--   1 gideon  staff   4898 16 Jul 13:53 README.md
drwxr-xr-x   9 gideon  staff    288 16 Jul 14:17 src
drwxr-xr-x   5 gideon  staff    160 16 Jul 13:53 test
gideon@Gideons-MacBook-Pro-2 rc522 % 

5. Modified CMakeLists.txt to:

idf_component_register(SRCS "read_write.c"
                    INCLUDE_DIRS "."
                    REQUIRES rc522)

[kanflo]

Could you please add some logs inside the rc522_spi_transaction_pre_cb callback to see if it is called to assert and deassert the soft CS?

[gid204]

added some comment:

static void rc522_spi_transaction_pre_cb(spi_transaction_t *trans)
{
    rc522_spi_config_t *conf = (rc522_spi_config_t *)trans->user;
    RC522_LOG_ON_ERROR(gpio_set_level(conf->ncs_io_num, RC522_DRIVER_NCS_PIN_SELECT));
    ESP_LOGI(TAG, "Inside rc522_spi_transaction_pre_cb");
}

static void rc522_spi_transaction_post_cb(spi_transaction_t *trans)
{
    rc522_spi_config_t *conf = (rc522_spi_config_t *)trans->user;
    RC522_LOG_ON_ERROR(gpio_set_level(conf->ncs_io_num, !RC522_DRIVER_NCS_PIN_SELECT));
    ESP_LOGI(TAG, "Inside rc522_spi_transaction_post_cb");

and logs is showing this:

I (322) main_task: Started on CPU0
I (332) main_task: Calling app_main()
I (332) rc522: Inside rc522_spi_transaction_pre_cb
I (332) rc522: Inside rc522_spi_transaction_post_cb
I (352) rc522: Inside rc522_spi_transaction_pre_cb
I (352) rc522: Inside rc522_spi_transaction_post_cb
I (352) rc522: Inside rc522_spi_transaction_pre_cb
I (352) rc522: Inside rc522_spi_transaction_post_cb
I (362) rc522: Inside rc522_spi_transaction_pre_cb
I (362) rc522: Inside rc522_spi_transaction_post_cb
I (372) rc522: Inside rc522_spi_transaction_pre_cb
I (372) rc522: Inside rc522_spi_transaction_post_cb
I (382) rc522: Inside rc522_spi_transaction_pre_cb
I (392) rc522: Inside rc522_spi_transaction_post_cb
E (392) rc522: rc522_pcd_rw_test(292): FIFO length missmatch after write
E (402) rc522: rc522_start(86): rw test failed
I (402) main_task: Returned from app_main()

[gid204]

Unsure if it's related (as it hasn't fixed the issue) but I noticed in rc522_driver_init_ncs_pin it references rst_io_num and not ncs_io_num. Is this intentional?
ee3b31b#diff-5869e84e136b45b36a75159d317ada46f641a62a4314c7c36d6c78daa9e63b4e

[gid204]

@kanflo I managed to get it working with some relatively major changes...

I followed the rabbit hole from my last comment above and realised ncs init wasn't correct and then realised it never actually gets called.

I confirmed this by printing the gpio_get_level in both pre_cb and post_cb and it always stayed low....

This led me to try and figure out where init should be called...

Not sure the best way to action this but what I did was:
In rc522_driver.c I made the ncs init this:

esp_err_t rc522_driver_init_ncs_pin(gpio_num_t ncs_io_num)
{
    gpio_config_t io_conf = {   
        .intr_type = GPIO_INTR_DISABLE,
        .mode = GPIO_MODE_OUTPUT,
        .pin_bit_mask = (1ULL << ncs_io_num),
        .pull_down_en = GPIO_PULLDOWN_DISABLE,
        .pull_up_en = GPIO_PULLUP_DISABLE,
    };

    RC522_RETURN_ON_ERROR(gpio_config(&io_conf));
    RC522_RETURN_ON_ERROR(gpio_set_level(ncs_io_num, !RC522_DRIVER_NCS_PIN_SELECT));

    return ESP_OK;
}

Called the init here in rc522_spi.c

if (conf->dev_config.spics_io_num == -1) {
        RC522_CHECK(conf->ncs_io_num == -1);
        conf->dev_config.spics_io_num = -1;
        conf->dev_config.pre_cb = &rc522_spi_transaction_pre_cb;
        conf->dev_config.post_cb = &rc522_spi_transaction_post_cb;

        // Initialize the software CS pin
        RC522_RETURN_ON_ERROR(rc522_driver_init_ncs_pin(conf->ncs_io_num));
    }

and finally in rc522_driver_internal.h

esp_err_t rc522_driver_init_ncs_pin(gpio_num_t ncs_io_num);

I'm curious how you have yours working in your environment, do you by some chance have a more up to date local copy?

[kanflo]

Well that was a huge omission on my part, sorry about that. I had the nCS GPIO setup in the general GPIO setup for LEDs and stuff in my app. Not calling rc522_driver_init_ncs_pin is an obvious bug, please fell free to push your changes to get a well deserved co commit ;)

[gid204]

@kanflo I'm relatively new to this, what would be the best approach here?
Fork your branch, make the changes and then create a new PR (referencing you as the main/co contributor)?

[kanflo]

You can fork my branch, add your changes and then make a PR. All referencing will be done automatically. I highly recommend you try it, it's a fun thing to make one's first contribution to someone else's project :)

[abobija]

PR continued in #90