Optimize code

Hello.

I have tried optimizing the following code, but there is must be more. There is a lot of (almost) repeated code for setting hors, mins and secs, but I can not figure out how to cut it down.

Code is shown in two messages, due to length.
Thanks

//byte sounder = 5;
//byte button_Op = A0;
//byte button_Ned = A1;
//byte button_Select = A2;
byte second, minute, hour;
byte menuCounter = 0;
byte buttonState = 0;
byte lastButtonState = 0;
byte buttonState3 = 0;
byte buttonState4 = 0;
byte lastButtonState3 = 0;
byte lastButtonState4 = 0;
byte check_Burn_In_Var = 0;
byte function = 1; // funktions nr vi er nået til, i funktions menu
byte dim_Start_Hours = 0; // start timer for dimmimg af rør, lav lys
byte dim_Stop_Hours = 0; // stop timer for dimminmg af rør, går over til høj lys
byte color_Run = 0; // bruges i funktions menu, for at sikre at farve skift kode kun køres een gang
byte dim_State = 0; // er rør sat til hi eller low lys
byte cfg_Dim_Start_Hours = 0;
byte cfg_Dim_Stop_Hours = 1;
byte cfg_Dim_State = 2;
byte delay_For_Number_Run = 0;
unsigned long check_Burn_In_PreviousMillis = 0;
const long check_Burn_In_Interval = 900000; // kør hvert 15. minut
byte ej[10] = { 0, 1, 2, 3, 4, 6, 7, 8, A3, 13 };
/////////////////////////////////////////////////////////////////////////////
#include <EEPROM.h>
#include "Wire.h"
#include "VFDTube.h"
#define TUBE_COUNT 6
VFDTube tube(12, 11, 10, 9, TUBE_COUNT);
// DIN on pin #12, OE on pin #11, STCP on pin #10, SHCP on pin #9,
// 6 for sections count in serial
// Pin #11 is with PWM output, so the brightness is adjustable

