Seven segment display flickering when reading GPS through serial

Hi, I'm using an arduino mega pro and a GPS NEO-6M

I'm trying to show the GPS speed and altitude on two seven segment displays.

The problem is the GPS read is very slow and the displays flicker while reading data from the serial.

This is my code:

#include <TinyGPS++.h>
#include <SoftwareSerial.h>

static const int RXPin = 53, TXPin = 52;
static const uint32_t GPSBaud = 19200;

TinyGPSPlus gps;
SoftwareSerial ss(RXPin, TXPin);

int digitDelay = 0;

int SPEED = 0;
int ALTITUDE = 0;

int thousands1;
int hundreds1;
int tens1;
int units1;

int thousands2;
int hundreds2;
int tens2;
int units2;

int counter;

int gpsAltitudePin1 = A1;
int gpsAltitudePin2 = A2;
int gpsAltitudePin3 = A3;
int gpsSpeedPin1 = A4;

int digitPins1 [4] = {2, 4, 6, 8};
int segmentPins1 [7] = {3, 5, 7, 9, 11, 13, 15};

int digitPins2 [4] = {10, 12, 14, 16};
int segmentPins2 [7] = {17, 19, 21, 23, 25, 27, 29};

int zero [7] =  {1, 1, 1, 1, 1, 1, 0};
int one [7] =   {0, 1, 1, 0, 0, 0, 0};
int two [7] =   {1, 1, 0, 1, 1, 0, 1};
int three [7] = {1, 1, 1, 1, 0, 0, 1};
int four [7] =  {0, 1, 1, 0, 0, 1, 1};
int five [7] =  {1, 0, 1, 1, 0, 1, 1};
int six [7] =   {1, 0, 1, 1, 1, 1, 1};
int seven [7] = {1, 1, 1, 0, 0, 0, 0};
int eight [7] = {1, 1, 1, 1, 1, 1, 1};
int nine [7] =  {1, 1, 1, 1, 0, 1, 1}; 

int numberArrays [10] = {zero, one, two, three, four, five, six, seven, eight, nine};

void setup() {

  ss.begin(GPSBaud);
  Serial.begin(9600);
  
  for (int i = 0; i < 4; i++) {
    pinMode(digitPins1[i], OUTPUT);
    pinMode(digitPins2[i], OUTPUT);
  }
  for (int i = 0; i < 7; i++) {
    pinMode(segmentPins1[i], OUTPUT);
    pinMode(segmentPins2[i], OUTPUT);
  }

  counter = 0;
}

void clear(int dp[4], int sp[7]) {
  for (int i = 0; i < 4; i++) {
    digitalWrite(dp[i], LOW);
  }
  for (int i = 0; i < 7; i++) {
    digitalWrite(sp[i], HIGH);
  }
}

void setDigit(int digit[7], int sp[7]) {
  for (int i = 0; i < 7; i++) {
    if (digit[i] == 1) {
      digitalWrite(sp[i], LOW);
    }
  }
}

void setNumbers(int number1, int number2, int dp1[4], int dp2[4], int sp1[7], int sp2[7]) {

  if (number1 > 9999) {
    number2 = 9999;
  }

  if (number2 > 9999) {
    number2 = 9999;
  }

  clear(dp1, sp1);
  clear(dp2, sp2);
  
  thousands1 = number1 / 1000;
  hundreds1 = (number1 - thousands1 * 1000) / 100;
  tens1 = (number1 - thousands1 * 1000 - hundreds1 * 100) / 10;
  units1 = (number1 - thousands1 * 1000 - hundreds1 * 100 - tens1 * 10);
  
  thousands2 = number2 / 1000;
  hundreds2 = (number2 - thousands2 * 1000) / 100;
  tens2 = (number2 - thousands2 * 1000 - hundreds2 * 100) / 10;
  units2 = (number2 - thousands2 * 1000 - hundreds2 * 100 - tens2 * 10);

  if (thousands1 != 0) {
    digitalWrite(dp1[0], HIGH);
    setDigit(numberArrays[thousands1], sp1);
  }
  
  if (thousands2 != 0) {
    digitalWrite(dp2[0], HIGH);
    setDigit(numberArrays[thousands2], sp2);
  }

  delay(digitDelay);
  clear(dp1, sp1);
  clear(dp2, sp2);

  if (thousands1 != 0 || hundreds1 != 0) {
    digitalWrite(dp1[1], HIGH);
    setDigit(numberArrays[hundreds1], sp1);
  }
  
  if (thousands2 != 0 || hundreds2 != 0) {
    digitalWrite(dp2[1], HIGH);
    setDigit(numberArrays[hundreds2], sp2);
  }

  delay(digitDelay);
  clear(dp1, sp1);
  clear(dp2, sp2);

  
 

  if (thousands1 != 0 || hundreds1 != 0 || tens1 != 0) {
    digitalWrite(dp1[2], HIGH);
    setDigit(numberArrays[tens1], sp1);
  }
  
  if (thousands2 != 0 || hundreds2 != 0 || tens2 != 0) {
    digitalWrite(dp2[2], HIGH);
    setDigit(numberArrays[tens2], sp2);
  }
  
  delay(digitDelay);
  clear(dp1, sp1);
  clear(dp2, sp2);



  digitalWrite(dp1[3], HIGH);
  setDigit(numberArrays[units1], sp1);

  digitalWrite(dp2[3], HIGH);
  setDigit(numberArrays[units2], sp2);

  delay(digitDelay);
  clear(dp1, sp1);
  clear(dp2, sp2);
}


