Loops

Projects Programming
1

Perhaps I'm asking too many questions all at once.

This is the function I call resetTime():

void resetTime(){

//global variables: year, month, date, hour, minute, wait, edit, YesPin, NoPin.



byte rst;  //variable of 'for' loop

  //1=year. 2=month, 3=date, 4=hour, 5=minute.

  for (rst = 0; rst <=5; rst++) {


    switch (rst) {

      case 0:

      lcd.clear();


     break;


      case 1: // year


        
        while (edit == 1) {

          lcd.print("The year is 20");
          lcd.print(year);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");

          if (digitalRead(NoPin) == 1) {
            year = year + 1;
            if (year > 21)year = 16;
            delay(250);

            lcd.clear();
          }

          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }

        } // End year loop

        break;

      case 2:

        edit = 1;

        lcd.clear();
        while (edit == 1) {
          lcd.print("The month is ");
          if (month < 10)lcd.print(" ");
          lcd.print(month);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");
          if (digitalRead(NoPin) == 1) {
            month = month + 1;
            if (month > 12)month = 1;
            delay(500);

            lcd.clear();
          }
          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }
        } // End month loop

        break;

      case 3:

        edit = 1;

        lcd.clear();
        while (edit == 1) {
          lcd.print("The date is ");
          if (date < 10)lcd.print(" ");
          lcd.print(date);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");
          if (digitalRead(NoPin) == 1) {
            date = date + 1;
            if (date > 31)date = 1;
            delay(500);

            lcd.clear();
          }
          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }
        } // End date loop

        break;

      case 4:

        edit = 1;

        lcd.clear();
        while (edit == 1) {
          lcd.print("The hour is ");
          if (hour < 10)lcd.print(" ");
          lcd.print(hour);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");
          if (digitalRead(NoPin) == 1) {
            hour = hour + 1;
            if (hour > 23)month = 0;
            delay(500);

            lcd.clear();
          }
          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }
        } // End hour loop

        break;

      case 5:

        edit = 1;

        lcd.clear();
        while (edit == 1) {
          lcd.print("The minute is ");
          if (minute < 10)lcd.print(" ");
          lcd.print(month);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");
          if (digitalRead(NoPin) == 1) {
            minute = minute + 1;
            if (minute > 59)minute = 0;
            delay(500);

            lcd.clear();
          }
          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }
        } // End minute loop

        break;
    }
  }

}

What I am imagining is that the first time through the for loop, the display is cleared (switch case 0).
'break' then closes switch case and so we return to the 'for' loop which increments rst to 1.
Switch case is re-entered at case 2 and the global year variable is edited. Then break takes us out and back to 'for' which increments rst to 2.
Case 2 edits the month variable. and so on until rst is 5 when minute is edited, then the function closes.

The trouble is that it doesn't work like that so there's something I'm missing. Sometimes case 1 is presented, sometimes it goes straight to case 5 but never to the other cases.

2

Sometimes case 1 is presented, sometimes it goes straight to case 5 but never to the other cases.

I still suspect that your switches are not wired correctly. Post a schematic of the wiring, and a picture of it.

3

There is an enormous amount of repetition in your code. All that common stuff should go into a function that can be called for each situation. As well as shortening the program it means there is less scope for errors and only one piece that needs testing and debugging.

Personally I would put all the button stuff in its own function that gets called eacj time through loop(). Of course that implies that I would not be using WHILE.

Have a look at Planning and Implementing a Program

...R

4

If you press 'yes', your loop 'immediately' finishes the current iteration and next continues with the next iteration of the loop. If you still have pressed it (and that might be a matter of milliseconds), it will also accept the next one; so you have to wait till the button is released again.

In its simplest form, you can achieve this with something like

  for (rst = 0; rst <= 5; rst++)
  {
    while (digitalRead(YesPin) == 1);

    switch(rst)
    ...
    ...
  }

This simply waits till the button is released.

You might also suffer from bouncing where the button state goes quickly high-low-high-low etc for a while. You can do a search how to debounce a button.

5

sterretje's suggestion overcame all but one problem - case 5 doesn't respond!

