DS3231 RTC returns a 0 when setting time through serial????

Hi all I’m new to this and am trying to write a program for setting the realtime clock or even checking it but once set it bypasses this option after a certain time, I’m using an Ardunio UNO and version 1.8.10.
at present I’ve managed to get it to read and display the time on the serial monitor the problem I’m having is when I wish to reset the time it goes to settime() and then sets the 0 for hours and asks for the minutes, I think I’m missing something. The project will form a bigger project using a SD card to log and time and date along with sensor readings for short periods, and taking the date to form the data file in .csv format.
the switch works far better than the if statements and quicker, is there a way I can use a logical or statement as one case eg (‘y’ || ‘Y’ ).

my code is below.
any help would be greatly appreciated:-

include <Wire.h>
#define DS3231_I2C_ADDRESS 0x68

byte decToBcd(byte val){
  return((val/10*16) + (val%10));
}
byte bcdToDec(byte val){
  return((val/16*10) + (val%16));
}
byte sec,mins,hr,wkday,date,mon,yr;
byte sec1,mins1,hr1,wkday1,date1,mon1,yr1;
word var;

void setup(){ 
  // put your setup code here, to run once:
  Wire.begin();
  Serial.begin(9600);
      Serial.print("The Current Date and Time is = ");
      readtime();
      displaytime();
      Serial.println("Do you wish to change the Date and Time Y/N?");
      //enter here to ask if you wish to change date and time

 while (var <= 500){
     var ++;
      if (!Serial.available()) delay(10);
        int inByte = Serial.read();
        switch (inByte){
          case 'y':
          settime();
           break; 
          case 'Y':
          settime();
           break;
          case 'n':
           Serial.println("The Date and Time Hasn't Changed:");
           displaytime();
           break;
          case 'N':
           Serial.println("The Date and Time Hasn't Changed:");
           displaytime();
           break;
           }
         }
         var = 0;
       }
       
void settime(){
  Serial.println("Enter Current Hour from 00 - 23");
  hr = readByte();
  Serial.println(hr);
  Serial.println("Enter Current Minutes from 0 - 59");
  mins = readByte();
  Serial.println(mins);
  sec = 00;
  Serial.println("Enter Current Day of the week 1-7");
  Serial.println("1 = Sun, 2 = Mon, 3 = Tues, 4 = Weds, 5 = Thur, 6 = Fri, 7 = Sat");
  wkday = readByte();
  Serial.println(wkday);
  Serial.println("Enter Current date from 1 - 31");
  date = readByte();
  Serial.println(date);
  Serial.println("Enter Current Month from 1 - 12");
  mon = readByte();
  Serial.println(mon);
  Serial.println("Enter Current Year from 00 - 99");
  yr = readByte();
  Serial.println(yr);
  Wire.beginTransmission(DS3231_I2C_ADDRESS);
  Wire.write(0);
  Wire.write(decToBcd(sec));
  Wire.write(decToBcd(mins));
  Wire.write(decToBcd(hr));
  Wire.write(decToBcd(wkday));
  Wire.write(decToBcd(date));
  Wire.write(decToBcd(mon));
  Wire.write(decToBcd(yr));
  Wire.endTransmission();  
}

byte readByte(){
 while(!Serial.available()) delay(10);
  byte reading = 0;
  byte incomingByte = Serial.read();
  while(incomingByte != '\n'){
    if (incomingByte >='0' && incomingByte <='9')
     reading = reading*10 + (incomingByte - '0');
     else;
     incomingByte = Serial.read();
  }
  Serial.flush();
  return reading;
}

void readtime(){
  Wire.beginTransmission(DS3231_I2C_ADDRESS);
  Wire.write(0);
  Wire.endTransmission();
  Wire.requestFrom(DS3231_I2C_ADDRESS,7);
  sec1 = bcdToDec(Wire.read() & 0x7f);
  mins1 = bcdToDec(Wire.read());
  hr1 = bcdToDec(Wire.read() & 0x3f);
  wkday1 = bcdToDec(Wire.read());
  date1 = bcdToDec(Wire.read());
  mon1 = bcdToDec(Wire.read());
  yr1 = bcdToDec(Wire.read());
}