// Convert normal decimal numbers to binary coded decimal
byte decToBcd(byte val)
{
  return ( (val / 10 * 16) + (val % 10) );
}
// Convert binary coded decimal to normal decimal numbers
byte bcdToDec(byte val)
{
  return ( (val / 16 * 10) + (val % 16) );
}
/////////////////////////////////////////////////////////////////////
void set_Time()
{
  bip();
  Wire.beginTransmission(0x68);
  Wire.write (0x00);
  Wire.write(decToBcd(second) & 0x7f);    // 0 to bit 7 starts the clock
  Wire.write(decToBcd(minute));
  Wire.write(decToBcd(hour));
  Wire.endTransmission();
}
void get_Time()
{
  // Reset the register pointer
  Wire.beginTransmission(0x68);
  Wire.write(0x00);
  Wire.endTransmission();
  Wire.requestFrom(0x68, 7);

  // A few of these need masks because certain bits are control bits
  second     = bcdToDec(Wire.read() & 0x7f);
  minute     = bcdToDec(Wire.read());
  hour       = bcdToDec(Wire.read() & 0x3f);  // Need to change this if 12 hour am/pm
}
///////////////////////////////////////////////////////////////////////////////////
void bip()
{
  PORTD |= (1 << PD5); //sounder pÃ¥ port PD5, digital pin 5 high
  _delay_ms(100);
  PORTD = PORTD & (~(1 << 5)); // low
}
//////////////////////////////////////////////////////////////////////////////////
void set_Hour()
{
  tube.setBackgroundColor (0, Blue); // tænd lys i timer rør
  tube.setBackgroundColor (1, Blue);
  buttonState3 = PINC & (1 << PC0); // port PC0, arduino digital A0            //digitalRead(button_Op);
  if (buttonState3 != lastButtonState3) {
    if (buttonState3 == LOW) {
      hour++;
      if (hour >= 24) {
        hour = 0;
      }
      set_Time();
    }
  }
  buttonState4 = PINC & (1 << PC1); // port PC1, arduino digital A1. button ned
  if (buttonState4 != lastButtonState4) {
    if (buttonState4 == LOW) {
      hour--;
      if (hour == 255) {
        hour = 23;
      }
      set_Time();
    }
  }
  lastButtonState4 = buttonState4;
  lastButtonState3 = buttonState3;
}
///////////////////////////////////////////////////////////////////////////////////
void set_Minute()
{
  tube.setBackgroundColor (0, Black); // sluk lys i timer
  tube.setBackgroundColor (1, Black);
  tube.setBackgroundColor (2, Blue); // tænd lys i minutter rør
  tube.setBackgroundColor (3, Blue);
  buttonState3 = PINC & (1 << PC0); // port PC0, arduino digital A0. button op
  if (buttonState3 != lastButtonState3) {
    if (buttonState3 == LOW) {
      minute++;
      if (minute >= 60) {
        minute = 0;
      }
      set_Time();
    }
  }
  buttonState4 = PINC & (1 << PC1); // port PC1, arduino digital A1. button ned
  if (buttonState4 != lastButtonState4) {
    if (buttonState4 == LOW) {
      minute--;
      if (minute == 255) {
        minute = 59;
      }
      set_Time();
    }
  }
  lastButtonState4 = buttonState4;
  lastButtonState3 = buttonState3;
}
void set_Second()
{
  tube.setBackgroundColor (Black); // sluk lys i rør
  tube.setBackgroundColor (4, Blue); // tænd lys i sekunder rør
  tube.setBackgroundColor (5, Blue);
  buttonState3 = PINC & (1 << PC0); // port PC0, arduino digital A0. button op
  if (buttonState3 != lastButtonState3) {
    if (buttonState3 == LOW) {
      second++;
      if (second >= 60) {
        second = 0;
      }
      set_Time();
    }
  }
  buttonState4 = PINC & (1 << PC1); // port PC1, arduino digital A1. button ned
  if (buttonState4 != lastButtonState4) {
    if (buttonState4 == LOW) {
      second--;
      if (second == 255) {
        second = 59;
      }
      set_Time();
    }
  }
  lastButtonState4 = buttonState4;
  lastButtonState3 = buttonState3;
}
///////////////////////////////////////////////////////////
void set_Start_Hours() {
  dim_Start_Hours = EEPROM.read(cfg_Dim_Start_Hours);
  dim_Stop_Hours = EEPROM.read(cfg_Dim_Stop_Hours);
  function = 1;
  tube.printf("FN%01d. %02d", function, dim_Start_Hours);
  tube.display();
  if (color_Run == 0) {
    color_Run = 1; // set color run til 1, sÃ¥ nedenstÃ¥ende kun køres een gang
    for (byte tube_Number = 0; tube_Number < 6; tube_Number++) { // disse linier laver løbe LED lys i rør
      tube.setBackgroundColor (tube_Number, Cyan);
      tube.display();
      _delay_ms(100);
    }
  }// color run end bracket
  buttonState3 = PINC & (1 << PC0); // port PC0, arduino digital A0
  if (buttonState3 != lastButtonState3) {
    if (buttonState3 == LOW) {
      dim_Start_Hours++;
      bip();
      if (dim_Start_Hours >= 24) {
        dim_Start_Hours = 0;
      }
    }
  }
  buttonState4 = PINC & (1 << PC1); // port PC1, arduino digital A1
  if (buttonState4 != lastButtonState4) {
    if (buttonState4 == LOW) {
      dim_Start_Hours--;
      bip();
      if (dim_Start_Hours == 255) {
        dim_Start_Hours = 23;
      }
    }
  }
  lastButtonState4 = buttonState4;
  lastButtonState3 = buttonState3;
  EEPROM.update(cfg_Dim_Start_Hours, dim_Start_Hours);
}
///////////////////////////////////////////////////////////
void set_Stop_Hours()
{
  function = 2;
  tube.printf("FN%01d. %02d", function, dim_Stop_Hours);
  tube.display();
  if (color_Run == 1) {
    color_Run = 2; // set color run til 2, sÃ¥ nedenstÃ¥ende kun køres een gang
    for (byte tube_Number = 0; tube_Number < 6; tube_Number++) {
      tube.setBackgroundColor (tube_Number, Green);
      tube.display();
      _delay_ms(100);
    }
  }// color run end bracket
  buttonState3 = PINC & (1 << PC0); // port PC0, arduino digital A0
  if (buttonState3 != lastButtonState3) {
    if (buttonState3 == LOW) {
      dim_Stop_Hours++;
      bip();
      if (dim_Stop_Hours >= 24) {
        dim_Stop_Hours = 0;
      }
    }
  }
  buttonState4 = PINC & (1 << PC1); // port PC1, arduino digital A1
  if (buttonState4 != lastButtonState4) {
    if (buttonState4 == LOW) {
      dim_Stop_Hours--;
      bip();
      if (dim_Stop_Hours == 255) {
        dim_Stop_Hours = 23;
      }
    }
  }
  lastButtonState4 = buttonState4;
  lastButtonState3 = buttonState3;
  EEPROM.update(cfg_Dim_Stop_Hours, dim_Stop_Hours);
}
////////////////////////////////////////////////////////////////////////////////////////
void set_Dimming() {
  function = 3;
  if (dim_State == 0) {
    tube.printf("FN%01d. HI", function);
  }
  if (dim_State == 1) {
    tube.printf("FN%01d. LO", function);
  }
  if (color_Run == 2) {
    color_Run = 3; // set color run til 2, sÃ¥ nedenstÃ¥ende kun køres een gang
    for (byte tube_Number = 0; tube_Number < 6; tube_Number++) {
      tube.setBackgroundColor (tube_Number, Yellow);
      tube.display();
      _delay_ms(100);
    }
  }// color run end bracket
  buttonState3 = PINC & (1 << PC0); // port PC0, arduino digital A0
  if (buttonState3 != lastButtonState3) {
    if (buttonState3 == LOW) {
      dim_State = 0;
      bip();
    }
  }
  buttonState4 = PINC & (1 << PC1); // port PC1, arduino digital A1
  if (buttonState4 != lastButtonState4) {
    if (buttonState4 == LOW) {
      dim_State = 1;
      bip();
    }
  }
  lastButtonState4 = buttonState4;
  lastButtonState3 = buttonState3;
}
////////////////////////////////////////////////////////////////////////////////////////
void set_Dimming_Mode() { // her kontrolleres hvad dimming flag er sat til og tilpasser dimming on/off efter det
  if (dim_State == 1) {
    tube.setBrightness(0x06);
  }
  if (dim_State == 0) {
    tube.setBrightness(0xff);
  }
  tube.display(); // denne SKAL være her, for at display virker.
  EEPROM.update(cfg_Dim_State, dim_State);
}
///////////////////////////////////////////////////////////////////////////////////////
void check_Burn_In() // sætter alle segmenter i alle display on/off hver xx minut, for at afhjælpe burn-in
{
  unsigned long check_Burn_In_CurrentMillis = millis();
  if (check_Burn_In_CurrentMillis - check_Burn_In_PreviousMillis >= check_Burn_In_Interval) {
    check_Burn_In_PreviousMillis = check_Burn_In_CurrentMillis;

    while (check_Burn_In_Var < 80) { //kør nedenstÃ¥ende kode 70 gange
      hour = hour + 12; // lææger 12,13,14 til timer,minutter og sekunder, sÃ¥ alle segmenter bliver aktiveret
      if (hour >= 100) {
        hour = 0;
      }
      minute = minute + 13;
      if (minute >= 100) {
        minute = 0;
      }
      second = second + 14;
      if (second >= 100) {
        second = 0;
      }
      delay (delay_For_Number_Run);// delay for at display løber langsommere og langsommere. kan ikke bruge rigtig c delay, da det kun virker med en constant tids angivelse
      tube.printf("%02d%02d%02d", hour, minute, second); // vis løbende tal og sluk "." (dot) i alle rør, mens tal løber
      tube.display();
      delay_For_Number_Run =  delay_For_Number_Run + 2; // øg med 2 for hvert gennemløb og delay bliver større og større
      check_Burn_In_Var++;
    }
  }
  check_Burn_In_Var = 0;
  delay_For_Number_Run = 0;
}
////////////////////////////////////////////////////////////////////////////////////////
void check_Time_For_Dim ()
{
  if (hour == dim_Start_Hours && minute == 0 && second < 5) {
    dim_State = 1;
  }
  if (hour == dim_Stop_Hours && minute == 0 && second < 5) {
    dim_State = 0;
  }
}
////////////////////////////////////////////////////////////////////////////////////////
void setup()
{
  DDRD = DDRD | B00100000; //set port pd5 til output, hvor sounder sidder. er arduino digital pin 5
  TCCR2B = TCCR2B & B11111011 | B00000001;// set PWM frekvens til . Dette for at undgÃ¥ at spoler pÃ¥ display moduler hviner/hyler i hørbart omrÃ¥de,
  for (byte i = 0; i < 11; i++) digitalWrite(ej[i], HIGH);
  dim_Start_Hours = EEPROM.read(cfg_Dim_Start_Hours); // læs konfiguration ved opstart
  dim_Stop_Hours = EEPROM.read(cfg_Dim_Stop_Hours);
  dim_State = EEPROM.read(cfg_Dim_State);
  if (dim_State >= 2) { // sikrer vi altid ligger inde for korrekt grænse, ogsÃ¥ ved eeprom fejl
    dim_State = 1;
  }
  set_Dimming_Mode();// kør sub rutine for at stille dimming flag, efter det er udlæst af eeprom
  tube.setBackgroundColor (Black);
  tube.display();
  _delay_ms(500); // slow start med at vise segmenter og ingen back light, for at mindste start strøm
  Wire.begin();
}
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
void loop()
{
  set_Dimming_Mode();
  check_Time_For_Dim ();
  check_Burn_In(); // kør rutine for at aktivere alle segenter i alle display, hvert xx minut, for at mindske burn-in
  if (menuCounter < 4) { // viser kun tid i normal mode, samt ved tidsindstilling. Viser ikke tid, i funktions menuer.
    get_Time(); //henter tid fra rtc
    tube.printf("%02d.%02d.%02d.", hour, minute, second);
  }
  buttonState = PINC & (1 << PC2); // port PC2, arduino digital A2. læs select button
  if (buttonState != lastButtonState) {
    if (buttonState == LOW) {
      menuCounter++;
      bip();
    }
  }
  lastButtonState = buttonState; // her vælges menu nr og kontrol hvor vi er i menu
  if (menuCounter == 24) {
    menuCounter = 0;
  }
  switch (menuCounter) {
    case 0:
      {
        tube.setBackgroundColor (Black);
      }
      break;
    case 1:
      {
        set_Hour();
      }
      break;
    case 2:
      {
        set_Minute();
      }
      break;
    case 3:
      {
        set_Second();
      }
      break;
    case 4:
      {
        menuCounter = 21;
        color_Run = 0; // nulstil color rum tæller, tæller op i de næste cases
      }
      break;

    case 21:
      {
        set_Start_Hours();
      }
      break;
    case 22:
      {
        set_Stop_Hours();
      }
      break;

    case 23:
      {
        set_Dimming();
      }
      break;
  }
}

