'A' Part or Top Part of Seven Segment Not Working Properly

nexuscryo · March 29, 2025, 3:17pm

The code is working as intended functionality wise, and the problem is the 'A' part of the seven segment, or the top LED line is not lighting up from digits 0 to 9, with the exception of 7. I do not know why is this happening, maybe using byte mode somehow messing up the D0 pin? Maybe perhaps the way I store counts on each button press? I appreciate any help since I am a beginner.

This is what I am trying to accomplish:

Here is my code:

#define PORTD_CONFIG (0xFF)

#define SEG_0  (0x40)  // 01000000 - All segments except g (a, b, c, d, e, f)
#define SEG_1  (0x79)  // 01111001 - Only segments b and c
#define SEG_2  (0x24)  // 00100100 - Segments a, b, d, e, g
#define SEG_3  (0x30)  // 00110000 - Segments a, b, c, d, g
#define SEG_4  (0x19)  // 00011001 - Segments b, c, f, g
#define SEG_5  (0x12)  // 00010010 - Segments a, c, d, f, g
#define SEG_6  (0x02)  // 00000010 - Segments a, c, d, e, f, g
#define SEG_7  (0x78)  // 01111000 - Only segments a, b, c
#define SEG_8  (0x00)  // 00000000 - All segments a-g
#define SEG_9  (0x10)  // 00010000 - Segments a, b, c, d, f, g  
#define STOP_DISP     (0xFF)
#define BUTTON1       (8)
#define SEG_DEL       delay(500)
int count = 0;

void setup() {

  DDRD = PORTD_CONFIG;
  pinMode(BUTTON1, INPUT);
  pinMode(0, OUTPUT);
  pinMode(1, OUTPUT);

  PORTD = STOP_DISP;
}

void loop() {
  
  unsigned int seg_disp[] = {SEG_0, SEG_1, SEG_2, SEG_3, SEG_4, SEG_5, SEG_6, SEG_7, SEG_8, SEG_9};
  bool buttonState = LOW;
  bool lastButtonState = LOW;

  // Read button state with debouncing
  buttonState = digitalRead(BUTTON1);
 
  if (buttonState != lastButtonState) {
    SEG_DEL;
 
    buttonState = digitalRead(BUTTON1);
  }
  
  // Check for button press (LOW to HIGH transition)
  if (buttonState == HIGH && lastButtonState == LOW) {
    count++;
    if (count >= 4) count = 0;
  }
  
  // Update the last button state
  lastButtonState = buttonState;
 switch (count) {
    case 0:  
      for (int i = 0; i < 10; i++) {
        PORTD = seg_disp[i];
        SEG_DEL;
      }
      break;

    case 1:  
      for (int i = 1; i <= 9; i += 2) {
        PORTD = seg_disp[i];
        SEG_DEL;
      }
      break;

    case 2:  
      for (int i = 9; i >= 0; i--) {
        PORTD = seg_disp[i];
        SEG_DEL;
      }
      break;

    case 3:  
      for (int i = 0; i <= 8; i += 2) {
        PORTD = seg_disp[i];
        SEG_DEL;
      }
      break;

    default:
      count = 0;
      break;
  }
}

Here are images:

ahsrabrifat · March 29, 2025, 4:57pm

Please test the 7-segment separately with the power supply/battery. Test all the segments. See if the 'A' segment lights up or not.

xfpd · March 29, 2025, 5:25pm

Your second image looks like the wiring is good. I would guess the "default" case is clearing all segments very quickly but "A" first, so you are seeing only that segment cleared.

alto777 · March 29, 2025, 5:26pm

You are using pins 0 and 1 in your effort to do things port wide at a time.

Using pins 0 and 1 is fraught. I bet if you weren't eveything would function to plan.

a7

alto777 · March 29, 2025, 5:42pm

Was my theory. But I did get your sketch to work.

I had to pinMode() all PORTD pins to OUTPUT.

The sense of your select line and the digits is wrong for both common anode and common cathode. The simple fix here was to turn on bit 7 and use a common anode display.

        PORTD = seg_disp[i] | 0x80;

which I only did in the case where you just count off.