void displaytime(){
  readtime();
  if (hr1 <=9){
    Serial.print("0");
  }
  Serial.print(hr1,DEC);
  Serial.print(":");
  if (mins1<=9){
    Serial.print("0");
  }
  Serial.print(mins1,DEC);
  Serial.print(":");
    if (sec1<=9){
    Serial.print("0");
  }
  Serial.print(sec1,DEC);
  Serial.print("  -  ");
  if (date1<=9){
    Serial.print("0");
  }
  Serial.print(date1,DEC);
  Serial.print("/");
  if (mon1<=9){
    Serial.print("0");
  }
  Serial.print(mon1,DEC);
  Serial.print("/");
  Serial.print("20");
  if (yr1<=9){
    Serial.print("0");
  }
  Serial.println(yr1,DEC);  
}

void loop() {                                                                                                                   // put your main code here, to run repeatedly:
     //Serial.println("this is the start of something amazing i hope");
displaytime();
delay(500);
}

The DS3231 is very accurate, and the answer to       Serial.println("Do you wish to change the Date and Time Y/N?");is most likely N, and you might use a separate programme to set the time when you need it. http://bildr.org/2011/03/ds1307-arduino/

grifnick: the problem I'm having is when I wish to reset the time it goes to settime() and then sets the 0 for hours and asks for the minutes, I think I'm missing something.

Does it also echo and set the value 0 for minutes, day of week, day, month etc. ?

Have you tested readByte() by itself?

Hi

It doesn't return a zero for anything else just the hours, I changed the program to Switch case as I thought it would be better than using loads of If Statements.

When I used

if(Serial.read() =='y' || Serial.read() == 'Y') {

}

etc etc

the readByte() function works fine.

I just wondered if I need to do something with the serial buffer before jumping to settime and readByte, but not sure how to do it??????

    if (!Serial.available())
      delay(10);
      
    int inByte = Serial.read();

This doesn't seem right. If there are no characters available, wait 1/100th of a second and assume a character has arrived?!? And you do the same thing in readByte()?!?

I think what is happening is that you read the 'y' or 'Y' and then the first character readByte() sees is the '\n' that follows the 'y' or 'Y'. You should clear the input buffer "while(Serial.available()) Serial.read();" after you prompt for input.

Hint: use either of the functions toupper() or tolower() to force the case of an ASCII character.

why not just set the time directly using code instead of input via serial?

Hi All thanks for all your help have managed to sort it with all your guidance,

I changed this part

byte readByte(){
 while(!Serial.available()) delay(10);
  byte reading = 0;
  byte incomingByte = Serial.read();
  while(incomingByte != '\n'){
    if (incomingByte >='0' && incomingByte <='9')
     reading = reading*10 + (incomingByte - '0');
     else;
     incomingByte = Serial.read();
  }
  Serial.flush();
  return reading;
}

to this

byte readByte(){
 while(Serial.available()) Serial.read();
  byte reading = 0;
  byte incomingByte = Serial.read();
  while(incomingByte != '\n'){
    if (incomingByte >='0' && incomingByte <='9')
     reading = reading*10 + (incomingByte - '0');
     else;
     incomingByte = Serial.read();
  }
  Serial.flush();
  return reading;
}

it now clears the Input Buffer and allows me to enter the hours as I wanted, the rest of it now works fine

Thanks it takes a little longer to start to learn something when your 50+

  byte incomingByte = Serial.read();
  while(incomingByte != '\n'){
    if (incomingByte >='0' && incomingByte <='9')
     reading = reading*10 + (incomingByte - '0');
     else;
     incomingByte = Serial.read();
  }

This is one of those places where it makes sense to use the ‘do-while’ statement. It’s similar to the ‘while’ except the test is done after the body of the loop instead of before. That way you don’t need Serial.read() in two places:

  byte incomingByte;
  do 
  {
     incomingByte = Serial.read();
     if (incomingByte >= '0' && incomingByte <= '9')
       reading = (reading * 10) + (incomingByte - '0');
  }
  while (incomingByte != '\n');

Thanks John I will give that a whirl makes alot of sense plus it uses, fewer lines of code.