Nested loop question and setting variable

I am going to dust off a Mega and use that to test a bunch of NOS 16x16 LED matrix and I wanted to do it in a "neat" way using variable resistor on one analog input. The analog can read from 0 to 1023 and I would use that directly to delay in between LED checking so I could do it slow to see each one or fast to light up the whole display one LED at a time.

The LED matrix is column cathode and if I am understanding LED dot matrix display correctly, row needs to be a 1 and column a 0 to turn on that one specific LED? If so, all row LED pin would be set to 0 to turn off all rows and column a 1 to turn off all column. There will be current limiting resistor.

I am sure multiplexer can be used but this would be temporary and I don't want to pull out a few ICs and empty a large breadboard just to test a dozen displays. 32 wires that has pins male to female to connect LED display, 16 direct to Mega256 and 16 to a breadboard for resistors and 16 male to male pins from breadboard to ATMega seems quicker to setup.

Is this a legal nested loop?

{
   for (int x=0;x <= 15; i++){
      digitalWrite(rled[x],1);
      for (int y=0;y <=15; i++){
        digitalWrite(cled[y], 0);
        delay(analogRead(0));  // whatever variable that reads analog pin, in straight ms up to one second per LED at slowest speed.
        digitalWrite(cled[y], 1);
      }
      digitalWrite(rled[x],0);
   } 
}

Also is analog read correct or does it need to be changed? I'd like it to be checked regularly and not once per entire loop because at slowest speed, it would take over 4 minutes to do whole 16x16 display before the analog pin is read again.

TIA~!

delay(analogRead(0));

That will give you a delay of between 0 and 1023ms or just barely over a second. If that's good for you then you're good to go.

If it were me I would read the analog value once per row and use delayMicroseconds(reading * 200UL); That would give delays from 0 to 204.6 milliseconds giving better control at the fast end and slow enough at the slow end to see the moving dot.

wilykat:
Also is analog read correct or does it need to be changed?

what happened when you tried it?

analogRead() is sort of slowish. you could update it less frequently using mod operator strategically located sort-of like this:

{
  int myDelay = analogRead(0);
  for (int x = 0; x <= 15; i++) 
  {
    digitalWrite(rled[x], 1);
    for (int y = 0; y <= 15; i++) 
    {
      myDelay = i % 3 ? : myDelay : analogRead(0);  //every 3 turns of 'y' for-loop
      digitalWrite(cled[y], 0);
      delay(myDelay);  // whatever variable that reads analog pin, in straight ms up to one second per LED at slowest speed.
      digitalWrite(cled[y], 1);
    }
    digitalWrite(rled[x], 0);
  }
}
myDelay = i % 3 ? : analogRead(0) : myDelay;  //every 3 turns of 'y' for-loop

This will read twice and then skip one. So it skips every third reading. 0 is false, 1 and 2 are true. (Also got an extra colon in there after the ? that shouldn't be there)

Maybe you meant:

myDelay = i % 3 == 0 ? analogRead(0) : myDelay;  //every 3 turns of 'y' for-loop

That will read every third time through.

EDIT: Or just reverse the end

myDelay = i % 3?  myDelay : analogRead(0);  //every 3 turns of 'y' for-loop

I fixed it before you posted your correction... man you have to type fast here!

Delta_G:

myDelay = i % 3 ? : analogRead(0) : myDelay;  //every 3 turns of 'y' for-loop

This will read twice and then skip one. So it skips every third reading. 0 is false, 1 and 2 are true. (Also got an extra colon in there after the ? that shouldn't be there)

Maybe you meant:

myDelay = i % 3 == 0 ? analogRead(0) : myDelay;  //every 3 turns of 'y' for-loop

That will read every third time through.

EDIT: Or just reverse the end

myDelay = i % 3?  myDelay : analogRead(0);  //every 3 turns of 'y' for-loop

I am getting error, expected ; before :

I ended up moving analogRead(0) to between x and y loops so it is read once per row rather than every single LED. No error when I tried this.

 byte cled[16];
 byte rled[16];
 int myAnalog = 512;
    
 void setup()
  {
   rled[0] = 22;
   rled[1] = 24;
   rled[2] = 26;
   rled[3] = 28;
   rled[4] = 30;
   rled[5] = 32;
   rled[6] = 34;
   rled[7] = 36;
   rled[8] = 38;
   rled[9] = 40;
   rled[10] = 42;
   rled[11] = 44;
   rled[12] = 46;
   rled[13] = 48;
   rled[14] = 50;
   rled[15] = 62;
   cled[0] = 23;
   cled[1] = 25;
   cled[2] = 27;
   cled[3] = 29;
   cled[4] = 31;
   cled[5] = 33;
   cled[6] = 35;
   cled[7] = 37;
   cled[8] = 39;
   cled[9] = 41;
   cled[10] = 43;
   cled[11] = 45;
   cled[12] = 47;
   cled[13] = 49;
   cled[14] = 51;
   cled[15] = 53;

 {
    for (int x=0;x <= 15; x++){
      pinMode(rled[x],OUTPUT);
      digitalWrite(rled[x],LOW);
    }
    for (int x=0;x <= 15; x++){
      pinMode(cled[x],OUTPUT);
      digitalWrite(cled[x],HIGH);
    }
}
  }

void loop()
{
   for (int x=0;x <16; x++)
   { 
      myAnalog=analogRead(0);
      digitalWrite(rled[x],1);
      for (int y=0;y <16; y++)
      {
        digitalWrite(cled[y], 0);
        delay(myAnalog);  
        digitalWrite(cled[y], 1);
      }
      digitalWrite(rled[x],0);
   } 
   
}

A tad lengthy due to defining 32 pins to variable containers. But it does use all of the 2x16 digital pins on the end of Mega board and leaving the rest of the board clear for easy access to fix loose wire and reset button. Now to wire this up and start testing.

interesting choice here:

   rled[0] = 22;
   rled[1] = 24;
   rled[2] = 26;
   rled[3] = 28;
   rled[4] = 30;
   rled[5] = 32;
   rled[6] = 34;
   rled[7] = 36;
   rled[8] = 38;
   rled[9] = 40;
   rled[10] = 42;
   rled[11] = 44;
   rled[12] = 46;
   rled[13] = 48;
   rled[14] = 50;
   rled[15] = 62;
   cled[0] = 23;
   cled[1] = 25;
   cled[2] = 27;
   cled[3] = 29;
   cled[4] = 31;
   cled[5] = 33;
   cled[6] = 35;
   cled[7] = 37;
   cled[8] = 39;
   cled[9] = 41;
   cled[10] = 43;
   cled[11] = 45;
   cled[12] = 47;
   cled[13] = 49;
   cled[14] = 51;
   cled[15] = 53;

what about the old fashioned way:

byte rled[16] = {22, 24, 26, 28, 30, 32, ...(rest of the values here) ..., 51, 53};

BulldogLowell:
what about the old fashioned way:

byte rled[16] = {22, 24, 26, 28, 30, 32, ...(rest of the values here) ..., 51, 53};

Just tried that. It makes for smaller file on my hard drive, and added about 300 bytes to total compiled size on Arduino. Seems like trying to make it look smaller and neater ends up needing more flash memory storage. It doesn't matter when I am using only 2k out of max 256k but for smaller chip like 328p a few hundred bytes can make a bigger difference.

That should have made the compiled code smaller not larger I would think.

If you're concerned with code size then note that the values all differ by 2. It's a nice pattern. So you could just compute the value from the index with no need for the array.