Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 4 of 4

Thread: ISR routine does not appear to be passing back boolean?

  1. #1

    ISR routine does not appear to be passing back boolean?

    Folks,

    I've using TeensyTimerTool and it is showing odd behavior. If I put a scope on the LED_PIN, in the code shown below, if I use loop(), it works fine. I get a very nice 8ms change in the LED_PIN. But in that version of loop() then I'm changing the volatile from the ISR which I thought was a no-no.

    If I change the second function loop1(), to loop() (and remove the original loop, then there is no pause, and the LED_PIN changes as rapidly as loop can er, loop.

    where am i going wrong?

    owen
    Code:
    #include "TeensyTimerTool.h"
    
    #define LED_PIN       13
    
    using namespace TeensyTimerTool;
    Timer timer1(TMR1);
    Timer timer2(GPT1); // First channel on TMR1 aka QUAD timer module. (TMR1 - TMR4, four 16bit channels each)
    
    // loop stuff
    volatile boolean _pause = false;
    boolean pause = false;
    
    // LED 
    boolean blinkOn = false;
    
    void setup() {
      pinMode(LED_PIN, OUTPUT); 
    
      timer1.beginPeriodic(tickLoop, 1); 
      timer2.beginPeriodic(foo, 8000);
      pause = false;
      _pause = false;
    }
    
    // this makes a nice 8 ms square wave
    void loop() {
      noInterrupts();
      pause = _pause;
      interrupts();
    
      while(_pause) {}
      _pause = true;
    
      digitalWrite(LED_PIN, blinkOn); 
      blinkOn = !blinkOn;
    }
    
    // this makes dogmeat
    void loop1() {   
      noInterrupts();
      pause = _pause;
      interrupts();
    
      while(pause) {}
      pause = true;
    
      digitalWrite(LED_PIN, blinkOn); 
      blinkOn = !blinkOn;
    }
    
    void foo() { 
      _pause = false;
    }
    
    void tickLoop() {
      // does lost of other things
    }

  2. #2
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    22,458
    Quote Originally Posted by owhite View Post
    But in that version of loop() then I'm changing the volatile from the ISR which I thought was a no-no.
    That's the purpose of volatile. It forces code to actually read the variable, so you *can* change it in the ISR and have your program detect the change.

    Without volatile the compiler makes a lot of assumptions about memory as it compiles each function. Data in memory doesn't just spontaneously change. So when you write something like "while (pause) {}", the compiler tries very hard to be smart. It knows "pause" can't change on its own, so it will eliminate that loop. It's trying to be smart and avoid unnecessary work, and why would it bother having the loop at all when it knows the result will always be the same.

    Using volatile tells the compiler that the data in memory *can* change spontaneously. It also tells the compiler that anything you write needs to be actually written out to memory ASAP (otherwise the compiler tries to be smart and may never actually do the write if it believes nothing else ever reads it). That's why you use it with variables shared with interrupts.

  3. #3
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    1,043
    While Paul's comments are certainly true, in this special case the problem is different (the code assigns the volatile _pause to pause, so it wont be optimized away).

    See my comments in the code for an explanation why it doesn't work.

    Code:
    // this makes dogmeat
    void loop()
    {
        noInterrupts();   // you never assign _pause other than in setup and the ISR...
        pause = _pause;   // ...since the ISR always sets _pause to false, pause will _always_ be set to false here
        interrupts();    
    
        while (pause) {}  // pause is always false -> never waits
        pause = true;     // will be overwritten to false in the next loop
    
        digitalWrite(LED_PIN, blinkOn);
        blinkOn = !blinkOn;
    }
    A few general remarks to your code:

    • The 'Timer' interface is depreciated and will be removed in one of the next versions of the library. Better to use 'PeriodicTimer' and 'OneShotTimer' instead. (library docu: https://github.com/luni64/TeensyTimerTool/wiki)
    • Since pause/_pause is boolean, assignment is atomic and there is no need to disable interrupts
    • You set up timer1 to call tickloop at 1MHz. A comment in tickloop says "does lost of other things". This is calling for trouble. Obviously the other things should be ready in less than 1Ás. Additionally the interrupt system is not very efficient. So, it will probably overload if you spend more than a couple of 100ns in the ISR.


    As an example, this is how I would code this:
    Code:
    #include "TeensyTimerTool.h"
    using namespace TeensyTimerTool;
    
    PeriodicTimer timer1(TMR1); // First channel on TMR1 aka QUAD timer module. (TMR1 - TMR4, four 16bit channels each)
    PeriodicTimer timer2(GPT1); 
    
    void setup()
    {
        pinMode(LED_BUILTIN, OUTPUT);
    
        timer1.begin(tickLoop, 1);
        timer2.begin(foo, 8000);
    }
    
    
    volatile boolean waitFlag;
    
    void loop()
    {
        waitFlag = true;
        while (waitFlag){}
    
        digitalToggle(LED_BUILTIN);
    }
    
    void foo()
    {
        waitFlag = false;
    }
    
    void tickLoop()
    {
        // does lost of other things
    }
    Last edited by luni; 08-03-2020 at 06:18 AM.

  4. #4
    thanks. digitalToggle() was a new one for me. I updated the the teens4 library and there it was.

    I'll look at the timing lengths of tickLoop() that may be what got me in trouble in the first place.

    preciate it.

Posting Permissions

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