Why doesn't this code for an 8x8 LED matrix work properly?

Question

There's an Arduino Uno (Keyestudio KS0078) and a 788BS 8x8 LED matrix. I built the circuit as on the image and ran this program:

#define MY_PERIOD 200
int displayColumn = 0;
uint32_t tmr1;
unsigned char Text[] = { 0x00, 0x1c, 0x22, 0x22, 0x22, 0x22, 0x22, 0x1c };
void Draw_point(unsigned char x, unsigned char y) {
 clear();
 digitalWrite(x + 2, HIGH);
 digitalWrite(y + 10, LOW);
 delay(1);
}
void show_num(void) {
 unsigned char i, j, data;
 for (i = 0; i < 8; i++) {
 data = Text[i];
 data <<= displayColumn;
 for (j = 0; j < 8; j++) {
 if (data & 0x01) {
 Draw_point(j, i);
 }
 data >>= 1;
 }
 }
}
void setup() {
 Serial.begin(9600);
 int i = 0;
 for (i = 2; i < 18; i++) {
 pinMode(i, OUTPUT);
 }
 clear();
}
void loop() {
 show_num();
 if (millis() - tmr1 >= MY_PERIOD) {
 tmr1 = millis();
 displayColumn++;
 if (displayColumn > 7) {
 displayColumn = 0;
 }
 }
}
void clear() {
 for (int i = 2; i < 10; i++)
 digitalWrite(i, LOW);
 for (int i = 10; i < 18; i++)
 digitalWrite(i, HIGH);
}

circuit

I got the code from the Keyestudio examples wiki and added a part with displayColumn and shifting to the left every 0.2 sec. But instead of fading out fully on the 7th step, one LED (x=0, y=6) stays on. Even if I stop executing show_num when displayColumn > 7 it stays on as long as there is power. Why?

UPD: I removed dynamic shifting in loop and set displayColumn in 5, 6 and 7 - the display showed correct result and was empty when displayColumn was equal to 7. Obviously the bug hides between "transitions".

UPD2: If I try the programm with other symbol, the bug on the step 7 always repeats with other LED which was the last lighted one on the step 6.

UPD3: Debug data by request of @thebusybee (repeated blocks removed)

x,y:2,1 x,y:3,1 x,y:4,1
x,y:1,2 x,y:5,2
x,y:1,3 x,y:5,3
x,y:1,4 x,y:5,4
x,y:1,5 x,y:5,5
x,y:1,6 x,y:5,6
x,y:2,7 x,y:3,7 x,y:4,7
displayColumn: 1
x,y:3,1 x,y:4,1 x,y:5,1
x,y:2,2 x,y:6,2
x,y:2,3 x,y:6,3
x,y:2,4 x,y:6,4
x,y:2,5 x,y:6,5
x,y:2,6 x,y:6,6
x,y:3,7 x,y:4,7 x,y:5,7
displayColumn: 2
x,y:4,1 x,y:5,1 x,y:6,1
x,y:3,2 x,y:7,2
x,y:3,3 x,y:7,3
x,y:3,4 x,y:7,4
x,y:3,5 x,y:7,5
x,y:3,6 x,y:7,6
x,y:4,7 x,y:5,7 x,y:6,7
displayColumn: 3
x,y:5,1 x,y:6,1 x,y:7,1
x,y:4,2
x,y:4,3
x,y:4,4
x,y:4,5
x,y:4,6
x,y:5,7 x,y:6,7 x,y:7,7
displayColumn: 4
x,y:6,1 x,y:7,1
x,y:5,2
x,y:5,3
x,y:5,4
x,y:5,5
x,y:5,6
x,y:6,7 x,y:7,7
displayColumn: 5
x,y:7,1
x,y:6,2
x,y:6,3
x,y:6,4
x,y:6,5
x,y:6,6
x,y:7,7
displayColumn: 6
x,y:7,2
x,y:7,3
x,y:7,4
x,y:7,5
x,y:7,6
displayColumn: 7
//many many empty strings
displayColumn: 8

