Calling radio.available() after failed transmission when using writeFast or startWrite causes interrupt loop about rf24 HOT 10 CLOSED

I'm not exactly sure, I kind of think this is 'undefined' behaviour, because you are using ack payloads, but not keeping the chip in the right mode.

Technically, you should be calling radio.txStandBy() at some point, if using these methods of writing with ack-payloads.

  if (tx) {  // Have we successfully transmitted?
    if (role == role_sender){
      Serial.println(F("Send:OK"));
      radio.txStandBy();
    }
    if (role == role_receiver)
      Serial.println(F("Ack Payload:Sent"));
  }

  if (fail) {  // Have we failed to transmit?
    if (role == role_sender){
      Serial.println(F("Send:Failed"));
      radio.txStandBy();
    }
    if (role == role_receiver)
      Serial.println(F("Ack Payload:Failed"));
  }

Does adjusting this code resolve the issue?

from rf24.

I don't see how calling available() could cause an extra interrupt. It only reads from registers, and you can't write to the IRQ flags (in the radio's STATUS register) that whatHappened() returns.

I would suggest using volatile bool states and handling their changes in the main loop() instead of the ISR (check_radio()) because ... per Arduino's attachinterrupt() docs:

Generally, an ISR should be as short and fast as possible. If your sketch uses multiple ISRs, only one can run at a time, other interrupts will be executed after the current one finishes in an order that depends on the priority they have. millis() relies on interrupts to count, so it will never increment inside an ISR. Since delay() requires interrupts to work, it will not work if called inside an ISR. micros() works initially but will start behaving erratically after 1-2 ms. delayMicroseconds() does not use any counter, so it will work as normal.

Calling txStandby() within the ISR will greatly increase the risk of violating the millis() concern because it uses millis() to measure the standby time.

Due to the delayMicroseconds(5) call twice per SPI transaction (when the CSN is toggled on non-Arduino platforms),

• whatHappened() can incur 10 µs delay because it does 1 SPI transaction
• available(void) will toggle the CSN pin 2x (10 µs)
• available(&pipe_num) will toggle the CSN 4x (20 µs)
• writeAckPayload() will toggle CSN 2x (10 µs) and now (since v1.3.11) returns the TX_FULL flag in the STATUS register.

This is a minimal example (based on the ping_par_irq example) which demonstrates the unexpected behavior (using only a sender, assuming the receiver is down):

The pingpair_irq example is a relic example from back when maniacbug was still involved in this library (10+ years ago). I haven't actually ran tests on those examples (now located in our examples/old_backups folder) since v1.3.11 when I rewrote the examples for simplicity (among other reasons).

from rf24.

Serial.print() tends to take a lot of time when output to the Serial Monitor, especially if the board uses a chip that doesn't natively support USB (including but not limited to Arduino Nano/Uno and ESP8266 boards). I should revise the newer InterruptConfigure.ino to adhere to Arduino's "short and fast as possible" advice.

from rf24.

I'm not exactly sure, I kind of think this is 'undefined' behaviour, because you are using ack payloads, but not keeping the chip in the right mode.

Technically, you should be calling radio.txStandBy() at some point, if using these methods of writing with ack-payloads.

  if (tx) {  // Have we successfully transmitted?
    if (role == role_sender){
      Serial.println(F("Send:OK"));
      radio.txStandBy();
    }
    if (role == role_receiver)
      Serial.println(F("Ack Payload:Sent"));
  }

  if (fail) {  // Have we failed to transmit?
    if (role == role_sender){
      Serial.println(F("Send:Failed"));
      radio.txStandBy();
    }
    if (role == role_receiver)
      Serial.println(F("Ack Payload:Failed"));
  }

Does adjusting this code resolve the issue?

Thank you, this actually solves the problem of the interrupt loop but as @2bndy5 pointed out this might be problematic, too.

EDIT: actually adding more code brought back the problem. Still, I think your answer goes in the right direction, since disabling AutoAck seems to prevent going into the undefined state. I guess adding radio.txStandBy() just changed the timing in the IRQ a bit so that the code did not get stuck in the interrupt loop anymore...

from rf24.

I don't see how calling available() could cause an extra interrupt. It only reads from registers, and you can't write to the IRQ flags (in the radio's STATUS register) that whatHappened() returns.

I also have no clue, why exactly the radio.available() triggers this behaviour, but if you try the code you could verify it.

EDIT:

Actually you are right. It seems that it was just a coincidence. I tested again in a more elaborated example and leaving out radio.available() does not seem to change the behaviour. I have no clue what else could cause the interrupt loop. So far, the only way to prevent it reliable seems to be disabling AutoAck.

from rf24.

I would suggest using volatile bool states and handling their changes in the main loop() instead of the ISR (check_radio()) because ... per Arduino's attachinterrupt() docs:

Generally, an ISR should be as short and fast as possible.

I know, I will try your proposal. I used interrupts because from another application where I streamed audio, I remember this was the only way of detecting fast enough without calling available() every loop cycle and thus introducing too much delay. Could you give a hint how to notice the arrival of a package without using an interrupt and without calling functions which introduce a delay?

from rf24.

Serial.print() tends to take a lot of time when output to the Serial Monitor, especially if the board uses a chip that doesn't natively support USB (including but not limited to Arduino Nano/Uno and ESP8266 boards)

I know, I only put it for demonstrating reasons, I don't use it in production code. During debugging on a teensy 4.0 it did not cause troubles so far (probably because of native USB support).

from rf24.

It honestly sounds like you are causing undefined behavior by doing too much in the ISR. Trying to point blame at a specific function will be little more than guesses that yield nothing conclusive.

It helps to know you're using a teensy board. According to their docs:

The simplest and most common strategy is to keep all interrupt service routines short and simple, so they execute quickly, and to minimize time the main program disables interrupts. Virtually all examples follow this model.

When the hardware calls an interrupt service routine, it clears the global interrupt flag, so that no other interrupt routine may be called. The return from an interrupt service routine automatically reenables interrupts, and if any other interrupt flags are set, the hardware will call the next pending interrupt routine rather than returning to the main program.

I strongly suggest you try the volatile bool strategy instead making the ISR more and more complex.

from rf24.

It honestly sounds like you are causing undefined behavior by doing too much in the ISR. Trying to point blame at a specific function will be little more than guesses that yield nothing conclusive.

I did not want to blame any function, I just wanted to understand what's happening. Honestly, I never experienced an extra interrupt being triggered because I was doing a single Serial.print() in the ISR.

It helps to know you're using a teensy board. According to their docs:

The simplest and most common strategy is to keep all interrupt service routines short and simple, so they execute quickly, and to minimize time the main program disables interrupts. Virtually all examples follow this model.
When the hardware calls an interrupt service routine, it clears the global interrupt flag, so that no other interrupt routine may be called. The return from an interrupt service routine automatically reenables interrupts, and if any other interrupt flags are set, the hardware will call the next pending interrupt routine rather than returning to the main program.

I strongly suggest you try the volatile bool strategy instead making the ISR more and more complex.

I get your point. I read your first answer as "use volatile bool instead of the ISR". After having a closer look at InterruptConfigure.ino I understand what you meant. I will close the issue and refactor my code to use the 'volatile bool' strategy. Sorry for the noise...

from rf24.

There is a good reason why ISRs should be fast and simple. Looking back on my Assembly language class, I suspect a complex ISR might cause stack overflows which would explain the undefined behavior. To fully understand what that means would take considerable knowledge about how the compiler allocates memory for functions and how functions (in binary) are executed at runtime (and that's just focused on global functions - methods scoped to a data structure have more implications).

from rf24.