Make variable global in class

I want to make a class to connect the ESP32 to a PN532 module, i made a header file:

Cardreader.h:

#ifndef Cardreader_h
#define Cardreader_h

#include <Arduino.h>
#include <SPI.h>
#include <PN532_SPI.h>
#include <PN532.h>
#include <IPSDisplay.h>

class Cardreader
{
  public:
    Cardreader(IPSDisplay ipsDisplay);
    void begin();
    static void scanCard(void * parameter);
  private:
    TaskHandle_t task_ScanCard;
    IPSDisplay _ipsDisplay;
    PN532_SPI pn532spi(SPI, 5);
    PN532 nfc(pn532spi);
};

#endif

With a Cardreader.cpp file:

#include <Arduino.h>
#include <Cardreader.h>
#include <SPI.h>
#include <PN532_SPI.h>
#include <PN532.h>
#include <IPSDisplay.h>

Cardreader::Cardreader(IPSDisplay ipsDisplay) 
{
    _ipsDisplay = ipsDisplay;
}

void Cardreader::begin() {

    nfc.begin();

    uint32_t versiondata = nfc.getFirmwareVersion();
    if (! versiondata) {
        Serial.print("Didn't find PN53x board");
        _ipsDisplay.show("/i003");
        delay(5000);
        begin();
    }
    else {
        // Got ok data, print it out!
        Serial.print("Found chip PN5"); Serial.println((versiondata>>24) & 0xFF, HEX); 
        Serial.print("Firmware ver. "); Serial.print((versiondata>>16) & 0xFF, DEC); 
        Serial.print('.'); Serial.println((versiondata>>8) & 0xFF, DEC);

        // configure board to read RFID tags
        nfc.SAMConfig();

        Serial.println("Waiting for an ISO14443A Card ...");
        xTaskCreatePinnedToCore(this->scanCard, "scanCard", 5000, this, 1, &task_ScanCard, 0);
    }
}

void Cardreader::scanCard(void * parameter) {
  for(;;) {
    uint8_t success;
    uint8_t uid[] = { 0, 0, 0, 0, 0, 0, 0 };  // Buffer to store the returned UID
    uint8_t uidLength;                        // Length of the UID (4 or 7 bytes depending on ISO14443A card type)
        
    // Wait for an ISO14443A type cards (Mifare, etc.).  When one is found
    // 'uid' will be populated with the UID, and uidLength will indicate
    // if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
    success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);
    
    if (success) {
        // Display some basic information about the card
        Serial.println("Found an ISO14443A card");
        Serial.print("  UID Length: ");Serial.print(uidLength, DEC);Serial.println(" bytes");
        Serial.print("  UID Value: ");
        nfc.PrintHex(uid, uidLength);
        Serial.println("");
        
        if (uidLength == 7)
        {
            // We probably have a Mifare Ultralight card ...
            Serial.println("Seems to be a Mifare Ultralight tag (7 byte UID)");
            
            // Try to read the first general-purpose user page (#4)
            Serial.println("Reading page 4");
            uint8_t data[32];
            success = nfc.mifareultralight_ReadPage (4, data);
            if (success)
            {
                // Data seems to have been read ... spit it out
                nfc.PrintHexChar(data, 4);
                Serial.println("");
                
                // Wait a bit before reading the card again
                delay(1000);
            }
            else
            {
                Serial.println("Ooops ... unable to read the requested page!?");
            }
        }
    }
  }
}

I am getting some errors i don’t know how to fix:

In file included from src\main.cpp:7:0:
lib\Cardreader/Cardreader.h:19:24: error: 'SPI' is not a type
     PN532_SPI pn532spi(SPI, 5);
                        ^
lib\Cardreader/Cardreader.h:19:29: error: expected identifier before numeric constant
     PN532_SPI pn532spi(SPI, 5);
                             ^
