Trying to make my sketch more efficient.

Using Arduino Programming Questions
#1

Hello!

This is my first post on the Arduino forums, so bear with me as I get acclimated. I am working on a project that is basically a simplistic clone of a CatEye style bicycle computer. I want to monitor speed and distance and so on. Ive been making great progress recently, and I have a pretty complete first version. The next goal is clean up some quirks and maybe get some slop out of the sketch. As you’ll see in my sketch, MOST of my processing happens when the sensor activate.

A few quirks, I’d like help with:

  • I run lcd.clear() every time I switch screens, and it isn’t a very quick method. Is there a faster way?
  • There is a boolean called ‘checked’ that I use to prevent the main logic from calculating more then once when the reed switch closes, is that the best method?
  • On the speed screen, the mph doesn’t update until the reed switch closes, so if you stop, it will sit with a false reading until you move. Someone it needs to display zero if you stop, more specifically after a set delay.
  • Is a switch/case the best method for handling my separate screens?
  • In the switch/case where I display the output, it seems unneeded to repeatedly reprint the labels for the values, can easily prevent that?
  • In the same area, when I display the values, I print out space characters to make sure the previous value is fully erased. That cant be proper.

This is my first major sketch and I think its looking pretty good. Hopefully you guys will have some input on improving it.

I tried to throw in some helpful comments, so the code is easier to parse.

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
//button reference
int but1 = 1;
int but2 = 0;
int but3 = 10;
int but4 = 6;

int rotations = 0; //track wheel rotations for averaging

int screen = 2; //value to differenciate screens in switch statement

double curMils = 0; //current milliseconds
double prevMils = 0; //previously stored milliseconds
double mph = 0;
double dist = 0;

//values for tracking average speed
double mphTotal = 0;
double mphAvg = 0;

boolean checked = false; //prevents multiple calcultions when the reed closes

void setup() {
  //print splash screen
  lcd.begin(16, 2);
  lcd.print("Mason's     v0.1");
  lcd.setCursor(3,1);
  lcd.print("Bike Computer");
  
  //set reed switch pin to input
  pinMode(8, INPUT);
  //set button pins to inputs
  pinMode(but1, INPUT);
  pinMode(but2, INPUT);
  pinMode(but3, INPUT);
  pinMode(but4, INPUT);
}

void loop() {
  //reset distance traveled or average speed if button 4 is pressed
  if (digitalRead(but3) == LOW) {
    switch(screen) {
      case 0:
        mphTotal = 0;
        rotations =0;
        break;
      case 1:
        dist = 0.00;
        break;
    }
  }
  //change screen value if button is pressed
  if (digitalRead(but1) == LOW) {
    lcd.clear();
    screen = 0;
  } else if (digitalRead(but2) == LOW) {
    lcd.clear();
    screen = 1;
  }
  
  //do calculations when the reed closes
  if (digitalRead(8) == LOW && !checked) {
    //keep track of rotations of wheel
    rotations++;
    
    //calculate current mph 
    curMils = millis();
    mph = (4873.591953413029) / (curMils - prevMils);
    prevMils = curMils;
    
    //calcult distance traveled
    dist = dist + 0.000430921;
    
    //calculate the average speed
    mphTotal = mphTotal + mph;
    mphAvg = mphTotal / rotations;
  
    //mark that calculations have occures for this rotation
    checked = true;
    
    //display output to lcd
    switch(screen) {
      case 0:
        lcd.setCursor(0,0);
        lcd.print("MPH: ");
        lcd.print("      ");
        lcd.setCursor(5,0);
        lcd.print(mph);
        lcd.setCursor(0,1);
        lcd.print("Avg: ");
        lcd.print("      ");
        lcd.setCursor(5,1);
        lcd.print(mphAvg);
        break;
      case 1:
        lcd.setCursor(0,0);
        lcd.print("Total Distance: ");
        lcd.setCursor(7,1);
        lcd.print(dist);
        lcd.print("mi");
        break;
      default:
        lcd.print("Mason's     v0.1");
        lcd.setCursor(3,1);
        lcd.print("Bike Computer");
        break;
    }
  }
  if (digitalRead(8) == HIGH && checked) {
    checked = false;
  }
  if ((millis() - prevMils) >= 1000) {
    mph = 0;
    //lcd.print("      ");
    //lcd.setCursor(5,0);
    //lcd.print(mph);
  }
}
#2 
double curMils = 0;

sp. "unsigned long"

#3

Jeebiss:

  • I run lcd.clear() every time I switch screens, and it isn't a very quick method. Is there a faster way?

Do you have to clear the entire screen, or just changes parts of it?

  • There is a boolean called 'checked' that I use to prevent the main logic from calculating more then once when the reed switch closes, is that the best method?

This:

if (digitalRead(8) == HIGH && checked) {
    checked = false;
  }

Can just be this:

else if (digitalRead(8) == HIGH) {
    checked = false;
  }

If checked is already false, there is no harm in setting it to false.

  • On the speed screen, the mph doesn't update until the reed switch closes, so if you stop, it will sit with a false reading until you move. Someone it needs to display zero if you stop, more specifically after a set delay.

When a change is made, make note of the time using millis(). Outside within your loop() function, check if it's the proper "screen" and check if it's been XX amount of time since the time you made note of. If it has, print 0.

  • Is a switch/case the best method for handling my separate screens?

Yes.

  • In the switch/case where I display the output, it seems unneeded to repeatedly reprint the labels for the values, can easily prevent that?

Don't clear the screen unless you are switching screens and only update the parts of the screen that need updating.

  • In the same area, when I display the values, I print out space characters to make sure the previous value is fully erased. That cant be proper.

It is. Another option would be to use sprintf(). While is reduces the number of lines of code, it's less efficient.

#4