Help reducing code size

Using Arduino Programming Questions
#1

I have written a code whose size is 7292 bytes…
as I am new to coding I may have done some things in a long way.
Friends please help and guide me in places where the code can be made shorter to reduce the size, as I am planning to write this code to an Atmega 88…

#include <IRremote.h>
#include <EEPROM.h>
#include "Seg7.h"
int pin1 = 8;
int pin3 = 9;
int RECV_PIN = 11;
int fadered = 25;
int pin1State = LOW;
int pin2State = LOW;
int val;
int state;
int state1;
int btn1_Cnt = LOW;
int btn2_Cnt;
Seg7 s7;
int digit;

IRrecv irrecv(RECV_PIN);
decode_results results;

void setup()
{
   s7.attach(0, 1, 2, 3, 4, 5, 6, 7);
  s7.write(digit);
  irrecv.enableIRIn();
  pinMode(pin1, OUTPUT);
  pinMode(pin3, OUTPUT);
  fadered = EEPROM.read(8);

  /********** THIS IS TO READ WHAT WAS THE STATE OF PIN3 SAVED LAST TIME ***********/

  btn2_Cnt = EEPROM.read(4);

  if(btn2_Cnt % 2 == 0){
    analogWrite(pin3, 0);
}
  else{
    analogWrite(pin3, fadered);
  } 
   
   s7.write(digit);


  /********** THIS IS TO READ WHAT WAS THE STATE OF PIN1 SAVED LAST TIME ***********/

  btn1_Cnt = EEPROM.read(0);

  if(btn1_Cnt % 2 == 0){
    digitalWrite(pin1, LOW);
  }
  else{
    digitalWrite(pin1, HIGH); 
  }
  
      digit = map(fadered,24, 249, 0, 9);
    s7.write(digit);


}
void loop() {
  if (irrecv.decode(&results)) {

    /********** THIS PART IS FOR PIN1 ************/

    if(results.value == 0xffb24d){
      btn1_Cnt++;
      if(btn1_Cnt % 2 == 0)
        digitalWrite(pin1, LOW);
      else
        digitalWrite(pin1, HIGH);

    }

    /********** THIS PART IS FOR PIN3 ************/

    if(results.value == 0xff7887 && fadered <= 230){
      fadered = fadered + 25;
      analogWrite(pin3, fadered);
    }

    if(results.value == 0xff50af && fadered >= 25){
      fadered = fadered - 25;
      analogWrite(pin3, fadered);
    }
    
    
        if(results.value == 0xff38c7){
      btn2_Cnt++;
      if(btn2_Cnt % 2 == 0)
        analogWrite(pin3, 0);
      else
        analogWrite(pin3, fadered); 
    }
    digit = map(fadered,24, 249, 0, 9);
    s7.write(digit);

    /********** THIS IS FOR SAVING THE BRIGHTNESS VALUE IN EEPROM ************/

    val = fadered;
    if(val != EEPROM.read (8)){
      EEPROM.write(8, val);
     
    }

    /********** THIS IS TO SAVE THE LAST STATE OF THE PIN3 ************/

    state = btn2_Cnt;
    if(state != EEPROM.read (4)){
      EEPROM.write(4, state);
   
    }

    /********** THIS IS TO SAVE THE LAST STATE OF THE PIN1 ************/

    state1 = btn1_Cnt;
    if(state1 != EEPROM.read (0)){
      EEPROM.write(0, state1);
  
    }
    
   
    irrecv.resume(); 
  }
}
#2