What do you think optimize means? What does it mean to you? Are you optimizing speed? Memory usage? Source code size? What?

Paul

Your code reads fine. What do you think you need to optimize and why? Don't worry about how many lines it is in an editor, look at the compiled size.

What you've written is what we call simple code, an art lost to many today as neo-programmers try to obfuscate as a demonstration their in-depth knowledge of the language. You could roll up some of your if statements but the compiler may roll them back down again if it makes for more efficient code.

You should however, move your include directives to the top of you listing, avoid potential naming conflicts with variables you've already defined.

I would take out the references to PIN, PORT, and DDR registers and just use Arduino pin numbers with digitalRead() and digitalWrite(). I would assign names to pin numbers.

If there are two functions that look very similar it is usually possible to figure out in what way they are different and use function parameters to allow one function to work in both places. For example, set_Hour(), set_Minute(), and set_Second() use the same two buttons to increment or decrement a value.

int adjustValue(int currentValue, int backgroundIndex)
{
  tube.setBackgroundColor (0, backgroundIndex==0?Blue:Black);
  tube.setBackgroundColor (1, backgroundIndex==0?Blue:Black);

  tube.setBackgroundColor (2, backgroundIndex==1?Blue:Black);
  tube.setBackgroundColor (3, backgroundIndex==1?Blue:Black);
  tube.setBackgroundColor (4, backgroundIndex==2?Blue:Black);
  tube.setBackgroundColor (5, backgroundIndex==2?Blue:Black);

  buttonState3 = digitalRead(A0); // arduino digital A0. button op
  if (buttonState3 != lastButtonState3)
  {
    lastButtonState3 = buttonState3;

    if (buttonState3 == LOW)
      currentValue++;
  }


  buttonState4 = digitalRead(A1) // arduino digital A1. button ned
  if (buttonState4 != lastButtonState4)
  {
    lastButtonState4 = buttonState4;
    if (buttonState4 == LOW)
      currentValue--;
  }


  return currentValue;
}




    case 1:
      hour = adjustValue(hour, 0) % 24;
      set_Time();
      break;
      
    case 2:
      minute = adjustValue(minute, 1) % 60;
      set_Time();
      break;
      
    case 3:
      second = adjustValue(second, 2) % 60;
      set_Time();
      break;

