next topic »

[on:]

Hello! I have a curious question about exiting a "for" loop which is driving me a bit nuts. I think I am either using the wrong logic altogether or making a basic error in how I'm approaching this problem.

The code is simple, when you run it and type "H" into the serial monitor, the system is on, when you type "L" the system is off, and when you type "A" the system blinks an LED on pin 13 ten times.

My question is this, when you press "A" and the system is running through the "for" loop to blink the LED ten times, I want to be able to turn off the system by typing "L" and end the loop immediately without waiting for all ten LED blinks.

I have tried "break;" statements, as well as "continue" and "return", but can't seem to figure out the correct flow.

Code:

int incoming_byte;
int led_pin = 13;
int power = 0;

void setup() {
  Serial.begin(9600);
  pinMode(led_pin, OUTPUT);
}

void loop() {
  // Read the last byte in the serial stream
  if (Serial.available() > 0) {
    incoming_byte = Serial.read();
    if (incoming_byte == 'H') {
      power = 1;
    }
    if (incoming_byte == 'L') {
      power = 0;
    }
    if (incoming_byte == 'A') {
      blink();
    }
  }
  
  // If the system is on, print the "on" message, otherwise print an "off" message
  if (power == 1){
    power_on();
  }
  else if(power == 0){
    Serial.println("------off------");
    delay(200);
  }
  
}

void power_on(){
  // Print a message that the power is on
  Serial.println("Power On");
  delay(200);
}

void blink (){
  // Blink the LED 10 times
  for(int x = 1; x<=10; x++){
    digitalWrite(led_pin, HIGH);
    delay(400);
    digitalWrite(led_pin, LOW);
    delay(100);
    Serial.println(x);
  }

}

