[2.0.0] Adds the setRXInterrupt

[timkoers]

Should fix pull request #3376

I haven't tested the code yet myself, but feel free to review

[timkoers]

I'll take a look at the failing tests after X-mass

[AlessandroAU]

Thanks for doing this! I've wanted this feature for awhile

[timkoers]

I'm working on a rewrite, which makes it easier to manage the interrupt functions.

[me-no-dev]

Please note my comments and revers all code style changes. Stick to the style in the files and change only what is necessary for this to work. I can not review this request as-is.

[timkoers]

Are you able to merge it?

[ianr34]

it looks good - and when this issue was first raised (in another post, a year or two ago) I got so frustrated I just junked the whole serial class and did my own.. Polling for input in your top program is just so not the way to go..

[me-no-dev]

Coming in 2.0.0 almost everything will be event based. I'm thinking that events are probably the best solution here also. They are thread-safe and can still access the RX Queue. Events can come based on different factors (byte received, transmission done, queue full, etc.). What do you think?

[ianr34]

Yes, Yes, and Yes. Events are the way to go (not interrupts, that was OK for a short term solution) - the one I wrote to replace the arduino-esp32 one was event based, with a queue.

We have to get everyone away from thinking about a single task with polling, it simply isn't the way to use hardware this powerful, let alone the way to use more than 1 core, and all those nice hardware bits.. And events are much nicer than interrupts in most cases.

[timkoers]

Coming in 2.0.0 almost everything will be event based. I'm thinking that events are probably the best solution here also. They are thread-safe and can still access the RX Queue. Events can come based on different factors (byte received, transmission done, queue full, etc.). What do you think?

For the sake of unified code, I think that is good practice. Are you going to change it up to events?

[me-no-dev]

@timkoers yes I will :) do not close this PR so it reminds me of it.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[mycbtec]

@timkoers I tested the code and it is working fine. I have only one issue at this point during Setup initilization... it sometimes stop at setrxInterrupt function. When I delete Serial2.begin and Serial2.setRxInterrupt -> upload the code -> then add Serial2.begin and Serial2.setRxInterrupt -> upload the code, everything starts to work fine again... I'm trying to figure out why this is happening...

[SuGlider]

SerialHardware and HAL have changed to work over IDF 4.4 layer.
This PR won't fit anymore in the new refactoring effort.

Another way to solve it would be creating Serial.onReceive(callback_function)
This follows Arduino style and works as way to setRCInterrupt as well.

Example:

A potential solution is being implemented.
It's the Serial.onReceive(function)

As soon as any data arrives UART RX, function() will called.

Example:

void UART_RX_IRQ(){
  uint16_t size = Serial.available();
  Serial.printf("Got %d bytes on Serial to read\n", size);
  while(Serial.available()) {
    Serial.write(Serial.read());
  }
  Serial.printf("\nSerial data processed!\n");
}

void setup() {
  Serial.begin(115200);
  Serial.onReceive(UART_RX_IRQ);
  Serial.println("Send data to UART0 in order to activate the RX callback");
}

void loop() {
  Serial.println("Sleeping for 10 seconds...");
  delay(10000);
}

[ianr34]

I I said above - it shouldn't be an interrupt, it should be either a queue that you can wait on ie xQueueReceive, or an event that you can also wait on.

I wrote my own quite a while ago, you do a uart_driver_install, then have a task waiting on that queue, then a task that waits on that to copy it over to another queue for the user program (doing a logical packet at a time), with an optional event. Works great and NO polling OR interrupts..

Waiting on something is much better than interrupts for these types of things, though maybe I think that because I've been writing multi threaded programs for 40 years... It's too limiting trying to process things in an interrupt, which often means you'd just have to raise an event in that anyway... (And I don't mean a volatile bool that you do polling on type of event!!)

[SuGlider]

I I said above - it shouldn't be an interrupt, it should be either a queue that you can wait on ie xQueueReceive, or an event that you can also wait on.

I wrote my own quite a while ago, you do a uart_driver_install, then have a task waiting on that queue, then a task that waits on that to copy it over to another queue for the user program (doing a logical packet at a time), with an optional event. Works great and NO polling OR interrupts..

Waiting on something is much better than interrupts for these types of things, though maybe I think that because I've been writing multi threaded programs for 40 years... It's too limiting trying to process things in an interrupt, which often means you'd just have to raise an event in that anyway... (And I don't mean a volatile bool that you do polling on type of event!!)

Hi @ianr34

The Arduino API will be the Serial.onReceive(callback_func) but internally it will use a task waiting for UART Events to know when to execute the UART callback. IDF UART has an EventQueue that is populated from the UART ISR, which will be read by a separated task.

So everything will be done exactly as you described above. It will work correctly with FreeRTOS, IDF and Arduino.
You will be able to see it in the code next week, when I publish this PR.

Thanks for your contribution!

[ianr34]

thanks - I'd missed the fact it was a callback, not an interrupt (or a callback from an interrupt) - which means they can spend as much time in it as they need to.. Though of course they will need to be processing fast enough that the serial queue doesn't fill up waiting for this callback to return (I assume in most cases they will be reading the queue in this callback) .. :-)

[SuGlider]

thanks - I'd missed the fact it was a callback, not an interrupt (or a callback from an interrupt) - which means they can spend as much time in it as they need to.. Though of course they will need to be processing fast enough that the serial queue doesn't fill up waiting for this callback to return (I assume in most cases they will be reading the queue in this callback) .. :-)

Yes, exactly. The only potential issue is when the callback never returns and blocks the UART Event Task. But it won't cause any issue to the UART ISR and neither to the regular Arduino Sketch that is running in a separated task as well (or to any other task running within the application). Maybe some WDT will complain about the UART Event Task in the FreeRTOS runtime.

I think it's a safe solution. Let me know if you think about any other potential issue in order to consider it when implementing it.
Thanks again!