how can i improve my code?

Projects Programming
1

Hi all, i'm new around here :slight_smile:
i'm 17, and work as a security engineer by trade, however i've always been interested in electronics and making various things in my spare time.

i've had an arduino for years but only recently started properly using it.

this is my first 'proper' project using one, i have some electricity meters that have a transistor output which pulses for every one watt-hour of electricity used. i have one of these connected to pin 2 of my arduino mega.
i've modified the debounce example to make a sketch which measures the amount of watt hours used on a 5 minute cycle,
it also gives a rolling average based on the last 15 mins of power usage.

i do have an RTC which i plan to use to time stamp the data, and i have an ethernet shield which i'm intending to use to allow me to store data onto an SD card, and also enable the data to be accessed via TCP/IP requests.

const int meterA = 2;    // pin meter A is connected to.
const int ledPin = 13;      // the number of the LED pin

// Variables will change:
int ledState = HIGH;         // the current state of the output pin
int buttonState;             // the current reading from the input pin
int lastButtonState = LOW;   // the previous reading from the input pin
int val = 0;
int valold = 0;
int start;
int potNumber = 0;
unsigned int count = 0;
int mill = millis();

// the following variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long cycleTime = 300000;        //time of each measuring cycle 300000ms = 5min
long lastDebounceTime = 0;  // the last time the output pin was toggled
long debounceDelay = 50;    // the debounce time; increase if the output flickers
long nextUpdate;
long pot0 = 0;              //pot to store the third most recent value
long pot1 = 0;              //pot to store the second most recent value
long pot2 = 0;              //pot to store the most recent value.
long potTotal = 0;          //total value of all the pots.
long potAverage = 0;        //average value of the last three pots.

void setup() {
  Serial.begin(9600);
  valold = digitalRead(meterA);
  nextUpdate = millis() + cycleTime; 
  pinMode(meterA, INPUT);
  pinMode(ledPin, OUTPUT);

  // print startup message
  Serial.print ("arduino electricity logger V1.0 created by Luke.\nprogram running\n");
}

void loop() 
{
  val = 0; //reset val
  nextUpdate = 1;
//  count = 0; //reset counter
//  start = millis(); //take the start time
  //do the following loop for 60000ms = 1min
  if (millis() >= nextUpdate){
    nextUpdate = millis() + cycleTime;
  }
  if (nextUpdate < 0){
    nextUpdate = millis() + cycleTime;
  }
  while (millis() <= nextUpdate){
    // read the state of the switch into a local variable:
    int reading = digitalRead(meterA);
    // check to see if you just pressed the button 
    // (i.e. the input went from LOW to HIGH),  and you've waited 
    // long enough since the last press to ignore any noise:  
  
    // If the switch changed, due to noise or pressing:
    if (reading != lastButtonState) {
      // reset the debouncing timer
      lastDebounceTime = millis();
    } 
    
    if ((millis() - lastDebounceTime) > debounceDelay) {
      // whatever the reading is at, it's been there for longer
      // than the debounce delay, so take it as the actual current state:
  
      // if the button state has changed:
      if (reading != buttonState) {
        buttonState = reading;
  
        // only toggle the LED if the new button state is HIGH
        if (buttonState == LOW) {
          ledState = HIGH;
        //  Serial.print("1WH\n");
          val = val + 1;
        }
        else{
          ledState = LOW;
        }
      }
    }
    // save the reading.  Next time through the loop,
    // it'll be the lastButtonState:
    lastButtonState = reading;
  }
  Serial.println("\n**new cycle**");
  Serial.print("watts used in last 5 mins: ");
  Serial.println(val);
  if (potNumber == 0) {            //if potNumber = 0, save val to pot0 and then increment potNumber.
    pot0 = val;
    potNumber = potNumber + 1;
  }
  else{
   if (potNumber == 1) {           //if potNumber = 1, save val to pot1 and then increment potNumber.
    pot1 = val;
    potNumber = potNumber + 1;
   }   
    else{
     if (potNumber >= 2) {         //if potNumber = 2, save val to pot2 and then reset PotNumber to 0.
      pot2 = val;
      potNumber = 0;
     }
    }
  }
  potTotal = (pot0 + pot1 + pot2);
  potAverage = (potTotal / 3);
  Serial.print("total watts used in last 15 mins: ");
  Serial.println(potTotal);
  Serial.print("average watts used last 5 mins: ");
  Serial.println(potAverage);
}

now my questions!
1: i intend to have 5 meters connected when the project is finished, what is the best way to do this in the code? i know i could replicate everything 5 times... but there MUST be a simpler, more efficient way?

2: how can i clean the sketch up? i know its quite messy in some places, that's a result of me chopping and changing various examples between writing my own code, but, for example:

  Serial.print("watts used in last 5 mins: ");
  Serial.println(val);

it seems silly that i have to use two lines of code for this, is there a way to Serial.print text, AND a variable on the same line?
also, as i say this is my first sketch so IS pretty bulky, any tips on how to make it less bulky would be great! :slight_smile:

i think that's all i have to ask for now, i'll no doubt be back soon to ask for advice on the RTC, SD, and IP connection (i'm aiming to have a simple web interface)

thanks in advance! :slight_smile:
-Luke

2

You should look into interrupts. I don't think you need to debounce anything if it just sends out a pulse, but if you want efficiency then interrupts are the way to go.

3

any tips on how to make it less bulky would be great

Use appropriately-sized variables, keep variables that should be local local, don't waste RAM by putting constant strings in RAM.
Make sure variables that should be unsigned are unsigned.

4

HazardsMind:
You should look into interrupts. I don't think you need to debounce anything if it just sends out a pulse, but if you want efficiency then interrupts are the way to go.

unfortunately, i do need to debounce. the very first sketch i did didn't use debounce and the readings were always about 7X higher than they should of been.
will look into interrupts now, thanks! :slight_smile:

5

1: i intend to have 5 meters connected when the project is finished, what is the best way to do this in the code? i know i could replicate everything 5 times... but there MUST be a simpler, more efficient way?

Create a class that implements your meter interface and then instance 5 of them.

6

unfortunately, i do need to debounce. the very first sketch i did didn't use debounce and the readings were always about 7X higher than they should of been.

Do you have an oscilloscope? I'm curious as to what the meter is actually outputting.

7

If you have several meters the simplest way to keep track of everything is with a few arrays. For example one array would hold the pin number that each meter is connected to and another the data collected. Then you just need a FOR loop to iterate through all the meters with the same piece of code.

If the pulse from the meter stays HIGH (or LOW as the case may be) for two hundred microseconds or so you won't need to use interrupts. The Arduno will be perfectly able to check the condition of the input pins much faster than that so it won't miss anything. If the pulses are much shorter you probably will need to use interruts in case two pulses occur close together.

If you are a beginner, avoid interrupts if you can as they can be hard to debug. If you do need to use interrupts make sure the code in the interrupy routine is very very short.

...R

8 
  valold = digitalRead(meterA);
  nextUpdate = millis() + cycleTime; 
  pinMode(meterA, INPUT);

Read from the pin, and then make it an input. Novel approach.

9

HazardsMind:

unfortunately, i do need to debounce. the very first sketch i did didn't use debounce and the readings were always about 7X higher than they should of been.

Do you have an oscilloscope? I'm curious as to what the meter is actually outputting.

[url=https://www.youtube.com/watch?v=uPMGtORsbmk[this video](http://this video) is old, i made it before i even started messing with the arduino interface... does it show you what you want? if not i can make another one but we're redoing the house at the mo so getting the scope out would be a pain.