[SOLVED] Doing do...while wrongly ? [LED multiplexing related ?]

have suceeded in getting one letter to display multiplexed via 2 x 74HC595 shift registers on an 8x8 LED matrix, but now want to start showing more letters, one at a time -i can’t seem to get the control structure correct for the delay before the next letter shows up.

int latchPin = 8;
int clockPin = 12;
int dataPin = 11;

byte letterABC[3][8] = {
  {  60, 102, 102, 126, 102, 102, 102, 0  },
  { 124, 102, 102, 124, 102, 102, 124, 0  },
  {  60, 102,  96,  96,  96, 102,  60, 0, }
};

void setup() {
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);

  Serial.begin(9600);  
}

unsigned long timeNOW;

void loop() {

  for (int letter = 0; letter < 3; letter++) {
    int row = 128;
    unsigned long timeGO = millis();
    do {
      for (int i = 0; i < 8; i++) {
        digitalWrite(latchPin, LOW);
        shiftEMout(255-letterABC[letter][i]);  // this is the COL '0' is what's ON !! (ie. which gets pulled to GND, shunted on to 2nd Register)
        shiftEMout(row);     // light EVERY in row on, which stays on determined by what pulled to GND
        row = row / 2;      // for the NEXT row

        digitalWrite(latchPin, HIGH);
        delay(3);  // at '4' can start to notice, at '5' flickering obvious
      }
    timeNOW = millis();

//DEBUG
Serial.print("tineNOW = ");
Serial.print(timeNOW);
Serial.print(", timeGO was : ");
Serial.println(timeGO);
delay(500);

    }
    //delay(1800); STOP THIS !!
    while((timeNOW - timeGO) < 1800);
  }
}
void shiftEMout(byte dataOut) {
  // Shift out 8 bits LSB first, on rising edge of clock
  boolean pinState;
  //clear shift register read for sending data
  digitalWrite(dataPin, LOW);
  digitalWrite(clockPin, LOW);
  // for each bit in dataOut send out a bit
  for (int i=0; i<=7; i++) {
    //set clockPin to LOW prior to sending bit
    digitalWrite(clockPin, LOW);
    // if the value of DataOut and (logical AND) a bitmask
    // are true, set pinState to 1 (HIGH)
    if ( dataOut & (1<<i) ) {
      pinState = HIGH;
    }                 
    else {            
      pinState = LOW;
    }
    //sets dataPin to HIGH or LOW depending on pinState
    digitalWrite(dataPin, pinState);
    //send bit out on rising edge of clock
    digitalWrite(clockPin, HIGH);
    digitalWrite(dataPin, LOW);
  }
  //stop shifting
  digitalWrite(clockPin, LOW);
}

i can just make out the letters briefly but they don’t seem to stay on for the 1800 uSec - what am i doing wrong ?

For one thing, you are waiting 1800 mS, not microseconds.

For the other it is hard to tell with your uneven indentation.

KeithRB: For one thing, you are waiting 1800 mS, not microseconds.

yes, that is how long i do want to wait !

KeithRB: For the other it is hard to tell with your uneven indentation.

how so ? it's been "Ctrl-T'd - except for that //DEBUG section.

how so ? it’s been "Ctrl-T’d - except for that //DEBUG section.

You really should try putting the { on new lines.

void setup()
{ // Down here, where it belongs
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);

  Serial.begin(9600);  
}
      for (int i = 0; i < 8; i++) 
      {
        digitalWrite(latchPin, LOW);
        shiftEMout(255-letterABC[letter][i]);  // this is the COL '0' is what's ON !! (ie. which gets pulled to GND, shunted on to 2nd Register)
        shiftEMout(row);     // light EVERY in row on, which stays on determined by what pulled to GND
        row = row / 2;      // for the NEXT row

        digitalWrite(latchPin, HIGH);
        delay(3);  // at '4' can start to notice, at '5' flickering obvious
      }

It is, in my opinion, far easier to see the range of a statement when the { and } line up than when there are several inches apart horizontally and vertically.

And the debug session is right where I really care about the indenting!

PaulS:
You really should try putting the { on new lines.

void setup()

{ // Down here, where it belongs
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);

Serial.begin(9600); 
}

yes sir !
will do !!
i guess i’ve seen the curly bracket on the end many times because of setup() & loop() so went with that “convention”

