Looping program Problem.

Hi all I have a question that is doing my nut in >:( .
I have a project on the go that I plan to be automatic bird feeder, which will ideally feed at a set time in the code.
Now I have the code and the hardware set up.
It is set to read the time from an RTC (I think?) and feed at a time which is 9am in the code.
The code has verified all Ok and uploaded to an Arduino Mega.
Now, I have looked through this code and cannot see why it is constantly looping.
The arm I have fashioned for the servo is constantly moving every 10 seconds from its open to closed position…
I am fairly new to this and this is only my third major project and the most complex to date.

After the looping is sorted I plan to add in an override so I can operate via Bluetooth.
I have the code for this already and tried it and it works Ok…
Any help would be appreciated.

looping code.txt (2.61 KB)

It looks to me like getTime returns true or false, but you're comparing that to a time.

Your code is a mess. Please use the AutoFormat tool of the IDE to format it correctly. You will probably find out then that the two functions getTime() and BirdsToBeFed() got mixed up and the later returns it’s result for the former. Even worse the constants that are compared are never set, so the result is always false and the two times are always 0 so the if statements always match and OpenFeeder() and CloseFeeder() are always called. The 10 seconds result from the delay() inside these two functions.

Thanks I will give that a go... Im Fairly new to coding so I am still trying to get to grips with it....

I have tried the auto formatting as suggested and I am still getting the software loop..... ::slight_smile:

AWOL:
It looks to me like getTime returns true or false, but you're comparing that to a time.

Could you explain this Further AWOL?

SpookyFlash:
I have tried the auto formatting as suggested and I am still getting the software loop… ::slight_smile:

Auto formatting just makes the code more readable. It doesn’t change the content.
Did you not read through it after auto-formatting it?

Post your re-formatted code so we can all see it - use code tags (the scroll with <> on top)

so it looks like this

…R

Hi Robin … Yes I have looked through it after auto-formatting and I am still at a loss… :frowning:

#include <Servo.h>
#include <Wire.h>
const byte DS1307_CTRL_ID = 0x68; // address of the DS1307 real-time clock
const byte constNumberOfFields = 6; // the number of fields (bytes) to request from the RTC
const int constServoSignalPin = 9;  // Servo connected to Digital 9
const int constTimeToFeed_min = 9*60;  // 9:00 am
const int constTimeToNot_min = 8*60 + 30;  // 8:30AM
int Second ;
int Minute;
int Hour;
int Day;
int Month;
int Year;
int currentTime;
int timeToClose;
int timeToFeed;
int Time
Servo myservo;  // create servo object to control a servo
// variable to store the initial servo position
void setup()
{
  Serial.begin(9600);
  Wire.begin();
}
// Convert Binary Coded Decimal (BCD) to Decimal
byte bcd2dec(byte num){
  return ((num/16 * 10) + (num % 16));
}
int getTime()
{
  Wire.beginTransmission(DS1307_CTRL_ID);
  Wire.write((uint8_t)0x00);
  Wire.endTransmission();
  Wire.requestFrom(DS1307_CTRL_ID, constNumberOfFields); 
  Second = bcd2dec(Wire.read() );  // (0-59)
  Minute = bcd2dec(Wire.read() );  // (0-59)
  Hour   = bcd2dec(Wire.read() );  //assumes 24hr clock (0-23)
  Day    = bcd2dec(Wire.read() );  // (1-31)
  Month  = bcd2dec(Wire.read() );  // (1-12)
  Year   = bcd2dec(Wire.read() );
  Year   = Year + 2000; // RTC year 0 is year 2000
  bool BirdsToBeFed(int time);
  if (Time < timeToFeed && Time > timeToClose)
    return true;
  else
    return false;
}
void OpenFeeder(){
  myservo.attach(constServoSignalPin);  // attaches the servo on pin 9 to the servo object
  myservo.write(57);    // tell servo to go to position in variable 'angle'
  delay(10000);                       // delay in ms 
  myservo.detach(); 
}
void CloseFeeder(){
  myservo.attach(constServoSignalPin);  // attaches the servo on pin 9 to the servo object
  myservo.write(28);    // tell servo to go to position in variable 'angle'
  delay(10000);                       // waits 10 seconds between servo movement  
  myservo.detach();
}
// utility function for clock 
void printDigits(int digits)
{
  Serial.print(":");
  if(digits < 10)
    Serial.print('0');
  Serial.print(digits);
}
void digitalClockDisplay(){// digital clock display of the time
  Serial.print(Hour);
  printDigits(Minute);
  printDigits(Second);
  Serial.print(" ");
  Serial.print(Day);
  Serial.print(" ");
  Serial.print(Month);
  Serial.print(" ");
  Serial.print(Year);
  Serial.println();
}
void loop()
{
  currentTime = getTime();   //if (BirdShoudlBeFed)
  if (currentTime == timeToClose)
    CloseFeeder();
  if (currentTime == timeToFeed)
    OpenFeeder();
  digitalClockDisplay();
}

So now you can see that currentTime will be false (zero) because Time is zero, and timeToClose and timeToFeed are also zero.
You've got a function prototype for birdsToBeFed in getTime that isn't doing anything useful.

I have removed the function prototype :slight_smile: I didn't realise that would be a problem.
Still have a loop though..... I will keep looking at it..

It wasn't a problem, but it was probably confusing.

When the clock runs, what output do you see from the call to digitalClockDisplay()?

Let’s look at this function to get the time: (I’ve taken out the useless function prototype)

int getTime()
{
  Wire.beginTransmission(DS1307_CTRL_ID);
  Wire.write((uint8_t)0x00);
  Wire.endTransmission();
  Wire.requestFrom(DS1307_CTRL_ID, constNumberOfFields); 
  Second = bcd2dec(Wire.read() );  // (0-59)
  Minute = bcd2dec(Wire.read() );  // (0-59)
  Hour   = bcd2dec(Wire.read() );  //assumes 24hr clock (0-23)
  Day    = bcd2dec(Wire.read() );  // (1-31)
  Month  = bcd2dec(Wire.read() );  // (1-12)
  Year   = bcd2dec(Wire.read() );
  Year   = Year + 2000; // RTC year 0 is year 2000

  if (time < timeToFeed && time > timeToClose)
    return true;
  else
    return false;
}

So this pulls in all the pieces of the time, then does nothing with them. Instead it either returns true or false. What time is true? What time is false?

Really though, it always returns false. Since timeToFeed and timeToClose were declared at global scope and have never been given any value anywhere else in the code, both of those variables are 0. The variable Time is also initialized to 0 and is never changed anywhere else that I can find in the program. So all three variables are 0, Time is not less than timeToFeed and the if statement comes out false so getTime returns false. Always. Never will it return anything other than false (or actually 0 since it is declared to return an int and not a bool).

Now let’s carry that result through to this code:

void loop()
{
  currentTime = getTime();   //if (BirdShoudlBeFed)
  if (currentTime == timeToClose)
    CloseFeeder();
  if (currentTime == timeToFeed)
    OpenFeeder();
  digitalClockDisplay();
}

Now getTime returns 0 so currentTime will always be 0. So is timeToClose and timeToFeed since you’ve never given them any values anywhere. So both if statements come out true since 0 == 0. So this loop will run the functions CloseFeeder() and OpenFeeder() over and over and over again.

CloseFeeder will :

void CloseFeeder(){
  myservo.attach(constServoSignalPin);  // attaches the servo on pin 9 to the servo object
  myservo.write(28);    // tell servo to go to position in variable 'angle'
  delay(10000);                       // waits 10 seconds between servo movement  
  myservo.detach();
}

move the servo and wait ten seconds.

Then OpenFeeder will also move the servo and wait ten seconds.

So it sounds like with the program as written, the servo should move to one position, wait ten seconds, then move to the other position and wait ten seconds, then repeat ad infinitum.

Now, to me it would make sense to have the getTime function actually return a time instead of true or false. It would make sense that timeToClose and timeToFeed should have some value other than 0.

econjack:
When the clock runs, what output do you see from the call to digitalClockDisplay()?

I see the current time hh:mm:ss and current date updated every second...

SpookyFlash:
I see the current time hh:mm:ss and current date updated every second...

And well you should, getTime() does a great job of populating the variables Minute, Second, Hour, etc. Now if only you would use that time to determine whether or not to feed the birds instead of false.

I'm still looking on my watch to figure out what time false is.

Why are you attaching and detaching the servo each time the functions are called ? Attach it once in setup() and leave it attached.

How is the servo powered ?

UKHeliBob:
Why are you attaching and detaching the servo each time the functions are called ? Attach it once in setup() and leave it attached.

How is the servo powered ?

I plan to power it by battery as it has to be mobile....
So enabling it each time saves battery power I was thinking instead of having it permanently powered on....

I am no longer looping now as I made some modifications to the code and it stopped.
I ma now unsure if I am getting any useful times to judge whether to feed or not ???
I changed the feeding time to test it and it didn’t cycle at all…

#include <Servo.h>
#include <Wire.h>
const byte DS1307_CTRL_ID = 0x68; // address of the DS1307 real-time clock
const byte constNumberOfFields = 6; // the number of fields (bytes) to request from the RTC
const int constServoSignalPin = 9;  // Servo connected to Digital 9
const int constTimeToFeed = 21*60 + 02;  // 9:00 am
const int constTimeToNot = 8*60 + 30;  // 8:30AM
int Second;
int Minute;
int Hour;
int Day;
int Month;
int Year;
int currentTime;
int Time;
Servo myservo;  // create servo object to control a servo
// variable to store the initial servo position
void setup()
{
  Serial.begin(9600);
  Wire.begin();
}
// Convert Binary Coded Decimal (BCD) to Decimal
byte bcd2dec(byte num){
  return ((num/16 * 10) + (num % 16));
}
int getTime(){
  Wire.beginTransmission(DS1307_CTRL_ID);
  Wire.write((uint8_t)0x00);
  Wire.endTransmission();
  Wire.requestFrom(DS1307_CTRL_ID, constNumberOfFields); 
  Second = bcd2dec(Wire.read() );  // (0-59)
  Minute = bcd2dec(Wire.read() );  // (0-59)
  Hour   = bcd2dec(Wire.read() );  //assumes 24hr clock (0-23)
  Day    = bcd2dec(Wire.read() );  // (1-31)
  Month  = bcd2dec(Wire.read() );  // (1-12)
  Year   = bcd2dec(Wire.read() );
  Year   = Year + 2000; // RTC year 0 is year 2000
  if (Time == constTimeToFeed && Time < constTimeToNot)
    OpenFeeder;
  else
    CloseFeeder;
}
void OpenFeeder(){
  myservo.attach(constServoSignalPin);  // attaches the servo on pin 9 to the servo object
  myservo.write(57);    // tell servo to go to position in variable 'angle'
  delay(10000);                       // delay in ms 
  myservo.detach(); 
}
void CloseFeeder(){
  myservo.attach(constServoSignalPin);  // attaches the servo on pin 9 to the servo object
  myservo.write(28);    // tell servo to go to position in variable 'angle'
  delay(10000);                       // waits 10 seconds between servo movement  
  myservo.detach();
}
// utility function for clock 
void printDigits(int digits)
{
  Serial.print(":");
  if(digits < 10)
    Serial.print('0');
  Serial.print(digits);
}
void digitalClockDisplay(){// digital clock display of the time
  Serial.print(Hour);
  printDigits(Minute);
  printDigits(Second);
  Serial.print(" ");
  Serial.print(Day);
  Serial.print(" ");
  Serial.print(Month);
  Serial.print(" ");
  Serial.print(Year);
  Serial.println();
}
void loop()
{
  currentTime = getTime();   //if (BirdShoudlBeFed)
  if (currentTime == constTimeToNot)
    CloseFeeder();
  if (currentTime == constTimeToFeed)
    OpenFeeder();
  digitalClockDisplay();
}

Let’s look at this function one more time.

int getTime(){
  Wire.beginTransmission(DS1307_CTRL_ID);
  Wire.write((uint8_t)0x00);
  Wire.endTransmission();
  Wire.requestFrom(DS1307_CTRL_ID, constNumberOfFields); 
  Second = bcd2dec(Wire.read() );  // (0-59)
  Minute = bcd2dec(Wire.read() );  // (0-59)
  Hour   = bcd2dec(Wire.read() );  //assumes 24hr clock (0-23)
  Day    = bcd2dec(Wire.read() );  // (1-31)
  Month  = bcd2dec(Wire.read() );  // (1-12)
  Year   = bcd2dec(Wire.read() );
  Year   = Year + 2000; // RTC year 0 is year 2000
  if (Time == constTimeToFeed && Time < constTimeToNot)
    OpenFeeder;
  else
    CloseFeeder;
}

So now constTimeToFeed and constTimeToNot have values. But Time still does not. It’s still 0. So now, 0 is indeed less than constTimeToNot, but it can never equal constTimeToFeed, so this if at the end will never ever be true. So it will always hit the else clause. Which in this case just has a useless statement of a pointer to the function CloseFeeder. If you would put some parenthesis behind that and make it an actual function call, CloseFeeder();, then it would at least call that function over and over again.

You’ve also removed the return statement. So now when you call this function in loop(), currentTime will now be 0 because there is no return from getTime(). So you’re still in the same situation in loop() but for a different reason. You were returning 0 to set currentTime, but now you’re not returning anything at all.

I would also suggest you give some serious thought to your logic on the time thing. Even if this worked it wouldn’t work.

There are two places where you are trying to do the same thing different ways. In the getTime() function you have this:

if (Time == constTimeToFeed && Time < constTimeToNot)

where constTimeToFeed is 21 * 60 + 2 == 1262 //(how is this 9am)
and constTimeToNot is 8 * 60 + 30 == 510 // (if this is 8:30)

so in order for that if to be true Time would have to be 1262 AND also be less than 510. 1260 is never less than 510. I hope you can see how that would never be true.

Then in the loop you get a little closer trying to do the same thing. Let’s assume that you fixed getTime and the current time is now coming in right. Let’s also assume that you fix the constant values to actually be something that represent 9am and 8:30am.

if (currentTime == constTimeToNot)
    CloseFeeder();
  if (currentTime == constTimeToFeed)
    OpenFeeder();

So when it is 8:30 am, it will close the feeder and then when it is 9am it will open it. Then it will be until 8:30am the next morning before either one of those is true again and the servo moves again. So the feeder will be open all the time and will only be closed from 8:30am to 9am. That doesn’t sound like what you described in the OP.