I need help speeding up my led matrix functions

You are thinking of only one iteration of my shiftText() function.

It has 24 iterations, so before it was 89 calls (89 x 24 = 2136). (I previously made an error!)

And now its 20 calls (20 x 24 = 480). ( I also previously made an error!)

So the diffrence is (2136 - 480 = 1656) 1656 calls less per 24 iterations of my shiftText() function.

So it is (1656 / 2136 x 100 = 77.53) 77.53% saving. You were right!

(Btw, you forgot to add the two more for the 87 one ((87 + 2) x 24 = 2136)

Should I convert size to int16_t?

Give it a try.

Yep, it worked!

So next I'll try the timer and direct port manipulation! (probably not today)

Well, I'm not sure direct port manipulation is needed now we have that 77% saving. As for the timer, yes, something is needed to control the update rate of the display, so that the Arduino has time left to do other things, and those things don't cause flickering. That could be done with a hardware timer, but maybe it could also be done with micros(), which is more portable.

If you can avoid using hardware timers and direct port manipulation, then your code will be easy to adapt to other types of Arduino in the future.

I am already using micros() in my code. But if I add a long delay in my code, it will still have to pass the delay before displaying the information. So that's why I need to use a hardware timer, so the display could be shifted periodically.

But for the direct port manipulation, I would still like to use it, because I am using the digitalWrite() function 480 times in one information display. Shorten the time for one digitalWrite(), multiply it by 480 and it saves quite sometime. (1.75 us - 0.06 us * 480 = 811.2 us). I would save 811.2 us (it's a lot!)

Oh, please post the updated code, we have not seen that version.

So you want to use a hardware timer because you plan to add a long delay to your code? Why would you add a long delay? You have spent a lot of time trying to speed your code up, so why would you do that now? If you don't add the delay, you won't need the hardware timer.

My bad, i'm using millis() not micros(). I will implement it later.

The long delay would be if ever I want to implement some code that waits for, let's say, the serial monitor to get input. In that case it will be a long delay, and I need to display the information in the mean time.

Then don't! There's no need to wait, you can check if there are any characters waiting to be read, and, if not, continue updating the display. So, again, no need for hardware timers.

I'm sure it'll come in handy. And besides, I wont need to worry about delays anymore, so it will be easier to modify. I will do it, just for the sake of flexibility.

[EDIT]: I just researched how to do direct port manipulation, and it seems like it would just complicate my life, so I won't do it.

Hey guys! If you want to make your LEDs brighter, the key is to drive them harder! :bulb:If you're planning on using LEDs, make sure to check out the data sheet to figure out the power supply requirements.

I don't see the purpose of this reply, it was already stated. And anyways, that's not the problem.

Even if I wanted to, I could only brighten it a little bit since the registers can only supply a max of 35 mA, and one row draws 20 mA. Also, the project is already soldered up together with the 1 KΩ resistors.

But the display is bright enough for my purpose, so I'm happy.

I just put a demo pattern in my timer1 interrupt service routine. But, it takes too much time from my Arduino. I can't even print to the serial monitor. I think the interval time is too high. What would be a suitable interval time, and if that is not the problem, I would like to know what it is.

Code:

/*
  24 x 5 led matrix
  Version 2.2
  by: ArduMasterPro44
*/

const uint8_t alphabet[95][4] = {
  { 3, 0x00, 0x00, 0x00 },  // " "
  { 1, 0x17, 0x00, 0x00 },  // "!"
  { 3, 0x03, 0x00, 0x03 },  // """
  { 3, 0x1F, 0x0A, 0x1F },  // "#"
  { 3, 0x0B, 0x1F, 0x0D },  // "$"
  { 3, 0x19, 0x04, 0x13 },  // "%"
  { 3, 0x0A, 0x15, 0x1A },  // "&"
  { 1, 0x03, 0x00, 0x00 },  // "'"
  { 2, 0x0E, 0x11, 0x00 },  // "("
  { 2, 0x11, 0x0E, 0x00 },  // ")"
  { 3, 0x05, 0x02, 0x05 },  // "*"
  { 3, 0x04, 0x0E, 0x04 },  // "+"
  { 1, 0x18, 0x00, 0x00 },  // ","
  { 3, 0x04, 0x04, 0x04 },  // "-"
  { 1, 0x10, 0x00, 0x00 },  // "."
  { 3, 0x18, 0x04, 0x03 },  // "/"
  { 3, 0x1F, 0x11, 0x1F },  // "0"
  { 3, 0x12, 0x1F, 0x10 },  // "1"
  { 3, 0x19, 0x15, 0x12 },  // "2"
  { 3, 0x11, 0x15, 0x1F },  // "3"
  { 3, 0x07, 0x04, 0x1F },  // "4"
  { 3, 0x17, 0x15, 0x1D },  // "5"
  { 3, 0x1E, 0x15, 0x1D },  // "6"
  { 3, 0x01, 0x01, 0x1F },  // "7"
  { 3, 0x1F, 0x15, 0x1F },  // "8"
  { 3, 0x17, 0x15, 0x1F },  // "9"
  { 1, 0x0A, 0x00, 0x00 },  // ":"
  { 1, 0x1A, 0x00, 0x00 },  // ";"
  { 3, 0x04, 0x0A, 0x11 },  // "<"
  { 3, 0x0A, 0x0A, 0x0A },  // "="
  { 3, 0x11, 0x0A, 0x04 },  // ">"
  { 3, 0x01, 0x15, 0x03 },  // "?"
  { 3, 0x0E, 0x11, 0x17 },  // "@"
  { 3, 0x1F, 0x05, 0x1F },  // "A"
  { 3, 0x1F, 0x15, 0x1B },  // "B"
  { 3, 0x1F, 0x11, 0x11 },  // "C"
  { 3, 0x1F, 0x11, 0x0E },  // "D"
  { 3, 0x1F, 0x15, 0x11 },  // "E"
  { 3, 0x1F, 0x05, 0x01 },  // "F"
  { 3, 0x1F, 0x11, 0x1D },  // "G"
  { 3, 0x1F, 0x04, 0x1F },  // "H"
  { 3, 0x11, 0x1F, 0x11 },  // "I"
  { 3, 0x18, 0x10, 0x1F },  // "J"
  { 3, 0x1F, 0x04, 0x1B },  // "K"
  { 3, 0x1F, 0x10, 0x10 },  // "L"
  { 3, 0x1F, 0x06, 0x1F },  // "M"
  { 3, 0x1F, 0x01, 0x1F },  // "N"
  { 3, 0x1F, 0x11, 0x1F },  // "O"
  { 3, 0x1F, 0x05, 0x07 },  // "P"
  { 3, 0x0F, 0x19, 0x0F },  // "Q"
  { 3, 0x1F, 0x05, 0x1A },  // "R"
  { 3, 0x17, 0x15, 0x1D },  // "S"
  { 3, 0x01, 0x1F, 0x01 },  // "T"
  { 3, 0x1F, 0x10, 0x1F },  // "U"
  { 3, 0x0F, 0x10, 0x1F },  // "V"
  { 3, 0x1F, 0x0C, 0x1F },  // "W"
  { 3, 0x1B, 0x04, 0x1B },  // "X"
  { 3, 0x07, 0x1C, 0x07 },  // "Y"
  { 3, 0x19, 0x15, 0x13 },  // "Z"
  { 2, 0x1F, 0x11, 0x00 },  // "["
  { 3, 0x03, 0x04, 0x18 },  // "\"
  { 2, 0x11, 0x1F, 0x00 },  // "]"
  { 3, 0x02, 0x01, 0x02 },  // "^"
  { 3, 0x10, 0x10, 0x10 },  // "_"
  { 2, 0x01, 0x02, 0x00 },  // "`"
  { 3, 0x0c, 0x12, 0x1E },  // "a"
  { 3, 0x1F, 0x12, 0x0C },  // "b"
  { 3, 0x1E, 0x12, 0x12 },  // "c"
  { 3, 0x0C, 0x12, 0x1F },  // "d"
  { 3, 0x0C, 0x1A, 0x14 },  // "e"
  { 3, 0x04, 0x1F, 0x05 },  // "f"
  { 3, 0x12, 0x15, 0x0E },  // "g"
  { 3, 0x1F, 0x04, 0x18 },  // "h"
  { 1, 0x1D, 0x00, 0x00 },  // "i"
  { 2, 0x10, 0x0D, 0x00 },  // "j"
  { 3, 0x1F, 0x04, 0x1A },  // "k"
  { 1, 0x1F, 0x00, 0x00 },  // "l"
  { 3, 0x1E, 0x04, 0x1E },  // "m"
  { 3, 0x1E, 0x02, 0x1E },  // "n"
  { 3, 0x1E, 0x12, 0x1E },  // "o"
  { 3, 0x1F, 0x09, 0x06 },  // "p"
  { 3, 0x06, 0x09, 0x1F },  // "q"
  { 3, 0x1E, 0x02, 0x04 },  // "r"
  { 3, 0x14, 0x16, 0x0A },  // "s"
  { 3, 0x02, 0x0F, 0x12 },  // "t"
  { 3, 0x1E, 0x10, 0x1E },  // "u"
  { 3, 0x0E, 0x10, 0x0E },  // "v"
  { 3, 0x1E, 0x08, 0x1E },  // "w"
  { 3, 0x1A, 0x04, 0x1A },  // "x"
  { 3, 0x17, 0x14, 0x0F },  // "y"
  { 3, 0x1A, 0x12, 0x16 },  // "z"
  { 3, 0x04, 0x1F, 0x11 },  // "{"
  { 1, 0x1F, 0x00, 0x00 },  // "|"
  { 3, 0x11, 0x1F, 0x04 },  // "}"
  { 3, 0x04, 0x0C, 0x08 }   // "~"
};

const uint8_t Data1 = 3;
const uint8_t Clock1 = 4;
const uint8_t Data2 = 5;
const uint8_t Clock2 = 6;
const uint8_t Latch = 7;
const uint16_t delayTime = 125;
const uint8_t rows = 24;
const uint8_t columns = 5;
uint32_t time;
char message[] = "Aaaaaaaaaaaaaaaaaaaaaaaaa";
int16_t size = 0;
uint16_t length = strlen(message);
uint8_t* bitMap;
volatile uint8_t displayBuffer[rows] = {
  0x01,
  0x02,
  0x04,
  0x08,
  0x10,
  0x08,
  0x04,
  0x02,
  0x01,
  0x02,
  0x04,
  0x08,
  0x10,
  0x08,
  0x04,
  0x02,
  0x01,
  0x02,
  0x04,
  0x08,
  0x10,
  0x08,
  0x04,
  0x02
};

void setup() {

  // Uncomment to use pinMode() / comment to use data direction register
  // pinMode(Data1, OUTPUT);
  // pinMode(Clock1, OUTPUT);
  // pinMode(Data2, OUTPUT);
  // pinMode(Clock2, OUTPUT);
  // pinMode(Latch, OUTPUT);

  // Comment to use pinMode() / uncomment to use data direction register
  DDRD |= B11111000;
  DDRB |= (1 << PB5);

  // ──────── Timer configuration ────────┐
  //                                      │
  // Reset Timer1 Control Reg A           │
  TCCR1A = 0;  // ────────────────────────┤
  //                                      │
  // Set CTC mode                         │
  TCCR1B &= ~(1 << WGM13);  // ───────────┤
  TCCR1B |= (1 << WGM12);   // ───────────┤
  //                                      │
  // Set to prescaler of 1                │
  TCCR1B &= ~(1 << CS12);  //  ───────────┤
  TCCR1B &= ~(1 << CS11);  //  ───────────┤
  TCCR1B |= (1 << CS10);   //  ───────────┤
  //                                      │
  // Reset Timer1 a and set compare value │
  TCNT1 = 0;               //  ───────────┤
  OCR1A = 4000;            //  ───────────┤
  //                                      │
  // Enable Timer1 compare interrupt      │
  TIMSK1 = (1 << OCIE1A);  //  ───────────┤
  //                                      │
  // Enable global interrupts             │
  sei();                   //  ───────────┘

  for (uint16_t i = 0; i < length; i++) {  // Loop for every character in the message
    size += alphabet[message[i] - 32][0];
    if (i < length - 1) {  // Check if it isn't the last character of the message
      size++;
    }
  }

  bitMap = new uint8_t[size];

  uint16_t x = 0;                                                 // Keeps track of the X-Axis bit
  for (uint16_t i = 0; i < length; i++) {                         // Loop for every character in the message
    for (uint8_t j = 0; j < alphabet[message[i] - 32][0]; j++) {  // Loop for every byte in the character
      bitMap[x] = alphabet[message[i] - 32][j + 1];               // Store the byte in the bitMap[] array
      x++;
    }
    if (i < length - 1) {  // Check if it isn't the last character of the message
      bitMap[x] = 0;       // Store a 0x00 byte in the bitMap[] array
      x++;
    }
  }
  Serial.begin(9600);
}

void loop() {

  // int16_t i = -24;
  // while (i < size) {
  //   if (millis() - time >= delayTime) {
  //     i++;
  //     time = millis();
  //   }
  //   shiftText(i, rows, columns);
  // }
  PORTB ^= (1 << 5);
  Serial.println("Good");
}

void shiftText(int16_t idx, uint8_t bitsX, uint8_t bitsY) {
  for (uint8_t i = 0; i < bitsX; i++) {
    digitalWrite(Latch, LOW);
    if (i == 0) {
      digitalWrite(Data1, HIGH);
      digitalWrite(Clock1, HIGH);
      digitalWrite(Clock1, LOW);
      digitalWrite(Data1, LOW);
    } else {
      digitalWrite(Clock1, HIGH);
      digitalWrite(Clock1, LOW);
    }
    shiftBits(Data2, Clock2, LSBFIRST, ~stringToChar(i + idx), bitsY);
    digitalWrite(Latch, HIGH);  // Shift the byte to the matrix
  }
}

uint8_t stringToChar(int16_t bit) {  // Converts one peice of the character according to the bit variable
  if (bit >= size) {
    return 0x00;
  } else if (bit < 0) {
    return 0x00;
  } else {
    return bitMap[bit];
  }
}

void shiftBits(uint8_t data, uint8_t clock, uint8_t bitOrder, uint32_t value, uint8_t bits) {
  for (uint8_t i = 0; i < bits; i++) {
    if (bitOrder == LSBFIRST) {
      digitalWrite(data, value & 1);
      value >>= 1;
    } else if (bitOrder == MSBFIRST) {
      digitalWrite(data, (value & bit(bits - 1)) != 0);
      value <<= 1;
    }
    digitalWrite(clock, HIGH);
    digitalWrite(clock, LOW);
  }
}

ISR(TIMER1_COMPA_vect) {
  for (uint8_t i = 0; i < rows; i++) {
    PORTD &= ~(1 << Latch);
    if (i == 0) {
      PORTD |= (1 << Data1);
      PORTD |= (1 << Clock1);
      PORTD &= ~(1 << Clock1);
      PORTD &= ~(1 << Data1);
    } else {
      PORTD |= (1 << Clock1);
      PORTD &= ~(1 << Clock1);
    }
    shiftBits(Data2, Clock2, LSBFIRST, ~displayBuffer[i], columns);
    PORTD |= (1 << Latch);  // Shift the byte to the matrix
  }
}

I just realised that using the hardware timer affects the timing of micros(), and that it also dims the matrix if I lower the frequency, so I think I will follow @PaulRB's advice and not use a hardware timer, but instead, the micros() function.

There is nothing wrong or complex about using a hardware timer to control display multiplexing. However, you appear to be doing far too much in that ISR.
I understood that, with the way you had now structured the solution, based on the idea suggested by @PaulRB, taking advantage of your "2 shift register chain" design , only the following would be required.

On each call of the ISR, you would (a) open the latch (b) push 1 bit out to the column shift register chain (c) write 1 byte to the row shift register from displayBuffer[ ] and (d) close the latch. That is, you do the minimum in the ISR to display the next column.

As it is, you have a loop in the ISR and on each iteration of that loop, you call a function which invokes yet another loop which itself makes multiple calls to digitalWrite().

@6v6gt is correct about the ISR.

The ISR should set up the scan for 1 led column per call. It is currently looping over all 24 columns.

ISR(TIMER1_COMPA_vect) {
  static uint8_t i = 0;
  PORTD &= ~(1 << Latch);
  if (i == 0) {
    PORTD |= (1 << Data1);
  } else {
    PORTD &= ~(1 << Data1);
  }
  PORTD |= (1 << Clock1);
  PORTD &= ~(1 << Clock1);
  shiftBits(Data2, Clock2, LSBFIRST, ~displayBuffer[i], columns);
  PORTD |= (1 << Latch);  // Shift the byte to the matrix
  if (++i >= rows) i = 0;
}

The ISR looks like it is set to trigger every 4000 clock cycles, which at 16MHz is every 250us. So even if only one column is lit for each ISR call, all 24 will be lit over 6ms, which is a frame rate of 166 per second. @6v6gt am I correct?

If I'm right, that seems too fast, you could set the trigger for 8000 or even more without the display flickering.

The reason I opted for a higher frequency, is that the first bit was always too bright since the off time happened when that column was on. But doing it one row at a time will eliminate that issue. Therefore I will be able to do it at a lower frequency.

[EDIT]: @PaulRB, at each timer interrupt you are declaring the i variable to 0 and then at the end checking if it is at the end of the array. But if you declare it 0 at the beginning, it will always be 0.

[EDIT1]: It still works even with the i variable being declared 0 at the beginning of th ISR, why does it still work?

To help speed up your LED matrix functions, there are several techniques you can try:

  1. Use an optimized LED library: Make sure you're using an LED library that is optimized for your specific LED matrix. There are many LED libraries available, and some are better optimized for certain types of matrices than others.

Yes. That is how I see ir also. A complete display refresh every 5ms.
@ardumasterpro44 look up initialisation of static variables. This is done only once.