Stuck in a loop

The Project:

5" dia circle with 16 5mm leds chasing, 4 (90deg from each other) on at any one time powered by 4 aa batteries. It's controlled by an attiny 825 bootloaded with Arduino. Everything works great. I guy that I'm creating this for now asked if I could add remote on/off functionality. I said I'll look into it.
I found an old tv remote and got the codes using Raw IR Commander from the adafriut.com site.

The Problem:

I changed the code as needed and uploaded it from an Arduino to the 825 and ran it on a breadboard using one light. It worked as expected by turning on and off 1 led. When I hooked up all 16 lights it would not work. I'm guessing it’s too much for the 825. When I ran the same sketch on a Arduino it ran all 16 lights. The road block that I’m facing is, I can turn the lights on with the remote but not turn them off. The program looks for a button push signal and when it finds one it turns on the lights and gets stuck in a loop. How can I get the sketch to continue to run the loop but pass the focus back to the button push to turn the speed of rotation up/down or off with the leds? A coworker told me I need a DoEvents function but Arduino doesn't have that.
I tried adding a timer but I’m not sure how that works.

Any thoughts?

#define IRpin_PIN      PIND
#define IRpin          2


#define MAXPULSE 65000
#define NUMPULSES 50




#define RESOLUTION 20 


#define FUZZINESS 20


uint16_t pulses[NUMPULSES][2];   
uint8_t currentpulse = 0; 


#include "ircodes.h"

int Speed = 1;
int DelayP = 40*Speed;
int DelayS = 10*Speed;
int On;

void setup(void) {
  //    Timer1.initialize(500000);
  //  Timer1.attachInterrupt(callback);
  for (int thisPin = 4; thisPin < 8; thisPin++)  {
    pinMode(thisPin, OUTPUT);      
  }

}

void loop(void) {
  int numberpulses;

  numberpulses = listenForIR();


  if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
    On = 1;
    if (On == 1) {
      do{
        for (int thisPin = 4; thisPin < 8; thisPin++) {
          // turn the pin on:
          digitalWrite(thisPin, HIGH);  
          delay(DelayP);                  
          // turn the pin off:
          digitalWrite(thisPin, LOW);    
        }
      } 
      while(On ==1);

    }
  }
  if (IRcompare(numberpulses, Speedup,sizeof(Speedup)/4)) {
    Speed /= .5;
  }
  if (IRcompare(numberpulses, Slowdown,sizeof(Slowdown)/4)) {
    ++Speed;
  }
  if (IRcompare(numberpulses, Stop,sizeof(Stop)/4)) {
    On = 0;
  }
  delay(500);

}

//KGO: added size of compare sample. Only compare the minimum of the two
boolean IRcompare(int numpulses, int Signal[], int refsize) {
  int count = min(numpulses,refsize);

  for (int i=0; i< count-1; i++) {
    int oncode = pulses[i][1] * RESOLUTION / 10;
    int offcode = pulses[i+1][0] * RESOLUTION / 10;


    if ( abs(offcode - Signal[i*2 + 1]) <= (Signal[i*2 + 1] * FUZZINESS / 100)) {

    } 
    else {
      return false;
    }
  }
  // Everything matched!
  return true;
}

int listenForIR(void) {
  currentpulse = 0;

  while (1) {
    uint16_t highpulse, lowpulse;  // temporary storage timing
    highpulse = lowpulse = 0; // start out with no pulse length
    while (IRpin_PIN & (1 << IRpin)) {
      // pin is still HIGH

      // count off another few microseconds
      highpulse++;
      delayMicroseconds(RESOLUTION);

      // If the pulse is too long, we 'timed out' - either nothing
      // was received or the code is finished, so print what
      // we've grabbed so far, and then reset

      // KGO: Added check for end of receive buffer
      if (((highpulse >= MAXPULSE) && (currentpulse != 0))|| currentpulse == NUMPULSES) {
        return currentpulse;
      }
    }
    // we didn't time out so lets stash the reading
    pulses[currentpulse][0] = highpulse;

    // same as above
    while (! (IRpin_PIN & _BV(IRpin))) {
      // pin is still LOW
      lowpulse++;
      delayMicroseconds(RESOLUTION);
      // KGO: Added check for end of receive buffer
      if (((lowpulse >= MAXPULSE)  && (currentpulse != 0))|| currentpulse == NUMPULSES) {
        return currentpulse;
      }
    }
    pulses[currentpulse][1] = lowpulse;

    // we read one high-low pulse successfully, continue!
    currentpulse++;
  }
}
//void callback()
//{
//  digitalWrite(10, digitalRead(10) ^ 1);
//}