Thank you for all the answers!

Paul; Code size. Just by replacing some of the Arduino “Delay”, “digital.Read/Write” and a few other things, compiled code was cut down by over 7 kb.

DKWatson; Thanks, thought my code was hopeless, but sounds not as bad as I thought. And I have moved the includes up to the top.

johnwasser; Good idea with the function parameters, will try that.

Is it possible to assign names the AVR PIN numbers ? EX: PORTD |= (1<<PD5). Replace “PD5” with “sounder” ? Tried, but compiler nags…

Best regards
Christian

After a little RTFM, I found:

#define sounder PC5

PORTD |= (1<<sounder);

Makes code a little more easy to understand

Have you checked out the "bit()" macros?

bitSet(PORTD,sounder);

https://www.arduino.cc/reference/en/language/functions/bits-and-bytes/bitset/
Just about as fast but easier (for me :slight_smile: ) to code.

Have you looked at the macro?

#define bitSet(value, bit) ((value) |= (1UL << (bit)))

Up to you what you want to call it.

@DKWatson:
Do you mean this, from Arduino.h?

#define bitRead(value, bit) (((value) >> (bit)) & 0x01)
#define bitSet(value, bit) ((value) |= (1UL << (bit)))
#define bitClear(value, bit) ((value) &= ~(1UL << (bit)))
#define bitWrite(value, bit, bitvalue) (bitvalue ? bitSet(value, bit) : bitClear(value, bit))

What’s your point?