Optimize code

RobNorthen · July 11, 2018, 6:10pm

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;
}

RobNorthen · July 11, 2018, 6:16pm

////////////////////////////////////////////////////////////////////////////////////////
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;
  }
}

Paul_KD7HB · July 12, 2018, 1:45am

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

Paul

DKWatson · July 12, 2018, 2:00am

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.

johnwasser · July 12, 2018, 3:09am

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;

RobNorthen · July 12, 2018, 3:10pm

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

RobNorthen · July 12, 2018, 3:28pm

After a little RTFM, I found:

#define sounder PC5

PORTD |= (1<<sounder);

Makes code a little more easy to understand

FredScuttle · July 12, 2018, 6:41pm

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 ) to code.

DKWatson · July 12, 2018, 6:55pm

Have you looked at the macro?

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

Up to you what you want to call it.

FredScuttle · July 12, 2018, 7:05pm

@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?

system · May 5, 2021, 11:16pm