int simplification

Hi I was wondering how to simplify this

int total; 
int readings; 
int irnow;
int average;
int newaverage;
int ButtonValue = 0;
int Button = 3;
int ButtonValue2 = 0;
int Button2 = 8;
int ButtonValue3 = 0;
int Button3 = 10;

Help much appreciated!! Squidsirymchenry

You can delete whichever of those variables you don't actually use. Other than that, what sort of simplification are you expecting?

Pete

  • you could change Button, Button2 and Button3 to char instead of int which will save 3 bytes of ram.

Pete

You can use a ‘struct’ to combine the pin pin number and the pin value in one variable. If you use software debouncing, you can also store the info related to debouncing in that struct.

See e.g. Data structures - C++ Tutorials

You can use an array of those structs as well.

I actually need all of the variables, via my code below,and please put examples, as i’m new to this, it’s confusing.

int total; 
int readings; 
int irnow;
int average;
int newaverage;
int ButtonValue = 0;
int Button = 3;
int ButtonValue2 = 0;
int Button2 = 8;
int ButtonValue3 = 0;
int Button3 = 10;
void setup() {
  Serial.begin(9600);
  pinMode(2, OUTPUT);
  pinMode(Button, INPUT_PULLUP);

             }
void loop() {
  irnow= 0;
    while ( irnow < 9)
    {
      readings = analogRead(A1);
      total = total + readings;
      irnow = irnow +1;
    }
    average = total / 9;
    newaverage = map(average, 0, 1023, 500, 200);

       if (newaverage < 390)
        { 
          if (newaverage > 385)
          {
           digitalWrite(2, HIGH);
           delay(500);
          }
               
           if (newaverage <= 370)
           {
           digitalWrite(2, HIGH);
           delay(100);
           }
        }
          else
        digitalWrite(2,LOW);

ButtonValue = digitalRead(Button);
ButtonValue2 = digitalRead(Button2);

  if(ButtonValue != 0){
    digitalWrite(2,LOW);
   delay(30000);
  }
   else if(ButtonValue2 != 0)
   {
    digitalWrite(2,LOW);
    delay(300000);//5 minutes 60 * 5 300 and then * 1000 = 300000
   }
ButtonValue3 = digitalRead(Button3);
  if(ButtonValue3 != 0){
    digitalWrite ( 2, LOW);
    while(true);
  }
   
  average = 0; 
  total = 0;
  newaverage = 0;
 }

Why are you wondering how to simplify that code? It is such a small program that "simplifying" it isn't going to gain you anything.

If you use the Tools|Auto Format menu item it will straighten out the indentation in your code which will at least make it look better.

Pete

Hi, How you format your code in the IDE with indents, comments, functions and variable/constant names is basically important for the programmer, so he/she can see the flow of their code.

The compiler that checks the code and converts it to machine code for the controller, doesn't really care about indents and comments and how many declarations you put on a line, it makes its own code form yours.

So when you write code, the text appearance needs to basically make your code easy to read for us humans to read ,the compiler makes it easy for the controller.

CTRL-T (Auto Format) is a wonderful IDE function.

Tom..... :)

Hi, You can check your variables. If your variable is just HIGH or LOW, 1 or 0, TRUE or FALSE you can declare it as a bool

bool Button;

If more than just HIGH or LOW you can declare different variable types. For example "int" has its limits, the link below lists the other variable types.

https://www.arduino.cc/en/Reference/Int

Different variable types use different amounts of memory.

Hope it helps Tom.. :)

Wait... Tom so you're saing that

  if (ButtonValue != 0) {
    digitalWrite(2, LOW);
    delay(30000);
  }
  else if (ButtonValue2 != 0)
  {
    digitalWrite(2, LOW);
    delay(300000);//5 minutes 60 * 5 300 and then * 1000 = 300000
  }
  ButtonValue3 = digitalRead(Button3);
  if (ButtonValue3 != 0) {
    digitalWrite ( 2, LOW);
    while (true);
  }
[code/]
can be replaced by
bool? where do I put it? sorry I'm a rookie, excuse my program and my knowledge

There is no gain in using bool compared to the byte that was suggested in the first reply.