Can you see how this:

    state = btn2_Cnt;
    if(state != EEPROM.read (4)){
      EEPROM.write(4, state);

looks a lot like this:

    state1 = btn1_Cnt;
    if(state1 != EEPROM.read (0)){
      EEPROM.write(0, state1);

? Quite often that says "time for a function". Won't always make your code smaller, but it'll make it easier to debug.

(it is also worth pointing out that "btn1_Cnt" is an "int", and doesn't fit in an EEPROM byte)

#3

There are a number of small changes you can make - variables like state that aren't necessary, AWOL's function, transforming the if statements that digitalwrite HIGH or LOW to a single digitalwrite that uses the ifs condition directly. These aren't going to get you much though - the libraries you are using are likely the cause of the size of your code.

I see that the 88 has 8K available - how much does the bootloader occupy and so how much do you need to reduce it?

#4

Overall I think it’s pretty good, just a few comments

   val = fadered;
    if(val != EEPROM.read (8)){
      EEPROM.write(8, val);
     
    }

val isn’t used anywhere else, no point assigning fadered to it just for this piece of code.

    if(fadered != EEPROM.read (8)){
      EEPROM.write(8, fadered);
     
    }

Likewise with “state1” and “state”.

This

fadered = fadered + 25;
.....
fadered = fadered - 25;

could be

fadered += 25;
.....
fadered -= 25;

No smaller but slight simpler source code.

int RECV_PIN = 11;

This is a constant, either use “const” or “#define”.

Rob

#5 
int fadered = 25;
....
  fadered = EEPROM.read(8);

Two bytes could be saved by not initialising "fadered" when declared!

Make the pin constants constant.

#6

Hmm… you could also try changing the if/else chain to a switch/case, which may reduce size ( I believe switch/case compiles to a jump table, whereas if/else compiles to a bunch of jump-if-compare or jump-if-not-compare statements ).

Since you’re storing an on/off value for pin1 and pin3, you can also use a single byte for both of them. You also set pin1 and pin3 multiple times without changing the values, which is a waste - just set it once at the end. Moreover, there is repeated code between setup and loop - encapsulate that into a function to save space.

#include <IRremote.h>
#include <EEPROM.h>
#include "Seg7.h"
const int pin1 = 8;
const int pin3 = 9;
const int RECV_PIN = 11;
uint8_t fadered = 25;

Seg7 s7;
int digit;

IRrecv irrecv(RECV_PIN);
decode_results results;

uint8_t stateReg;
const int PIN1_MSK = ( 1 << 0 );
const int PIN3_MSK = ( 1 << 1 );

const int FADE_REG = 8;
const int STATE_REG = 4;

void setup()
{
  s7.attach(0, 1, 2, 3, 4, 5, 6, 7);
  s7.write(digit);
  irrecv.enableIRIn();
  pinMode(pin1, OUTPUT);
  pinMode(pin3, OUTPUT);
  
  fadered = EEPROM.read( FADE_REG );
  stateReg = EEPROM.read( STATE_REG );
  
  setStates();

  digit = map(fadered, 24, 249, 0, 9);
  s7.write(digit);
}

void setStates() {
	// set pin3
	if ( stateReg & PIN3_MSK ) {
		analogWrite( pin3, 0 );
	} else {
		analogWrite( pin3, fadered );
	}
	// set pin1
	digitalWrite( pin1, ( stateReg & PIN1_MSK ) ); 
}

void loop() {
  if (irrecv.decode(&results)) {

	switch ( results.value ) {
	case 0xffb24d: // pin1
		stateReg ^= PIN1_MSK;
		break;
	case 0xff38c7: // pin3
		stateReg ^= PIN3_MSK;
		break;
	case 0xff7887:
		if ( fadered <= 230 ) {
			fadered += 25;
		}
		break;
	case 0xff50af:
		if ( fadered >= 25 ) {
			fadered -= 25;
		}
		break;
	}
	
    digit = map(fadered, 24, 249, 0, 9);
    s7.write(digit);
	
	setStates(); // set the state of pin1 and pin3

    if ( fadered != EEPROM.read ( FADE_REG ) ) {
      EEPROM.write( FADE_REG, fadered );
    }
	if ( stateReg != EEPROM.read( STATE_REG ) ) {
	  EEPROM.write( STATE_REG, stateReg );
	}
    
    irrecv.resume(); 
  }
}

Note: I have no clue if this code compiles, since I don’t have Seg7.h. But it should work, and the concepts are all thee.

#7

Aeturnalus:
Hmm… you could also try changing the if/else chain to a switch/case, which may reduce size ( I believe switch/case compiles to a jump table, whereas if/else compiles to a bunch of jump-if-compare or jump-if-not-compare statements ).

Since you’re storing an on/off value for pin1 and pin3, you can also use a single byte for both of them. You also set pin1 and pin3 multiple times without changing the values, which is a waste - just set it once at the end. Moreover, there is repeated code between setup and loop - encapsulate that into a function to save space.

#include <IRremote.h>

#include <EEPROM.h>
#include “Seg7.h”
const int pin1 = 8;
const int pin3 = 9;
const int RECV_PIN = 11;
uint8_t fadered = 25;

Seg7 s7;
int digit;

IRrecv irrecv(RECV_PIN);
decode_results results;

uint8_t stateReg;
const int PIN1_MSK = ( 1 << 0 );
const int PIN3_MSK = ( 1 << 1 );

const int FADE_REG = 8;
const int STATE_REG = 4;

void setup()
{
  s7.attach(0, 1, 2, 3, 4, 5, 6, 7);
  s7.write(digit);
  irrecv.enableIRIn();
  pinMode(pin1, OUTPUT);
  pinMode(pin3, OUTPUT);
 
  fadered = EEPROM.read( FADE_REG );
  stateReg = EEPROM.read( STATE_REG );
 
  setStates();

digit = map(fadered, 24, 249, 0, 9);
  s7.write(digit);
}

void setStates() {
// set pin3
if ( stateReg & PIN3_MSK ) {
analogWrite( pin3, 0 );
} else {
analogWrite( pin3, fadered );
}
// set pin1
digitalWrite( pin1, ( stateReg & PIN1_MSK ) );
}

void loop() {
  if (irrecv.decode(&results)) {

switch ( results.value ) {
case 0xffb24d: // pin1
	stateReg ^= PIN1_MSK;
	break;
case 0xff38c7: // pin3
	stateReg ^= PIN3_MSK;
	break;
case 0xff7887:
	if ( fadered <= 230 ) {
		fadered += 25;
	}
	break;
case 0xff50af:
	if ( fadered >= 25 ) {
		fadered -= 25;
	}
	break;
}

digit = map(fadered, 24, 249, 0, 9);
    s7.write(digit);

setStates(); // set the state of pin1 and pin3

if ( fadered != EEPROM.read ( FADE_REG ) ) {
      EEPROM.write( FADE_REG, fadered );
    }
if ( stateReg != EEPROM.read( STATE_REG ) ) {
  EEPROM.write( STATE_REG, stateReg );
}
   
    irrecv.resume();
  }
}




Note: I have no clue if this code compiles, since I don't have Seg7.h. But it should work, and the concepts are all thee.

Thank you Aeturnalus… :slight_smile:

Your code just worked fine… :slight_smile:
it was really written in a clean way and in short…

I am actually learning a lot for ur code pattern…
I am still going through the code and trying to understand how it is actually working…:slight_smile:

#8