Robin2, I agree but I have not as yet been able to understand how I would change the variable names if I used one section of the code as a function.

I promise you PaulS that my switches are wired correctly - I've been designing and building electronic circuitry for decades ... that is until I retired! It's just this darn new-fangled C stuff that's finishing me off.

Out of frustration I returned to a previously saved sketch from before Christmas. It works fine as far as resetTime() is concerned, there's a lesson there somewhere.

6

I could be wrong but I think you have a syntax error with your if statements.

if (hour < 10)lcd.print(" ");

perhaps I'm wrong, but it was my understanding that an if statment without curly's will only execute the next line of code which in this case would be cd.print(" "). So maybe something like this would work better?

 if (hour < 10){
      lcd.print(" ");
      lcd.print(hour);
      lcd.setCursor(6, 2);
      lcd.print("Correct?");
      lcd.setCursor(0, 3);
      lcd.print("No               Yes");
  }

I'll look a bit more but at first glance that was standing out. If I missed the mark let me know I'll try and readjust my aim.

7

See if this works, I was unable to compile it because I didn't declare lcd but I think it should if I had the library's.

void resetTime(){

//global variables: year, month, date, hour, minute, wait, edit, YesPin, NoPin.



byte rst;  //variable of 'for' loop

  //1=year. 2=month, 3=date, 4=hour, 5=minute.

  for (rst = 0; rst <=5; rst++) {


    switch (rst) {

      case 0:
      lcd.clear();
     break;

      case 1: // year 
        while (edit == 1) {

          lcd.print("The year is 20");
          lcd.print(year);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");

          if (digitalRead(NoPin) == 1) {
            year += 1;
            if (year > 21)year = 16;
            delay(250);

            lcd.clear();
          }

          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }

        } // End year loop

        break;

      case 2:
        edit = 1;

        lcd.clear();
        while (edit == 1) {
          lcd.print("The month is ");
          if (month < 10){
		  lcd.print(" ");
          lcd.print(month);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");
		  }
		  
          if (digitalRead(NoPin) == 1) {
            month += 1;
            if (month > 12)month = 1;
            delay(500);

            lcd.clear();
          }
          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }
        } // End month loop

        break;

      case 3:

        edit = 1;

        lcd.clear();
        while (edit == 1) {
          lcd.print("The date is ");
          if (date < 10){
		  lcd.print(" ");
          lcd.print(date);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");
		  }
          if (digitalRead(NoPin) == 1) {
            date += 1;
            if (date > 31)date = 1;
            delay(500);

            lcd.clear();
          }
          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }
        } // End date loop

        break;

      case 4:

        edit = 1;

        lcd.clear();
        while (edit == 1) {
          lcd.print("The hour is ");
          if (hour < 10){
			lcd.print(" ");
			lcd.print(hour);
			lcd.setCursor(6, 2);
			lcd.print("Correct?");
			lcd.setCursor(0, 3);
			lcd.print("No               Yes");
		  }
		  
          if (digitalRead(NoPin) == 1) {
            hour = hour + 1;
            if (hour > 23)month = 0;
            delay(500);

            lcd.clear();
          }
          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }
        } // End hour loop

        break;

      case 5:

        edit = 1;

        lcd.clear();
        while (edit == 1) {
          lcd.print("The minute is ");
          if (minute < 10){
		  lcd.print(" ");
          lcd.print(month);
          lcd.setCursor(6, 2);
          lcd.print("Correct?");
          lcd.setCursor(0, 3);
          lcd.print("No               Yes");
		  }
		  
          if (digitalRead(NoPin) == 1) {
            minute = minute + 1;
            if (minute > 59)minute = 0;
            delay(500);

            lcd.clear();
          }
          if (digitalRead(YesPin) == 1) {
            lcd.clear();

            edit = 0;
          }
        } // End minute loop

        break;
    }
  }

}
8

ZBay:
I could be wrong but I think you have a syntax error with your if statements.

if (hour < 10)lcd.print(" ");

perhaps I'm wrong, but it was my understanding that an if statment without curly's will only execute the next line of code which in this case would be cd.print(" "). So maybe something like this would work better?

