CC1101: RX Interrupt triggered when Transmitting

[studiofuga]

Describe the bug It seems that when transmitting a packet using transmit(), an IRQ is asserted. If the receiver is used with IRQ, a spurious read state is triggered.

To Reproduce

The foollowing code is built on the receiver_interrupt example.

Just added a periodic transmission.

// include the library
#include <RadioLib.h>

// CC1101 has the following connections:
// CS pin:    10
// GDO0 pin:  2
// RST pin:   unused
// GDO2 pin:  3 (optional)
CC1101 radio = new Module(10,2, RADIOLIB_NC,3);

// flag to indicate that a packet was received
volatile bool receivedFlag = false;

// disable interrupt when it's not needed
volatile bool enableInterrupt = true;

unsigned long lastSend = 0;

bool enableSend = true;

// this function is called when a complete packet
// is received by the module
// IMPORTANT: this function MUST be 'void' type
//            and MUST NOT have any arguments!
void setFlag(void) {
    // check if the interrupt is enabled
    if(!enableInterrupt) {
        return;
    }

    // we got a packet, set the flag
    receivedFlag = true;
}

void setup() {
    Serial.begin(38400);

    // initialize CC1101 with default settings
    Serial.print(F("[CC1101] Initializing ... "));
    int state = radio.begin(868.3, 38.4, 21.0, 200.0, 10, 32);
    if (state == ERR_NONE) {
        Serial.println(F("success!"));
    } else {
        Serial.print(F("failed, code "));
        Serial.println(state);
        while (true);
    }

    // set the function that will be called
    // when new packet is received
    radio.setGdo0Action(setFlag);

    // start listening for packets
    Serial.print(F("[CC1101] Starting to listen ... "));
    state = radio.startReceive();
    if (state == ERR_NONE) {
        Serial.println(F("success!"));
    } else {
        Serial.print(F("failed, code "));
        Serial.println(state);
        while (true);
    }

    // if needed, 'listen' mode can be disabled by calling
    // any of the following methods:
    //
    // radio.standby()
    // radio.sleep()
    // radio.transmit();
    // radio.receive();
    // radio.readData();

    lastSend = millis();
    enableSend = true;
}

void loop() {
    unsigned long now = millis();

    if (enableSend && now - lastSend > 5000) {
        // Send something
            Serial.println(F("Sending..."));
            uint8_t data[] = {0x00, 0x20, 0x00, 0x01 };
            size_t datalen = 4;
            radio.transmit(data, datalen);

            lastSend = now;
            radio.startReceive();
    }

    // check if the flag is set
    if(receivedFlag) {
        // disable the interrupt service routine while
        // processing the data
        enableInterrupt = false;

        // reset flag
        receivedFlag = false;

        // you can read received data as an Arduino String
        auto len = radio.getPacketLength();
        uint8_t str[255];
        int state = radio.readData(str, 255);

        // you can also read received data as byte array
        /*
          byte byteArr[8];
          int state = radio.readData(byteArr, 8);
        */

        if (state == ERR_NONE) {
            // packet was successfully received
            Serial.println(F("[CC1101] Received packet!"));

            // print data of the packet
            Serial.print(F("[CC1101] Data:\t\t"));
            for (auto i = 0; i < len; ++i){
                PrintHex8(&str[i],1);
                if (i % 16 == 15) {
                    Serial.print("\n");
                }
            }
            Serial.println();

            // print RSSI (Received Signal Strength Indicator)
            // of the last received packet
            Serial.print(F("[CC1101] RSSI:\t\t"));
            Serial.print(radio.getRSSI());
            Serial.println(F(" dBm"));

            // print LQI (Link Quality Indicator)
            // of the last received packet, lower is better
            Serial.print(F("[CC1101] LQI:\t\t"));
            Serial.println(radio.getLQI());

        } else if (state == ERR_CRC_MISMATCH) {
            // packet was received, but is malformed
            Serial.println(F("CRC error!"));

        } else {
            // some other error occurred
            Serial.print(F("failed, code "));
            Serial.println(state);

        }

        // put module back to listen mode
        radio.startReceive();

        // we're ready to receive more packets,
        // enable interrupt service routine
        enableInterrupt = true;
    }

}

Expected behavior Transmission should happen every 5 seconds, nothing should be received.

Screenshots A spurious packet is received.

Sending...                                                                      
[CC1101] Received packet!                                                       
[CC1101] Data:          00 00 00 10 00 00 41 40 0B 01 C1 01 5F AF 27 F1         
                                                                        65 F2 3 
[CC1101] RSSI:          -26.50 dBm                                              
[CC1101] LQI:           47          

