# Beginner struggling with basic two-digit 7-segment led display

I have just started playing around with 7-segment led displays.

My objective is to display a voltage of 0 to 5 presented to one of the analogue inputs (Arduino Uno) to a reading of 0 to 99 on a dual digit display. In my experiments to sort out the basics I am using two common-cathode display elements.

I am copying hereunder what I have got so far in the way of code (parts cribbed from what I have found on this forum – thank you). In the set-up procedure the program correctly steps through (and displays) the numbers 0 to 99, so the “void setSegments(byte digit)” part of the code (as also the hardware) seems to be working okay.

It correctly displays 00 when the A0 pin is grounded, and the first digit (‘tens’) steps from 0 to 1 to 2 as I increase the voltage. For the rest I get gibberish (both ‘tens’ and ‘units’). I suspect the line “int displayValue = sensorValue*99/1024” is the source of the trouble.

Can someone here please point me in the right direction.

//       --a--
//    f |     | b
//      |--g--|
//    e |     | c
//       --d--  O h

// define the pins connected to the segments
#define segment_a 3
#define segment_b 4
#define segment_c 5
#define segment_d 6
#define segment_e 7
#define segment_f 8
#define segment_g 9
#define segment_h 10

//define the common sink pins
#define sink_1 11  // enables 1st digit
#define sink_2 12  // enables 2nd digit

int digit;

byte segmentPin[] = {
segment_a, segment_b, segment_c, segment_d, segment_e, segment_f, segment_g, segment_h};

byte segmentPattern_1[] = {
0x3f, 0x06, 0x5b, 0x4f, 0x66, 0x6d, 0x7d, 0x07, 0x7f, 0x6f, 0x80, 0x40};

void setup()
{
pinMode (segment_a, OUTPUT);
pinMode (segment_b, OUTPUT);
pinMode (segment_c, OUTPUT);
pinMode (segment_d, OUTPUT);
pinMode (segment_e, OUTPUT);
pinMode (segment_f, OUTPUT);
pinMode (segment_g, OUTPUT);
pinMode (segment_h, OUTPUT);

pinMode (sink_1, OUTPUT);
pinMode (sink_2, OUTPUT);

//counts from 00 to 99 and calls for display the two digits after each increment
for(int i=0; i<100; i++){
for(int j=0; j<10; j++){
digit = i/10;  //determines value of 'tens' digit
digitalWrite(sink_1,HIGH);
digitalWrite(sink_2,LOW);
setSegments(digit);
delay (5);
digit = i - digit*10;  //determines value of 'units' digit
digitalWrite(sink_1,LOW);
digitalWrite(sink_2,HIGH);
setSegments(digit);
delay (5);
}
}
}
void loop ()
{
int sensorValue = analogRead(A0);
int displayValue = sensorValue*99/1024;

//display sensed value
for(int j=0; j<50; j++){
digit = displayValue/10;  //determines value of 'tens' digit
digitalWrite(sink_1,HIGH);
digitalWrite(sink_2,LOW);
setSegments(digit);
delay (5);
digit = displayValue - displayValue*10;  //determines value of 'units' digit
digitalWrite(sink_1,LOW);
digitalWrite(sink_2,HIGH);
setSegments(digit);
delay (5);
}
}
// setting the bits of the digit to be displayed
void setSegments(byte digit){
byte mask = 1;
for(int i=0; i<8; i++){
if((segmentPattern_1[digit] & mask) == 0) digitalWrite(segmentPin[i],LOW);
else digitalWrite(segmentPin[i],HIGH);
}
}

I suspect the line “int displayValue = sensorValue*99/1024” is the source of the trouble.

Without looking at the rest of the code that is a problem.
It is an interger and dividing 99 by 1024 will always give you zero.

Not sure if it's pretty or not. But it works.

In my case I'm using ShiftRegisters for the 7 Segments, so you will have to adjust one part of the code.

I'm going to make a guide on how I connected those 7 segments to the Shift Registers and Arduino.

int potPin = A0;
int potValueMapped;
int digitOne;
int digitTwo;

const int latchPin = 8;  // Pin connected to Pin 12 of 74HC595 (Latch)
const int dataPin  = 11;  // Pin connected to Pin 14 of 74HC595 (Data)
const int clockPin = 12;  // Pin connected to Pin 11 of 74HC595 (Clock)

void setup() {
Serial.begin(9600); //Remove after testings

pinMode(latchPin, OUTPUT);
pinMode(clockPin, OUTPUT);
pinMode(dataPin, OUTPUT);
}

void loop() {

potValueMapped = map(analogRead(potPin), 0, 1023, 0, 50);

digitOne = (potValueMapped/10);

Serial.println(digitOne); //Remove after testings
displayDigitOne(digitOne);
delay(500); //This delay you have to remove it's just so I can see both numbers in only 1 7 Segments

digitTwo = (potValueMapped - (10*digitOne));

Serial.println(digitTwo); //Remove after testings
displayDigitTwo(digitTwo);
delay(500); // This is your refresh time
}

