Non blocking delay and analog read

Hello everyone I am trying to implement a non blocking delay.

Basically I read a value from analog pin every second and if current value is higher than previous I blink LED with higher frequency, opposite is for the currentValue < previousValue.

If currentValue is equal to the previousValue I turn off the LED

Code seems to work, besides part when I need to turn off the LED. I even tried to write digitalWrite(LED_PIN, LOW); when previousValue == currentValue, but that messed up the whole program.

unsigned long timeOne;
unsigned long timeTwo;

uint8_t currentTemp;
uint8_t previousTemp;

unsigned long previousLedTimer = 0;
unsigned long interval  = 1000UL;

#define LED_DDR     DDRB
#define LED_TOGGLE  (PORTB ^= (1 << PINB5))
#define ONE_SECOND  1000UL

bool isToggleEnabled = false;

void setup() 
{
  pinMode(11, OUTPUT);
  pinMode(A0, INPUT);
  Serial.begin(9600);
  timeOne = millis();
}

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

  timeTwo = millis();
  // Check if One second has passed and read analog pin if so
  if (timeTwo - timeOne > ONE_SECOND)
  {
    timeOne = millis();
    currentTemp = analogRead(A0);
    Serial.print("Current temp: ");
    Serial.print(currentTemp);
    Serial.print("    Previous temp: ");
    Serial.println(previousTemp);
  }

  // if Current temperature is higher than the previous we need to fast blink
  if (currentTemp > previousTemp)
  {
    interval = 100UL;
    isToggleEnabled = true;
  }
  // If no current temperature is higher we need to blink slowly
  else if (currentTemp < previousTemp)
  {
    interval = 500UL;
    isToggleEnabled = true;
  }
  // If no change in the temperature we need to turn OFF the LED
  else
  {
    isToggleEnabled = true;
  }

  // THIS PART DOESNT WORK!
  // If I write if statement like this it simply doesn't work anymore
  //if(currentLedTimer - previousLedTimer > interval && isToggleEnabled)
  if(currentLedTimer - previousLedTimer > interval) 
  {
    // save the last time you blinked the LED 
    previousLedTimer = currentLedTimer;   
    LED_TOGGLE;
  }
  previousTemp = currentTemp;   
}

Think about previousTemp and currentTemp. You've created a non-blocking delay. That's good, but it means that when you read a different temperature then a few microseconds later you repeat the loop but the temperatures are now equal so it turns off your led blinking. I would move the line that sets previousTemp to currentTemp up to the spot right before you set a new currentTemp.

After I moved it to the where you suggested

if (timeTwo - timeOne > ONE_SECOND)
  {
    timeOne = millis();
    currentTemp = analogRead(A0);
    Serial.print("Current temp: ");
    Serial.print(currentTemp);
    Serial.print("    Previous temp: ");
    Serial.println(previousTemp);
  }
  previousTemp = currentTemp;

but LED seems to blink with only one frequency after that (lower frequency , interval = 500UL)

else if (currentTemp < previousTemp)
  {
    interval = 500UL;
    isToggleEnabled = true;
  }
if(currentLedTimer - previousLedTimer > interval ) 
  {
    // save the last time you blinked the LED 
    previousLedTimer = currentLedTimer;   
    LED_TOGGLE;
  }

If I change the above if statement into this one

if(currentLedTimer - previousLedTimer > interval && isToggleEnabled)

LED never turns ON.

I really do not see what I am doing wrong with this one.

That's not quite where I suggested you put it. Now think about it. When you set a new currentTemp you immediately then make previousTemp equal to that currentTemp. So again, they'll always be equal.

I said put the line that sets previousTemp to currentTemp to the line right before you set a new currentTemp. You only want previousTemp changing if there is a new currentTemp being read.

Sorry I did not read carefully.

I got it working right now, big thanks, I was really wondering why it isn’t working, I guess I should have checked the logic without arduino using VS or some other IDE with a debugger.

unsigned long timeOne;
unsigned long timeTwo;

uint8_t currentTemp;
uint8_t previousTemp;

unsigned long previousLedTimer = 0;
unsigned long interval  = 1000UL;

#define LED_DDR     DDRB
#define LED_TOGGLE  (PORTB ^= (1 << PINB5))
#define ONE_SECOND  1000UL

bool isToggleEnabled = false;

void setup() 
{
  pinMode(11, OUTPUT);
  pinMode(A0, INPUT);
  Serial.begin(9600);
  timeOne = millis();
}

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

  timeTwo = millis();
  // Check if One second has passed and read analog pin if so
  if (timeTwo - timeOne > ONE_SECOND)
  {
    timeOne = millis();
    previousTemp = currentTemp;
    currentTemp = analogRead(A0);
    Serial.print("Current temp: ");
    Serial.print(currentTemp);
    Serial.print("    Previous temp: ");
    Serial.println(previousTemp);
  }
  
  
  // if Current temperature is higher than the previous we need to fast blink
  if (currentTemp > previousTemp)
  {
    interval = 100UL;
    isToggleEnabled = true;
  }
  // If no current temperature is higher we need to blink slowly
  else if (currentTemp < previousTemp)
  {
    interval = 500UL;
    isToggleEnabled = true;
  }
  // If no change in the temperature we need to turn OFF the LED
  else
  {
    isToggleEnabled = false;
  }

  // THIS PART DOESNT WORK!
  // If I write if statement like this it simply doesn't work anymore
  //if(currentLedTimer - previousLedTimer > interval)
  if(currentLedTimer - previousLedTimer > interval && isToggleEnabled) 
  {
    // save the last time you blinked the LED 
    previousLedTimer = currentLedTimer;   
    LED_TOGGLE;
  }  
}

OnesAndZeroes: Sorry I did not read carefully.

I got it working right now, big thanks, I was really wondering why it isn't working, I guess I should have checked the logic without arduino using VS or some other IDE with a debugger.

You don't need to do that. For a simple program like this, print it out and follow through it with your finger and think about the values of variables as you go. You can debug short code like this in your head if you take your time.

Yeah I was kind of doing that, you can see that I printed out the previous and current values

  if (timeTwo - timeOne > ONE_SECOND)
  {
    timeOne = millis();
    
    currentTemp = analogRead(A0);
    Serial.print("Current temp: ");
    Serial.print(currentTemp);
    Serial.print("    Previous temp: ");
    Serial.println(previousTemp);
  }

So since I printed different values for both current and previous Temp. And after that line I was convinced that one these conditions should be true

  // if Current temperature is higher than the previous we need to fast blink
  if (currentTemp > previousTemp)
  {
    interval = 100UL;
    isToggleEnabled = true;
  }
  // If no current temperature is higher we need to blink slowly
  else if (currentTemp < previousTemp)
  {
    interval = 500UL;
    isToggleEnabled = true;
  }
  // If no change in the temperature we need to turn OFF the LED
  else
  {
    isToggleEnabled = false;
  }

I think what tricked my logic was having previousTemp = currentTemp at the end of the loop function.