With those changes, it counts pererctly.

Without those changes, I have no idea how you were seeing anything that made any sense at all...

Try it here:

And here's your code, my adjustments and srsly, get off pins 0 and 1 so you have the serial monitor available. I never understand ppl who try to get along with the first line of information on what the sketch is actual doing.

I did not test the button or the other cases. I just wanted to see that 'A' segment properly illuninating. Which it does.

// https://wokwi.com/projects/426781912952646657

#define PORTD_CONFIG (0xFF)

#define SEG_0  (0x40)  // 01000000 - All segments except g (a, b, c, d, e, f)
#define SEG_1  (0x79)  // 01111001 - Only segments b and c
#define SEG_2  (0x24)  // 00100100 - Segments a, b, d, e, g
#define SEG_3  (0x30)  // 00110000 - Segments a, b, c, d, g
#define SEG_4  (0x19)  // 00011001 - Segments b, c, f, g
#define SEG_5  (0x12)  // 00010010 - Segments a, c, d, f, g
#define SEG_6  (0x02)  // 00000010 - Segments a, c, d, e, f, g
#define SEG_7  (0x78)  // 01111000 - Only segments a, b, c
#define SEG_8  (0x00)  // 00000000 - All segments a-g
#define SEG_9  (0x10)  // 00010000 - Segments a, b, c, d, f, g  
#define STOP_DISP     (0xFF)
#define BUTTON1       (8)
#define SEG_DEL       delay(500)
int count = 0;

void setup() {

  DDRD = PORTD_CONFIG;
  pinMode(BUTTON1, INPUT);
  pinMode(0, OUTPUT);
  pinMode(1, OUTPUT);
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);

  PORTD = STOP_DISP;
}

void loop() {
  
  unsigned int seg_disp[] = {SEG_0, SEG_1, SEG_2, SEG_3, SEG_4, SEG_5, SEG_6, SEG_7, SEG_8, SEG_9};
  bool buttonState = LOW;
  bool lastButtonState = LOW;

  // Read button state with debouncing
  buttonState = digitalRead(BUTTON1);
 
  if (buttonState != lastButtonState) {
    SEG_DEL;
 
    buttonState = digitalRead(BUTTON1);
  }
  
  // Check for button press (LOW to HIGH transition)
  if (buttonState == HIGH && lastButtonState == LOW) {
    count++;
    if (count >= 4) count = 0;
  }
  
  // Update the last button state
  lastButtonState = buttonState;
 switch (count) {
    case 0:  
      for (int i = 0; i < 10; i++) {
        PORTD = seg_disp[i] | 0x80;
        SEG_DEL;
      }
      break;

    case 1:  
      for (int i = 1; i <= 9; i += 2) {
        PORTD = seg_disp[i];
        SEG_DEL;
      }
      break;

    case 2:  
      for (int i = 9; i >= 0; i--) {
        PORTD = seg_disp[i];
        SEG_DEL;
      }
      break;

    case 3:  
      for (int i = 0; i <= 8; i += 2) {
        PORTD = seg_disp[i];
        SEG_DEL;
      }
      break;

    default:
      count = 0;
      break;
  }
}

HTH

a7

ShermanP · March 29, 2025, 5:46pm

This may be a long shot, but...

It looks like you are using a single current-limiting resistor on the common anode pin of the display. If there is something not quite right about the connection to segment A, and since all of the lit segments have to share the fixed current, then it may be that not enough current is getting to A to light it up. And if A is a bit dim even when 7 is displayed, that may be because 7 is the only digit where only two other segments are lit up along with A, so at least a little current is getting to A on a 7.