//The reason for 2 "functions" is bc the numbers on digit one, will always have the decimal point
//This code should be replaced depending on how you are sending data to the 7 segments
//In this code, it's done for using 2 Shift Registers

void displayDigitOne(int value) {
switch (value) {
case 0:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11111101);
digitalWrite(latchPin, HIGH);
break;
case 1:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b01100001);
digitalWrite(latchPin, HIGH);
break;
case 2:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11011011);
digitalWrite(latchPin, HIGH);
break;
case 3:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11110011);
digitalWrite(latchPin, HIGH);
break;
case 4:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b01100111);
digitalWrite(latchPin, HIGH);
break;
case 5:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b10110111);
digitalWrite(latchPin, HIGH);
break;
case 6:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b10111111);
digitalWrite(latchPin, HIGH);
break;
case 7:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11100001);
digitalWrite(latchPin, HIGH);
break;
case 8:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11111111);
digitalWrite(latchPin, HIGH);
break;
case 9:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11100111);
digitalWrite(latchPin, HIGH);
break;
case '.':
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b00000001);
digitalWrite(latchPin, HIGH);
break;
case 'OFF':
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b00000000);
digitalWrite(latchPin, HIGH);
break;
default:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b00000000);
digitalWrite(latchPin, HIGH);
break;
}
}

void displayDigitTwo(int value) {
switch (value) {
case 0:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11111100);
digitalWrite(latchPin, HIGH);
break;
case 1:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b01100000);
digitalWrite(latchPin, HIGH);
break;
case 2:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11011010);
digitalWrite(latchPin, HIGH);
break;
case 3:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11110010);
digitalWrite(latchPin, HIGH);
break;
case 4:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b01100110);
digitalWrite(latchPin, HIGH);
break;
case 5:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b10110110);
digitalWrite(latchPin, HIGH);
break;
case 6:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b10111110);
digitalWrite(latchPin, HIGH);
break;
case 7:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11100000);
digitalWrite(latchPin, HIGH);
break;
case 8:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11111110);
digitalWrite(latchPin, HIGH);
break;
case 9:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b11100110);
digitalWrite(latchPin, HIGH);
break;
case '.':
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b00000001);
digitalWrite(latchPin, HIGH);
break;
case 'OFF':
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b00000000);
digitalWrite(latchPin, HIGH);
break;
default:
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0b00000000);
digitalWrite(latchPin, HIGH);
break;
}
}

Grumpy_Mike:

I suspect the line “int displayValue = sensorValue*99/1024” is the source of the trouble.

Without looking at the rest of the code that is a problem.
It is an interger and dividing 99 by 1024 will always give you zero.

It's ok, he is making *99

BTW on my code, I thought you were looking to display a number from 0 to 5 with the decimals.

So now you have it easier:

This line will be potValueMapped = map(analogRead(potPin), 0, 1023, 0, 99);

And you can send out the data with the same function, as no decimal point is needed

There is a problem in this line:

int displayValue = sensorValue*99/1024;

The * and / will be evaluated in that order because they have equal precedence, and equal precedence operators are evaluated from left to right. However, the maximum value of sensorValue99 is 102399 = 101277 which is too large to fit in an int. Try:

unsigned int displayValue = (unsigned int)(((unsigned long)sensorValue * 99UL)/1024U);

btw there is no need to make variable 'digit' global. It would be better programming style to declare it locally instead, and this would probably make the code smaller.

This line will be potValueMapped = map(analogRead(potPin), 0, 1023, 0, 99);

Thank you. That did the trick! As simple as that.

The loop section of my program now reads (I also had to fix a bug):

void loop ()
{
int sensorValue = analogRead(A0);
sensorValue = map (sensorValue, 0, 1023, 0, 99);

//display sensed value
for(int j=0; j<50; j++){
digit = sensorValue/10;  //determines value of 'tens' digit
digitalWrite(sink_1,HIGH);
digitalWrite(sink_2,LOW);
setSegments(digit);
delay (5);
digit = sensorValue - digit*10;  //determines value of 'units' digit
digitalWrite(sink_1,LOW);
digitalWrite(sink_2,HIGH);
setSegments(digit);
delay (5);
}
}

BTW, I also have a few shift registers and am planning to try them out. Once I have done that I can decide which system will best suit my purpose.

dc42:
There is a problem in this line:

int displayValue = sensorValue*99/1024;

The * and / will be evaluated in that order because they have equal precedence, and equal precedence operators are evaluated from left to right. However, the maximum value of sensorValue99 is 102399 = 101277 which is too large to fit in an int. Try:

unsigned int displayValue = (unsigned int)(((unsigned long)sensorValue * 99UL)/1024U);

btw there is no need to make variable 'digit' global. It would be better programming style to declare it locally instead, and this would probably make the code smaller.

Thank you for your helpful comments. I was wondering about the order of operations. I did try previously to change the order by bracketing but it didn’t help. You have now explained why.

I prefer American2020’s solution. The ‘map’ statement seems to have been designed exactly for this purpose.

You’re right. I have now declared the variable ‘digit’ locally and it reduced the sketch size from 2158 to 2090.