enter image description here

• jsotola: add debugging code to your program
• Mik: @jsotola I'v tried to get print of x, y from some code parts in some times and I have no idea anymore. After step 6 I found nothing activating the pixel. So seems the pixel still acive after previous teps but why so and why one? My experience is too poor to get it.
• the busybee: @6v6gt The second statement shifts the value in data. It is not a simple assignment overwriting the first. In contrast, the first statement is necessary to set a defined value in data.
• Mik: @6v6gt yes, kinda. I don't know it looks odd or not cause I'm newbee in Arduino. But I see no logical contradictions in the order of assignment and besides it works strange but almost properly. The first assignment sets next byte ("the display line") in data, the second shifts it to left, and every 0.2 sec displayColumn increases by 1 ("one display column"). I try to animate original code slightly to understand better how to cook displays
• the busybee: Your code looks fine to me, I checked the algorithm with debug prints, but no output to an LED matrix. -- Your wiring according to the drawing looks fine, too. -- Please edit your question to clarify: How long does this single erroneous LED light? Your code repeats endlessly. Does this happen?

Answer 1

As found out during the discussion, the display must be cleaned before drawing any new row. And since the clear() calling was at the beginning of the Draw_point() and this function was not used in the last step, cleaning was not executed before the last step. This is correct for static rendering but for scrolling it will be correct to move the clear() to the end of the Draw_point() then the project works correctly.

#define MY_PERIOD 200
int displayColumn = 0;
uint32_t tmr1;
unsigned char Text[] = { 0x00, 0x1c, 0x22, 0x22, 0x22, 0x22, 0x22, 0x1c };
void Draw_point(unsigned char x, unsigned char y) {
 digitalWrite(x + 2, HIGH);
 digitalWrite(y + 10, LOW);
 delay(1);
 clear();
}
void show_num(void) {
 unsigned char i, j, data;
 for (i = 0; i < 8; i++) {
 data = Text[i];
 data <<= displayColumn;
 for (j = 0; j < 8; j++) {
 if (data & 0x01) {
 Draw_point(j, i);
 }
 data >>= 1;
 }
 }
}
void setup() {
 Serial.begin(9600);
 int i = 0;
 for (i = 2; i < 18; i++) {
 pinMode(i, OUTPUT);
 }
}
void loop() {
 show_num();
 if (millis() - tmr1 >= MY_PERIOD) {
 tmr1 = millis();
 displayColumn++;
 if (displayColumn > 7) {
 displayColumn = 0;
 }
 }
}
void clear() {
 for (int i = 2; i < 10; i++)
 digitalWrite(i, LOW);
 for (int i = 10; i < 18; i++)
 digitalWrite(i, HIGH);
}

Thanks to users @6v6gt and @thebusybee for their help in finding the solution.

Answer 2

As an addition to your answer, this is for future visitors who want to learn more.

1. Programming by Accident

Your analysis is correct, mine in an early comment is wrong.

The last LED of the second last "picture" is not cleared when the last "picture" is drawn. This is because you clear the matrix only, if you switch on an LED.

This is your original code:

void Draw_point(unsigned char x, unsigned char y) {
 clear();
 digitalWrite(x + 2, HIGH);
 digitalWrite(y + 10, LOW);
 delay(1);
}

In your solution you clear the complete matrix after the delay, which results in the expected effect.

However, you did not analyze the underlying to the end. You changed something after a random suggestion, and it "works." I have seen this so many times.

We know that Draw_point() handles only one LED. So we do not need to reset all pins to their respective off levels, 14 of the 16 are already fine. It suffices to do this for the single LED just switched on.

void Draw_point(unsigned char x, unsigned char y) {
 digitalWrite(x + 2, HIGH);
 digitalWrite(y + 10, LOW);
 delay(1);
 digitalWrite(x + 2, LOW);
 digitalWrite(y + 10, HIGH);
}

2. Driving the LEDs in Reverse

As I mentioned in a comment, you drive the LEDs of the matrix in reverse, when they are off. We all know that comments will not be read, mostly. So I add this here. ;-)