Additional info (please complete):

• MCU: Arduino NANO 328 (old bootloader)
• Wireless module type: CC1101

Notes: Likely it is possible to workaround this issue by disabling the GDO action before triggering the transmission and re-enabling after the transmission is completed.

[jgromes]

You seem to be mixing two fundamentally differing approaches here: blocking and interrupt-driven. You're using a blocking transmit method together with a non-blocking interrupt-based startReceive/readData. What you're seeing is an expected behavior in this case.

First, you enable non-blocking receive. While doing that, you define an interrupt service routine setFlag, which is set to trigger on GDO0 activity regardless of its cause. Next, at some point, you transmit a packet, which causes activity on the GDO0 pin, which then obviously triggers the setFlag ISR.

I would argue that the whole approach of mixing blocking and non-blocking methods is incorrect. Either you should only use the blocking methods, in which case the library manages the GDO0 states, or you should only use interrupt-based approach, and handle all interrupts by yourself - including those generated when packet transmission is finished.

[studiofuga]

Hi Jgromes, thank you for your comment.

I know that the interrupt handler change the flags regardless of the cause, and for a good reason: there's no way for the client code to identify the cause. It can be derived indirectly by reading the state of the IO pin configuration, but this breaks the "isolation" of the internal states. if the value is changed in the future, the client code will be broken. So likely an access function to check the cause may be helpful.

For this reason, usually one use BOTH pins, GDO0 AND GDO2. The library seems to be ready to use both GDO0 and GDO2, for example GDO0 for receive and GDO2 for transmit, but then transmit code is broken because it should use IOCFG2 instead of IOCFG0.

Second point, I don't totally agree with you with the sync/async approach. Receive and Transmit are implemented using FIFO so the real aysnchronous part is handled by hardware, so there's no reason to avoid using this approach, that is very natural by the way. Transmit are not really synchronous, because the wait is implemented internally by the library, not a requirement by the client code. Also note that even if I would use the "interrupt driven" approach for transmit, the code would be nearly identical (except for the issue above).

Third, if this approach is wrong, then the library should enforce the correct way to implement, or document the limitation.

Anyway, thank you for your comment. Let me know if I can do anything to improve the library, if it falls in the purpose of my code I can try to implement it and share.

Kind regards

[jgromes]

To answer the points:

So likely an access function to check the cause may be helpful

I agree, though haven't had the time to add it into the public API. Other modules would probably also benefit from some getIRQ method.

Receive and Transmit are implemented using FIFO so the real aysnchronous part is handled by hardware, so there's no reason to avoid using this approach

In most cases, reception and transmission are using the same FIFO buffer in its entirety. While some radio modules can be configured to handle Rx and Tx in the same FIFO buffer (by splitting it into Rx and Tx part), this is not done in RadioLib for the sake of simplicity, as well as compatibility across various modules (and to get the maximumm possible packet length). So the asynchronicity must be handled by software, hence the need to either handle everything within the library (which is the blocking approach), or let user handle the interrupts (the interrupt approach). The need to strictly distinguish between the two approaches arises from that - either you let the library handle everything internally, or you have to handle it yourself.

Third, if this approach is wrong, then the library should enforce the correct way to implement, or document the limitation.

I don't see a reason or a clean way to enforce this (without resorting to some compile-time flag that would select between the aproaches). Obviously more documentation would be nice, so perhaps it would be worth to extend the Basics wiki page.

[jgromes]

@studiofuga sorry it took so long to get back to this. In summary, I tried the following options to resolve this.

1. Providing a getIRQ method so that the user may check the source of the interrupt in Arduino sketch. This didn't work because CC1101 has no "interrupt source" register. I tried emulating it by using a combination of other register (e.g. packet/radio status), but that is very unreliable as those registers update.

2. I also tried the approach you proposed in #358, however, it's incomplete - the one shown in that PR only solves the case of interrupt receive colliding with blocking transmit. Theoretically, it is also possible to have interrupt transmit collide with blocking receive (althoug it seems very unpractical to use in that way). It also does not resolve the case of using both receive and transmit as interrupts.

Finally, I somewhat conceded that the best way forward is to separate the GDOx pins, so from now, GDO0 is used to signal Rx interrupts while GDO2 is used for Tx. This is working reliably, since now the interrupt source is determined by the physical pin. The downside is that no other module in RadioLib works this way, but I'm afraid that's something I will have to accept because of the missing interrupt source register.

@phretor I know you guys are using CC1101 in the RFQuack project. I don't think this change should break anything, but I'm letting you know just in case.