help with loop code

I am having so much trouble with this program I am trying to make.

I am trying to create a project where the frequency of a blinking LED increases until the blinking can’t be seen anymore. At that time I want to press a button, the blinking stops, and the frequency (measured in Hz) is printed on an LCD.

Even after looking at BlinkWithoutDelay and “Demonstration code for several things at the same time”. I still don’t know how to make the light blink faster.

Particularly, I am having trouble with the increasing interval between blinks. I want to have 1-second of constant interval for each increment of frequency, with about 95 increments between 5 and 100 Hz. For loops and delays won’t work because I need to read the button simultaneously.

Basically, could anyone give me the code to increase the frequency of a blinking light (with 1 Hz increments) at 1 second intervals or even point me in the right direction, because I am totally lost.

this probably won’t help but this is what I have so far.

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
float interval=500;
int LEDpin=13;
int Buttonpin=6;
unsigned long previousMillis=0;
int LEDstate= LOW;

void setup() {
  
  lcd.begin(16, 2);
  
  lcd.print("frequency");
  while (digitalRead(Buttonpin) == HIGH) {
  
    
    unsigned long currentMillis = millis(); 
    
    if (currentMillis - previousMillis >= interval) {
    // save the last time you blinked the LED
    previousMillis = currentMillis;

    // if the LED is off turn it on and vice-versa:
    if (LEDstate == LOW) {
      LEDstate = HIGH;
    } else {
      LEDstate = LOW;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(LEDpin, LEDstate);
  }
    
    
    
    

  
  lcd.setCursor(0, 1);
  
  lcd.print(1);
}
}
void loop() {
  
}

I would like to move the declaration part outsude Setup.

    unsigned long currentMillis = millis();

Inside Setup: currentMillis = millis();

You have this line of code

 if (currentMillis - previousMillis >= interval) {

You need another line of code to change the value in the variable interval. Maybe

interval -= 10;

which would change it by 10 millisecs every time the led changes state.

Also note that interval should be defined as unsigned long.

...R

PS ... the program should work without the changes suggested by @Railroader but I agree with him that it seems strange to create previousMillis as a global variable and currentMillis as a local variable.

This should get you started

unsigned long currentMillis;
unsigned long startTime;
int period = 1000;
const byte ledPin = 3;
byte ledState = LOW;

void setup()
{
  Serial.begin(115200);
  pinMode(ledPin, OUTPUT);
}

void loop()
{
  currentMillis = millis();
  if (currentMillis - startTime >= period)  //time for a change ?
  {
    digitalWrite(ledPin, ledState);
    if (ledState == LOW)
    {
      ledState = HIGH;
    }
    else
    {
      ledState = LOW;
    }
    startTime = currentMillis;
    period = period - 16;
    if (period < 0)
    {
      period = 1000;
    }
  }
}

The loop() function runs freely so you can read your button input and react to it at any time. You will, of course, need to adjust the timings to suit your needs

I am trying to create a project where the frequency of a blinking LED increases by 1 Hertz every 1 second until the blinking can’t be seen anymore. At that time I want to press a button, the blinking stops, and the frequency (measured in Hz) is printed on an LCD.

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
int ledPin = 13;
int Buttonpin=6;

unsigned long currentMillis;
unsigned long startTime;
float period = 100;
byte ledState = LOW;
unsigned long startTime2;


void setup() {
  
  lcd.begin(16, 2);
  
  lcd.print("frequency");
  
  pinMode(ledPin, OUTPUT);
  
  while (digitalRead(Buttonpin) == HIGH) {
  
do {  
    currentMillis = millis();
    startTime2 = 0;
  if (currentMillis - startTime >= period)  //time for a change ?
  {
    digitalWrite(ledPin, ledState);
    if (ledState == LOW)
    {
      ledState = HIGH;
    }
    else
    {
      ledState = LOW;
    }
    startTime = currentMillis;
  }
} while (currentMillis - startTime2 <=1000);
    
    startTime2= millis();
    period = period - 1;
    if (period <= 0)
    {
      period = 100;
    }
  
    
  }
  
  lcd.setCursor(0, 1);
  
  lcd.print(-1 * period + 105);
}

void loop() {
  
}

However, the blinking doesn’t seem to be at the right frequency I want it to be and it doesn’t reset once the “period” variable becomes 0. When I press the button the frequency displayed is also messed up and doesn’t always increase the longer I wait to press the button

Can anyone give me a code that works properly or tell me what I need to change in order to make the code work properly? Attached is a picture of my circuit.

Topcs merged

UKHeliBob:
This should get you started

unsigned long currentMillis;

unsigned long startTime;
int period = 1000;
const byte ledPin = 3;
byte ledState = LOW;

void setup()
{
  Serial.begin(115200);
  pinMode(ledPin, OUTPUT);
}

void loop()
{
  currentMillis = millis();
  if (currentMillis - startTime >= period)  //time for a change ?
  {
    digitalWrite(ledPin, ledState);
    if (ledState == LOW)
    {
      ledState = HIGH;
    }
    else
    {
      ledState = LOW;
    }
    startTime = currentMillis;
    period = period - 16;
    if (period < 0)
    {
      period = 1000;
    }
  }
}




The loop() function runs freely so you can read your button input and react to it at any time. You will, of course, need to adjust the timings to suit your needs

but how do I make each increment repeat fro only 1 second. I tried this code but it doesn’t work.

do {  
    currentMillis = millis();
    startTime2 = 0;
  if (currentMillis - startTime >= period)  //time for a change ?
  {
    digitalWrite(ledPin, ledState);
    if (ledState == LOW)
    {
      ledState = HIGH;
    }
    else
    {
      ledState = LOW;
    }
    startTime = currentMillis;
  }
} while (currentMillis - startTime2 <=1000);
    
    startTime2= millis();
    period = period - 1;
    if (period <= 0)
    {
      period = 100;
    }

how do I make each increment repeat fro only 1 second. I tried this code but it doesn’t work.

You need to describe exactly the pattern that you want, perhaps something like this

ON 1000, OFF 1000, ON 990, OFF 1000, ON 980, OFF 1000 and so on

Incidentally, this
while (currentMillis - startTime2 <=1000);is just
delay(1000);in disguise and blocks the free running of loop() and thus the button cannot be read during the while loop.

What I specifically want is:

100 ON 100 OFF for 1 second
99 ON 99 OFF for 1 second. Cutting off the pattern at 1 second even so it would look like this:

99 ON 99 OFF
99 ON 99 OFF
99 ON 99 OFF
99 ON 99 OFF
99 ON 99 OFF
10 ON (for a total of 1000ms or 1 second)

98 ON 98 OFF for 1 second

so on and so on until

5 ON 5 OFF for 1 second

And then at any time if the button is pressed then the frequency of the blinking at that moment:

x ON x OFF

is recorded and displayed on the LCD in Hz using the linear equation:

-1 * x + 105

You need 2 periods. One fixed at 1000 milliseconds and another starting at 100 and decreasing. let's name them longPeriod and shortPeriod. You also need two start times. Let's name them longStartTime and shortStartTime.

Assuming that all the variables have been declared and initialised, that the LED is in its starting state (probably off) and that loop() starts by getting the current value of millis() to a variable named currentMillis. In loop() you need 2 separate tests.

Test 1 - is it time to change the state of the LED ?
if shortPeriod has ended
change the state of the LED
save currentMillis to shortStartTime
end if

Test 2 - is it time to change the flash rate of the LED ?
if longPeriod has ended
turn off the LED
save currentMillis to shortStartTime
save currentMillis to longStartTime
reduce the value of shortPeriod
end if

You can read the value of the input and act on it at any time because loop() is not blocked

Robin2:
PS ... the program should work without the changes suggested by @Railroader but I agree with him that it seems strange to create previousMillis as a global variable and currentMillis as a local variable.

It makes perfect sense to me. If a variable is only needed in one function, that's where it should be defined. Programming best practices dictate that you should always limit the scope of variables to the minimum required.

EDIT:
But, looking again at OP's original code, I see it's all in the setup() function. That really doesn't make sense either. Or, at least, it's not the "Arduino Way". @UKHeliBob is attempting to move OP in the right direction.