lib\Cardreader/Cardreader.h:19:29: error: expected ',' or '...' before numeric constant
lib\Cardreader/Cardreader.h:20:15: error: 'pn532spi' is not a type
     PN532 nfc(pn532spi);
               ^
Archiving .pio\build\esp32dev\lib90f\libAdafruit BusIO.a
Archiving .pio\build\esp32dev\libf36\libAdafruit PN532.a
Archiving .pio\build\esp32dev\libd7b\libIPSDisplay.a
Archiving .pio\build\esp32dev\lib449\libInternet.a
Compiling .pio\build\esp32dev\lib428\PN532\llcp.cpp.o
Compiling .pio\build\esp32dev\lib428\PN532\mac_link.cpp.o
Compiling .pio\build\esp32dev\lib428\PN532\snep.cpp.o
*** [.pio\build\esp32dev\src\main.cpp.o] Error 1
====================================================

Thanks for the help already

Hm yes I see the caveat. You want to keep it object-oriented with the member variables (per-object), but FreeRTOS neeeds a static function as the task function – but a static function does not have an object reference. But I also see that you’ve nearly got it there, you already passed this as the parameter object in the xTaskCreatePinnedToCore function. You just need to refactor it a bit so that the function accessing the member variables happens in member function.

I think the best cause of action is to refactor the actual “processing” that interacts with member-variables into its own member functions, and then call into that function, given the actual object in parameter.

Also a few things need to be fixed up with initilization of the member variables, you can’t write

but just

private:
    TaskHandle_t task_ScanCard;
    IPSDisplay _ipsDisplay;
    PN532_SPI pn532spi;
    PN532 nfc;

and then call the sub-constructors in the constructor call of the class.

Cardreader.h

#ifndef Cardreader_h
#define Cardreader_h

#include <Arduino.h>
#include <SPI.h>
#include <PN532_SPI.h>
#include <PN532.h>
#include <IPSDisplay.h>

class Cardreader
{
  public:
    Cardreader(IPSDisplay ipsDisplay);
    void begin();
    static void scanCard(void * parameter);
  private:
    void process();
    TaskHandle_t task_ScanCard;
    IPSDisplay _ipsDisplay;
    PN532_SPI pn532spi;
    PN532 nfc;
};

#endif

Cardreader.cpp

#include <Arduino.h>
#include <Cardreader.h>
#include <SPI.h>
#include <PN532_SPI.h>
#include <PN532.h>
#include <IPSDisplay.h>

/* constructor initializes member variables with appropriate constructor arguments */
Cardreader::Cardreader(IPSDisplay ipsDisplay) : pn532spi(SPI, 5), nfc(pn532spi)
{
    _ipsDisplay = ipsDisplay;
}

void Cardreader::begin()
{

    nfc.begin();

    uint32_t versiondata = nfc.getFirmwareVersion();
    if (!versiondata)
    {
        Serial.print("Didn't find PN53x board");
        _ipsDisplay.show("/i003");
        delay(5000);
        begin();
    }
    else
    {
        // Got ok data, print it out!
        Serial.print("Found chip PN5");
        Serial.println((versiondata >> 24) & 0xFF, HEX);
        Serial.print("Firmware ver. ");
        Serial.print((versiondata >> 16) & 0xFF, DEC);
        Serial.print('.');
        Serial.println((versiondata >> 8) & 0xFF, DEC);

        // configure board to read RFID tags
        nfc.SAMConfig();

        Serial.println("Waiting for an ISO14443A Card ...");
        xTaskCreatePinnedToCore(this->scanCard, "scanCard", 5000, this, 1, &task_ScanCard, 0);
    }
}