[code]


Here is the timer code:

[code]
/*
 *  Interrupt and PWM utilities for 16 bit Timer1 on ATmega168/328
 *  Original code by Jesse Tane for http://labs.ideo.com August 2008
 *  Modified March 2009 by Jérôme Despatis and Jesse Tane for ATmega328 support
 *  Modified June 2009 by Michael Polli and Jesse Tane to fix a bug in setPeriod() which caused the timer to stop
 *  Modified June 2011 by Lex Talionis to add a function to read the timer
 *  Modified Oct 2011 by Andrew Richards to avoid certain problems:
 *  - Add (long) assignments and casts to TimerOne::read() to ensure calculations involving tmp, ICR1 and TCNT1 aren't truncated
 *  - Ensure 16 bit registers accesses are atomic - run with interrupts disabled when accessing
 *  - Remove global enable of interrupts (sei())- could be running within an interrupt routine)
 *  - Disable interrupts whilst TCTN1 == 0.  Datasheet vague on this, but experiment shows that overflow interrupt 
 *    flag gets set whilst TCNT1 == 0, resulting in a phantom interrupt.  Could just set to 1, but gets inaccurate
 *    at very short durations
 *  - startBottom() added to start counter at 0 and handle all interrupt enabling.
 *  - start() amended to enable interrupts
 *  - restart() amended to point at startBottom()
 * Modiied 7:26 PM Sunday, October 09, 2011 by Lex Talionis
 *  - renamed start() to resume() to reflect it's actual role
 *  - renamed startBottom() to start(). This breaks some old code that expects start to continue counting where it left off
 *
 *  This program is free software: you can redistribute it and/or modify
 *	it under the terms of the GNU General Public License as published by
 *	the Free Software Foundation, either version 3 of the License, or
 *	(at your option) any later version.
 *
 *	This program is distributed in the hope that it will be useful,
 *	but WITHOUT ANY WARRANTY; without even the implied warranty of
 *	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *	GNU General Public License for more details.
 *
 *	You should have received a copy of the GNU General Public License
 *	along with this program.  If not, see <http://www.gnu.org/licenses/>.
 *
 *  See Google Code project http://code.google.com/p/arduino-timerone/ for latest
 */
#ifndef TIMERONE_h
#define TIMERONE_h

#include <avr/io.h>
#include <avr/interrupt.h>

#define RESOLUTION 65536    // Timer1 is 16 bit

class TimerOne
{
  public:
  
    // properties
    unsigned int pwmPeriod;
    unsigned char clockSelectBits;
	char oldSREG;					// To hold Status Register while ints disabled

    // methods
    void initialize(long microseconds=1000000);
    void start();
    void stop();
    void restart();
	void resume();
	unsigned long read();
    void pwm(char pin, int duty, long microseconds=-1);
    void disablePwm(char pin);
    void attachInterrupt(void (*isr)(), long microseconds=-1);
    void detachInterrupt();
    void setPeriod(long microseconds);
    void setPwmDuty(char pin, int duty);
    void (*isrCallback)();
};

extern TimerOne Timer1;
#endif

[/code][/code]

i can clearly see why your arduino 'stops', or better, seems to stop.

it on this line:

 if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
    On = 1;
    if (On == 1) {
      do{
        for (int thisPin = 4; thisPin < 8; thisPin++) {
          // turn the pin on:
          digitalWrite(thisPin, HIGH);  
          delay(DelayP);                  
          // turn the pin off:
          digitalWrite(thisPin, LOW);    
        }
      } 
      while(On ==1);         //<<<<<<<<<<<<<<HERE is the mistake

    }
  }

as you tell it to do nothing while on==1. on will never change in that loop, so once it gets an IR code on is 1 and it will be stuck in the while loop. just remove this one and it will be fine

i don't thinks this runs fine on an arduino UNO though, it will react exactly the same way on the while, so i can see no differences

Steen:
i can clearly see why your arduino 'stops', or better, seems to stop.

it on this line:

 if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {

On = 1;
    if (On == 1) {
      do{
        for (int thisPin = 4; thisPin < 8; thisPin++) {
          // turn the pin on:
          digitalWrite(thisPin, HIGH); 
          delay(DelayP);                 
          // turn the pin off:
          digitalWrite(thisPin, LOW);   
        }
      }
      while(On ==1);        //<<<<<<<<<<<<<<HERE is the mistake

}
  }