So the solution may be to correct the bad connection to A. It could be inside the display, but more likely a bad connection on the breadboard, or a bad jumper (I've wasted hours on bad jumpers).

But you really need to put resistors on each segment line, and connect the common anode to the 5V rail. Then each segment is equally bright no matter how many are lit up.

I said it was a long shot.

alto777 · March 29, 2025, 5:58pm

I like it!

I am no longer in the lab so I cannot repeat my early experiments, but even the simulator which needs no stinkin' resistors did not show the segments properly.

At least part of the problem was the non-output nature of pins 3 .. 7.

The other was the incorrect pin 7 common lead. It has to be opposite of the segment select bits for current to flow.

Which made me wonder what @nexuscryo has in front of her and what she is actually seeing.

a7

ShermanP · March 29, 2025, 9:47pm

Ok. I set up a Nano with a common anode display, and flashed the OP's exact code to the Nano. I initially got D0 and D1 reversed because on the Nano, TX is the first pin, but is D1. Anyway, after getting that right...

I get exactly the same result as the OP. No segment A, except dimly on "7". However, segment B is also a bit dim, but not as dim as A.

I think the explanation is so simple that I'm embarrassed. It's the current drain through the Nano's onboard Tx and Rx LEDs, which of course correspond to D0 and D1. I don't know why there's a difference between A and B. I thought the onboard current limiting resistors were the same value, but perhaps not.

Actually, this just dawned on me - they are the same value, but there's a difference at the UART chip, which is connected to both pins. D0 on the Nano is Rx, which is connected to the Tx pin of the UART chip, which is probably driven high, which prevents D0 from taking A low.

So, [a few minutes later] I've tested this on a Pro Mini, which has no LEDs on Tx and Rx, and doesn't have a UART chip. When powered from the computer through an FT232 UART adapter, I have the same problem with segment A, but when I power it from a separate dumb supply, everything works.

So this just further confirms that God doesn't want you to use D0 and D1 as GPIOs.

alto777 · March 30, 2025, 4:59pm

OK, I finally see

  DDRD = PORTD_CONFIG;

which takes care of the pin modes.

But I am baffled by @ShermanP's success with the unmodified code from #1.

Common anode means the segment cathodes must be taken LOW, and the common anode must be driven HIGH.

~~But all the constants while properly taking the segments to 0 leave the anode~~

Argh! Never mind, the common anode is just connect to Vcc. I did not sqwuint enough at the diagram.

So... yes, you prolly want a separate resistor for each segment and... when you ever use more than one even segment display, you can do the multiplexing by taking individual anodes HIGH one at a time...

I am losing my mind, but at least not in this case.

a7

ShermanP · March 30, 2025, 6:24pm

Having individual resistors on each segment is supposed to be standard practice, but to tell the truth, in my test everything looked pretty good. Yes, the segments on a "1" are brighter than the segments on an "8", but you have to be looking for it to notice. But I guess if you had multiple digits, the difference would be more noticeable.

Anyway, I think the bottom line is to move the A and B segments to D8 and D9, which are the bottom two bits of port B, and move the button input to D10 or whatever. Then updating would just be a two-step process. I haven't tested this, except to compile it:

[Edit: This code doesn't work. See the following posts]

#define PORTD_CONFIG (0xFC)
#define PORTB_CONFIG (0x03)

#define SEG_0  (0x40)  // 01000000 - All segments except g (a, b, c, d, e, f)
#define SEG_1  (0x79)  // 01111001 - Only segments b and c
#define SEG_2  (0x24)  // 00100100 - Segments a, b, d, e, g
#define SEG_3  (0x30)  // 00110000 - Segments a, b, c, d, g
#define SEG_4  (0x19)  // 00011001 - Segments b, c, f, g
#define SEG_5  (0x12)  // 00010010 - Segments a, c, d, f, g
#define SEG_6  (0x02)  // 00000010 - Segments a, c, d, e, f, g
#define SEG_7  (0x78)  // 01111000 - Only segments a, b, c
#define SEG_8  (0x00)  // 00000000 - All segments a-g
#define SEG_9  (0x10)  // 00010000 - Segments a, b, c, d, f, g  
//#define STOP_DISP     (0xFF)
#define BUTTON1       (10)
#define SEG_DEL       delay(500)
int count = 0;

void setup() {

  DDRD = DDRD | PORTD_CONFIG;
  DDRB = DDRB | PORTB_CONFIG;
  
  pinMode(BUTTON1, INPUT);
//  pinMode(0, OUTPUT);
//  pinMode(1, OUTPUT);

  PORTD = PORTD | PORTD_CONFIG;
  PORTB = PORTB | PORTB_CONFIG;

}

void loop() {
  
  unsigned int seg_disp[] = {SEG_0, SEG_1, SEG_2, SEG_3, SEG_4, SEG_5, SEG_6, SEG_7, SEG_8, SEG_9};
  bool buttonState = LOW;
  bool lastButtonState = LOW;

  // Read button state with debouncing
  buttonState = digitalRead(BUTTON1);
 
  if (buttonState != lastButtonState) {
    SEG_DEL;
 
    buttonState = digitalRead(BUTTON1);
  }
  
  // Check for button press (LOW to HIGH transition)
  if (buttonState == HIGH && lastButtonState == LOW) {
    count++;
    if (count >= 4) count = 0;
  }
  
  // Update the last button state
  lastButtonState = buttonState;
 switch (count) {
    case 0:  
      for (int i = 0; i < 10; i++) {
        PORTD = PORTD & (seg_disp[i] | ~PORTD_CONFIG);
        PORTB = PORTB & (seg_disp[i] | ~PORTB_CONFIG);
        SEG_DEL;
      }
      break;

    case 1:  
      for (int i = 1; i <= 9; i += 2) {
        PORTD = PORTD & (seg_disp[i] | ~PORTD_CONFIG);
        PORTB = PORTB & (seg_disp[i] | ~PORTB_CONFIG);
        SEG_DEL;
      }
      break;

    case 2:  
      for (int i = 9; i >= 0; i--) {
        PORTD = PORTD & (seg_disp[i] | ~PORTD_CONFIG);
        PORTB = PORTB & (seg_disp[i] | ~PORTB_CONFIG);
        SEG_DEL;
      }
      break;

    case 3:  
      for (int i = 0; i <= 8; i += 2) {
        PORTD = PORTD & (seg_disp[i] | ~PORTD_CONFIG);
        PORTB = PORTB & (seg_disp[i] | ~PORTB_CONFIG);
        SEG_DEL;
      }
      break;

    default:
      count = 0;
      break;
  }
}

alto777 · March 30, 2025, 6:45pm

I did. I saw what you meant and fixed it for one case. You just want the 1s from the seg_disp array entry placed into the bits of the two registers. Everything is already lined up and ready to go:

 switch (count) {
    case 0:  
      for (int i = 0; i < 10; i++) {

/*
# define PORTD_CONFIG (0xFC)
# define PORTB_CONFIG (0x03)
*/

//        PORTD = PORTD & (seg_disp[i] | ~PORTD_CONFIG);
//        PORTB = PORTB & (seg_disp[i] | ~PORTB_CONFIG);

        PORTD = PORTD_CONFIG & seg_disp[i];
        PORTB = PORTB_CONFIG & seg_disp[i];

       SEG_DEL;
      }
      break;

I assume the other cases will function properly with the same change.

a7

ShermanP · March 30, 2025, 8:08pm

Well, your change messes with the bits not involved in the display, including D0 and D1, which I was trying to avoid. Did my code not work?

alto777 · March 30, 2025, 9:10pm

So it does. nice catch. It's been a minute since I've needed to care about bits I don't care about!

I can forgive you for wondering. I am not in the habit of fixing code that works, or fixing code I don't know is broken so no, your code did not work.

I won't claim it did not work for you. When you did not test it. Just now I wrote

        PORTD = PORTD & ~PORTD_CONFIG | seg_disp[i] & PORTD_CONFIG;
        PORTB = PORTB & ~PORTB_CONFIG | seg_disp[i] & PORTB_CONFIG;

which reads to me as preserving the non-config (output) bits, and inserting the bits from the seg_disp element.

And works.

a7

ShermanP · March 30, 2025, 9:23pm

I've tested my code, and no, it doesn't work. But yes, I arrived at the same solution. I also changed the array from int to byte, not that it really makes any difference. So here's the "final" version:

#define PORTD_CONFIG (0xFC)
#define PORTB_CONFIG (0x03)

#define SEG_0  (0x40)  // 01000000 - All segments except g (a, b, c, d, e, f)
#define SEG_1  (0x79)  // 01111001 - Only segments b and c
#define SEG_2  (0x24)  // 00100100 - Segments a, b, d, e, g
#define SEG_3  (0x30)  // 00110000 - Segments a, b, c, d, g
#define SEG_4  (0x19)  // 00011001 - Segments b, c, f, g
#define SEG_5  (0x12)  // 00010010 - Segments a, c, d, f, g
#define SEG_6  (0x02)  // 00000010 - Segments a, c, d, e, f, g
#define SEG_7  (0x78)  // 01111000 - Only segments a, b, c
#define SEG_8  (0x00)  // 00000000 - All segments a-g
#define SEG_9  (0x10)  // 00010000 - Segments a, b, c, d, f, g  
//#define STOP_DISP     (0xFF)
#define BUTTON1       (10)
#define SEG_DEL       delay(500)
int count = 0;

void setup() {

  DDRD = DDRD | PORTD_CONFIG;
  DDRB = DDRB | PORTB_CONFIG;
  
  pinMode(BUTTON1, INPUT);
//  pinMode(0, OUTPUT);
//  pinMode(1, OUTPUT);

  PORTD = PORTD | PORTD_CONFIG;
  PORTB = PORTB | PORTB_CONFIG;

}

void loop() {
  
  byte seg_disp[] = {SEG_0, SEG_1, SEG_2, SEG_3, SEG_4, SEG_5, SEG_6, SEG_7, SEG_8, SEG_9};
  bool buttonState = LOW;
  bool lastButtonState = LOW;

  // Read button state with debouncing
  buttonState = digitalRead(BUTTON1);
 
  if (buttonState != lastButtonState) {
    SEG_DEL;
 
    buttonState = digitalRead(BUTTON1);
  }
  
  // Check for button press (LOW to HIGH transition)
  if (buttonState == HIGH && lastButtonState == LOW) {
    count++;
    if (count >= 4) count = 0;
  }
  
  // Update the last button state
  lastButtonState = buttonState;
 switch (count) {
    case 0:  
      for (int i = 0; i < 10; i++) {
        PORTD = (PORTD & ~PORTD_CONFIG) | (seg_disp[i] & PORTD_CONFIG);
        PORTB = (PORTB & ~PORTB_CONFIG) | (seg_disp[i] & PORTB_CONFIG);
        SEG_DEL;
      }
      break;

    case 1:  
      for (int i = 1; i <= 9; i += 2) {
        PORTD = (PORTD & ~PORTD_CONFIG) | (seg_disp[i] & PORTD_CONFIG);
        PORTB = (PORTB & ~PORTB_CONFIG) | (seg_disp[i] & PORTB_CONFIG);
        SEG_DEL;
      }
      break;

    case 2:  
      for (int i = 9; i >= 0; i--) {
        PORTD = (PORTD & ~PORTD_CONFIG) | (seg_disp[i] & PORTD_CONFIG);
        PORTB = (PORTB & ~PORTB_CONFIG) | (seg_disp[i] & PORTB_CONFIG);
        SEG_DEL;
      }
      break;

    case 3:  
      for (int i = 0; i <= 8; i += 2) {
        PORTD = (PORTD & ~PORTD_CONFIG) | (seg_disp[i] & PORTD_CONFIG);
        PORTB = (PORTB & ~PORTB_CONFIG) | (seg_disp[i] & PORTB_CONFIG);
        SEG_DEL;
      }
      break;

    default:
      count = 0;
      break;
  }
}

I guess nobody has tested the button press logic. At least I haven't.

system · September 26, 2025, 9:24pm

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.

Topic		Replies	Views
7 Segment Countdown General Guidance	27	126	February 23, 2026
Malfunctioning 4-digit,7 segment display LEDs and Multiplexing	72	1231	April 15, 2025
The easiest way to control a multiplexed 7-segment display unit! Showcase	45	782	February 5, 2026
Seven segment display is wrong LEDs and Multiplexing	9	189	November 17, 2025
2 digit display with Sevseg library - "A" segment staying on Programming	18	540	August 31, 2024

'A' Part or Top Part of Seven Segment Not Working Properly

Related topics