Updates to new yield.cpp code

[froeber]

Paul, I'm merging in changes from TeensyDuino 1.20 release for Teensy 3.x into my baseline code.. I saw that there is new yield.cpp code. There was a comment in that code that asked if it needed to be atomic. Maybe the code doesn't absolutely need to be atomic depending on how perfectly reentrant the serial code is that gets called. But since it's easier to be safe why not avoid the question? It's an easy/efficient fix to do it right as per reworked version I did.

Hey, I have to say I really appreciate how good your code is. Too many people write less than stellar code these days.

void yield(void) __attribute__ ((weak));
void yield(void)
{
#ifdef ORIGINAL

    static uint8_t running=0;

    if (running) return; // TODO: does this need to be atomic?
    running = 1;
    if (Serial.available()) serialEvent();
    if (Serial1.available()) serialEvent1();
    if (Serial2.available()) serialEvent2();
    if (Serial3.available()) serialEvent3();
    running = 0;

#else /* NEW VERSION */

    static bool running=false;
    bool was_running;

    __disable_irq();
    was_running = running;
    running = true;
    __enable_irq();

    if (was_running) return;
    if (Serial.available()) serialEvent();
    if (Serial1.available()) serialEvent1();
    if (Serial2.available()) serialEvent2();
    if (Serial3.available()) serialEvent3();
    running = false;
#endif /* FIXED */
};

 

[stevech]

Isn't serialEvent() called from the ISR, and runs in ISR context?
Rather than polling with yield() ?

 

[froeber]

Hey Steve, From what I know serialEvent is an Arduino concept where you can code a function of that name to handle serial data. It's supposed to be called once per "loop()" iteration. It wasn't in TeensyDuino till recently. The yield code that was added just provides a mechanism for invoking user supplied serialEvent functions for each serial device. The weakly linked functions that the BSP code provides do nothing and are just place holders for user code. I think Paul provided the "note to self" comment about atomic operation in the yield function code because of the notion that some people see the yield() function as a good thing to call liberally in their code to make sure they "keep things moving along". It shouldn't be called from interrupt code but you never know what people might do. So I thought putting the atomic protection in wasn't a bad thought and so coded it up as more of a thought exercise.

I thought Paul could use it or not as he saw fit. Now I've got to get back to trying to figure out how to cleanly shut down USB. Argh.

 

[PaulStoffregen]

Isn't serialEvent() called from the ISR, and runs in ISR context?

No, serialEvent() is not called from ISR context.

Rather than polling with yield() ?

Yes, it's polled from yield().

This is an intentional, and controversial, design decision. It trades off low latency for ease of use.

I've heard many dismissive arguments for optimizing serialEvent() latency, but I've seen little evidence that typical Arduino users (or even moderately experienced software developers) can deal with the many issues involved with properly processing data from ISR context. Even just implementing a fifo buffer, written only by ISR and read only by main program, is tricky enough. Doing the sorts of processing people usually attempt with incoming serial data, and also navigating the minefield of ISR-unsafe core library and 3rd party libraries, is nearly impossible, even for experts.

Anyone capable of dealing with ISR issues can copy or modify the code from the core library.