Minimal Blink fails with void yield()

I thought the same Frank - Maybe I made a bad start of it - but I tried it once and it wasn't faster? And yes, lots of overhead to allocate a bool, set the bool, test the bool: a few times over.

It is nice to just kill the 'weak' yield() - but that is broken. As noted "EventResponder is a WIP it seems PJRC might incorporate best answer" ... but alas a week after posting and no answer . . .

Manitou first noted in p#3 that it was EventResponder - but referred to IDE build - but this is a bug in TeensyDuino build when the hack to yield() was introduced?

It was 3 months back - I just posted a comment on github: This yield.cpp change for EventResponder::runFromYield(); breaks removal of 'weak' yield and results in a non-functional upload when that common practice is done. and linked back to this thread.

Testing one bool is for sure way faster than 7 times calling a function "SerialX.available()"..
In addition even the tests in the brackets if (!running && flag) {if SerialX.available()...} can be saved if the flag is used in a smart way: Bitwise. Bit #0 for Serial1, bit#1 for Serial2... and setting back to 0 in read()...


Edit: Maybe something like this:
Code:
void yield(void)
{
    static uint8_t running=0;

    if (running) return; // TODO: does this need to be atomic? [B]<- no.[/B]
    running = 1;
int flag = RealNameOfTheVolatileFlag;
if (flag) {
    if (flag & 0x01) serialEvent();
    if (flag & 0x02) serialEvent1();
    if (flag & 0x04) serialEvent2();
    if (flag & 0x08) serialEvent3();
#ifdef HAS_KINETISK_UART3
    if (flag & 0x10) serialEvent4();
#endif
#ifdef HAS_KINETISK_UART4
    if (flag & 0x20) serialEvent5();
#endif
#if defined(HAS_KINETISK_UART5) || defined (HAS_KINETISK_LPUART0)
    if (flag & 0x40) serialEvent6();
#endif
}
    running = 0;

    EventResponder::runFromYield();
};
(should'nt be running = 0 after EventResponder::runFromYield();, or is this intended ?)
EventResponder::runFromYield(); needs a similar bit in the flag - and its own timer.

Edit: Doesn't this prevent including of the SerialX Code too ?
 
Last edited:
There was another discussion about this maybe a month or two ago.

At the time I proposed doing something similar... But at the time it sounded like maybe should wait until we could implement it with the event objects.

Some of the stuff at the time, I thought, would set a bit when for example Serial1 received a character and as you mentioned the code really simply first check if FLAG== 0 return...

I also that that maybe Serial1, would only set this flag if there was if there was a real user implementation for SerialEvent1. The default implementation would simply set/clear flag to tell the Serial1 ISR not to bother setting the flag anymore...


Some complications that I saw included, when can you clear this flag? You maybe can not clear it when you call serialEvent1 as they may choose not to process all of the outstanding input characters and may choose to defer to process some/all of it on another call... Could maybe be cleared in serial1.read()?
 
Seems the Arduino serialEvent() compatibility the only main() runtime compromise to the 'bare metal'?

If not used it should just go away - like void yield(void) did before. If a callback Event can be user registered that could be nice.

As noted Frank - my simple test must have been misapplied - somehow it was not any faster as I did it to check a BOOL.

Maybe a 'weak' set of serialEvent()'s that set the flag to ZERO. They are set to 1 at compile time. If the user provides serialEvent() that won't set to ZERO and then it will be called in yield? If not replaced it will only be called once?

<edit>: glanced at code and see this won't work as suggested - until Serial#.available - no call to serialEvent#() is made - this would only help on active ports where serialEvent was not used, unless a second flag were set on any receive - which would require maintaining and checking two flag bits to prevent calling serialEvent#()

Code:
void serialEvent(void) __attribute__ ((weak));
void serialEvent( flag&=!(0x01) );
void serialEvent1(void) __attribute__ ((weak));
void serialEvent1( flag&=!(0x02) );
void serialEvent2(void) __attribute__ ((weak));
void serialEvent2( flag&=!(0x04) );
// ...
 
Last edited:
Hi Frank, was simply trying to remember the conversation from a month or two ago. At the time I was trying to make it such that there would be very little performance gain by providing your own yield. My approach at the time was probably almost identical to what you are proposing.

As for Benefit: The idea I believe is to come up with a consistent solution....

Also remember saying event Objects does not necessarily imply Systick. Systick is only I believe used for the MillisTimer code.

There are options to run immediate. Likewise to be called by low priority interrupt, called by yield, ....

Like you said set back to zero in read (when queue is empty). Likewise if the user calls clear()...
 
Ok :)
I think I made my suggestions. I also think you'll find a good solution. I just hope I can argue in the future that Teensyduino is as good as bare-metal. It would be a shame if it wasn't. I always thought we agreed that no OS should be created. With a systick with high int- priority doing more than minmum, or that yield(), - with tasks that you can't get rid of easyly - times are changing...
That is just my opinion on this, and I do not think I need to go into it any further. And I don't want to waste our time on this either.

EOD (End of discussion for me) :)

Have fun,
Frank
 
Last edited:
This just bit me with my Zilch library. I too think that event responder stuff needs to be optional like any other library and leave the core as bare metal as possible.
 
Does my post above in conjunction with Frank's tests make it work to have yield() effect on serialEvent check's reduced? Could be extended with serial buffering code mods even when used - but would minimize the test effort in the bare metal case.

