Forum Rule: Always post complete source code & details to reproduce any issue!
Page 1 of 3 1 2 3 LastLast
Results 1 to 25 of 74

Thread: Minimal Blink fails with void yield()

  1. #1
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    6,834

    Minimal Blink fails with void yield()

    In response to a query about code/RAM used on base USB sketch - I did this to see the current numbers without USB. I started with Teensy :: arduino-1.8.4\examples\01.Basics\Blink

    This fails on a T_3.1 - 'No USB' - compiled 'smallest code' { with and without LTO } [also fails FAST w/LTO}.

    Uploading with TeensyLoader 1.39 and IDE 1.8.4 on Windows, Larger code executed with no problem when using PJRC yield(). NOTE: Another recent post with a 'void yield()' in library code was dying on start, removing that allowed it to run.

    Blink as below fails to show 'LED' signs of running when using this to further reduce base code: void yield(void) {};

    Code:
    void yield(void) {}; 
    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
    }
    
    // USB type : No USB
    // ------------------------------
    // Smallest Code - PJRC Yield - T_3.1 :: Uploads and Blinks
    // Sketch uses 3508 bytes (1%)
    // Global variables use 900 bytes (1%)
    
    // Smallest Code - void Yield - T_3.1 :: Uploads, then Blink fails
    // Sketch uses 2280 bytes (0%)
    // Global variables use 488 bytes (0%)
    
    // Smallest Code w/LTO - void Yield - T_3.1 :: Uploads, then Blink fails
    // Sketch uses 1680 bytes (0%)
    // Global variables use 480 bytes (0%)
    
    // Fast w/LTO - void Yield - T_3.1 :: Uploads, then Blink fails
    // Sketch uses 3104 bytes (1%)
    // Global variables use 1544 bytes (2%)

  2. #2
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    6,834
    This is the thread where void yield() stopped another project until removed from external library - in that case the user was using USB Serial: Need-Help-with-Teensy-3-6-Serial-COM-port-disappears-after-uploading-code

  3. #3
    Senior Member+ manitou's Avatar
    Join Date
    Jan 2013
    Posts
    1,584
    With 1.8.3/1.37 blink works with your yield() and Smallest on T3.2 (and no USB). I'm wondering if EventResponder stuff that was added to 1.8.4/1.38 is affecting things? 1.8.4 yield() has an EventResponder check.

    can you re-test with 1.8.3? or one could modify your in-sketch yield() to do various event checking pieces as taken from
    hardware/teensy/avr/cores/teensy3/yield.cpp

    ... further study required
    Last edited by manitou; 09-24-2017 at 12:33 AM.

  4. #4
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    3,652
    Yes - the system appears to depend on runFromYield() being called.

    Code:
    #include "EventResponder.h"
    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) {
        EventResponder::runFromYield();
      };

  5. #5
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    This can't be the whole truth.. I have sketches without, and they work. Don't know why.

  6. #6
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    3,652
    Quote Originally Posted by Frank B View Post
    This can't be the whole truth.. I have sketches without, and they work. Don't know why.
    Yep - I think the whole truth is, that the yield code called runfromYield... So the event responder code was brought in.

    The actual code that is really needed was the systick timer ISR, which counted millis... Which is now in EventResponder.cpp
    Code:
    extern "C" volatile uint32_t systick_millis_count;
    void systick_isr(void)
    {
    	systick_millis_count++;
    	MillisTimer::runFromTimer();
    }
    So whole program right now is:
    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) {
    };
    extern "C" volatile uint32_t systick_millis_count;
    void systick_isr(void)
    {
      systick_millis_count++;
    }

  7. #7
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    Oh, wow.. thank you for your examination..
    That means.. if I don't want these additions, it is not enough to override yield() (i need nothing of the stuff there..), no, with the new versions I have to override (eh..no, it is not "weak"(?)) / redirect the systick too.
    Thanks Kurt !

    Would have been nice to mention that on a place that gets read..

  8. #8
    Senior Member
    Join Date
    Jul 2014
    Posts
    1,877
    Quote Originally Posted by Frank B View Post
    Would have been nice to mention that on a place that gets read..
    Some sort of Wiki? It will not happen... (or maybe, sometime,...)

  9. #9
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    3,652
    I am thinking out loud, that maybe the default systick timer code should be in a core place.

    Maybe only if someone uses the event code and sets the first event that uses call on yield, should it either replace the vector and/or the default one should have function pointer to call, that if non-null gets called...

  10. #10
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    Quote Originally Posted by WMXZ View Post
    Some sort of Wiki? It will not happen... (or maybe, sometime,...)
    +1

    Ok, in case someone reads this:
    To disable the new behaviour:
    Code:
    void mySystick_isr(void);
    
    void setup(){
        _VectorsRam[15] = mySystick_isr;
    }
    
    [...]
    
    extern "C" volatile uint32_t systick_millis_count;
    void mySystick_isr(void)
    {
        systick_millis_count++;    
    }
    (I hope there will be a better way to do this, in future)


    If you want to disable the stuff in yield() :
    Code:
    void yield(void){}
    But something like yield() can be useful sometimes, because it gets called not only after loop() but in some cases when the core waits for things, too.
    In this case:
    Code:
    void yield(void){
      static volatile uint8_t running = 0;
      if (running) return;
      running = 1;
     
      //Add your code here
    
      running = 0;
    }
    But be careful what you do here.

    For reference, this is the original yield():
    Code:
    void yield(void)
    {
        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();
    #ifdef HAS_KINETISK_UART3
        if (Serial4.available()) serialEvent4();
    #endif
    #ifdef HAS_KINETISK_UART4
        if (Serial5.available()) serialEvent5();
    #endif
    #if defined(HAS_KINETISK_UART5) || defined (HAS_KINETISK_LPUART0)
        if (Serial6.available()) serialEvent6();
    #endif
        running = 0;
        EventResponder::runFromYield();
    };
    (btw, isn't this a bug ? Shouldn't be "running = 0" AFTER EventResponder::runFromYield(); ??)
    Last edited by Frank B; 09-17-2017 at 06:28 PM.

  11. #11
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    Quote Originally Posted by KurtE View Post
    I am thinking out loud, that maybe the default systick timer code should be in a core place.
    It was there

  12. #12
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    Is it really necessary to start EventResponder::runFromYield(); 1000 times a second ? 1000 is minimum from systick - it gets called more often....in addition, in runs with every loop(), and way more often when the core waits...

    And yes, in EVERY Sketch. Regardless if it needed or not. In 99.999% a Sketch does not need it...!

  13. #13
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    6,834
    Quote Originally Posted by manitou View Post
    With 1.8.3 blink works with your yield() and Smallest on T3.2 (and no USB). I'm wondering if EventResponder stuff that was added to 1.8.4 is affecting things? 1.8.4 yield() has an EventResponder check.

    can you re-test with 1.8.3? or one could modify your in-sketch yield() to do various event checking pieces as taken from
    hardware/teensy/avr/cores/teensy3/yield.cpp

    ... further study required
    My 1.8.3 must have EventR...() Beta - but confirm IDE 1.8.1 builds and runs fine same sketch my machine.

    Wow - Glad I wrote that up . . . in the middle of stuff here looks like I started a fun PARTY ... will be back later.

    It doesn't add a lot of code - just a critical link that breaks - size is about the same:
    // IDE 1.8.1 :: Smallest Code - void Yield - T_3.1 :: Uploads and Blinks
    // Sketch uses 2304 bytes (0%)
    // Global variables use 488 bytes (0%)

  14. #14
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    6,834
    Good research @manitou!

    As a WIP - hopefully this can get cleaned up in a way to avoid overhead. As yield() indeed 'can be' unneeded overhead and as a weak is nice to disable, but now that no longer works as core function was moved in.

    IIRC : isn't EventResponder going to end up running/driving some core interfaces behind the scenes over time?

    > WIKI would be cool! Not sure it would have brought this out like a simple sketch did - but now known - it could be documented.
    > it seems "running = 0;" should be the last of yield() to prevent re-entry

    Windy storm and rain just blew in - must get out and tend to stuff.
    Last edited by defragster; 09-17-2017 at 09:46 PM.

  15. #15
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    Tim,

    I don't know your exact setting when blink failed - can you try this patch please : https://github.com/PaulStoffregen/cores/pull/254

  16. #16
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    3,652
    Quote Originally Posted by Frank B View Post
    Tim,

    I don't know your exact setting when blink failed - can you try this patch please : https://github.com/PaulStoffregen/cores/pull/254
    Frank, this might work, but there may be an interesting issue, with using Teensy Threads, that will also try to set that same vector. So it may be last one wins.

  17. #17
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    Ok, yes, depends on the order. Without the patch, Teensy Threads always wins, correct ? (I don't know what it does...)

    On c64, long years ago, we used a kind of chaining, where the new interrupt called the old routine. - if i remember correctly, after 30 years...or it wasn't the c64 but an other machine...lol..

  18. #18
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    3,652
    Yep - I have not looked at their stuff lately, but I remember there being an issue that was fixed during one of the recent betas...

    Yes - I believe old MSDOS programs used to chain interrupts as well. Hold on to the previous interrupt handler and put yours in, and then if appropriate pass an interrupt on down the chain...
    It was always fun on being able to remove the hooks. It was easy to remove the last one added, but I remember having code around that would have to leave a stub function if your program exited that had a hook and you were not the first one in the chain... Memories...

  19. #19
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    Oh yes, it was msdos.. you are right. It was fun.


    Anyway, I think the best way for both is, to use "IntervalTimer" instead. It handles such things better and doesnot influence myriads of innocent sketches
    ?..

  20. #20
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    6,834
    Frank - Indeed I hand made the edits and that code compiles and works - nothing special to compile - just creating a "void vield()":
    Code:
    // Smallest Code w/LTO - void Yield - T_3.1 :: Uploads and Blinks
    Sketch uses 1712 bytes (0%)
    Global variables use 480 bytes (0%)
    Chaining came to mind for Teensy Threads - but that can get ugly.

    Must be a good solution. When EventResponder finalizes it should be outside yield() as that has always been 'weak'. If it becomes 'always on' - perhaps 'EventResponder::runFromYield();' would be added to main() - renamed to EventResponder::runFromMain(); ?

  21. #21
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    Hi Tim,
    If it was me, I'd remove that yield() completely But I don't think that's up for debate.

    Chaining: I can imagine a small c++ library in the core, that can do it.. an, as a nice side-effect, for every interrupt.
    Teensy Threads: I've looked at the source. RE: Millis, It does almost exactly the same as Events.. would be better to use an other timer...PIT..or...FTM..

    But ok... my solution seems not to find friends.. correct ?

  22. #22
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    6,834
    Frank - your solution works - it would be up to Paul to decide the direction as it will tie up a resource? Since the EventResponder is a WIP it seems PJRC might incorporate best answer.
    yield() is an Arduino compatibility thing - so not likely to go away - though being able to remove that overhead since serialEvent#() isn't really as cool as it seemed when I first saw it.

  23. #23
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    4,511
    yield() :

    Why not - simply - set a flag when all the checks are needed ?
    Serial1...SerialX and even Event Responder could set a global volatile flag IN their INTERRUPT (which is called anyway).

    All SerialX can set the same Flag.

    So yield could look like

    if (!running && flag) {
    do_all_the _checks_and call_events_like_orginal_yield()
    }

    This would be *much* faster than now and would not hurt apps so much that are not interested in this stuff.. ;-)
    And 100% compatible.

    Nope this time no pullrequest... don't like to invest work for nothing when nobody even answers... even a "no, because" would be helpful or nice. but not nothing.
    Last edited by Frank B; 09-23-2017 at 04:23 PM.

  24. #24
    Senior Member
    Join Date
    Dec 2016
    Location
    Montreal, Canada
    Posts
    2,738
    such a precious waste of memory (for a bool? ), hopefully we have enough left over for our projects lol

  25. #25
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    6,834
    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.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •