I've gone as far as I can go, can't get multiplexing to work

7 segment display, atmega328 to control it. 4 buttons. The buttons control each digit respectively. If I keep pressing a button the appropriate digit will cycle through 0-9 functions and correctly displays 0-9.

Here's the problem, the " turnOffAllExcept" function I have doesn't do what it was supposed to do(I'm assuming it's the culprit) and when pressing the buttons, all the numbers change instead of just their specific ones. Now I thought the way it's supposed to work is turn on one digit, display what I want, turn off the rest, then turn on the next digit, display what I want, turn off the previous one and the next two, etc.etc. Then do that more than 50 times a second. what can I do to fix it? I've been looking at this for days and just can't see it. :confused:

//set integers for switches
const int S1 = A3;
const int S2 = A2;
const int S3 = A1;
const int S4 = A0;
//set integers for 7 segment display
const int SegA = 11;
const int SegB = 10;
const int SegC = 9;
const int SegD = 8;
const int SegE = 5;
const int SegF = 4;
const int SegG = 3;
const int SegDP = 2; //decimal point
const int SegCOL = 1; //colon, ":"
//set integers for digits.(i.e D1 is equal to digit 1 and also T1, transistor 1, on board. 
const int D1 = 13;
const int D2 = 12;
const int D3 = 7;
const int D4 = 6;
//track swith read value
int buttonState1 = HIGH;
int buttonState2 = HIGH;
int buttonState3 = HIGH;
int buttonState4 = HIGH;
int counterD1 = 0;
int counterD2 = 0;
int counterD3 = 0;
int counterD4 = 0;

void (*DigitFunctions[10])( int );
void turnOffAllExcept(int targetDigit)
{
  int digits[4] = {
    D1, D2, D3, D4    };
  for(int i=0; i<4; i++){
    if(i != targetDigit){
      digits[i] = LOW;

    }
  }
}

//N1
void turnOnNumber1(int targetDigit){
  digitalWrite(SegA, HIGH);
  digitalWrite(SegB, LOW);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, HIGH);
  digitalWrite(SegE, HIGH);
  digitalWrite(SegF, HIGH);
  digitalWrite(SegG, HIGH);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

//N2
void turnOnNumber2(int targetDigit){
  digitalWrite(SegA, LOW);
  digitalWrite(SegB, LOW);
  digitalWrite(SegC, HIGH);
  digitalWrite(SegD, LOW);
  digitalWrite(SegE, LOW);
  digitalWrite(SegF, HIGH);
  digitalWrite(SegG, LOW);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

void turnOnNumber3(int targetDigit){
  digitalWrite(SegA, LOW);
  digitalWrite(SegB, LOW);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, LOW);
  digitalWrite(SegE, HIGH);
  digitalWrite(SegF, HIGH);
  digitalWrite(SegG, LOW);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

void turnOnNumber4(int targetDigit){
  digitalWrite(SegA, HIGH);
  digitalWrite(SegB, LOW);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, HIGH);
  digitalWrite(SegE, HIGH);
  digitalWrite(SegF, LOW);
  digitalWrite(SegG, LOW);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

void turnOnNumber5(int targetDigit){
  digitalWrite(SegA, LOW);
  digitalWrite(SegB, HIGH);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, LOW);
  digitalWrite(SegE, HIGH);
  digitalWrite(SegF, LOW);
  digitalWrite(SegG, LOW);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

void turnOnNumber6(int targetDigit){
  digitalWrite(SegA, LOW);
  digitalWrite(SegB, HIGH);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, LOW);
  digitalWrite(SegE, LOW);
  digitalWrite(SegF, LOW);
  digitalWrite(SegG, LOW);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

void turnOnNumber7(int targetDigit){
  digitalWrite(SegA, LOW);
  digitalWrite(SegB, LOW);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, HIGH);
  digitalWrite(SegE, HIGH);
  digitalWrite(SegF, HIGH);
  digitalWrite(SegG, HIGH);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

void turnOnNumber8(int targetDigit){
  digitalWrite(SegA, LOW);
  digitalWrite(SegB, LOW);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, LOW);
  digitalWrite(SegE, LOW);
  digitalWrite(SegF, LOW);
  digitalWrite(SegG, LOW);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}
void turnOnNumber9(int targetDigit){
  digitalWrite(SegA, LOW);
  digitalWrite(SegB, LOW);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, HIGH);
  digitalWrite(SegE, HIGH);
  digitalWrite(SegF, LOW);
  digitalWrite(SegG, LOW);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

void turnOnNumber0(int targetDigit){
  digitalWrite(SegA, LOW);
  digitalWrite(SegB, LOW);
  digitalWrite(SegC, LOW);
  digitalWrite(SegD, LOW);
  digitalWrite(SegE, LOW);
  digitalWrite(SegF, LOW);
  digitalWrite(SegG, HIGH);
  digitalWrite(SegDP, HIGH);
  turnOffAllExcept(targetDigit);
}

void setup() {
  //set switches as inputs with internal pullup resistor
  pinMode(S1, INPUT_PULLUP);
  pinMode(S2, INPUT_PULLUP);
  pinMode(S3, INPUT_PULLUP);
  pinMode(S4, INPUT_PULLUP);
  pinMode(SegA, OUTPUT);
  pinMode(SegB, OUTPUT);
  pinMode(SegC, OUTPUT);
  pinMode(SegD, OUTPUT);
  pinMode(SegE, OUTPUT);
  pinMode(SegF, OUTPUT);
  pinMode(SegG, OUTPUT);
  pinMode(SegDP, OUTPUT);
  pinMode(SegCOL, OUTPUT);
  pinMode(D1, OUTPUT);
  pinMode(D2, OUTPUT);
  pinMode(D3, OUTPUT);
  pinMode(D4, OUTPUT);

  DigitFunctions[0] = &turnOnNumber0;
  DigitFunctions[1] = &turnOnNumber1;
  DigitFunctions[2] = &turnOnNumber2;
  DigitFunctions[3] = &turnOnNumber3;
  DigitFunctions[4] = &turnOnNumber4;
  DigitFunctions[5] = &turnOnNumber5;
  DigitFunctions[6] = &turnOnNumber6;
  DigitFunctions[7] = &turnOnNumber7;
  DigitFunctions[8] = &turnOnNumber8;
  DigitFunctions[9] = &turnOnNumber9;

}




void loop() {

  //Keeps colon sign on always
  digitalWrite(SegCOL, LOW);

  if(didPressButtonDown(S1, buttonState1)){
    if(counterD1 == 9){
      counterD1 = -1;// it will be incremented to 0 on the next line
    }
    turnOnNumberForDigit(++counterD1, D1);
  }
  if(didPressButtonDown(S2, buttonState2)){
    if(counterD2 == 9){
      counterD2 = -1;// it will be incremented to 0 on the next line
    }
    turnOnNumberForDigit(++counterD2, D2);
  }
  if(didPressButtonDown(S3, buttonState3)){
    if(counterD3 == 9){
      counterD3 = -1;// it will be incremented to 0 on the next line
    }
    turnOnNumberForDigit(++counterD3, D3);
  }
  if(didPressButtonDown(S4, buttonState4)){
    if(counterD4 == 9){
      counterD4 = -1;// it will be incremented to 0 on the next line
    }
    turnOnNumberForDigit(++counterD4, D4);
  }




  delay(250);  
}

bool didPressButtonDown(int button, int buttonState)
{
  int tempValue = digitalRead(button);
  if(tempValue == LOW &&  tempValue != buttonState){
    buttonState = LOW;
    return true;
  }
  else if(tempValue == HIGH && tempValue != buttonState){
    buttonState = HIGH;
  }
  return false;
}

void turnOnNumberForDigit(int value, int targetDigit)
{
  void (*turnOnNumber)(int targetDigit);
  turnOnNumber = DigitFunctions[value];//get function N# where # == value
  turnOnNumber(targetDigit);
}

and the schematic:

  int digits[4] = {
    D1, D2, D3, D4    };

Perhaps a better choice for the variable name would be

  int maryHadALittleLamb[4] = {
    D1, D2, D3, D4    };