You can change all declarations of your variables (that don't need an int) in the beginning of your code from int to byte. And if so inclined, all xxxVal variables to bool.

// late edit, see italics

class Button {
public:
  const byte pin;
  byte previousValue;
  byte value;

  Button(byte pinAttach) : pin(pinAttach) {}

  void read() {
    previousValue = value;
    value = digitalRead(pin);
  }

  boolean wasPressed() {
    return value == LOW && previousValue == HIGH;
  }
  
};

const int BUTTONS = 3;

Button button[BUTTONS] = {Button(3), Button(8), Button(10)};

Button& blueButton = button[0];
Button& triangleButton = button[1];
Button& stopButton = button[2];

void loop() {
  for(int i = 0; i < BUTTONS ; i++) {
    button[i].read();
  }

  if(triangleButton.wasPressed()) {
    // the operator pressed the triangle button. Do stuff.  
  }
}

Just for the fun of it, see how much this can be reduced. Should have the exact same functionality as the original - dropping the map() function was like half the size reduction. Lots of variables were used only in the line following assignment, and as such unnecessary.

Original:
Sketch uses 2726 bytes (8%) of program storage space. Maximum is 30720 bytes.
Global variables use 200 bytes (9%) of dynamic memory, leaving 1848 bytes for local variables. Maximum is 2048 bytes.

This sketch:
Sketch uses 2286 bytes (7%) of program storage space. Maximum is 30720 bytes.
Global variables use 186 bytes (9%) of dynamic memory, leaving 1862 bytes for local variables. Maximum is 2048 bytes.

int average = 0;
byte button = 3;
byte button2 = 8;
byte button3 = 10;

void setup() {
  Serial.begin(9600);
  pinMode(2, OUTPUT);
  pinMode(button, INPUT_PULLUP);
}

void loop() {

  int total = 0; // Original code: left uninitialised on first run of loop().
  for (uint8_t irnow = 0; irnow < 8; irnow++) { // Take 8 instead of 9 readings for more efficient calculation later.
    total = total + analogRead(A1); // Simplified: no need for variable readings.
  }
  average = total >> 3; // divide by 8 through a simple bitwise shift.
  //  newaverage = map(average, 0, 1023, 500, 200);

  if (average > 375) // Calculated from the map.
  {
    if (average < 392) // Calculated from the map.
    {
      digitalWrite(2, HIGH);
      delay(500);
    }

    if (average >= 443) // Calculated from the map.
    {
      digitalWrite(2, HIGH);
      delay(100);
    }
  }
  else
    digitalWrite(2, LOW);

  if (digitalRead(button)) { // you're not using those variables anywhere else!
    digitalWrite(2, LOW);
    delay(30000);
  }
  else if (digitalRead(button2))
  {
    digitalWrite(2, LOW);
    delay(300000);  //5 minutes 60 * 5 300 and then * 1000 = 300000
  }
  if (digitalRead(button3)) {
    digitalWrite ( 2, LOW);
    while (true); // This stops any further execution - are you sure?
  }
}

Here’s my new program, but how do I apply it? Sorry if this is stupid, but I haven’t completely learned c programming yet, and that looks like c+++++. You must realize I learned it over the course of 4 months, including3 months of school of those 4.

int total;
int readings;
int irnow;
int average;
int newaverage;
int potValue;
int pot = A3;
void setup() {
  Serial.begin(9600);
  pinMode(2, OUTPUT);
  pinMode(pot, INPUT_PULLUP);

}
void loop() {
  irnow = 0;
  while ( irnow < 9)
  {
    readings = analogRead(A1);
    total = total + readings;
    irnow = irnow + 1;
  }
  average = total / 9;
  newaverage = map(average, 0, 1023, 500, 200);
  Serial.println(newaverage);
  int pot = analogRead(A3);
  int mappedpot = map(pot, 0, 1023, 0, 90);


  if (newaverage < 390 - mappedpot)
  {
    if (newaverage > 385 - mappedpot)
    {
      digitalWrite(2, HIGH);
      delay(1000);
    }

    if (newaverage <= 370 - mappedpot)
    {
      digitalWrite(2, HIGH);
      delay(100);
    }
  }
  else  {
    digitalWrite(2, LOW);
    delay(100);
  }
  average = 0;
  total = 0;
  newaverage = 0;
}

int potValue; // seems you want to store the returned A/D value of the pot here int pot = A3; // and here you name the A3 pin "pot"

int pot = analogRead(A3); // ? now you use the pot*pin name* to store the A/D value

You declare it (again)as int locally (not needed), and don't use the potValue variable that you have created before.

pinMode(pot, INPUT_PULLUP); // why this. A pot doesn't need pull up.


You could declare it like this:

int potValue = 0; const byte potPin = A3;

Then in the sketch:

potValue = analogRead(potPin);


Or don't declare the potPin, and do this:

potValue = analogRead(A3);

Leo..

Squidsirymchenry: Here's my new program, but how do I apply it?

Apply what?

It's best to define all pins either as a const byte or as #define macro at the start of your sketch. That makes your code more readable (as you can refer to all your different pins by their descriptive name rather than number), and you can much easier move things to another pin later without having to hunt down every single use of those pins.