How to break out of a loop

Using Arduino Programming Questions
1

Hi all,
So, the project I am working on is a dog food dispenser. I gather user input through an LCD screen and switches and then process that data, so it is dispensed at the time the user selected. I'm working on obtaining that data and if the current time matches with the time selected by the user, dispense the food. When an IR sensor detects something, close the lid. The issue I'm having is that I can't seem to break out of the dispensing loop. I want to break out of the loop once the IR sensor detects something, so the lid stays closed even if the dispensing time matches the current time.

Here is the code that I have:

void Dispense::Timer(Menu screen)
{
  unsigned long currentTime = millis() / 1000; //start timer

  int Hours = screen.getHour() + numberOfHours(currentTime); //update hours
  int Minutes = screen.getMinute() + numberOfMinutes(currentTime); //update minutes
  int Seconds = numberOfSeconds(currentTime);

  for (int i = 0; i < 10; i++)
  {
    if (screen.getTimer(i, 0) != NULL) //if there is data in the array
    {
      if (screen.getTimer(i, 0) == Hours && screen.getTimer(i, 1) == Minutes) //if times match 
      {
        digitalWrite(greenLed, HIGH);
        digitalWrite(redLed, LOW);
        servoMotor.write(0); //open lid

        break;
      }
    }
  }

  if(digitalRead(IRsensor) == 0) //if IR sensor detects 
  {
    digitalWrite(greenLed, LOW);
    digitalWrite(redLed, HIGH);
    servoMotor.write(90); //close lid 
  }
  
  printTimer(Hours);
  printTimer(Minutes);
  Serial.println();
}

When the current time is equal to the dispensing time, the lid stays open regardless of if the IR sensor detected something previously. How can I make it so that it won't open if the IR sensor detected an object and stay closed?

2

How can I make it so that it won't open if the IR sensor detected an object and stay closed

check the IR before opening the lid

3

make the state of the IR sensor part of your decision to open or not

void Dispense::Timer(Menu screen)
{
  unsigned long currentTime = millis() / 1000; //start timer

  int Hours = screen.getHour() + numberOfHours(currentTime); //update hours
  int Minutes = screen.getMinute() + numberOfMinutes(currentTime); //update minutes
  int Seconds = numberOfSeconds(currentTime);

  for (int i = 0; i < 10; i++)
  {
    if (screen.getTimer(i, 0) != NULL) //if there is data in the array
    {
      if (screen.getTimer(i, 0) == Hours && screen.getTimer(i, 1) == Minutes) //if times match
      {
        if (digitalRead(IRsensor) == HIGH) //if IR sensor is okay
        {
          digitalWrite(greenLed, HIGH);
          digitalWrite(redLed, LOW);
          servoMotor.write(0); //open lid
        }
        break;
      }
    }
  }

  if (digitalRead(IRsensor) == 0) //if IR sensor detects
  {
    digitalWrite(greenLed, LOW);
    digitalWrite(redLed, HIGH);
    servoMotor.write(90); //close lid
  }

  printTimer(Hours);
  printTimer(Minutes);
  Serial.println();
}
4

there's always a goto, which in many cases makes the code very much clearer

void
func (...)
{
    {
        {
            {
                if (for some reason)
                    goto done;
            }
        }
    }
done:
}
5

OP's break will exit the for loop - that's not the issue there...

The challenge is that the condition to "open lid" includes checking the IR and it's not done....

6

You could check the IR sensor first and not even THINK about opening the feeder unless the IR sensor reads HIGH:

void Dispense::Timer(Menu screen)
{
  unsigned long currentTime = millis() / 1000; //start timer


  int Hours = screen.getHour() + numberOfHours(currentTime); //update hours
  int Minutes = screen.getMinute() + numberOfMinutes(currentTime); //update minutes
  int Seconds = numberOfSeconds(currentTime);


  if (digitalRead(IRsensor) == LOW) // if IR sensor detects
  {
    digitalWrite(greenLed, LOW);
    digitalWrite(redLed, HIGH);
    servoMotor.write(90); //close lid
  }
  else  // (digitalRead(IRsensor) == HIGH)
  {
    // See if it is one of the times to open the lid
    for (int i = 0; i < 10; i++)
    {
      if (screen.getTimer(i, 0) != NULL) //if there is data in the array
      {
        if (screen.getTimer(i, 0) == Hours && screen.getTimer(i, 1) == Minutes) //if times match
        {
          if (digitalRead(IRsensor) == HIGH) //if IR sensor is okay
          {
            digitalWrite(greenLed, HIGH);
            digitalWrite(redLed, LOW);
            servoMotor.write(0); //open lid
          }
          break;  // Already open so no need to check the other times.
        }
      }
    }
  }


  printTimer(Hours);
  printTimer(Minutes);
  Serial.println();
}
7