[Reply #1 on:]

In that last function with the for loop, first you're going to need to get rid of those delays.  You can't do anything while the processor is in a delay, so instead keep up with the time using millis() and turn the light on or off when it is time.

Then, if you want the for loop to respond to something coming in on serial, then that for loop is going to have to have a line that reads the serial.  Then check that read in an if statement and if it is the letter you want to break the for loop, then use break to kill the loop.

[Reply #2 on:]

Start by look at the blink without delay example.

Then get rid of your blink function.

your code needs to look like this

Code:

Boolean blinking = false; // ADDITION

void loop() {
  // Read the last byte in the serial stream
  if (Serial.available() > 0) {
    incoming_byte = Serial.read();
    if (incoming_byte == 'H') {
      power = 1;
    }
    if (incoming_byte == 'L') {
      power = 0;
    }
    if (incoming_byte == 'A') {
      if (blinking == true){ blinking =false}else {blinking=true);// ADDTION
      blinking = true;
    }
  }
  ADDTION
 IF its time to change the state of the LED doit!
  END ADDITION
 
 
 
}

Mark

[Reply #3 on:]

Code:

     if (blinking == true){ blinking =false}else {blinking=true);// ADDTION

Without removing the next line, this one is as useless as it is verbose and incorrect (should be a } on the end, not a )).

Code:

blinking = !blinking;

is far simpler.

[Reply #4 on:]

Great, thanks to everyone for your help and quick turnaround, very cool!! I didn't think about using "millis" instead of "delay", which seems to create a state machine of sorts. I will have to wrap my head around that further.

I updated the code based on everyone's feedback, but still have a problem with looping the blink function, perhaps one of you will catch it. I will definitely try again when I am less sleep deprived!

Code:

const int ledPin =  13;       // LED pin
int ledState = LOW;           // Is the LED ON or OFF?
long previousTime = 0;        // Store the previous time (in millis)
long timeInterval = 1000;     // The time that is allowed to pass (in millis)
int incoming_byte;            // The last byte in the serial buffer
int power = 0;                // Is the system On or OFF?
boolean blinking = false;     // Should the LED be blinking?

void setup() {
  Serial.begin(9600);         // Enable serial transmission
  pinMode(ledPin, OUTPUT);    // Initialize Pin 13 for the LED
}

void loop()
{
  // Read the last byte in the serial stream
  if (Serial.available() > 0) {
    incoming_byte = Serial.read();
    if (incoming_byte == 'H') {
      power = 1;
    }
    if (incoming_byte == 'L') {
      power = 0;
    }
    if (incoming_byte == 'A') {
      blinking = !blinking;
      blinking = true;
      blink();
      //Serial.println(x);
    }
  }

  // If the system is on, print the "on" message, otherwise print an "off" message
  if (power == 1){
    // Print a message that the power is on
    Serial.println("Power On");
    delay(200);
  }
  else if(power == 0){
    Serial.println("------off------");
    delay(200);
  }
}


void blink() {
  unsigned long currentTime = millis();
  //Serial.println(currentTime - previousTime);
  // Blink the LED 10 times
  for(int x = 1; x<=10; x++){
    if(currentTime - previousTime > timeInterval) {
      previousTime = currentTime;
      if (ledState == LOW){
        ledState = HIGH;
      }
      else {
        ledState = LOW;
      }
      digitalWrite(ledPin, ledState);
    }
  }
}

[Reply #5 on:]

You can't use a for loop like that in your blink function.
You need a separate state machine for the ten blinks.

[Reply #6 on:]

You still have the problem of not handling the serial commands while blinking. That's the only way you'll get it to power off while blinking.

I actually wrote up a sketch for this last night while watching Pawn Stars.  I'll try to think to post it later.

I'm always somewhat reluctant to post code as it undermines the pedagogical approach that is popular on this forum. Do you promise that I won't just be doing your homework? LOL

[Reply #7 on:]

You still have the problem of not handling the serial commands while blinking. That's the only way you'll get it to power off while blinking.

I actually wrote up a sketch for this last night while watching Pawn Stars.  I'll try to think to post it later.

I'm always somewhat reluctant to post code as it undermines the pedagogical approach that is popular on this forum. Do you promise that I won't just be doing your homework? LOL

This is not for a homework problem, but is instead a personal curiosity. I've had tunnel vision for a bit on this code and wanted to get some outside input. I'm all for struggling through problems, but also fond of seeing how others approach problems with much more efficient code and am able to remember solutions better when asking for help sometimes. Cheers!!

[Reply #8 on:]

Code:

if (incoming_byte == 'A') {
      blinking = !blinking;
      blinking = true;
      blink();
      //Serial.println(x);
    }

The "blinking = !blinking;" seems superfluous as blinking is set to true straight after.
Should this line be an "If" statement?

[Reply #9 on:]

You should have two state variables: the number of blinks so far and the current led status. Every time the state machine executes, you invert led status, and if it says "on", you increment the counter. You probably need also a flag to know whether the blink procedure is "running" or not. At the bottom of loop() you drive the led based on the led status variable.
When does the state machine execute ? This is where millis() comes into play. Study the "blink without delay" example. It shows how to execute actions at specified intervals while letting the program do other stuff (in you case, testing Serial.available()).
When you receive a command through the serial interface you can change the state variables to affect the blinking procedure. For example, you can alter the "running" flag to stop it.

[Reply #10 on:]

what i see is that you are not reading the serial port when for loop is working so its not going to detect your keys so all you have to do is add serial.read() in the for loop and then apply the condition on serial byte... if it L then raise the value of x to 11 and the loop will break u can simply use break function as well and it will work fine

Code:

int incoming_byte;
int led_pin = 13;
int power = 0;

void setup() {
  Serial.begin(9600);
  pinMode(led_pin, OUTPUT);
}

void loop() {
  // Read the last byte in the serial stream
  if (Serial.available() > 0) {
    incoming_byte = Serial.read();
    if (incoming_byte == 'H') {
      power = 1;
    }
    if (incoming_byte == 'L') {
      power = 0;
    }
    if (incoming_byte == 'A') {
      blink();
    }
  }
  
  // If the system is on, print the "on" message, otherwise print an "off" message
  if (power == 1){
    power_on();
  }
  else if(power == 0){
    Serial.println("------off------");
    delay(200);
  }
  
}

void power_on(){
  // Print a message that the power is on
  Serial.println("Power On");
  delay(200);
}

void blink (){
  // Blink the LED 10 times
  for(int x = 1; x<=10; x++){
    incoming_byte = Serial.read();
                              // u have to read the serial as well when loop is working it is not going to serial.read() so u have to call that fucntion in            
   digitalWrite(led_pin, HIGH);     //in here and then just increase the value of x and condition will be false
    delay(400);
    digitalWrite(led_pin, LOW);
    delay(100);
    Serial.println(x);
 if (incoming_byte == 'L')
           x=11;
  }

}

[Reply #11 on:]

Please use code tags when positing code. Click on the # button in the editor toolbar.

Also, please use CTRL+T inside the arduino IDE before copy-n-pasting code. It makes the code easier to read.

You still use delay()s inside you sketch, so you either didn't read blink without delay or there's something about it you don't understand. Feel free to ask.

[Reply #12 on:]

I almost forgot about this. Here's the code I knocked out for it. I did have it using multiple if statements as you had before but just had to change that to a switch. This could likely be streamlined more, there are a couple of things that might be a bit redundant.

Code:

byte led_pin = 13;                                             // led pin
boolean ledOn = false;                                         // led state
boolean blinkOn = false;                                       // blinking state
byte blinkCount = 0;                                           // blink counter
unsigned long blinkDuration = 1000;                            // blink duration millis
unsigned long toggleTimer = 0;                                 // for timing blinks
boolean powerOn = false;                                       // our machines state

// Set up stuff
void setup() {
  Serial.begin(9600);
  pinMode(led_pin, OUTPUT);
}

void loop() {

  byte incoming_byte = 0;                                      // clear storage for serial read

  if (Serial.available() > 0) incoming_byte = Serial.read();   // read serial if available

  switch (incoming_byte)                                       // act on input
  {

  case 72:                                                     // H turns it on
    if (!powerOn)                                              // only turn on if needed
    {
      powerOn = true;                                          // set power flag
      Serial.println("Power is On.");
    }             
    break;

  case 76:                                                     // L turns power off
    if (powerOn)                                               // only turn off if needed
    {
      powerOn = false;                                         // clear power flag
      blinkOn = false;                                         // turn off blink
      if (ledOn) toggleLed();                                  // turn off led if needed
      Serial.println("Power is Off.");
    }     
    break;

  case 65:                                                     // A starts the blinking
    if (powerOn)                                               // we can only blink if machine is on
    {                                                         
      blinkOn = true;                                          // set blink flag
      blinkCount = 0;                                          // clear counter
      toggleTimer = blinkDuration + millis();                  // set timer for blink length
    }
  }

  // DO the LED stuff
  if (blinkOn && millis() > toggleTimer)                       // check if we should blink
  {
    toggleLed();                                               // switches led state
    toggleTimer = blinkDuration + millis();                    // set timer for next blink length
    if (++blinkCount == 21)                                    // stop after ten whole blinks
    {
      blinkOn = false;                                         // turn blink off
      if (ledOn) toggleLed();                                  // be sure led is off
    }
  }
}

// switches led state
void toggleLed(){
  if (ledOn)                                                   // LED is on
  {
    digitalWrite(led_pin, LOW);                                // so turn it off
    ledOn = false;                                             // clear flag
  }
  else                                                         // otherwise its off
  {
    digitalWrite(led_pin, HIGH);                               // turn it on
    ledOn = true;                                              // set the flag
  }
}

[Reply #13 on:]

If you used character constants in your case statements, your code would be much easier to read.

[Reply #14 on:]

I think I had trouble at one time so I just use the ASCII code but you are right it doesn't help readability.

So in place of 65 I could just use 'A' for example ?