void loop() {

//  while (ss.available() > 0) {
//    if (gps.encode(ss.read())) {
//      ALTITUDE = gps.altitude.meters();
//      SPEED = gps.speed.kmph();
//    } 
//  }

  while (ss.available() > 0) {
    byte gpsData = ss.read();
    Serial.write(gpsData);
  }


  
  setNumbers(ALTITUDE, SPEED, digitPins1, digitPins2, segmentPins1, segmentPins2);

}

I've tried changing the baud rate but even at maximum, the flickering is still visible. I was thinking on using and aditional arduino nano to read the gps data and send it to the main araduino through analogwrite or serial, but I don't think analogwrite works that way and serial would be as slow as the GPS so I will have the same problem.

Even if I replace the while loop for an if, the flickering still appears:

void loop() {
  if (ss.available() > 0) {
    byte gpsData = ss.read();
    Serial.write(gpsData);
  }
  
  setNumbers(ALTITUDE, SPEED, digitPins1, digitPins2, segmentPins1, segmentPins2);

}

I'm running out of ideas, how can I do it?

Eliminate the clear() calls. There is no need to blank the display between writes. Ultimately, you will need to control the update frequency so you don’t get digits jumping all over the place.

#include <SoftwareSerial.h>
static const int RXPin = 53, TXPin = 52;

Why?
A Mega (Pro) has four hardware serial ports.
Leo..

The suggestions above are valid, BUT, since you are multiplexing the display within loop(), you must be aware that receiving and parsing serial messages ‘takes time’.
That time spent handling the serial stream interrupts the muxing of the display rows & columns - if even only for several microseconds per received character, it can be visible as the LEDs respond very quickly.

If you can’t reduce or mask the effect of this timing, the 'easiest' other way is to offload the display multiplexing to a dedicated multiplexer chip like the MAX7219 or similar, which will hold the LED states between updates.

EDIT: 'Respect' to the following suggestion of using timers to slice between the serial interrupts.

I guess you have to change your multiplexing strategy, if you want to do everything from the mega.

You could try this:
Set up a global array which is image of what is to appear on the displays (for example 8 digits).
You maintain this array by the calculations in the loop.
Set up a hardware timer with say a 1ms period. In the timer callback routine, you read the display image array and display the next digit on each display. Repeat this with each call, setting the index of the next digit to display.
If the timer period is greater than about 5ms, you may get flicker. You control the brightness of the display by periodically blanking digits in the multiplexing cycle.

That way, the display cycle is independent of the loop. Further, you don’t need the inter digit delays, because the displayed digit remains until the next call of the callback routine.

Minor observation but..

void setNumbers(int number1, int number2, int dp1[4], int dp2[4], int sp1[7], int sp2[7]) {

  if (number1 > 9999) {
    number2 = 9999;
  }

This range check looks odd to me.….

You should skip updating either the displayed number, or a digit unless it changes.

All those "int" arrays used to store single bits are an incredible waste of precious RAM, and waste some amount of CPU time and code space as well.

int seven [7] = {1, 1, 1, 0, 0, 0, 0};

Hint: the above information can be put into a single byte, and the individual bits tested or shifted out as needed.

char seven = 0b01110000;

I second 6v6gt's suggestion to refresh the display via timer interrupt. And if you can use a single processor port for all the segment lines, then the ISR could be very short indeed, and not likely to cause problems anywhere else. So the ISR would just have four steps:

  1. Turn off the digit currently being displayed (i.e. - digit n).
  2. Increment the digit pointer n, and AND it with 7 so it will roll from 7 to zero..
  3. Change the segment values by writing a single byte to the segment port. The byte value would have already been calculated for digit n in loop().
  4. Turn on the new digit n.

So the loop would be responsible for calculating the new segment values for each digit each time the value changes, or less often if you prefer, but the actual refresh would be handled by the ISR. The ON period of each digit would be the refresh interrupt rate. Since you have 8 digits, you probably would need to have the timer interrupt every 3, or maybe 2 ms, to keep it from flickering. If it isn't bright enough, you could change the resistors to a lower value.

I don't know if this interrupt would interfere with the other stuff, but you already have one interrupt firing every ms or so - the millis interrupt. If that doesn't cause problems, another even shorter interrupt probably wouldn't either.

There is a chance the millis interrupt would interfere with the display interrupt, but I think that's unlikely. But if it does, you could even rewrite the millis interrupt to incorporate the display update.

The millis interrupt is driven off the overflow interrupt for timer 0, you can generate an additional interrupt off the same timer using the compare register, and use that to drive the display refresh. Using direct port instructions will take less time, as well as arranging the wiring so all the segments are driven off a single port.

As suggested in reply#8, you can use Timer0 to multiplex the display.

Adafruit has a nice tutorial showing you how to set that up, without interfering with the normal millis() operation.

Thanks for all the insight guys, I started trying the easiest ideas first, an it turned out that, as Wawa suggested I was using SoftwareSerial in vain.

Switching to the arduino hardware serial ports removes all the lag and the thing work flawlessly now.

Thank you very much.

Aw, that's too bad. I thought we had you headed for some really elegant code, but now it's merely good enough. Well, maybe next time.