PaulS:
It is, in my opinion, far easier to see the range of a statement when the { and } line up than when there are several inches apart horizontally and vertically.

that’s a good point - okay, count me in with the New Line Left Curly Bracket Gang.

KeithRB:
And the debug session is right where I really care about the indenting!

okay then, as requested…

int latchPin = 8;
int clockPin = 12;
int dataPin = 11;

byte letterABC[3][8] = {
  {  
    60, 102, 102, 126, 102, 102, 102, 0        }
  ,
  { 
    124, 102, 102, 124, 102, 102, 124, 0        }
  ,
  {  
    60, 102,  96,  96,  96, 102,  60, 0,       }
};

void setup() 
{
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);

  Serial.begin(9600);  
}

unsigned long timeNOW;

void loop() 
{
  for (int letter = 0; letter < 3; letter++) 
  {
    int row = 128;
    unsigned long timeGO = millis();
    do 
    {
      for (int i = 0; i < 8; i++) {
        digitalWrite(latchPin, LOW);
        shiftEMout(255-letterABC[letter][i]);  // this is the COL '0' is what's ON !! (ie. which gets pulled to GND, shunted on to 2nd Register)
        shiftEMout(row);     // light EVERY in row on, which stays on determined by what pulled to GND
        row = row / 2;      // for the NEXT row

        digitalWrite(latchPin, HIGH);
        delay(3);  // at '4' can start to notice, at '5' flickering obvious
      }
      timeNOW = millis();

      //DEBUG
      Serial.print("tineNOW = ");
      Serial.print(timeNOW);
      Serial.print(", timeGO was : ");
      Serial.println(timeGO);
      delay(500);

    }
    //delay(1800); STOP USING THIS !!
    while((timeNOW - timeGO) < 1800);
  }
}

void shiftEMout(byte dataOut) 
{
  // Shift out 8 bits LSB first, on rising edge of clock
  boolean pinState;
  //clear shift register read for sending data
  digitalWrite(dataPin, LOW);
  digitalWrite(clockPin, LOW);
  // for each bit in dataOut send out a bit
  for (int i=0; i<=7; i++) {
    //set clockPin to LOW prior to sending bit
    digitalWrite(clockPin, LOW);
    // if the value of DataOut and (logical AND) a bitmask
    // are true, set pinState to 1 (HIGH)
    if ( dataOut & (1<<i) ) {
      pinState = HIGH;
    }                 
    else {            
      pinState = LOW;
    }
    //sets dataPin to HIGH or LOW depending on pinState
    digitalWrite(dataPin, pinState);
    //send bit out on rising edge of clock
    digitalWrite(clockPin, HIGH);
    digitalWrite(dataPin, LOW);
  }
  //stop shifting
  digitalWrite(clockPin, LOW);
}

can i get a witness now ?..
please…

I think you should first explain how

unsigned long timeGO = millis();
do
{
    // Do something
    unsigned long timeNOW = millis();
}
while (timeNOW - timeGO < 1800);

is better then

   // Do something
   delay(1800);

when “Do something” is exactly the same thing, over and over.

Your fooling yourself if you think your implementation of delay is somehow better then delay().

because in the 2nd example, the "do something" occurs just once and then waits. this doesn't work with multiplexing because the display loop has to keep running - hence the use of Do...While (time is being checked). so my idea was "keep repeating the multiplexing" until the display time (of 1800 uSecs) has passed. i think the control structure is correct, it could be a matter of the multiplexing method itself that is flawed ?

hello again, can anyone make some more comments ?
i still can’t see where my control structure is in error.

Thanks

EDIT:
ok, i pared down the above code to just the control structure and made do_something() as simple as;

void do_something()
{
  for (int i=0; i<3; i++)
  {
    digitalWrite(13,HIGH);
    delay(800);
    digitalWrite(13,LOW);
    delay(500);
  }
}

i see why the code looks “strange” - it baffled me too in it’s simple form, the do_while loop didn’t “delay” 1800 uSecs and just kept on “doing_something”.

then i realised this is actually what i wanted to do - “keep doing_something” !

my problem was the outer for loop (per letter) - it should not be used as the do_while loop is already the repeating loop for letter.

so, i simply took out that for loop, and just added a check at the exit of the do_while loop with;

  letter += 1;
  if(letter > 2) letter = 0;

and now IT WORKS ! 8)