With OP : I just discovered yield() effect from Events breaks old behavior - but had otherwise figured PJRC would do it right in the end.

To the same end when the sketch calls the first Event.begin {or .attach /whatever} it could set a 'flag' bit to 1 to call the EventResponder::runFromYield(); in similar fashion - that doesn't break taking out the 'weak' yield().

I just posted this on the TD 1.39 released thread:
EventResponder::runFromYield(); in yield breaks standing use of replacing/removing the 'weak' yield() to keep Teensy Bare Metal.

Minimal-Blink-fails-with-void-yield()
 
Ok :)
I think I made my suggestions. I also think you'll find a good solution. I just hope I can argue in the future that Teensyduino is as good as bare-metal. It would be a shame if it wasn't. I always thought we agreed that no OS should be created. With a systick with high int- priority doing more than minmum, or that yield(), - with tasks that you can't get rid of easyly - times are changing...
That is just my opinion on this, and I do not think I need to go into it any further. And I don't want to waste our time on this either.

I think we are on the same page! I had a partial implementation back then doing more or less exactly what you suggested. I did have an extra test at the start, that tested the whole flag to zero and returned immediate... Figured that would probably be the majority case, especially with my hacks that if default SerialEvent functions were called then it never again sets that bit ... I don't think I have that code sitting around any more...

Maybe one of us should try it again and see if Paul will accept a Pull Request.

As for your other Pull request as I mentioned I wondered if there would be an issue with TeensyThreads...

So my thoughts before you did it was to move the systick timer back to the source file it originally was in and put in call that is conditional. Something like:
Code:
void   (*run_from_timer_func)() = nullptr;
void setRunFromTimerFunc(void (*f)()) {run_from_timer_func = f;}

extern "C" volatile uint32_t systick_millis_count;

void systick_isr(void)
{
	systick_millis_count++;
        if (run_from_timeer_func) {
            (*run_from_timer_func)();
        }
}

Then have the MillisTimer code code do a call with it's ( MillisTimer::runFromTimer()) function on the first call to begin (or beginRepeating)...

So in the default case we have a slight overhead to test for NULL and jump...
 
I'm looking at this today.

Sorry about the delayed reaction. This thread started last week while Robin was out of town. Normally she does most of the day-to-day operational stuff to run PJRC, so I can focus on tech questions and (sometimes) software dev. When she's out to town, I fall pretty far behind on tech issues. Usually I prioritize answering custom support questions first. Software bugs and actually developing anything new pretty much grinds to a halt. I know it's frustrating, to discuss a serious bug and not get any reply from me. Please try to understand PJRC is only 4 people, 2 of whom test and package products, so when we're down 1 person it really shows. I try to always keep up with the forum, but sometimes it's just not practical.
 
Good to see you Paul - hopefully our neurotic posting helps . . .

Won't the EventResponder have some initial wake (.attach) up code called in or before setup()? It could one time replace the pointer to systick_isr(void) with systick_isr_ER(void)? Where the _ER function is without conditional:
Code:
void systick_isr_ER(void)
{
	systick_millis_count++;
        (*run_from_timer_func)();
}

Of course if that vector is not == systick_isr() - then it is a chaining rabbit hole for cases that already redefine that : Zilch or TeensyThreads . . .
 
EDIT: It does work, disregard below.



I updated the core (1.39 / T3.6 / 180MHz / Faster / Serial) with Franks patches here and tested it with the sketch below but overriding yield function still crashes the system, no usb no blink:

Code:
int led = 13;

void setup() {
  // initialize the digital pin as an output.
  pinMode(led, OUTPUT);
  digitalWriteFast(led, HIGH);   // turn the LED on (HIGH is the voltage level)
}

void loop() {
  digitalWriteFast(led, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(1000);               // wait for a second
  digitalWriteFast(led, LOW);    // turn the LED off by making the voltage LOW
  delay(1000);               // wait for a second
}

void yield(void) {

};
 
Last edited:
So it looks like on my computer the Teensydunio core gets cached and any edits after the first compile to the core are not picked up, so frank your patch does work now! Not sure why the core is cached on my system, macOS Sierra but that another issue for me.
 
Glad you re-produced the issue Paul.

re #41: I think Frank #40 was saying to duff - his attempt to use Frank's solution failed to fully rebuild after he made the edits and that resulted in his post #37 saying it didn't work.

<edit> : confirmed in Cross Post - builder didn't catch changed files for core rebuild.
 
It Works.and Blinks.

On my IDE 1.8.3 I reinstalled TD 1.39.
edited 'weak' line ~#148 in mk20dx128.c to :: void systick_isr(void);

Smallest w/LTO and NO SERIAL::
BlinkMin with :: // void yield(void) {};
BlinkMin with :: void yield(void) {};

It grew 200 bytes from 1712 bytes ???
Sketch uses 1920 bytes (0%)
Global variables use 492 bytes (0%)

Also worked Smallest w/LTO and SERIAL
Sketch uses 4564 bytes (1%)
Global variables use 2036 bytes (3%)
 
Yeah, it increases code size slightly. Part is essential code that was getting incorrectly optimized away. The rest is the static parts of MillisTimer and EventResponder.

I know right now the value of these experimental features may seem questionable. But I'm confident in the long term a couple hundred bytes a code space and a dozen bytes of RAM will seem like a very small price to pay for allowing many libraries to interoperate easily.
 
Back
Top