im trying to switch number on the display using a potentiometer, it is just counting from 0/9. All help is much appreciated!

int SegmentA = 6;
int SegmentB = 7;
int SegmentC = 8;
int SegmentD = 9;
int SegmentE = 10;
int SegmentF = 11;
int SegmentG = 12;


const int PP = A0;


void setup()
{
  pinMode (SegmentA, OUTPUT);
  pinMode (SegmentB, OUTPUT);
  pinMode (SegmentC, OUTPUT);
  pinMode (SegmentD, OUTPUT);
  pinMode (SegmentE, OUTPUT);
  pinMode (SegmentF, OUTPUT);
  pinMode (SegmentG, OUTPUT);


}

void loop() {

  int POD = analogRead(PP);
  int SM = map(POD, 0, 1023, 0 , 8);
  switch (SM) {


    case 0:
      digitalWrite (SegmentA, HIGH);
      digitalWrite (SegmentB, HIGH);
      digitalWrite (SegmentC, HIGH);
      digitalWrite (SegmentD, HIGH);
      digitalWrite (SegmentE, HIGH);   //0
      digitalWrite (SegmentF, HIGH);
      digitalWrite (SegmentG, LOW);

      delay(1000);

    case 1:
      digitalWrite (SegmentA, LOW);
      digitalWrite (SegmentB, HIGH);
      digitalWrite (SegmentC, HIGH);
      digitalWrite (SegmentD, LOW);
      digitalWrite (SegmentE, LOW);   //1
      digitalWrite (SegmentF, LOW);
      digitalWrite (SegmentG, LOW);

      delay(1000);

    case 2:
      digitalWrite (SegmentA, HIGH);
      digitalWrite (SegmentB, HIGH);
      digitalWrite (SegmentC, LOW);
      digitalWrite (SegmentD, HIGH);
      digitalWrite (SegmentE, HIGH);   //2
      digitalWrite (SegmentF, LOW);
      digitalWrite (SegmentG, HIGH);

      delay(1000);

    case 3:
      digitalWrite (SegmentA, HIGH);
      digitalWrite (SegmentB, HIGH);
      digitalWrite (SegmentC, HIGH);
      digitalWrite (SegmentD, HIGH);
      digitalWrite (SegmentE, LOW);   //3
      digitalWrite (SegmentF, LOW);
      digitalWrite (SegmentG, HIGH);

      delay(1000);

    case 4:
      digitalWrite (SegmentA, LOW);
      digitalWrite (SegmentB, HIGH);
      digitalWrite (SegmentC, HIGH);
      digitalWrite (SegmentD, LOW);
      digitalWrite (SegmentE, LOW);   //4
      digitalWrite (SegmentF, HIGH);
      digitalWrite (SegmentG, HIGH);

      delay(1000);

    case 5:
      digitalWrite (SegmentA, HIGH);
      digitalWrite (SegmentB, LOW);
      digitalWrite (SegmentC, HIGH);
      digitalWrite (SegmentD, HIGH);
      digitalWrite (SegmentE, LOW);   //5
      digitalWrite (SegmentF, HIGH);
      digitalWrite (SegmentG, HIGH);

      delay(1000);

    case 6:
      digitalWrite (SegmentA, HIGH);
      digitalWrite (SegmentB, LOW);
      digitalWrite (SegmentC, HIGH);
      digitalWrite (SegmentD, HIGH);
      digitalWrite (SegmentE, HIGH);   //6
      digitalWrite (SegmentF, HIGH);
      digitalWrite (SegmentG, HIGH);

      delay(1000);

    case 7:
      digitalWrite (SegmentA, HIGH);
      digitalWrite (SegmentB, HIGH);
      digitalWrite (SegmentC, HIGH);
      digitalWrite (SegmentD, LOW);
      digitalWrite (SegmentE, LOW);   //7
      digitalWrite (SegmentF, LOW);
      digitalWrite (SegmentG, LOW);

      delay(1000);

    case 8:
      digitalWrite (SegmentA, HIGH);
      digitalWrite (SegmentB, HIGH);
      digitalWrite (SegmentC, HIGH);
      digitalWrite (SegmentD, HIGH);
      digitalWrite (SegmentE, HIGH);   //8
      digitalWrite (SegmentF, HIGH);
      digitalWrite (SegmentG, HIGH);

  }

}
  • 1
    I would shorten the delay (1000) to be much less - otherwise you turn the pot, nothing happens, you turn it some and finally something happens but now you've overshot what you wanted, and you overdo it the other way trying to get to where you wanted. Try a delay of 50 to 100, have it be much more reactive to turning the knob. You can then move on to using a rotary decoder with detents, one click for one number change. Here's a poorly lit example with 2 digits and rotary encoder. In this case, a shift register was used to drive each display (vs multiplexing). – CrossRoads Nov 13 at 17:53
  • youtube.com/watch?v=f51eSlcZt-g The link. – CrossRoads Nov 13 at 17:54
up vote 9 down vote accepted

You miss the break statement at the end of each case. e.g.:

  digitalWrite (SegmentF, LOW);
  digitalWrite (SegmentG, HIGH);

  delay(1000);

  break;

case 4:

This causes each case to perform, and continue with the next case. However, since there is a delay of 1 second, it looks like it is counting.

Actually, I would expect it counts from a certain value depending on the pot meter to 7, since there is no digit 9 implemented, and the delay for the 8 digit is missing too.

To further improve your code, you can add the delay not in every case but at the end, since I assume there always will be one digit selected.

Also you can make a function void showDigit with a parameter for each of the possible segment states and replace all (almost) duplicated code by that function call, or send the digit and use showDigit to set internally the states of each segment (by using a boolean array[nr_of_digits][nr_of_segments], somewhat like Nic Hartley proposes below.

  • Thank you, this fixes my problem! – user51180 Nov 13 at 18:39
  • 2
    I would add that you don't even need that switch -- you can easily have a const bool digits[7 * 10], then pass digits[7 * digit], and in the function pull from param[0], param[1], etc. – Nic Hartley Nov 13 at 21:42
  • @NicHartley Thanks ... I think a two dimensional array is than even better, but the use of an array is better in general. – Michel Keijzers Nov 14 at 9:10
  • 1
    Actually, strictly speaking, you could also do a const uint8_t digits[10];, then access the individual segments with digits[n] & (1 << segment), if you wanted it to be the least readable possible code. I wouldn't recommend that over the array (probably 2D array) approach, though, unless you have some extremely tight memory requirements. – Nic Hartley Nov 14 at 17:54
  • 1
    @MichelKeijzers I suggested it only for the very rare circumstances where you find yourself exhausting all 2KB and need to compact things down even more. It's definitely not a good thing to start with, I agree. – Nic Hartley Nov 14 at 21:08

You'll want a single delay (if any) at the bottom of the loop() function. You may not need it since it will keep displaying the current digit until you move the potentiometer. Exception: if the pot is near the edge between 2 digits, you could see some dithering.

Replace the delay-calls in the cases with break; statements. Otherwise whichever case you call will fall through into the case below it, and so on, displaying subsequent each digit in turn, which isn't what you wanted (and is what you are already seeing).

Change the map() call to map(POD, 0, 1024, 0 , 10). Otherwise there will be no '9' band on potentiometer and the '8' band will be only 1 value wide (out of 1023): exactly the value "1023". You'd like the '9' band to be about as wide as all of the others. It's even possible that a given potentiometer won't get close enough to zero resistance for you to get a 1023 reading.

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.