as you tell it to do nothing while on==1. on will never change in that loop, so once it gets an IR code on is 1 and it will be stuck in the while loop. just remove this one and it will be fine

i don't thinks this runs fine on an arduino UNO though, it will react exactly the same way on the while, so i can see no differences

If I change the while(On ==1); to a while(On ==0); the program runs only once. I want it to stay in the loop(keep on flashing) untill I press the stop button.
I'm not sure what you mean by it won't run fine on an UNO. Thats what it is currently running on.

POSENG1911:
If I change the while(On ==1); to a while(On ==0); the program runs only once. I want it to stay in the loop(keep on flashing) untill I press the stop button.
I'm not sure what you mean by it won't run fine on an UNO. Thats what it is currently running on.

You need some way to update On before you hit the while(On==0); line.

Arrch:

POSENG1911:

You need some way to update On before you hit the while(On==0); line.

I'm not sure how to do that.

You need to define the conditions that make "On" true, and then assign the value based on that.

Try taking this part:

    if (On == 1) {
//      do{    // get rid of this loop, you won't need it
        for (int thisPin = 4; thisPin < 8; thisPin++) {
          // turn the pin on:
          digitalWrite(thisPin, HIGH);  
          delay(DelayP);                  
          // turn the pin off:
          digitalWrite(thisPin, LOW);    
        }
//      } 
//      while(On ==1);         //<<<<<<<<<<<<<<HERE is the mistake

out of this part:

 if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
    On = 1;

so that changing the value of On is a separate block from running the lights if (On == 1).

The only connection between control and action is then the variable(s) set by control and used by action. Once you get that, a whole lot of code becomes simpler and loop() is your friend.

GoForSmoke:
Try taking this part:

    if (On == 1) {

//      do{    // get rid of this loop, you won't need it
        for (int thisPin = 4; thisPin < 8; thisPin++) {
          // turn the pin on:
          digitalWrite(thisPin, HIGH); 
          delay(DelayP);                 
          // turn the pin off:
          digitalWrite(thisPin, LOW);   
        }
//      }
//      while(On ==1);        //<<<<<<<<<<<<<<HERE is the mistake




out of this part:



if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
    On = 1;




so that changing the value of On is a separate block from running the lights if (On == 1).

The only connection between control and action is then the variable(s) set by control and used by action. Once you get that, a whole lot of code becomes simpler and loop() is your friend.

if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
On = 1;
if (On == 1) {
// do{
for (int thisPin = 4; thisPin < 8; thisPin++) {
// turn the pin on:
digitalWrite(thisPin, HIGH);
delay(DelayP);
// turn the pin off:
digitalWrite(thisPin, LOW);
}

// while(On ==1);

}
}

That is the code that I used with the do{ and while(On == 1); commented out but now when I press the on button, the leds only flash once. I need them to keep flashing untill I press the off button.

Then follow the directions in the post you quoted.

Could you simplfiy it because I'm apperently just not getting it. I thought by commenting out the Do and While lines it work.

Is this what I should do? If it is the program does nothing.

void loop(void) {
  int numberpulses;

  numberpulses = listenForIR();


  if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
    On = 1;
    
    }
  if (IRcompare(numberpulses, Speedup,sizeof(Speedup)/4)) {
    Speed /= .5;
  }
  if (IRcompare(numberpulses, Slowdown,sizeof(Slowdown)/4)) {
    ++Speed;
  }
  if (IRcompare(numberpulses, Stop,sizeof(Stop)/4)) {
    On = 0;
  }
  delay(500);

}

//    if (On == 1) {
////     do{
//        for (int thisPin = 4; thisPin < 8; thisPin++) {
//          // turn the pin on:
//          digitalWrite(thisPin, HIGH);  
//         delay(DelayP);                  
//          // turn the pin off:
//          digitalWrite(thisPin, LOW);    
//        }
//       
//   // while(On ==1);

I think the solution to the problem would be to attach an interrupt to the IR pin, because the problem itself is that the when the while loop is active there is no way to deactivate it. As I am rather new to them myself, suggestions for implementation would be appreciated.

POSENG1911:
Is this what I should do? If it is the program does nothing.

Where in that code do any digitalWrite to 'thisPin' (terribly unhelpful variable name by the way)?

LeviathansWake0:
I think the solution to the problem would be to attach an interrupt to the IR pin, because the problem itself is that the when the while loop is active there is no way to deactivate it. As I am rather new to them myself, suggestions for implementation would be appreciated.

No, the solution is to not use any while loops or delays. The Blink Without Delay example code should give you a good idea on how to make something blink without stalling the program completely; it's best to master every aspect of that code before trying to implement it.

POSENG1911:

GoForSmoke:
Try taking this part:

    if (On == 1) {

//      do{    // get rid of this loop, you won't need it
        for (int thisPin = 4; thisPin < 8; thisPin++) {
          // turn the pin on:
          digitalWrite(thisPin, HIGH); 
          delay(DelayP);                 
          // turn the pin off:
          digitalWrite(thisPin, LOW);   
        }
//      }
//      while(On ==1);         //<<<<<<<<<<<<<<HERE is the mistake




out of this part:



if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
    On = 1;




so that changing the value of On is a separate block from running the lights if (On == 1).

The only connection between control and action is then the variable(s) set by control and used by action. Once you get that, a whole lot of code becomes simpler and loop() is your friend.

if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
On = 1;
if (On == 1) {
// do{
for (int thisPin = 4; thisPin < 8; thisPin++) {
// turn the pin on:
digitalWrite(thisPin, HIGH);
delay(DelayP);
// turn the pin off:
digitalWrite(thisPin, LOW);
}

// while(On ==1);

}
}

That is the code that I used with the do{ and while(On == 1); commented out but now when I press the on button, the leds only flash once. I need them to keep flashing untill I press the off button.

// this part sets the control variable 
  if (IRcompare(numberpulses, Play,sizeof(Play)/4)) 
  {
    On = 1;
  }
  else 
  {
    On = 0; // because logically.... do I really have to spell it out?
  }

// and this part works the leds --- it has been taken OUT of the control change { } block
  if (On == 1) 
  {
    for (int thisPin = 4; thisPin < 8; thisPin++) 
    {
          // turn the pin on:
          digitalWrite(thisPin, HIGH);  
         delay(DelayP);                  
          // turn the pin off:
          digitalWrite(thisPin, LOW);    
    }
  }    // notice how the opens { and closes } have easy to see matching indents?

The led being on is controlled by the return of IRcompare(numberpulses, Play,sizeof(Play)/4).
Is that going to be enough?

GoForSmoke, \

Thanks very much for the help.

I changed the program as you sugested but i still face the same problem. The code only runs once for each button press. I want to press the on button and the leds should start to chase on stop untill i press the stop button.

Then change what makes On = 0.
Perhaps make the stop button do that instead of the start button not being pressed?

Because you see I hadn't gone through your whole sketch, just pointed out a way to get the action separated from the control.

You really seem lost in details. Maybe you should write what you want to do in comments before writing the code to make that happen and then check your steps before you code.

// loop()
// detect start button pressed and set On to 1
// detect stop button pressed and set On to 0
// make the leds do their dance if On is 1
// end loop

Does that make sense to you? Can you see how it would work or where it would not or if it's missing anything?
Do you see how you can add more actions without needing to jam them in to the steps outlined? By changing the value of On, any code section can start or stop the led chase. If you want to change how the led chase runs, what would you do?

herp derp. if you read his code, you'd see that he already has stop coded to make on = 0 and start coded to make on = 1, the problem lies with the fact that the LED's are not continuously blinking, because his for statement only runs once before hitting a massive delay. Thus he started the topic asking how to make the chip pay attention to the ir sensor while the While loop was running so he could manipulate the variable on via button press. But getting rid of the while loop causes a massive delay as the chip processes everything and only runs the if once. It isn't about "Making sense" yes it makes sense but your not looking at what he wrote.

He had the led chase code dependent on the code that sets On = 1 just having been run.
I gave him a piece of the fix, separating action code from control code.
Now he needs to change what makes On = 0... too bad I stuck that else in, I was -only- trying to show how to disentangle the code blocks because if he knew he needs to do that then he wouldn't have the problem in the first place.

POSENG, get rid of that else I wrote that sets On = 0, does that do the trick? I really wasn't and still am not sure what IRcompare() does, and I'd rather not be if I don't need to. I saw you lost and I'd rather leave you knowing more how to find your way than to just give you a code fix.

I removed the else statment and no the code still runs just once per button push. I am tring to turn on my project with an old tv remote. I have the correct codes to do this.

This code

]if (IRcompare(numberpulses, Play,sizeof(Play)/4)) {
    On = 1;
    if (On == 1) {
      do{
        for (int thisPin = 4; thisPin < 8; thisPin++) {
          // turn the pin on:
          digitalWrite(thisPin, HIGH);  
          delay(DelayP);                  
          // turn the pin off:
          digitalWrite(thisPin, LOW);    
        }
      } 
      while(On ==1);         //<<<<<<<<<<<<<<HERE is the mistake

    }
  }

makes the leds chase with a on button push. The problem is I can't turn them off with the wireless remote. The IRcommamd reads the codes sent from the remote.

Here is a copy of the complete code:

#define IRpin_PIN      PIND
#define IRpin          2


#define MAXPULSE 65000
#define NUMPULSES 50




#define RESOLUTION 20 


#define FUZZINESS 20


uint16_t pulses[NUMPULSES][2];   
uint8_t currentpulse = 0; 


#include "ircodes.h"

int Speed = 1;
int DelayP = (40*Speed);
int On = 0;

void setup(void) {
  for (int thisPin = 4; thisPin < 8; thisPin++)  {
    pinMode(thisPin, OUTPUT);      
  }
}

void loop(void) {
  int numberpulses;

  numberpulses = listenForIR();


  if (IRcompare(numberpulses, Play,sizeof(Play)/4)) 
  {
    On = 1;
  }
//  else 
//  {
//    On = 0; // because logically.... do I really have to spell it out?
//  }
  if (IRcompare(numberpulses, Speedup,sizeof(Speedup)/4)) {
  Speed = Speed /= .5;
  }
  if (IRcompare(numberpulses, Slowdown,sizeof(Slowdown)/4)) {
   Speed = ++Speed;
  }
  if (IRcompare(numberpulses, Stop,sizeof(Stop)/4)) {
    On = 0;
  }
  
// and this part works the leds --- it has been taken OUT of the control change { } block
  if (On == 0) 
  {
    for (int thisPin = 4; thisPin < 8; thisPin++) 
    {
          // turn the pin on:
          digitalWrite(thisPin, HIGH);  
         delay(40);                  
          // turn the pin off:
          digitalWrite(thisPin, LOW);    
    }
  }    // notice how the opens { and closes } have easy to see matching indents?

}

//KGO: added size of compare sample. Only compare the minimum of the two
boolean IRcompare(int numpulses, int Signal[], int refsize) {
  int count = min(numpulses,refsize);

  for (int i=0; i< count-1; i++) {
    int oncode = pulses[i][1] * RESOLUTION / 10;
    int offcode = pulses[i+1][0] * RESOLUTION / 10;


    if ( abs(offcode - Signal[i*2 + 1]) <= (Signal[i*2 + 1] * FUZZINESS / 100)) {

    } 
    else {
      return false;
    }
  }
  // Everything matched!
  return true;
}

int listenForIR(void) {
  currentpulse = 0;

  while (1) {
    uint16_t highpulse, lowpulse;  // temporary storage timing
    highpulse = lowpulse = 0; // start out with no pulse length
    while (IRpin_PIN & (1 << IRpin)) {
      // pin is still HIGH

      // count off another few microseconds
      highpulse++;
      delayMicroseconds(RESOLUTION);

      // If the pulse is too long, we 'timed out' - either nothing
      // was received or the code is finished, so print what
      // we've grabbed so far, and then reset

      // KGO: Added check for end of receive buffer
      if (((highpulse >= MAXPULSE) && (currentpulse != 0))|| currentpulse == NUMPULSES) {
        return currentpulse;
      }
    }
    // we didn't time out so lets stash the reading
    pulses[currentpulse][0] = highpulse;

    // same as above
    while (! (IRpin_PIN & _BV(IRpin))) {
      // pin is still LOW
      lowpulse++;
      delayMicroseconds(RESOLUTION);
      // KGO: Added check for end of receive buffer
      if (((lowpulse >= MAXPULSE)  && (currentpulse != 0))|| currentpulse == NUMPULSES) {
        return currentpulse;
      }
    }
    pulses[currentpulse][1] = lowpulse;

    // we read one high-low pulse successfully, continue!
    currentpulse++;
  }
}
//void callback()
//{
//  digitalWrite(10, digitalRead(10) ^ 1);
//}

The way you have listenForIR() written makes it a blocking function. It's not going to return until it receives a valid signal.

Your highpulse loop will run until it sees a pulse, so your sketch is blocked there till you send it a command. Your lowpulse would do the same thing, if the highpulse weren't blocking first.

jraskell:
The way you have listenForIR() written makes it a blocking function. It's not going to return until it receives a valid signal.

Your highpulse loop will run until it sees a pulse, so your sketch is blocked there till you send it a command. Your lowpulse would do the same thing, if the highpulse weren't blocking first.

Not sure what you mean.

I have been stuck on this for weeks before I dicided to post.

Is there anyway that you could fix the program so I could better understand what I did wrong?