Please critique my code

This is my first project and would like to know what I can do to improve my code. This is a pet feeder that uses rtc and also has a lcd display for time. I wrote 4 alarms to trigger the continuous rotation servo motor to dispense the food for my cat. Currently the sketch is 5728 bytes I would be interested to see how small we can make it without loosing any functionality. I know the motor is set to run once when the code starts. Basically it will cycle the servo when plugged in or on returning from a power loss.

Thanks in advance:

// Pet feeder based on:
// Simple clock based on a DS3231 RTC and simple 16x2 LCD with continuous rotation servo SM-S4303R
// 15th July 2015, Andrew W. - arwhitus.weebly.com
// 26th Febuary 2018, Mike Burton

//include LCD, Servo, and Wire library
#include <LiquidCrystal.h>
#include <Wire.h>
#include <Servo.h>

// Declare the Servo pin
// Create a servo object
const int servoPin = 13;
Servo Servo1;
//----------------------------------------------------------
int run1 = 340; //wait  seconds-----may need to be adjusted depending on how much food is dispensed
int turn1 = 50; //clockwise speed-- may need to be adjusted depending on how much food is dispensed
//----------------------------------------------------------

//define RTC address
#define DS3231_I2C_ADDRESS 0x68

//define LCD pins and initialize LCD library
LiquidCrystal lcd(2, 3, 4, 5, 6, 7);

//convert normal decimal numbers to binary coded decimals
byte decToBcd(byte val)
{
return ( (val / 10 * 16) + (val % 10) );
}
//convert binary coded decimal to normal decimal numbers
byte bcdToDec(byte val)
{
return ( (val / 16 * 10) + (val % 16) );
}
//code that runs once at setup
void setup() 
{
// We need to attach the sevo to the used pin number #13
Servo1.attach(servoPin);
pinMode(servoPin, OUTPUT);
Servo1.write(90);
Servo1.write(turn1);//turn clockwise at full power
delay(run1);//wait  seconds--may need to be adjusted depending on how much food is dispensed
Servo1.write(90);//turn off servo
//start wire and lcd
Wire.begin();
lcd.begin(16,2); //(col, rows)
  
//Use below code to set time and date once from code
//it will run at every reset and you will lose the time on the RTC
//format is (sec, min, hr, day of week, day of month, month, year)
//------------------------------------------------------------------------------------------------------------------------------------------------------------------------
//setRTCTime(30, 25, 21, 6, 3, 3, 18);
//------------------------------------------------------------------------------------------------------------------------------------------------------------------------
}

//code that runs on a constant loop
void loop() 
{
printTime();
}
//easy and dirty way to clear the LCD
void clearLCD ()
{
lcd.setCursor(0,0);
lcd.print("                ");
lcd.setCursor(0,1);
lcd.print("                ");
lcd.setCursor(0,0);
}
//set the time and date to the RTC
void setRTCTime(byte second, byte minute, byte hour, byte dayOfWeek, byte day, byte month, byte year)
{
// sets time and date data to DS3231
Wire.beginTransmission(DS3231_I2C_ADDRESS);
Wire.write(0); // set next input to start at the seconds register
Wire.write(decToBcd(second)); // set seconds
Wire.write(decToBcd(minute)); // set minutes
Wire.write(decToBcd(hour)); // set hours
Wire.write(decToBcd(dayOfWeek)); // set day of week (1=Sunday, 7=Saturday)
Wire.write(decToBcd(day)); // set date (1 to 31)
Wire.write(decToBcd(month)); // set month
Wire.write(decToBcd(year)); // set year (0 to 99)
Wire.endTransmission();
}
//read the time and date from the RTC
void readRTCTime(byte *second, byte *minute, byte *hour, byte *dayOfWeek, byte *day, byte *month, byte *year)
{
Wire.beginTransmission(DS3231_I2C_ADDRESS);
Wire.write(0); // set DS3231 register pointer to 00h
Wire.endTransmission();
Wire.requestFrom(DS3231_I2C_ADDRESS, 7);
// request seven bytes of data from DS3231 starting from register 00h
*second = bcdToDec(Wire.read() & 0x7f);
*minute = bcdToDec(Wire.read());
*hour = bcdToDec(Wire.read() & 0x3f);
*dayOfWeek = bcdToDec(Wire.read());
*day = bcdToDec(Wire.read());
*month = bcdToDec(Wire.read());
*year = bcdToDec(Wire.read());
}
//reads the RTC time and prints it to the top of the LCD
void printTime()
{
byte second, minute, hour, dayOfWeek, day, month, year;
//retrieve time
readRTCTime(&second, &minute, &hour, &dayOfWeek, &day, &month, &year);
  {
  //print to lcd top
  lcd.setCursor(0,0);
  lcd.print("TIME:   ");
    if (hour<10)
  lcd.print("0");
  lcd.print(hour, DEC);
  lcd.print(":");
    if (minute<10)
  lcd.print("0");
  lcd.print(minute, DEC);
  lcd.print(":");
    if (second<10)
  lcd.print("0");
  lcd.print(second, DEC);
  lcd.setCursor(0,1);
     switch (dayOfWeek)               // Friendly printout the weekday
    {
    case 1:
      lcd.print("MONDAY  ");
       break;
    case 2:
      lcd.print("TUESDAY ");
      break;
    case 3:
      lcd.print("WED     ");
      break;
    case 4:
      lcd.print("THURS  ");
      break;
    case 5:
      lcd.print("FRIDAY  ");
      break;
    case 6:
      lcd.print("SAT     ");
      break;
    case 7:
      lcd.print("SUNDAY  ");
      break;
    }
    if (month<10)
    lcd.print("0");    
  lcd.print(month, DEC);
  lcd.print("/");
    if (day<10)
    lcd.print("0");
  lcd.print(day, DEC);
  lcd.print("/");
  lcd.print(year, DEC);
  delay(1000);
  }
  {
  //for daylight savings time adjustment--spring ahead
  if (dayOfWeek == 7 && month == 3 && day >= 8 && day <= 14 && hour == 2 && minute == 00 && second == 00)
    {
    //format is (sec, min, hr, day of week, day of month, month, year)
    setRTCTime(01, 00, 3, 1, day, month, year);
    }
  //for daylight savings time adjustment--fall back
  else if (dayOfWeek == 7 && month == 11 && day >= 1 && day <= 7 && hour == 2 && minute == 00 && second == 00)
    {
     //format is (sec, min, hr, day of week, day of month, month, year)
    setRTCTime(01, 00, 2, 1, day, month, year);
    lcd.setCursor(0,0);
    lcd.print("Waiting for DST!");
    lcd.setCursor(0,1);
    lcd.print("                ");
    delay(3601000);
    }
  }
  {
  if (hour == 5 && minute == 30 && second == 00) 
    { 
    Servo1.write(turn1);
    delay(run1);
    Servo1.write(90);//turn off servo}
    }
  else if (hour == 9 && minute == 45 && second == 00)
    {
    Servo1.write(turn1);
    delay(run1);
    Servo1.write(90);//turn off servo
    }
  else if (hour ==14 && minute == 00 && second == 00)
    {     
    Servo1.write(turn1);
    delay(run1);
    Servo1.write(90);//turn off servo
    }
  else if (hour == 18 && minute == 15 && second == 00)
    {
    Servo1.write(turn1);
    delay(run1);
    Servo1.write(90);//turn off servo
    }
  }
}
/*
-Commands for a continuous rotation servo
------------------------------------------------------------------------------------
Servo1.write(0) will make the servo spin counter clockwise at full speed
Servo1.write(90) stops the servo from spinning
Servo1.write(180) will make the servo spin clockwise at full speed
 */