void Cardreader::process()
{
    uint8_t success;
    uint8_t uid[] = {0, 0, 0, 0, 0, 0, 0}; // Buffer to store the returned UID
    uint8_t uidLength;                     // Length of the UID (4 or 7 bytes depending on ISO14443A card type)

    // Wait for an ISO14443A type cards (Mifare, etc.).  When one is found
    // 'uid' will be populated with the UID, and uidLength will indicate
    // if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
    success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);

    if (success)
    {
        // Display some basic information about the card
        Serial.println("Found an ISO14443A card");
        Serial.print("  UID Length: ");
        Serial.print(uidLength, DEC);
        Serial.println(" bytes");
        Serial.print("  UID Value: ");
        nfc.PrintHex(uid, uidLength);
        Serial.println("");

        if (uidLength == 7)
        {
            // We probably have a Mifare Ultralight card ...
            Serial.println("Seems to be a Mifare Ultralight tag (7 byte UID)");

            // Try to read the first general-purpose user page (#4)
            Serial.println("Reading page 4");
            uint8_t data[32];
            success = nfc.mifareultralight_ReadPage(4, data);
            if (success)
            {
                // Data seems to have been read ... spit it out
                nfc.PrintHexChar(data, 4);
                Serial.println("");

                // Wait a bit before reading the card again
                delay(1000);
            }
            else
            {
                Serial.println("Ooops ... unable to read the requested page!?");
            }
        }
    }
}

void Cardreader::scanCard(void *parameter)
{
    //recover concrete Cardreader object given to us
    Cardreader* pCardreader = (Cardreader*) parameter;
    //little error check..
    if(pCardreader == nullptr) { 
        Serial.println("Internal error: scanCard() has received nullptr as parameter..");
        vTaskDelete(nullptr); //delete ourselves
    }
    for (;;)
    {
        //call internal process function where the object has access to all of its member variables
        pCardreader->process();
    }
}

Thanks a lot for your amazing help! It compiles so tomorrow i will test if it works.
Thanks again and have a good night

I uploaded the code but i do get an error, after a few seconds i get this message and the ESP reboots

Waiting for an ISO14443A Card ...
E (12082) task_wdt: Task watchdog got triggered. The following tasks did not reset the watchdog in time:
E (12082) task_wdt:  - IDLE0 (CPU 0)
E (12082) task_wdt: Tasks currently running:
E (12082) task_wdt: CPU 0: internetState
E (12082) task_wdt: CPU 1: loopTask
E (12082) task_wdt: Aborting.
abort() was called at PC 0x400e58ff on core 0

ELF file SHA256: 0000000000000000

Backtrace: 0x40085b5c:0x3ffbe550 0x40085dd9:0x3ffbe570 0x400e58ff:0x3ffbe590 0x400841f5:0x3ffbe5b0 0x400f4fcf:0x3ffb4450 0x400eb51b:0x3ffb4480 0x400eb536:0x3ffb44a0 0x400eca37:0x3ffb44c0 0x400eca60:0x3ffb44f0 0x400ecce5:0x3ffb4520 0x400ec494:0x3ffb4540 0x400ebc4b:0x3ffb4580 0x400ebcd5:0x3ffb45a0 0x400ecf83:0x3ffb45d0 0x400ed2c7:0x3ffb4630 0x400ed375:0x3ffb4660 0x400eb645:0x3ffb4680 0x400d229c:0x3ffb46c0 0x400d1539:0x3ffb46e0 0x40086dea:0x3ffb4700

Rebooting...

It also doesn’t detect a card but that may be another problem (maybe?)
Also the rfid card reader cannot be found for 95% of the time, while it can find it 100% of the time with the default example (with everything connected the same way). Can it be possible that the initialization cannot be in the constructor but needs to be intialized later?

Can you test this slightly modified, minimal example with debug output and show the output? GitHub - maxgerhardt/pio-nfc-test

Thanks for the example, i tried it and i get this as output:

