Direct Port Manipulation Hang?

I just learned how to use port manipulation, so I wrote some simple code to control a 7-segment LED display:

/*Flashes each segment on a 7 segment display for 250 milliseconds using
 direct port manipulation.
 
 Note that due to the way the display works, you MUST disconnect the display
 from pins 0 and 1 before uploading.
 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 Pins:
 DP  0
 C  1
 D  2
 E  3
 B  4
 A  5
 F  6
 G  7
 
 Arrangement:
 |-A-|
 F   B
 |-G-|
 E   C  
 |-D-| DP
 
 */

int time; //stores elapsed time

void setup() {

  DDRB = DDRB & B11110111; //make pin 12 an input, leave everything else alone

  while((PINB & 1 << 3)) { //wait until pin 12 goes low, then start
  }

  DDRD = B11111111; //make port d (pins 0-7) outputs

}

void loop() {
  for(byte seg=0; seg <=7; seg++) { //byte seg holds pin to display

    PORTD = 1 << seg;  //make pin seg  high

    PORTD = ~PORTD;    //invert port D because display is common anode (5V),
                       //so pins that are high are turned off, low pins are on

    time = millis();  
    while(millis() - time != 250) { //wait 250 milliseconds.
    }

  }
}

It works pretty well, but after 5 or 6 runs of cycling through the display, the program stops on an arbitrary segment.
Any ideas as to the problem?

Thanks!
baum

while(millis() - time != 250) { //wait 250 milliseconds.

That looks suspicious to me. If you miss the exact single millis count required to break out the wait will hang for like 55 days.

Try while(millis() - time < 250) { //wait 250 milliseconds.

Or even just use delay(250); instead.

Lefty

OK. Just a question... what exactly does the delay() function do? If I didn't have arduino, how would I do the equivalent in avr-c++?

So here’s the new code:

/*Flashes each segment on a 7 segment display for 250 milliseconds using
 direct port manipulation.
 
 Note that due to the way the display works, you MUST disconnect the display
 from pins 0 and 1 before uploading.
 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 Pins:
 DP  0
 C  1
 D  2
 E  3
 B  4
 A  5
 F  6
 G  7
 
 Arrangement:
 |-A-|
 F   B
 |-G-|
 E   C  
 |-D-| DP
 
 */

int time; //stores elapsed time

void setup() {

  DDRB = DDRB & B11110111; //make pin 12 an input, leave everything else alone

  while((PINB & 1 << 3)) { //wait until pin 12 goes low, then start
  }

  DDRD = B11111111; //make port d (pins 0-7) outputs

}

void loop() {
  
  for(byte seg=0; seg <= 7; seg++) { //byte seg holds pin to display

    PORTD = 1 << seg;  //make pin seg high

    PORTD = ~PORTD;    //invert port D because display is common anode (5V),
                       //so pins that are high are turned off, low pins are on

    time = millis();
    while(millis() - time < 250) { //wait 250 milliseconds.
    }

  }
}

Now, after 17 cycles, it hangs again, but now it lights up EVERY segment.

baum

OK. Just a question... what exactly does the delay() function do?

delay(100) hangs in a loop doing nothing else until the 100 milliseconds has elapsed, then the sketch will continue from there.

If I didn't have arduino, how would I do the equivalent in avr-c++?

Well delay relies on core arduino timer functions so not portable to any c++ compiler. However most would likely have some kind of equivalent library function that would perform the same task. The disadvantage of using delay() is that no other useful work can be performed by the program while waiting for the delay to time out. However as you are using the while and millis function you are mimicking the same thing, a blocking operation until 250msec has elapsed. If you look at the arduino example sketch blink without delay you can see how you can use millis to time events without have to stop all further processing until it expires.

Lefty

But it still hangs!! (see above post)

baum: OK. Just a question... what exactly does the delay() function do? If I didn't have arduino, how would I do the equivalent in avr-c++?

Check avr-libc for _delay_ms() ... basically it just executes a short loop of NOPs and uses the known clock cycles per instruction to give the desired delay. There's a little more to it than that, as it takes the CPU clock speed into account so that the function gives the same delay for various clock speeds, but that's the basic idea.

baum:
But it still hangs!! (see above post)

did you try replacing your while(millis() - time < 250) { //wait 250 milliseconds.
statement with a simple delay(250); statement?

Lefty

If I put delay(250); in, it works. But why would the while() part not work? It seems that when the program "hangs" it is actually cycling through the segments one at a time, very fast, because there is no way all 8 could be low at once. This means that at some point, the while() loop is skipped. Weird.

millis() returns unsigned long, so declare the time variable as unsigned long. That's causing the comparison in the while loop to get hosed up.

baum: If I put delay(250); in, it works. But why would the while() part not work? It seems that when the program "hangs" it is actually cycling through the segments one at a time, very fast, because there is no way all 8 could be low at once. This means that at some point, the while() loop is skipped. Weird.

Well what symptom are you talking about. You first said to works for a few cycles then stops with just some segment lite. Now you say all segments turn on continously? At first, or after a period?

Which while statement are you now trying, your original posted code or the changed one I suggested?

Or just use the delay if it does what you want and leave the weirdness rest in peace. ;)

Lefty

  1. Here is the current code:
/*Flashes each segment on a 7 segment display for 250 milliseconds using
 direct port manipulation.
 
 Note that due to the way the display works, you MUST disconnect the display
 from pins 0 and 1 before uploading.
 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 Pins:
 DP  0
 C  1
 D  2
 E  3
 B  4
 A  5
 F  6
 G  7
 
 Arrangement:
 |-A-|
 F   B
 |-G-|
 E   C  
 |-D-| DP
 
 */

int time; //stores elapsed time

void setup() {

  DDRB = DDRB & B11110111; //make pin 12 an input, leave everything else alone

  while((PINB & 1 << 3)) { //wait until pin 12 goes low, then start
  }

  DDRD = B11111111; //make port d (pins 0-7) outputs

}

void loop() {

  for(byte seg=0; seg <= 7; seg++) { //byte seg holds pin to display

    PORTD = 1 << seg;  //make pin seg high

    PORTD = ~PORTD;    //invert port D because display is common anode (5V),
    //so pins that are high are turned off, low pins are on

    /*time = millis();
     while(millis() - time < 250) { //wait 250 milliseconds
     }
     */
    delay(250);
  }
}

If I comment out the delay(250) and use the original statement, the code runs for 17-18 cycles, then displays all of them.

  1. I always use int for millis(), why don’t any other programs hang?

I bet it runs for about 33 seconds before it hangs? That’s when millis() will exceed the capacity of an int, 32767.

Yep. that's it. With long, it works. (now I will need to change this in all my programs that mysteriously stopped working! :) )

Thanks alot!

baum

baum: 2. I always use int for millis(), why don't any other programs hang?

Can't comment without seeing the other programs, that's not an offer XD

But ... the time variable can never hold a value greater than 32,767. So once millis() exceeds 32,767+250, the expression (millis - time) will always be greater than 250, and it'll stick in the while loop until millis() rolls over 50 days later.

Mixing data types is usually a Bad Thing. It can be done, and there may even be reasons to do it, but a person had better understand what they are doing. This isn't a case where there's a reason to do it, actually quite to the contrary!

It will overflow and nasty things (tm) will happen.

In avr-gcc you can use the _delay_loop1 and 2 and also _delay_ms(x) and _delay_us(x) or set up a timer and do non blocking delays like using milis.

Thanks!