Your understanding of 'if statements' is correct. And that is exactly the intention of that code; 'left pad' the hour with a space (if needed). If the hour is less than 10, it will first print a space before printing the hour so the least significant digit is always in the same place.

9

Adrifran39:
sterretje's suggestion overcame all but one problem - case 5 doesn't respond!

Does that mean that you can not edit minutes or that you can not leave the for loop? In general, Serial.println in combination with Serial Monitor can be a great help in debugging.

10

Adrifran39:
Robin2, I agree but I have not as yet been able to understand how I would change the variable names if I used one section of the code as a function.

It will be easier to help you if make the attempt.

...R

11

ZBay, I've added your suggestion in place of my own code but the result is the same; we simply leave resetTime()! ( there's an error , year=year+1;)

Serretje, it means the function hangs in 'minutes'. The variable cannot be changed and the loop cannot be left.

Now then Robin2, I have taken on board your comment that there is too much repetition in the resetTime() function and I did attempt to write a fragment of code that changed the display and variables through the five repeats (if you understand what I mean) :

//Nano

#include <Wire.h>
#include<LiquidCrystal_I2C.h>
#include <OneWire.h>
LiquidCrystal_I2C lcd(0x3F, 2, 1, 0, 4, 5, 6, 7); // 0x3F is the LCD address
byte zero = 0x00; //Workround for issue #527
#define BACKLIGHT_PIN 3
byte edit;
byte year;
byte month;
byte date;
byte hour;
byte minute;
byte NoPin;
byte YesPin;
byte button;
byte lastInput;


void setup() {
  Wire.begin();
  lcd.begin(20, 4);
  Serial.begin(9600);
  // lcd.setBacklightPin(BACKLIGHT_PIN, POSITIVE);
  lcd.setBacklight(HIGH);
}

void loop() {
  resetTime();
}

//year

void resetTime() {
  byte rst;
  byte x;
  byte variable;

  char* rstString[] = {"Year is 20", "Month is", "Date is", "Hour is", "Minute is"};

  lcd.setCursor(0, 0);
  lcd.clear();


  for (x = 0; x < 6; rst++) {

    while (digitalRead(YesPin) == 1);  //no buttons pressed

    lcd.print(rstString[x]);
    lcd.print(variable);

    lcd.setCursor(0, 1);
    lcd.print("Is this correct?");
    lcd.setCursor(0, 3);
    lcd.print("No               Yes");

    while ((digitalRead(NoPin) == LOW) && (digitalRead(YesPin) == LOW));

    //wait for button press

    if (digitalRead(YesPin) == HIGH) {
      button = YesPin;
      debounce();
      if (lastInput == HIGH) edit = 0; //end orbit
    }
    if (digitalRead(NoPin) == HIGH) {
      button = NoPin;
      debounce();
      if (lastInput == HIGH)
        variable = variable + 1;
      if (variable > 20)variable = 14; //to limit year range  THIS IS VARIABLE TOO
    }

  }  //continue year orbit
}

void debounce() {

}

My understanding of string handling is yet to be improved but I settled for the statement 'char* rstString[] =

{"Year is 20", "Month is", "Date is", "Hour is", "Minute is"}; to produce the first line.

The compiler(?) doesn't much like it! 'Deprecated conversion from string constant to 'char*' it declares yet I think I have copied it exactly from 'strings' in the Arduino reference.

The greatest difficulty I found, was in displaying the actual name of the current variable and adjusting its maximum value which in the case of 'year' I have as 21.

Please don't feel I am wanting you to do the work for me but I need hints and pointers, especially when a search seems to produce an error as above.

12

I had to replace #include LiquidCrystal_I2C.h by #include LiquidCrystal.h and removed the include of OneWire.h but after that your new code compiled.

You have one bug in your code. rstString contains 5 elements; indexed 0 .. 4. Your for loop however does 6 iterations which can happily cause havoc and create unexpected results.

13

Adrifran39:
My understanding of string handling is yet to be improved but I settled for the statement 'char* rstString[] =

{"Year is 20", "Month is", "Date is", "Hour is", "Minute is"}; to produce the first line.

I'm not good enough at C/C++ to deal with that without research and trial and error myself.