SAMConfig
write: 14 01 14 01
read: 15
Waiting for an ISO14443A Card …
Cardreader::scanCard() started with parameter 0x3ffbfe80
Calling into nfs.readPassiveTargetID()…
write: 4A 01 00
…done.
Waiting a second…
Calling into nfs.readPassiveTargetID()…
write: 4A 01 00
…done.
Waiting a second…
Calling into nfs.readPassiveTargetID()…
write: 4A 01 00
…done.
Waiting a second…
Calling into nfs.readPassiveTargetID()…
write: 4A 01 00
…done.
Waiting a second…
Calling into nfs.readPassiveTargetID()…
write: 4A 01 00

It seems to work but i am not sure where the write command comes from, i cannot find it in code

Ps: reading seems to work, when i place a card in front of the module i get the card info

This output comes from the additional debug output. It’s what the PN532 library writes via SPI (see code).

The output

Shows that it just wrote one command to the reader but there wasn’t any response / timeout. Was a card held over the reader at that time? (Otherwise it would have also shown “success”).

When i put a card in front of the reader, i get

Calling into nfs.readPassiveTargetID()…
write: 4A 01 00
read: 4B 01 01 00 44 00 07 04 53 7E 02 66 60 81
ATQA: 0x 44SAK: 0x 00
…done.
Found an ISO14443A card
UID Length: 7 bytes
UID Value: 04 53 7E 02 66 60 81

Seems to be a Mifare Ultralight tag (7 byte UID)
Reading page 4

So probably everything works with your example?

In my other code i have a few more things connected which may cause a problem?

I have:

  • DFPlayer mini
  • W5500 ethernet module
  • 1.28" round lcd display
  • GY-NEO6MV2
  • DS3231 RTC
  • INMP441 mic

I am not sue if i can connect everything together, maybe some components conflict with others?

Okay that looks very good. I thought there was a problem with the SPI connection.

Mhm if everything works when tested on its own there’s probably a problem with the integration of all modules.

I had the following thought: With multithreading / multiprocessing you have to be careful, because now you have that extra Cardreader::scanCard() task that is running on one CPU, and the code is such that it polls the status of the card reader via SPI constantly. So that one task is constantly doing SPI transfers. If another piece of code wants to access the same SPI bus to talk to another device via a differen slave-select for example, they must use a mechanism for synchronization so that not both pieces of code try to transfer something over the SPI bus at once, that’ll kill it. In FreeRTOS (or in any multithreaded system) you do that via mutexes (xSemaphoreCreateMutex () and friends). However, as I can read in the ESP32’s SPI implementation, there are already mutex locks in place (see e.g. here), so it should be threadsafe.

Another thought would be starvation / livelocks (here). The NFC reader code, when you ask for a tag, has this “wait for response and delay for 1ms each turn” logic in

This might really hog up the CPU and prevent it from running tasks effectively. Can e.g. try and increase that to delay(20); and time += 20; to give other threads more time to run, on that CPU.

In essence,

This tells you that the task internetState hogged up the CPU too much without delay()ing for too long, thus creating the watchdog panic. If that didn’t happen before you added the NFC module and thread, it might be due to starvation. You can also try and insert more delay(1) during the execution of that internetState thread (that I don’tk now that code of), to periodically feed the watchdog.

Otherwise remote analysis of this can be hard. If you are still stuck, you can maybe upload the whole code for inspection.

Thanks again for the help, i have integrated your example in my code and the ESP32 is able to connect to the cardreader. I only can’t get any information on the card, but i see i get an Invalid ACK message

Waiting for an ISO14443A Card …
Cardreader::scanCard() started with parameter 0x3ffbff40
Calling into nfs.readPassiveTargetID()…
write: 4A 01 00
Invalid ACK
…done.
Waiting a second…
12:47
true
Calling into nfs.readPassiveTargetID()…
write: 4A 01 00
Invalid ACK
…done.

Probebly this is because of what you explain in your reply?

Update:
I think i know where the problem is, i have a thread where i check internet connection via the W5500 and get the current time from a server. If i disable this part, it works perfectly.

As i can see now, the W5500 is conflicting with the cardreader

Mhm yeah that comes from

It seems to do a SPI write there and check for the packet answer.

