0

I need to write a string of number such as 1234567 on a 7 LED display. For example 1 will be lit up on display for 0.5 seconds, it will turn off then 2 will light up and so on. In the end I need to display "-" for 2 seconds. I also added the image of the given task.Given Task

The following is the code I wrote. I would like to ask if it is the correct, and are there any flaws.

  const int LEDa=13;
  const int LEDb=12;
  const int LEDc=11;
  const int LEDd=10;
  const int LEDe=9;
  const int LEDf=8;
  const int LEDg=7;

  int i;
  int a;

  void setup() {

    pinMode(LEDa, OUTPUT);
    pinMode(LEDb, OUTPUT);
    pinMode(LEDc, OUTPUT);
    pinMode(LEDd, OUTPUT);
    pinMode(LEDe, OUTPUT);
    pinMode(LEDf, OUTPUT);
    pinMode(LEDg, OUTPUT);

  }

  void loop() {
    int array[7]= {1,8,0,5,1,0,0};
    for(i=0;i<8;i++){
      a= array[i];
      }
        switch(a){
          case 1:
          digitalWrite(LEDb,HIGH);
          digitalWrite(LEDc,HIGH);
          delay(500);
          digitalWrite(LEDb,LOW);
          digitalWrite(LEDc,LOW);
          break;
          case 2:
          digitalWrite(LEDa,HIGH);
          digitalWrite(LEDb,HIGH);
          digitalWrite(LEDg,HIGH);
          digitalWrite(LEDe,HIGH);
          digitalWrite(LEDd,HIGH);
          delay(500);
          digitalWrite(LEDa,LOW);
          digitalWrite(LEDb,LOW);
          digitalWrite(LEDg,LOW);
          digitalWrite(LEDe,LOW);
          digitalWrite(LEDd,LOW);
          break;
          case 3:
          digitalWrite(LEDa,HIGH);
          digitalWrite(LEDb,HIGH);
          digitalWrite(LEDc,HIGH);
          digitalWrite(LEDg,HIGH);
          digitalWrite(LEDd,HIGH);
          delay(500);
          digitalWrite(LEDa,LOW);
          digitalWrite(LEDb,LOW);
          digitalWrite(LEDc,LOW);
          digitalWrite(LEDg,LOW);
          digitalWrite(LEDd,LOW);
          break;
          case 4:
          digitalWrite(LEDf,HIGH);
          digitalWrite(LEDg,HIGH);
          digitalWrite(LEDb,HIGH);
          digitalWrite(LEDc,HIGH);
          delay(500);
          digitalWrite(LEDf,LOW);
          digitalWrite(LEDg,LOW);
          digitalWrite(LEDb,LOW);
          digitalWrite(LEDc,LOW);
          break;
          case 5:
          digitalWrite(LEDf,HIGH);
          digitalWrite(LEDg,HIGH);
          digitalWrite(LEDd,HIGH);
          digitalWrite(LEDc,HIGH);
          digitalWrite(LEDa,HIGH);
          delay(500);
          digitalWrite(LEDf,LOW);
          digitalWrite(LEDg,LOW);
          digitalWrite(LEDd,LOW);
          digitalWrite(LEDc,LOW);
          digitalWrite(LEDa,LOW);
          break;
          case 6:
          digitalWrite(LEDf,HIGH);
          digitalWrite(LEDg,HIGH);
          digitalWrite(LEDa,HIGH);
          digitalWrite(LEDc,HIGH);
          digitalWrite(LEDe,HIGH);
          digitalWrite(LEDd,HIGH);
          delay(500);
          digitalWrite(LEDf,LOW);
          digitalWrite(LEDg,LOW);
          digitalWrite(LEDa,LOW);
          digitalWrite(LEDc,LOW);
          digitalWrite(LEDe,LOW);
          digitalWrite(LEDd,LOW);
          break;
          case 7:
          digitalWrite(LEDa,HIGH);
          digitalWrite(LEDb,HIGH);
          digitalWrite(LEDc,HIGH);
          delay(500);
          digitalWrite(LEDa,LOW);
          digitalWrite(LEDb,LOW);
          digitalWrite(LEDc,LOW);
          break;
          case 8:
          digitalWrite(LEDa,HIGH);
          digitalWrite(LEDb,HIGH);
          digitalWrite(LEDc,HIGH);
          digitalWrite(LEDd,HIGH);
          digitalWrite(LEDe,HIGH);
          digitalWrite(LEDf,HIGH);
          digitalWrite(LEDg,HIGH);
          delay(500);
          digitalWrite(LEDa,LOW);
          digitalWrite(LEDb,LOW);
          digitalWrite(LEDc,LOW);
          digitalWrite(LEDd,LOW);
          digitalWrite(LEDe,LOW);
          digitalWrite(LEDf,LOW);
          digitalWrite(LEDg,LOW);
          break;
          case 9:
          digitalWrite(LEDa,HIGH);
          digitalWrite(LEDb,HIGH);
          digitalWrite(LEDc,HIGH);
          digitalWrite(LEDd,HIGH);
          digitalWrite(LEDf,HIGH);
          digitalWrite(LEDg,HIGH);
          delay(500);
          digitalWrite(LEDa,LOW);
          digitalWrite(LEDb,LOW);
          digitalWrite(LEDc,LOW);
          digitalWrite(LEDd,LOW);
          digitalWrite(LEDf,LOW);
          digitalWrite(LEDg,LOW);
          break;
          case 0:
          digitalWrite(LEDa,HIGH);
          digitalWrite(LEDb,HIGH);
          digitalWrite(LEDc,HIGH);
          digitalWrite(LEDd,HIGH);
          digitalWrite(LEDe,HIGH);
          delay(500);
          digitalWrite(LEDa,LOW);
          digitalWrite(LEDb,LOW);
          digitalWrite(LEDc,LOW);
          digitalWrite(LEDd,LOW);
          digitalWrite(LEDe,LOW);
          digitalWrite(LEDf,LOW);
          break;
          }
          if(i=8){
            digitalWrite(LEDg,HIGH);
            delay(2000);
            digitalWrite(LEDg,LOW);
            }
          }
  • 1
    So did you actually try to see if the code worked? You are asking people simply to check the code for errors. – MichaelT Mar 31 at 18:30
  • 1
    the switch is outside for – Juraj Apr 1 at 4:52
  • I have not yet constructed a circuit. This was an educational type of code that I needed to write. Besides asking fo error check I was wondering how can I make it more concise without the need of using cases, which in this case I got the answer for it. – E.Canberk Apr 1 at 8:11
  • You might be interested in my implementation. I’m using a shift register instead of a bunch of the Arduino’s pins, but it might give you some ideas. Check out the tests to see how it’s used. github.com/rubberduck203/digital-thermometer/blob/master/src/… – RubberDuck Apr 1 at 11:04
  • my comment is kind of picky, but this is programming after all ...... following instructions exactly is important ....... your description of the problem and the description in the picture differ ...... you said it will turn off .... there is no such directive in the picture ...... perhaps, going from 1 to 2 means leave segment "b" on, turn off segment "c" and turn on segments "a","g" and "d" – jsotola Apr 2 at 4:21