gcjr:
there's always a goto, which in many cases makes the code very much clearer

Seriously? Don't use goto, use break, continue, return or in a system with exceptions, raise an exception.

The only place a goto is considered acceptable is when transliterating a state-transition diagram to
code - although often switch/case is perfectly able to handle this too.

Once people start playing with goto the result is spaghetti code than noone wants to deal with!

the example you give is a perfect fit for "return;"

8

MarkT:
Seriously?

yes, seriously

yes, return would be better in my example, but the goto handles the case when some cleanup is necessary.

as i said, when used appropriately, goto can make the code much easier

9

There are very few cases where goto makes sense, this has been an everlasting discussion in the C community (and C++ try/catch was not existing in C)

Exiting from deep nested loops and/or if/else without adding multiple error code check is one that is "tolerated" but there is no need for this here

10

J-M-L:
but there is no need for this here

maybe the subject line of the thread should be corrected

11

gcjr:
maybe the subject line of the thread should be corrected

yes because that's indeed not the issue

but to that question (assuming a loop statement) the quick answer wasbreak;not goto
if that was the loop(), then probably return was more appropriate

12

blh64:
make the state of the IR sensor part of your decision to open or not

void Dispense::Timer(Menu screen)

{
 unsigned long currentTime = millis() / 1000; //start timer

int Hours = screen.getHour() + numberOfHours(currentTime); //update hours
 int Minutes = screen.getMinute() + numberOfMinutes(currentTime); //update minutes
 int Seconds = numberOfSeconds(currentTime);

for (int i = 0; i < 10; i++)
 {
   if (screen.getTimer(i, 0) != NULL) //if there is data in the array
   {
     if (screen.getTimer(i, 0) == Hours && screen.getTimer(i, 1) == Minutes) //if times match
     {
       if (digitalRead(IRsensor) == HIGH) //if IR sensor is okay
       {
         digitalWrite(greenLed, HIGH);
         digitalWrite(redLed, LOW);
         servoMotor.write(0); //open lid
       }
       break;
     }
   }
 }

if (digitalRead(IRsensor) == 0) //if IR sensor detects
 {
   digitalWrite(greenLed, LOW);
   digitalWrite(redLed, HIGH);
   servoMotor.write(90); //close lid
 }

printTimer(Hours);
 printTimer(Minutes);
 Serial.println();
}

Hmmm, so I tried this, but still, I can't get to close the lid when the IR sensor detects something and stay closed afterward (even if the IR sensor is HIGH)

13

what does the IR sensor detect? an open lid?

and won't the condition to open the lid persist for 60 secs? should the code track whether it opened the lid?

14

ferchi1809:

Quote from: ferchi1809 Sun Mar 21 2021 13:42:17 GMT-0400 (Eastern Daylight Time)Hmmm, so I tried this, but still, I can't get to close the lid when the IR sensor detects something and stay closed afterward (even if the IR sensor is HIGH)

Sounds like the hardware isn't doing what you think you are telling it to do. I recommend using Serial to report every change in the hardware. When you find the IR Sensor is reading LOW, write a message. When you find the IR Sensor is reading HIGH, write a message. When you call servo.write(), write a message. Make sure the IR Sensor is reporting what you expect it to. Make sure the servo is moving to where you told it to.

void Dispense::Timer(Menu screen)
{
  unsigned long currentTime = millis() / 1000; //start timer

  int Hours = screen.getHour() + numberOfHours(currentTime); //update hours
  int Minutes = screen.getMinute() + numberOfMinutes(currentTime); //update minutes
  int Seconds = numberOfSeconds(currentTime);

  if (digitalRead(IRsensor) == LOW) // if IR sensor detects
  {
    Serial.println("IRsensor reads LOW");
    digitalWrite(greenLed, LOW);
    digitalWrite(redLed, HIGH);
    servoMotor.write(90); //close lid
    Serial.println("Servo moved to 90 (Closed)");
  }
  else  // (digitalRead(IRsensor) == HIGH)
  {
    Serial.println("IRsensor reads HIGH");
    // See if it is one of the times to open the lid
    for (int i = 0; i < 10; i++)
    {
      if (screen.getTimer(i, 0) != NULL) //if there is data in the array
      {
        if (screen.getTimer(i, 0) == Hours && screen.getTimer(i, 1) == Minutes) //if times match
        {

            Serial.print("Current time matches timer "); Serial.println(i);
            digitalWrite(greenLed, HIGH);
            digitalWrite(redLed, LOW);
            servoMotor.write(0); //open lid
            Serial.println("Servo moved to 0 (Open)");
          break;  // Already open so no need to check the other times.
        }
      }
    }
  }

  printTimer(Hours);
  printTimer(Minutes);
  Serial.println();
}
15

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.