No, wait, that doesn't make sense. When storing values, like pin numbers, the name should reflect what is being stored, shouldn't it?

    if(i != targetDigit){
      digits[i] = LOW;

How does changing the pin number help? Changing the STATE of the pin whose NUMBER is in the ith position might...

If you explained how it is supposed to work it would help.

It looks like you set the segments for a digit and then something else selects which diigit is illuminated.

Is it the case, for example, that setting pin13 HIGH causes digit 1 to illuminate? If so it should be pretty simple with an array of the 4 pin number to do something like this

byte digitPin[] = {13, 12, 7, 6};

byte selectedDigit = 2; // any of 0 .. 3

for (byte n = 0; n < 4; n++) {
    digitalWrite(digitPIn[n], LOW); // all off
}
digitalWrite(digitPin[selectedDigit], HIGH);

...R

PaulS:

  int digits[4] = {

D1, D2, D3, D4    };



Perhaps a better choice for the variable name would be


int maryHadALittleLamb[4] = {
    D1, D2, D3, D4    };




No, wait, that doesn't make sense. When storing values, like pin numbers, the name should reflect what is being stored, shouldn't it?

Each **DIGIT** of the 7 segment led is switched on and off by a transistor. Hence the naming and of pin being...digit as I associate D1 or digit one with the first digit of the 4 digit LED. Perhaps next time I should clear it with you whether or not the naming makes sense...**to me.**



if(i != targetDigit){
      digits[i] = LOW;



How does changing the pin number help? Changing the STATE of the pin whose NUMBER is in the ith position might...

I thought thats what this was doing, changing the state of the pin. Does this code:

int digits[4] = {
    D1, D2, D3, D4    };
  for(int i=0; i<4; i++){
    if(i != targetDigit){
      digits[i] = LOW;

not cycle through all 4 digits turning them off in secession expect for the digit that is called out as target digit?

Robin2:
If you explained how it is supposed to work it would help.

when you press a button, it reads the button state of one of 4 digits for the 4 digit 7 seg LED. the bool didpressbuttondown stores the value of true or false. The if function in the loop then, when pressing the button, uses the turnonnumberfordigit function to work within a function array which will then cycle through 10 functions that are for each number 0-9. They turn on and off the correct segment to form the number. When it gets to 9, it resets. to 0. The code in the beginning for turnoffAllExcept function is supposed to cycle through all four digits, turning them off, except for the targetDigit. The targetDigit is set by the digit D1, D2, etc, called out here:

turnOnNumberForDigit(++counterD2, D2);

But I don't think that's working as all the digits light up the same thing, which is how these devices work if you just turn on all the digits at once. But if I do function for number 1 and turn on digit one, then function for number 2 and turn off one and turn on 2, etc., etc., up to function 4; then it should display 1 2 3 4...providing it's flashing faster than 50 times a second.

But it doesn't.

Yes, setting any of the D pins causes the coresponding digit to illuminate. I'm still pretty new to coding in general and my brother wrote half the code for me. I just set up the pins and made the 0-9 functions. So on that note, I'm going to try what you said and see if I can integrate that into what I have. Thank you.

red913:
I thought thats what this was doing, changing the state of the pin. Does this code:

int digits[4] = {

D1, D2, D3, D4    };
  for(int i=0; i<4; i++){
    if(i != targetDigit){
      digits[i] = LOW;



not cycle through all 4 digits turning them off in secession expect for the digit that is called out as target digit?

No. digits is an array of pin numbers, not pin states. You have to use digitalWrite to change the state of a pin. This is a prime example of why PaulS made note of the fact that "digits" is not descriptive enough. It should be something like "digitPins", and that should make it more clear as to why assigning a state to a pin variable makes no sense.

oh ok, that makes sense. I never was unclear as to what the digits was doing it's just I was getting mixed up. I was seeing that I'm setting those digits to low but forgetting you have to put it as digitalWrite. In my mind it was changing the correct pin. Naming would have changed nothing for me.

But I put the code in you suggested, in lieu of what I had before I even tried:

   digitalWrite(digits[i],LOW);

Both solutions still left me with the same problem. :confused:

I was seeing that I'm setting those digits to low but forgetting you have to put it as digitalWrite. In my mind it was changing the correct pin.

Suppose that the array had been called digitalPinNumbers. Would you have made the same assumption? If so, programming may not be the career of choice for you.

Both solutions still left me with the same problem.

We can't see either code.