Most LEDs stand some voltage in reverse. According to the 788BS' data sheet it has 5V as absolute maximum rating. I would not dare to apply that really.

All professionals drive their LED matrices such that only forward current is applied. All other LEDs are not driven at all.

The solution here is to change the pin mode of the pins.

#define MY_PERIOD 200
int displayColumn = 0;
uint32_t tmr1;
unsigned char Text[] = { 0x00, 0x1c, 0x22, 0x22, 0x22, 0x22, 0x22, 0x1c };
void Draw_point(unsigned char x, unsigned char y) {
 pinMode(x + 2, OUTPUT);
 digitalWrite(x + 2, HIGH);
 pinMode(y + 10, OUTPUT);
 digitalWrite(y + 10, LOW);
 delay(1);
 pinMode(x + 2, INPUT);
 pinMode(y + 10, INPUT);
}
void show_num(void) {
 unsigned char i, j, data;
 for (i = 0; i < 8; i++) {
 data = Text[i];
 data <<= displayColumn;
 for (j = 0; j < 8; j++) {
 if (data & 0x01) {
 Draw_point(j, i);
 }
 data >>= 1;
 }
 }
}
void setup() {
}
void loop() {
 show_num();
 if (millis() - tmr1 >= MY_PERIOD) {
 tmr1 = millis();
 displayColumn++;
 if (displayColumn > 7) {
 displayColumn = 0;
 }
 }
}

You don't need to clear() in setup(), because all pins are initially in "input" mode already.

3. About Flicker

You tried with no delay() and see only a dim display. This is correct as the time the LED is on is short compared to the time of the rest of the program.

With delay(2) you experience flicker. Let's think about the timing here.

There are different "pictures" during the scroll, with the listed number of LEDs switch on:

displayColumn	Number of LEDs switched on
0	16
1	16
2	16
3	11
4	9
5	7
6	5
7	0

Because the Arduino is quite fast, we came approximate the calculations by neglecting the rest of the program.

Each single LED is shown for 2 ms according to delay(2), so a complete picture sums up to repetition periods between (nearly) 0 ms (displayColumn = 7) and 32 ms (displayColumn = 0 to 2). The reciprocal gives us the display frequency, many know the term "frames per second" or frame rate. The lowest frame rate is with the longest period: 1 / 0.032 s = 31.25 Hz. Our eyes see this as flickering. BTW, as soon as the "picture" has fewer LEDs, the display will stop flickering.

Commonly human eyes see no flicker if the frame rate exceeds about 50 to 60 Hz. With delay(1) the periods are only half as long as with delay(2), of course. The lowest frame rate is therefore 62.5 Hz, good enough for no flicker experience.

Please be aware that the frame rate drops with the number of LEDs switched on. If you want to have at least 50 Hz with delay(1), you cannot have more than 20 LEDs: 1 / 50 Hz = 20 ms, which corresponds to 20 LEDs.

The solution here is to show a complete row at the same time. Because then there are just 8 rows to sequence, you can raise to delay(2) again: 8 rows * 2 ms < 20 ms. The specific code is left as an exercise to the reader. Please be aware that you need to take electrical maximum ratings into account. The MCU of the Arduino has limits to the sourced current.

4. Final Note

You might wonder why the linked resource (the Keyestudio examples wiki) shows this "not-so-perfect" source code. Well, as we can see from the bad formatting of the example, it was hacked down in a hurry and without being aware that beginners will be misguided.

Take all such resources with a lot of caution. Unfortunately, this is absolutely necessary. (This includes my answer here, too.)

• Mik: Just like that, I thought whatever my programm was far from optimum. I read your comment about mode switch but decided to search for materials on this topic because I never saw this method before, so thank you for the explaining with working example! I also should to note that your first variant needs the clear() executed in the setup(). And one final question - why exactly does delay(1) work as it should? No delay - symbol is dim because we switch it on and off instantly, it's too little time. But the display begins to flicker when delay = 2 or higher. Why 1?