2

I see 3 errors:

  1. You define the array to have 7 elements, but in the for loop, you count 8 elements, by using i<8 (Array indizes start at 0, so 0 will be the first and 6 will be the last element). Currently you try to access an array element, that does not exist.

  2. The if statement is also not useful here. In your current code you check for i being equal to 8, but it will never be 8 inside of the for loop, because you used the condition i<8. To display the last - after the number, you should put the code after the for loop.

  3. In the same if statement you write i=8, but a single equality sign means assignment. The value 8 will be assigned to the variable i and this assignment is true everytime. If you want to test equality use == (double equality signs)


Tip: You can make your code smaller by defining functions for codes parts, that get repeated often (for example the code to turn off all LEDs).

Tip2: If you want the code to be very efficient, you can change the pins to use one single port. Then you can change all the pins of the port at once, by assigning a single byte to it. You can save these bytes in an array. This will make your code way shorter, but you also need a deeper understanding about port manipulations. You can choose, what you want to do.

0

It looks like it is mostly correct, although that is a very long-winded way to set 7 segments to multiple values. I would be more inclined to set up an array of bytes where 7 bits of each byte contain ones/zeros to control the LEDs. Then you could skip the massive switch statement and simply loop through the array of bytes.

One mistake: if(i=8) should be if (i == 8). (= is an assignment. You want the == comparison operator.) Your code won't work correctly until you fix that.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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