The problem that I see here that it’s non-atomic actually. After pulling the slave select low with digitalWrite(_ss, LOW); it does delay(1);, which might give execution control to another thread, e.g. your W5500 thread. Now the PN532 is selected (its slave-select is low) but the W5500 has control now and does maybe another SPI write. Now the W5500 and PN532 might be fighiting for the SPI bus.

The write() seems atomic and non-interruptable, but sadly the chip-selecting part before that is not and interruptable :confused:. So I’m pretty sure that the problem is also synchronization, due to that slave-select writing which happens outside the SPI library code (which is thread safe).

Solving that probably involves adding a synchroniziation layer where the PN532 and W5500 code can exclusively “lock” the SPI bus, on top of what is already happening in the SPI library.

Of course the problem can be avoided by putting the W5500 and PN532 on different SPI buses, then they can’t interfere with each other again.

I think it is not only the W5500, i have enother thread which is this:

void showClock(void * parameter) {
  for(;;) {
            char buff[BUFF_MAX];
    struct ts t;

    DS3231_get(&t);
    if((int)t.year >= 2021 && (int)t.mon >= 1 && (int)t.mon <= 12 && (int)t.mday >= 1 && (int)t.mday <= 31 && (int)t.hour >= 0 && (int)t.hour <= 23 && (int)t.min >= 0 && (int)t.min <= 59 && (int)t.sec >= 0 && (int)t.sec <= 59) {
      // display current time
      //snprintf(buff, BUFF_MAX, "%d.%02d.%02d %02d:%02d:%02d", t.year, t.mon, t.mday, t.hour, t.min, t.sec);
      snprintf(buff, BUFF_MAX, "%02d:%02d", t.hour, t.min);
      Serial.println(buff);
      ipsDisplay.show(buff);
    }
    else {
      ipsDisplay.show("no time");
      Serial.println("Received wrong time");
    }
  }
  delay(1000);
  
}

It gets the time from the DS3231 and displays the time, but this also makes the cardreader fail, i will see if i can use another spi bus, maybe this will be the easiest.

I also prefer to use the cardreader with an interrupt (like the RC522 module), i don’t have to poll every second for a card, but i am not sure if the PN532 module will be able to do this

That would definitely be prefferable, as I see in here the thing does have INT0 and INT1 pins for presumably interrupts, but the PN532 code you seem to be using doesn’t support using this interrupt pin :confused:

Mhm but the DS3231 is a I2C device? It shouldn’t be able to cause problems on the SPI bus… But maybe the ipsDisplay.show() does, if it interacts with the SPI bus. It also nicely delay()s.

That’s it! If i comment the ipsDisplay.show line it works perfect. I think i will see if can find an interrupt example or put the cardreader on a different spi bus.

The W5500 and the display seem to work together, i think the cardreader is causing a problem. The only other problem is that i don’t have many pins left :sweat_smile:

On a sidenote (I haven’t tested it), you may be able to modify the PN532 driver code so that it becomes uninterruptable by all CPUs (especially tricky since the ESP32 has two CPUs and you seem to be using both of them). There’s documentation here. The idea here is to enter a critical section before the chip-select is done, and exit the critical section after the SPI transfer is done (so before digitalWrite(_ss, LOW); and after digitalWrite(_ss, HIGH);).

That might however break SPI transfers or delay() if needs interrupts – you might be able to substitute delayMicroseconds() for delay() as a non-interrupt using function, but I don’t know about SPI.

If all other driver code for the other SPI devices is fine and their locking correctly (by hopefully specifying the chip-select with the SPI library itself so that its locks apply), that might work.

Otherwise you have a bit of work in front of you regarding synchronizing the SPI bus accesses with mutexes and modifying some drivers, but it can be done, and is also a cleaner way of solving it.

I think this is a bit too complicated for me but i will see if i can fix this or find another way. Thanks again for the good help, i really appreciate it