PROGMEM and for loops

Projects Programming
1

Hello,

I am trying to understand how and why my code is supposed to work. I can get it to do what I want, but I would stop short of saying I truly understand it. I am trying to create a lookup table using PROGMEM, and when I use a for loop, the program is locked in that loop and won't due anything else until the for loop is complete. I would like to track time(millis()) and Serial.print time for testing. I can get the for loop to do as it is intended, maybe there is a way around the for loop to do as I wanted, so I recreated the for loop using if statements. Everything works a lot better, however, it won't use the 1st step in the PROGMEM, and skips over it unless I force it to -1 "pgm_read_byte_near(LEDset + i - 1);"

I am attaching my code for advise, and so that I can learn more about what's going on. Thanks in advance for the help.

int LED_pin = 9;

int minBright = 50;		//I want to restrict brightness in later code, it works as is, but it won't do what I want long term as it will reduce the desired steps
int maxBright = 100;
int Brightness; 
int i;		//used to step through lookup table, needs to be global, not sure why

unsigned long previousMillis;


/******27 steps******/
const PROGMEM uint8_t LEDset[] = {
  0,10,20,30,40,50,60,70,80,90,100,
  110,120,130,140,150,160,170,180,190,200,
  210,220,230,240,250,255};


void setup(){
  pinMode(LED_pin, OUTPUT);
  
  Serial.begin(9600);
}


void loop(){
  unsigned long currentMillis = millis();
  unsigned long time = millis();
  
  if (currentMillis - previousMillis >= 1000){ 
    if ( i < 27){
        i++;
        Brightness = pgm_read_byte_near(LEDset + i - 1);
        analogWrite(LED_pin, Brightness= constrain(Brightness, minBright, maxBright));	//I want to restrict brightness in later code, it works as is, but it won't do what I want long term, as it will reduce the desired steps
        Serial.print("Time = ");
        Serial.println(time);
        Serial.print("Brightness = ");
        Serial.println(Brightness);
        Serial.print("i = ");
        Serial.println(i);
    /*for (byte i = 0; i < 27; i++){	//Cycles through the table as expected, using for loop locks up the program until it has finished. I want to be able to do other things while in the loop
        Brightness = pgm_read_byte_near(LEDset + i);  
        analogWrite(LED_pin, Brightness);
        Serial.print("Time = ");
        Serial.println(time);
        Serial.print("Brightness = ");
        Serial.println(Brightness);
    } */
    }
    previousMillis = currentMillis;     
  }
}
2 
int i;		//used to step through lookup table, needs to be global, not sure why

i does not have to be global, it can be a static local. Either way, you should initialize to 0 for good programming practice and you may want to consider naming it something else. Single character variables are hard to search on and if your program gets large it becomes cumbersome.

If you increment i after your processing then you don't have to use the -1.