You could extract all the time setting stuff and use a separate programme to do that.

http://bildr.org/2011/03/ds1307-arduino/

Good link. The only problem I see with that is that I would have 2 files then. That would be a valid option for decreasing the code though.

You have an RTC and you are doing delay(3601000); ???

Thats what I used to get over daylight savings time. The fall back part. If you shift the time back 1 hour would it not get stuck in that loop? example 2am it would switch back to 1am then at 2 it would do the same. These are the things I'm looking for suggestions on though. If you have an idea I would like to hear it.

why do you turn the servo in setup() ?

Why are you always printing? I could see printing once a every second for the the clock.

The animals get feed 4x a day ?

Your code looks good. Much better than some of these people coming on and asking "How do I write a library?"

noweare:
why do you turn the servo in setup() ?

To set it to an known initial position?

Better names would help - servo1 doesn't give much information about what it's used for.

There's not much point having a routine called printTime if it's the only thing called in loop - might as well just move all the code into loop. Don't though - make print time do what it says it does and move additional code into other functions. At the very least move the servo control code to its own function, even of you have to make all the time variables global to do it.

Get rid of the superfluous curly brackets e.g. line 161.

Generally, use of millis is preferred over delay. Since your system doesn't need to respond to any external inputs, it doesn't really matter but on the day that you add one, you'll have to refactor.

That delay(1000) is risky though - you have a lot of checks for second==0. That delay and the time taken to run the rest of your code might cause you to miss an action.

lcd.print("MONDAY  ");

use the F macro to save SRAM by putting your string literals in FLASH instead:

lcd.print(F("MONDAY  "));

noweare:
why do you turn the servo in setup() ?

If the power is out then it will run once when it comes back on. Just in case it has missed a cycle.

noweare:
The animals get feed 4x a day ?

yes

noweare:
Why are you always printing? I could see printing once a every second for the the clock.

What would you suggest?

Line 158:delay(1000);

 if (dayOfWeek == 7 && month == 11 && day >= 1 && day <= 7 && hour == 2 && minute == 00 && second == 00)

In general, it’s much easier to convert dates and times into a single serial number and work with that. For instance, convert these timestamps into a time serial:

long serial = month * 31L * 24L * 60L * 60L
   + day * 24L * 60L * 60L
   + hour * 60L * 60L
   + minute * 60L
   + second;

And do your checks against that number. Of course, these numbers will skip values for months with less than 31 days, but that doesn’t really matter unless you need to work out the size of intervals. You’d write a pair of functions to convert back and fourth, so the check becomes

if(day of week is sunday, and
   we aren't in daylight savings time, and
  current time serial >= convert_date_to_serial(11,1,2,0,0) /* Nov 1, 2AM */ ) {
  switch to daylight savings time;
}

Similarly, for scheduling things according to time of day it’s much easier to work with the second-in-day serial.

Actually - you wouldn’t write a function to convert, you’d use a macro. That way, the compiler can just crunch the numbers down to constants.

It will vastly simplify your code.

[/code]

Now I’m lost. I was still working on understanding wildbill I’m definitely a noob.

What would you suggest?

Line 158:delay(1000);
[/quote]

Sorry, didn't see that I was looking in loop() and did not see a delay.