Code optimization

This is my first attempt at controlling an LED matrix. I’m using a 5x7 matrix with a SN74LS164N shift register. Code to scroll "HELLO WORLD!!! " is below.

I tried running dispDraw() on interrupts with code shamefacedly copied and pasted, but only the first row would draw. Could this have been because a new interrupt was “triggered” before a draw was finished? (I’m on a 328, BTW)

Mostly, I’d deeply appreciate any general feedback on glaring inefficiencies, things done properly, etc…, because I’m sort of shooting in the dark :slight_smile: Also, if I’ve broken any etiquettes by the manner in which I’ve posted, or by my request, please do help me lose my ignorance.

Thanks very much in advance!

Datasheets:
(http://www.datasheetcatalog.org/datasheets2/15/157524_1.pdf)
(http://www.datasheetcatalog.org/datasheet2/8/0upt0oxpqizjodhogqshk59g1ucy.pdf)

int row[5] = {        //PINS USED FOR EACH ROW
  2, 3, 4, 5, 6};
int dataPin = 7;
int clockPin = 8;
int blankPin = 9;  //UNUSED

int letter[][15] = { //3X5 FONT, DEFINED FROM UPPER LEFT CORNER, MOVING ROW BY ROW 
  {
    0,1,0,1,0,1,1,1,1,1,0,1,1,0,1       } //A
  ,{
    1,1,0,1,0,1,1,1,1,1,0,1,1,1,0       } //B
  ,{
    0,1,0,1,0,1,1,0,0,1,0,1,0,1,0       } //C
  ,{
    1,1,0,1,0,1,1,0,1,1,0,1,1,1,0       } //D
  ,{
    1,1,1,1,0,0,1,1,0,1,0,0,1,1,1       } //E
  ,{
    1,1,1,1,0,0,1,1,0,1,0,0,1,0,0       } //F
  ,{
    0,1,1,1,0,0,1,0,1,1,0,1,0,1,1       } //G
  ,{
    1,0,1,1,0,1,1,1,1,1,0,1,1,0,1       } //H
  ,{
    1,1,1,0,1,0,0,1,0,0,1,0,1,1,1       } //I
  ,{
    0,1,1,0,0,1,0,0,1,1,0,1,0,1,0       } //J
  ,{
    1,0,1,1,0,1,1,1,0,1,0,1,1,0,1       } //K
  ,{
    1,0,0,1,0,0,1,0,0,1,0,0,1,1,1       } //L
  ,{
    1,0,1,1,1,1,1,1,1,1,0,1,1,0,1       } //M
  ,{
    1,1,1,1,0,1,1,0,1,1,0,1,1,0,1       } //N
  ,{
    0,1,0,1,0,1,1,0,1,1,0,1,0,1,0       } //O
  ,{
    1,1,0,1,0,1,1,1,0,1,0,0,1,0,0       } //P
  ,{
    0,1,0,1,0,1,1,0,1,0,1,1,0,0,1       } //Q
  ,{
    1,1,0,1,0,1,1,1,0,1,0,1,1,0,1       } //R
  ,{
    0,1,1,1,0,0,0,1,0,0,0,1,1,1,0       } //S
  ,{
    1,1,1,0,1,0,0,1,0,0,1,0,0,1,0       } //T
  ,{
    1,0,1,1,0,1,1,0,1,1,0,1,1,1,1       } //U
  ,{
    1,0,1,1,0,1,1,0,1,1,0,1,0,1,0       } //V
  ,{
    1,0,1,1,0,1,1,1,1,1,1,1,1,0,1       } //W
  ,{
    1,0,1,1,0,1,0,1,0,1,0,1,1,0,1       } //X
  ,{
    1,0,1,1,0,1,0,1,0,0,1,0,0,1,0        } //Y
  ,{
    1,1,1,0,0,1,0,1,0,1,0,0,1,1,1        } //Z
  ,{
    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0        } //SPACE
  ,{
    0,0,0,0,0,0,0,0,0,0,0,0,1,0,0      }  //PERIOD
  ,{
    1,0,0,1,0,0,1,0,0,0,0,0,1,0,0      }  //EXCLAMATION POINT

};

int message[] = {
  7,4,11,11,14,26,22,14,17,11,3,28,28,28,28,26};  //"HELLO WORLD!!! "
int messageLength = sizeof(message)/sizeof(int);
int messageIndex = 0;  //TO TRACK WHICH LETTER IS CURRENTLY BEING DRAWN
int scrollSpeed = 10;
int chunkTick = 0;     //WHICH OF THREE COLUMNS OF CURRENT LETTER (OR A SPACING COLUMN) IS BEING DRAWN
int disp[5][7] = {     //ARRAY STORES CURRENT DISPLAY, INITIALIZED WITH HATCH TEST PATTERN
  {
    1,0,1,0,1,0,1                                      }
  ,{
    0,1,0,1,0,1,0                                      }
  ,{
    1,0,1,0,1,0,1                                      }
  ,{
    0,1,0,1,0,1,0                                      }
  ,{
    1,0,1,0,1,0,1                                      }
};

void setup() {
  for (int i = 0; i < 5; i++) {   
    pinMode(row[i], OUTPUT);      
    digitalWrite(row[i], HIGH);   
  }
  pinMode(dataPin, OUTPUT);
  digitalWrite(dataPin, LOW);
  pinMode(clockPin, OUTPUT);
  digitalWrite(clockPin, LOW);
  pinMode(blankPin, OUTPUT);
  digitalWrite(blankPin, HIGH);
}

void loop() {
  for (int i = 0; i < scrollSpeed; i++) { //THIS IS IN LIEU OF GETTING INTERRUPTS TO WORK. LOOKS FINE ON A SINGE 5X7 MATRIX
    drawDisp();
  }
  switch (chunkTick) { //CHECKS TO SEE WHICH COLUMN OF LETTER NEEDS TO BE ADDED TO DISPLAY
  case 0: 
    {
      shiftDisp(); //MOVES DATA IN ARRAY "LEFT" ONE COLUMN
      for (int i = 0; i < 5; i++) {  //chunkTick IS 1, THEREFORE WRITE THE FIRST COLUMN OF A NEW LETTER TO THE FAR-RIGHT COLUMN OF DISP
        disp[i][6] = letter[message[messageIndex]][3*i];
      }
      chunkTick++;
      break;
    }
  case 1: 
    {
      shiftDisp();
      for (int i = 0; i < 5; i++) {    //WRITE SECOND COLUMN OF A LETTER
        disp[i][6] = letter[message[messageIndex]][3*i + 1];
      }
      chunkTick++;
      break;
    }
  case 2: 
    {
      shiftDisp();
      for (int i = 0; i < 5; i++) {    //THIRD COLUMN
        disp[i][6] = letter[message[messageIndex]][3*i + 2];
      }
      chunkTick++;
      messageIndex++;    //INDICATE LETTER IS COMPLETE, MOVE TO NEXT
      if (messageIndex >= messageLength) {    //RESTART MESSAGE IF COMPLETE
        messageIndex = 0;
      }
      break;
    }
  case 3: 
    {
      shiftDisp();
      for (int i = 0; i < 5; i++) {  //BLANK COLOUMN TO SPACE LETTERS
        disp[i][6] = 0;
      }
      chunkTick = 0;
      break;
    }
  }
}



void shiftDisp() {    //I KNOW THERE'S GOT TO BE A BETTER WAY TO DO THIS, BUT IF I'M ONLY DOING IT A FEW TIMES A SECOND...?
  for (int i = 0; i < 5; i++) {
    for (int j = 0; j < 6; j++) {
      disp[i][j] = disp[i][j+1];
    }
  }
}



void drawDisp() {    //"OPTIMIZED" FOR WHEN I WAS TRYING TO RUN ON A TIMER INTERRUPT
  for (int i = 0; i < 5; i++) {
    PORTD ^= (1 << row[i]);              //"SELECT" COLUMN BY BRINGING PIN LOW
    PORTD |= (1 << dataPin);             //WRITE ARBITRARY MSB, B/C MATRIX IS ONLY 7-WIDE
    pulseClock();
    for (int j = 0; j < 7; j++) {
      PORTD &= ~(1 << dataPin);          //COLUMN SELECT
      PORTD |= (disp[i][j] << dataPin);  //READ RESPECTIVE BIT FROM DISP[][] ARRAY
      pulseClock();                      //WRITE TO SHIFT REGISTER
    }
    delay(1);
    PORTD ^= (1 << row[i]);              //BRING ROW HIGH TO TURN OFF
  }
}



pulseClock() {
  PORTB |= (1 << (clockPin-8));
  PORTB &= ~(1 << (clockPin-8));
}

Glaring inefficiency #1:

int letter[][15] = { //3X5 FONT, DEFINED FROM UPPER LEFT CORNER, MOVING ROW BY ROW
  {
    0,1,0,1,0,1,1,1,1,1,0,1,1,0,1

Glaring inefficiency #2:

int message[] = {
  7,4,11,11,14,26,22,14,17,11,3,28,28,28,28,26};  //"HELLO

Glaring inefficiency #3:

int disp[5][7] = {     //ARRAY STORES CURRENT DISPLAY, INITIALIZED WITH HATCH TEST PATTERN
  {
    1,0,1,0,1,0,1

Ah. Is that because they're arrays..? Uhh.. pointers. Segfault? Buzzword buzzword buzzword!

I'm sorry, I appreciate the help, but that's slightly too vague for me to know what you mean :-/

I tried running dispDraw() on interrupts

Try to do it without. Interrupts add overhead, are difficult to get working correctly, and add no value to your application.

I see a "3*" in the code. Try to avoid multiplication.

Is that because they're arrays..?

Make them byte arrays. Try to avoid double dimensioned arrays (hidden multiplication).

I suspect AWOL had some other things in mind as well.

delay(1);

Try using delayMicroseconds with values smaller than 1000.

Is that because they're arrays..?

No, it's because they're arrays of 16 bit entities, holding values that could be held in a single bit, or at worst, eight bits.

Fantastic. Thanks, chaps.

No, it's because they're arrays of 16 bit entities, holding values that could be held in a single bit, or at worst, eight bits.

Bitmath for the win!! (interestingly enough, the arduino page is #1 for a google search for "bitmath" way to go guys!)

http://www.arduino.cc/playground/Code/BitMath

On-topic question: Is the reason behind using bitmath versus multi dimension arrays more for space issues, or is it faster?

Is the reason behind using bitmath versus multi dimension arrays more for space issues, or is it faster

Depends on the architecture. Usually microcontrollers are optimised for bit operations, because they're so memory-limited.

By all means, use "int"s to get it working, but when you start adding features and find you're running short of memory or too slowly, then, and only then, is it time to optimise.

Please read http://arduino.cc/en/Reference/PROGMEM for how to store your font and strings in the 32kb flash memory rather than using up the precious 2kb SRAM with static data.

I see a "3*" in the code. Try to avoid multiplication.

That multiplication is nothing to worry about. The compiler's strength-reduce optimization will most likely make it into an addition, and the 328 has a multiplication instruction that runs in 2 clock cycles anyway.