3 
    if ( i < 27) {
      i++;

Your array has 27 elements; the index goes from 0 .. 26. What happens when i equals 26? You will increment i and it will be 27; that's why you have to subtract 1 in below.

Brightness = pgm_read_byte_near(LEDset + i - 1);

Think about what happens when i equals 0.

4

All those Serial.print statement within the for loop will fill the xmit buffer, and cause the program to stall until some room is available. Since your baud rate is only 9600, it will take 64mSec to empty the 64-byte buffer. Either do away with the prints, or increase the baud rate to 115200, and things will probably speed up quite a bit.

5

ToddL1962:

int i;		//used to step through lookup table, needs to be global, not sure why

i does not have to be global, it can be a static local. Either way, you should initialize to 0 for good programming practice and you may want to consider naming it something else. Single character variables are hard to search on and if your program gets large it becomes cumbersome.

If you increment i after your processing then you don't have to use the -1.

I took your advice and changed it to StepCount. I did have plans to name i something else later, but for testing and out of laziness I was just left as was. I had i set local at first, but it was causing funky results. I was expecting it to step through the table, but instead gave me zero's and random other numbers.

If you increment i after your processing then you don't have to use the -1.

I'm not sure what you mean here.

6

sterretje:

    if ( i < 27) {

i++;



Your array has 27 elements; the index goes from 0 .. 26. What happens when *i* equals 26? You will increment i and it will be 27; that's why you have to subtract 1 in below.

Brightness = pgm_read_byte_near(LEDset + i - 1);



Think about what happens when *i* equals 0.

Silly me, I did originally have it set to 26, but while playing around with the code a bit, and getting it to do similar to my intended reactions, I never changed it back to 26. While playing around a bit, if I change it to 26, now the code skips over the final step of 255. I thought maybe it was due to the fact that I had "pgm_read_byte_near(LEDset + StepCount - 1)", so I removed the -1, and now it skips over the 0 step and goes to 2nd step of 10. Not sure why it skips the first step.

Think about what happens when i equals 0.

Setting i = to 0, had no noticeable change.

7

RayLivingston:
All those Serial.print statement within the for loop will fill the xmit buffer, and cause the program to stall until some room is available. Since your baud rate is only 9600, it will take 64mSec to empty the 64-byte buffer. Either do away with the prints, or increase the baud rate to 115200, and things will probably speed up quite a bit.

Done, although I had no intentions to leave the serial.prints in there. I was just using them to debug, it helps me to see what is going on.

8

timothypaul26:
I took your advice and changed it to StepCount. I did have plans to name i something else later, but for testing and out of laziness I was just left as was. I had i set local at first, but it was causing funky results. I was expecting it to step through the table, but instead gave me zero's and random other numbers.

That's because you didn't declare it as static.

timothypaul26:
I'm not sure what you mean here.

Increment StepCount AFTER you read and set the brightness.

Here is a version that should work for you:

int LED_pin = 9;

int minBright = 50;		//I want to restrict brightness in later code, it works as is, but it won't do what I want long term as it will reduce the desired steps
int maxBright = 100;
int Brightness;
int setCount;		//used to step through lookup table, needs to be global, not sure why

unsigned long previousMillis;


/******27 steps******/
const PROGMEM uint8_t LEDset[] = {
  0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100,
  110, 120, 130, 140, 150, 160, 170, 180, 190, 200,
  210, 220, 230, 240, 250, 255
};
const int numLeds = sizeof(LEDset) / sizeof(LEDset[0]);

void setup() {
  pinMode(LED_pin, OUTPUT);
  Serial.begin(9600);
  Serial.print("numLeds = ");
  Serial.println(numLeds);
}


void loop() {
  unsigned long currentMillis = millis();
  unsigned long time = millis();
  static int setCount = 0;		//used to step through lookup table, needs to be global, not sure why

  if (currentMillis - previousMillis >= 1000) {
    if ( setCount < numLeds) {
      Brightness = pgm_read_byte_near(LEDset + setCount);
      analogWrite(LED_pin, Brightness= constrain(Brightness, minBright, maxBright));	//I want to restrict brightness in later code, it works as is, but it won't do what I want long term, as it will reduce the desired steps
      Serial.print("Time = ");
      Serial.println(time);
      Serial.print("Brightness = ");
      Serial.println(Brightness);
      Serial.print("setCount = ");
      Serial.println(setCount);
      setCount++;
    }
    previousMillis = currentMillis;
  }
}

I also added this line to automatically compute the size of the LEDset array.

const int numLeds = sizeof(LEDset) / sizeof(LEDset[0]);
9

The part WITHOUT the 'for' loop does one entry every second. The part WITH the 'for' loop does all 27 entries every second. In either case, you can do other stuff between seconds but if you want to do other stuff between entries you either have to put that other stuff inside the 'for' loop or use the style without the 'for' loop.

WITHOUT the 'for' loop:

void loop()
{
  unsigned long currentMillis = millis();


  if (currentMillis - previousMillis >= 1000)
  {
    previousMillis += 1000;


    Brightness = pgm_read_byte_near(LEDset + i);
    Brightness = constrain(Brightness, minBright, maxBright);
    analogWrite(LED_pin, Brightness); //I want to restrict brightness in later code, it works as is, but it won't do what I want long term, as it will reduce the desired steps
    Serial.print("Time = ");
    Serial.print(currentMillis);
    Serial.print("   i = ");
    Serial.print(i);
    Serial.print("   Brightness = ");
    Serial.print(Brightness);


    i++
    if ( i >= 27)
      i = 0;
  }
}

// WITH the 'for' loop:

void loop()
{
  unsigned long currentMillis = millis();


  if (currentMillis - previousMillis >= 1000)
  {
    previousMillis += 1000;


    for (byte i = 0; i < 27; i++)   //Cycles through the table as expected, using for loop locks up the program until it has finished. I want to be able to do other things while in the loop
    {
      Brightness = pgm_read_byte_near(LEDset + i);
      Brightness = constrain(Brightness, minBright, maxBright);
      analogWrite(LED_pin, Brightness); //I want to restrict brightness in later code, it works as is, but it won't do what I want long term, as it will reduce the desired steps
      Serial.print("Time = ");
      Serial.print(currentMillis);
      Serial.print("   i = ");
      Serial.print(i);
      Serial.print("   Brightness = ");
      Serial.print(Brightness);
    }
  }
}
10

johnwasser:
The part WITHOUT the 'for' loop does one entry every second. The part WITH the 'for' loop does all 27 entries every second. In either case, you can do other stuff between seconds but if you want to do other stuff between entries you either have to put that other stuff inside the 'for' loop or use the style without the 'for' loop.

Good catch John. I assumed he wanted one increment every second. Bad assumption.

11

All those Serial.print statement within the for loop will fill the xmit buffer, and cause the program to stall until some room is available

See my tutorial on Arduino Serial I/O for the Real World for how to get non-blocking Serial debug statements.

My Multi-tasking in Arduino tutorial includes a loopTimer() class for tracking your loop() execution time, max, min, average over 5sec + max values so far.

12

ToddL1962:
That's because you didn't declare it as static.

Increment StepCount AFTER you read and set the brightness.

Here is a version that should work for you:

int LED_pin = 9;

int minBright = 50; //I want to restrict brightness in later code, it works as is, but it won't do what I want long term as it will reduce the desired steps
int maxBright = 100;
int Brightness;
int setCount; //used to step through lookup table, needs to be global, not sure why

unsigned long previousMillis;

/27 steps/
const PROGMEM uint8_t LEDset[] = {
 0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100,
 110, 120, 130, 140, 150, 160, 170, 180, 190, 200,
 210, 220, 230, 240, 250, 255
};
const int numLeds = sizeof(LEDset) / sizeof(LEDset[0]);

void setup() {
 pinMode(LED_pin, OUTPUT);
 Serial.begin(9600);
 Serial.print("numLeds = ");
 Serial.println(numLeds);
}

void loop() {
 unsigned long currentMillis = millis();
 unsigned long time = millis();
 static int setCount = 0; //used to step through lookup table, needs to be global, not sure why

if (currentMillis - previousMillis >= 1000) {
   if ( setCount < numLeds) {
     Brightness = pgm_read_byte_near(LEDset + setCount);
     analogWrite(LED_pin, Brightness= constrain(Brightness, minBright, maxBright)); //I want to restrict brightness in later code, it works as is, but it won't do what I want long term, as it will reduce the desired steps
     Serial.print("Time = ");
     Serial.println(time);
     Serial.print("Brightness = ");
     Serial.println(Brightness);
     Serial.print("setCount = ");
     Serial.println(setCount);
     setCount++;
   }
   previousMillis = currentMillis;
 }
}

Ok thank you for that, I see how it works now. I messed up when I first had i as int i = 0; local, and didn't think or even know how static worked. Now I've read a little about static and hopefully I won't make that mistake again.

I also added this line to automatically compute the size of the LEDset array.

const int numLeds = sizeof(LEDset) / sizeof(LEDset[0]);

That's a pretty neat sanity check, thank you.

13

The part WITHOUT the 'for' loop does one entry every second.

I think this is what I'm looking for as, I want to be able to stretch out a LED fading in and out over a period of time, maybe like 30 mins to an hour?.

The part WITH the 'for' loop does all 27 entries every second

This is too fast and it would not work for what I want. If I use the for loop, there is no way to spread out the fade. This is what I was finding, and having trouble with. I wanted to make sure that I was understanding the for loop properly, and not making a noob mistake.

Having the code do one entry every second is intentional here. But, not truly my desired outcome, in the end I want to be able to select a fade length.

Now in your code you write "previousMillis += 1000" is there a reason that I should be using that instead of setting previousMillis equal to currentMillis? Or is it a shorthand?

14

timothypaul26:
If I use the for loop, there is no way to spread out the fade. This is what I was finding, and having trouble with. I wanted to make sure that I was understanding the for loop properly, and not making a noob mistake.

It's one of the big hurdles of switching timing from delay() to millis(). You can, sort of, translate a 'for' loop containing a single delay into one that works with millis(), but it isn't pretty.

  for (int loopIndex=START_VALUE; loopIndex < COUNT_VALUE; loopIndex ++)
  {
    // Do Stuff
    delay(INTERVAL);
  }

First, we turn the 'for' loop into an equivalent-ish while() loop:

  int loopIndex = START_VALUE;
  while (loopIndex < COUNT_VALUE)
  {
    // Do Stuff
    delay(INTERVAL);
    loopIndex++;
  }

Then we change it from a 'while' to an 'if' so it only does one step each time

  static int loopIndex = START_VALUE;  // Make it 'static' so it doesn't get re-initialized
  if (loopIndex < COUNT_VALUE)
  {
    // Do Stuff
    delay(INTERVAL);
    loopIndex++;
  }
  else
  {
    // the 'loop' has ended so we set up for the next time
    loopIndex = START_VALUE;
    // If your main code needs to know the loop has finished, you can set a flag here
  }

Now that the 'for' loop has been deconstructed to only do one step, do that step only when the time is ripe.

  static int loopIndex = START_VALUE;  // Make it 'static' so it doesn't get re-initialized
  static unsigned long loopTimer = 0;
  if (loopIndex < COUNT_VALUE)
  {
    if (millis() - loopTimer >= INTERVAL)
    {
      loopTimer = millis();
      // Do Stuff
      loopIndex++;
    } // END if (millis) 
  }
  else
  {
    // the 'loop' has ended so we set up for the next time
    loopIndex = START_VALUE;
    // If your main code needs to know the loop has finished, you can set a flag here
  }

Alternatively, the timing can go around the 'loop':

  static unsigned long loopTimer = 0;
  if (millis() - loopTimer >= INTERVAL)
  {
    loopTimer = millis();
    static int loopIndex = START_VALUE;  // Make it 'static' so it doesn't get re-initialized
    if (loopIndex < COUNT_VALUE)
    {
      // Do Stuff
      loopIndex++;
    }
    else
    {
      // the 'loop' has ended so we set up for the next time
      loopIndex = START_VALUE;
      // If your main code needs to know the loop has finished, you can set a flag here
    }
  } // END if (millis)