However I don't see the code in Reply #10 as the sort of division into functions that I have in mind.

I was thinking that your code in loop() would be something like this

void loop() {
   readSwitches();
   updateLCD();
   updateTime();
   // other functions
}

For example the only place where LCD code would exist would be in the updateLCD function and the only place with digitalRead()s would be the readSwitches() function. That allows each function to be tested separately by putting it into a short test/development program.

...R

14

Yes, robin2. I see what you mean but I was trying to keep voidLoop() to just a list of the functions necessary and do all the nitty-gritty inside each function although I take your point about updateLCD and updateTime as stand alone functions.

This is why I was trying to condense the repetitious code in resetTime() to just one routine in a for loop which varied the significant data.

Perhaps somone else can tell me why the compiler objects to the use of 'char* rstString[] = {"Year is 20", "Month is", "Date is", "Hour is", "Minute is"};'

And how I can display the identity (not the value) of the relevant variable in an 'lcd.print() command.

And how I may change 'year = year + 1;' and 'if (year > 20)year = 14;' so as to be applicable to the other four situations. I refer to #0 and #10.

Personally, I think it is impossible and there is no other option than a long list of repetitive code.

15

Perhaps somone else can tell me why the compiler objects to the use of 'char* rstString[] = {"Year is 20", "Month is", "Date is", "Hour is", "Minute is"};'

What error does it give ?

16

UKHeliBob, it tells me 'Deprecated conversion from string constant to 'char*'

sterretje's suggestion - 'while (digitalRead(YesPin) == 1);' or an adaptation of it at strategic points proved to be the answer, so thank you sterretje.

The only point remaining is how to create a function to resetTime without all that repitition.

Thanks to everyone, my project is functioning as planned!

17

In C++ the type of an ordinary string literal is const char[], so you need

const char * rstString[] = {...};
18

Adrifran39:
The only point remaining is how to create a function to resetTime without all that repitition.

Repetition is not necessarily bad (my opinion) and can not be prevented completely. One thing that I immediately noticed when I looked at the code in the opening post is that every case has a 'prompt'. That is what I would have changed to a function.

/*
  display a prompt on the second line of the LCD display
*/
void displayPrompt()
{
  lcd.setCursor(6, 2);
  lcd.print("Correct?");
  lcd.setCursor(0, 3);
  lcd.print("No               Yes");
}

Next I might expand it with a function to display the data on the LCD as well.

/*
  display a message in the first line of the LCD
*/
void displayMessage(char *message)
{
  lcd.print(message);
}

You can add a call to displayPrompt() after the 'print'.

In your main loop you can now call display message

void loop()
{
  // message buffer for LCD; 20 characters max and terminating '\0'
  char messageBuffer[21];
  ...
  ...
  switch(rst)
  {
    case 0:
      ...
      ...
      break;
    case 1:
      while(edit == 1)
      {
        // create message
        snprintf(messageBuffer, sizeof(messageBuffer), "The year is 20%02d", year);
        displayMessage(messageBuffer);
        displayPrompt();
        ...
        ...
      }
      break;
    case 2:
      while(edit == 1)
      {
        // create message
        snprintf(messageBuffer, sizeof(messageBuffer), "The month is %2d", month);
        displayMessage(messageBuffer);
        displayPrompt();
        ...
        ...
      }
      break;
    // other cases
    ...
    ...
  }
}

Notes:

  1. this assumes that year is 2 digits max; it will append a leading zero if needed.
  2. for month, it will automatically pad with a leading space if needed.
    You can also write a function to check the limits; possibly something like
/*
  check limits for a value
  returns (adjusted) value that will be within the limits; e.g. for month set limits to 1 and 12 and the function will return a value between 1 and 12 (both included)
*/
int checkLimits(int value, int min, int max)
{
  if(value > max)
    return min;
  else if (value < min)
    return max;
  else
    return value;
}

and incorporate it in the code.

Hope this helps a bit.

// edit: added the notes

19

sterretje:
One thing that I immediately noticed when I looked at the code in the opening post is that every case has a 'prompt'.

You have expressed more clearly what I was trying to say.

...R