Function is not working when combined with others

This is an extension from previous topic where i wanted to store the maximum value from sensor within a specific period and then print that to an lcd.

I managed to make a code that will store the max value and it worked nicely but then i wanted to to add a blowing count down and a warm up time countdown and there came a new problem, both countdown codes worked like i wanted them but the code to get the max value is not working or idk it's just not being excuted. but when i separate it into another file and just run it individually it works fine

here is my code

#include <SoftwareSerial.h>
#include <String.h>
#include <LiquidCrystal.h>

const int Alcohol = 9; //the sensor connected to pin 9
int Alcoholvalue;
int Max;
int Mosfet = 10;
unsigned long previousMillis = 10005; //i set this to 10 sec because of the 10 seconds of the warming time
unsigned const long interval = 5000; //so here is the blowing time

LiquidCrystal lcd(A0, A1, A2, A3, A4, A5); //connecting the lcd
void setup() {
  // Setup size of LCD 20 characters and 4 lines
  lcd.begin(20, 4);
  lcd.setCursor(1, 2);
  lcd.setCursor(4, 3);
  lcd.setCursor(0, 0);
  Serial.begin(9600);
  pinMode(Alcohol, INPUT);
  pinMode (Mosfet, OUTPUT);
  lcd.print("warming up, wait");
}


void Warming() //this function will show a countdown timer for 10 seconds (i need the 10 sec as a warm up time for the sensor)
{
  while (millis() <= 10000) {
    lcd.setCursor(0, 1);
    int count = 10 - millis() / 1000;
    lcd.print (count);
    //to remove the 0 from the 10 before the 9
    if (count < 10) {
      lcd.print(' ');
    }
  }
}

void Blowing()    //thie is a countdown while exhaling or blowing towards the sensor for 5sec
{
  while (millis() >= 10000) { //after 10sec of warming
    //now i want this to run for 5 sec only so i timed it until 15005 millis
    //i gave the extra 5 millisec here to make sure i get to see all the countdown
    while (millis() <= 15005) {
      lcd.setCursor(0, 1);
      lcd.print(15 - millis() / 1000);
    }
  }
}



void DetectingTheMax() //function to get the maximum value during the whole time
//and then print it
{
  Max = 0;
  int Alcoholvalue = digitalRead(Alcohol);
  if (Alcoholvalue > Max) {
    Max = Alcoholvalue;
  }
  Serial.print("detected value: ");
  Serial.print(Max);
  lcd.clear();
  lcd.print("detected value:   ");
  lcd.print(Max);
  return Max;
}


void loop()
{

  Warming();
  lcd.clear(); //clearing the lcd so the blow word can appear
  lcd.setCursor(0, 0);
  lcd.print("blow");


  Blowing();
  lcd.clear();
  lcd.setCursor(0, 0);

  unsigned long currentMillis = millis();

  //i want this function to be excuted after the countdown,
  //basically at time = 15005 millisec, but it is not working,
  //i tried using while instead of if but it is still not working
  if (currentMillis - previousMillis == interval) {
    DetectingTheMax();
  }

}

and here is the separate code for getting the max and it works just fine !!

only getting max value
#include <LiquidCrystal.h>
const int Alcohol = 9; 
int Alcoholvalue;
int Max;
unsigned long previousMillis = 10000;
unsigned const long interval = 5000;

LiquidCrystal lcd(A0, A1, A2, A3, A4, A5); //connecting the lcd

void setup() {
    // Setup size of LCD 20 characters and 4 lines
  lcd.begin(20, 4);
  lcd.setCursor(1, 2);
  lcd.setCursor(4, 3);
  lcd.setCursor(0, 0);
  Serial.begin(9600); 
  pinMode(Alcohol, INPUT);
}



void DetectingTheMax()
{
Max = 0;
int Alcoholvalue = digitalRead(Alcohol); 
if (Alcoholvalue > Max){
Max = Alcoholvalue;
}
   Serial.print("detected value: ");
   Serial.print(Max);
   lcd.clear();
   lcd.print("detected value:   "); 
   lcd.print(Max);
   return Max; 
}

void loop() {
unsigned long currentMillis = millis();
 if (currentMillis - previousMillis == interval) {
 DetectingTheMax();
 } 

}

It will get stuck in function blowing(). There is no exit condition.
Please format your code in the IDE before publishing it.

1 Like

There is only one way to use millis():

unsigned long previousMillis;
const unsigned long interval = 1000;

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

  if( currentMillis - previousMillis >= interval)
  {
    previousMillis = currentMillis;

    // here the code within the millis-timer
    ...
  }
}

There are more ways, but most of them are just variations to the code above.

See also the Blink Without Delay.

This is not a millis-timer:

  if (currentMillis - previousMillis == interval) {
 DetectingTheMax();
 } 

The previousMillis is not updated and using == might fail.

I don't understand this code:

while(millis()>=10000) {
  while (millis()<=15005) { 

In my opinion, there is no use for such code.
If you want to do something just once after power on, then you should do that in the setup() function. You can use delay() in the setup().
If you want to warm up or blow during runtime, then you should make a millis-timer and never compare millis() directly with a value.

1 Like

I just did, sorry about that.

Isn't it the same for function Warming()?
do you know howcan i do the exit condition ?

The first is when millis() value reaches 10 seconds and the second one is for only until it reaches the 15005 basically 15 seconds. This is giving 5 seconds of blowing after the 10sec of warm up

i'm trying to make a condition which is when the warm up (10sec) and the blowing (5sec) are done,, i also tried to make it like if (currentMillis == 15005) and it didn't work.

please not that this code worked when separated

I think I see what you are trying to do:

Obtain several samples using this cycle:

  1. warm up (10 seconds)
  2. collect sample (5 seconds)
  3. calculate the max over all the samples collected this session.
  4. start again.

You probably need a button to restart the cycle, otherwise it will continue endlessly, collecting empty samples.

The way you have structured the code, it could only work once during an Arduino power on session.

The value of millis() is zero when the Arduino starts and you cannot reset it.
When you do a test like this:

while (millis() <= 10000) { //statements }

It will perform the statements for the first 10 seconds after the Arduino starts and never again until the next restart.

If this is entered after the Arduino has been running for 10 seconds, it will never stop until the Arduino restarts:

while (millis() >= 10000) { //statements }

Start with the famous "blink without delay()" sketch (already referenced by @Koepel) and adapt it for your 10 and 5 second periods and use that as a basis.

2 Likes

@khamis98, your topic has been moved to a more suitable location on the forum.

1 Like

this is exactly what i want

That's why added another while function under this function which says as long as we are under 15000 millisec

The thing is i cannot use the same way as blink without delay because of:

1- assuming that i made two intervals one is a interval1=10000 and the other one is interval2=5000
2- if i want to create a condition instead of this

then it would be like this
unsigned long currentMillis = millis();
if (currentMillis - previousMillis <= interval1) {
previousMillis = currentMillis;

3- the other condition is going to look like this
unsigned long currentMillis = millis();
if (currentMillis - previousMillis <= interval2) {
previousMillis = currentMillis;

The problem here is that both conditions are satisfied at the same time and the output is going to be so messy.
So i added one more condition before the blowing
if (currentMillis >= interval1) {

but i'm still getting a very messed up output.... the lcd is going crazy and it keeps reprinting blow
here is a complete code with trying to use the similar method:

click to open
#include <SoftwareSerial.h>
#include <String.h>
#include <LiquidCrystal.h>

const int Alcohol = 9; //the sensor connected to pin 9
int Alcoholvalue;
int Max;
unsigned long previousMillis = 0;
unsigned const long interval1 = 10000; //so here is the warm up time
unsigned const long interval2 = 5000; //so here is the blowing time

LiquidCrystal lcd(A0, A1, A2, A3, A4, A5); //connecting the lcd
void setup() {
  // Setup size of LCD 20 characters and 4 lines
  lcd.begin(20, 4);
  lcd.setCursor(1, 2);
  lcd.setCursor(4, 3);
  lcd.setCursor(0, 0);
  Serial.begin(9600);
  pinMode(Alcohol, INPUT);
  lcd.print("warming up, wait");
}


void Warming() //this function will show a countdown timer for 10 seconds (i need the 10 sec as a warm up time for the sensor)
{
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis <= interval1) {
    previousMillis = currentMillis;
    lcd.setCursor(0, 1);
    int count = 10 - millis() / 1000;
    lcd.print (count);
    //to remove the 0 from the 10 before the 9
    if (count < 10) {
      lcd.print(' ');
    }
  }
}

void Blowing()    //thie is a countdown while exhaling or blowing towards the sensor for 5sec
{
  unsigned long currentMillis = millis();
  if (currentMillis >= interval1) { //after 10sec of warming
    if (currentMillis - previousMillis <= interval2) {
      previousMillis = currentMillis;
      lcd.setCursor(0, 1);
      lcd.print(15 - millis() / 1000);
    }
  }

}



void DetectingTheMax() //function to get the maximum value during the whole time
//and then print it
{
  Max = 0;
  int Alcoholvalue = digitalRead(Alcohol);
  if (Alcoholvalue > Max) {
    Max = Alcoholvalue;
  }
  Serial.print("detected value: ");
  Serial.print(Max);
  lcd.clear();
  lcd.print("detected value:   ");
  lcd.print(Max);
  return Max;
}


void loop()
{

  Warming();
  lcd.clear(); //clearing the lcd so the blow word can appear
  lcd.setCursor(0, 0);
  lcd.print("blow");


  Blowing();
  lcd.clear();
  lcd.setCursor(0, 0);

  unsigned long currentMillis = millis();

  //i want this function to be excuted after the countdown,
  //basically at time = 15005 millisec, but it is not working,
  //i tried using while instead of if but it is still not working
  if (currentMillis - previousMillis >= interval2) {
    previousMillis = currentMillis;
    DetectingTheMax();
  }

}

If you want the following:

  1. Arduino starts
  2. it enters a 10 second warm up phase and prints a countdown message every second.
  3. it then collects samples over the next 5 seconds, testing for the maximum.
  4. it prints the maximum and goes into a wait loop.
  5. The operator the presses the reset button to start another test (if required)

Then you can do it like this pseudocode and all in setup()

void setup() {
  
  // print the start message

  for ( int i = 0; i< 10 ; i++ ) {
    // print warming coundown message every second (i)
    delay( 1000 ) 
  }
  
  // we are now 10 seconds into the cycle

  // Blowing

  // now every 100 ms for the next 5 seconds
  // take a sample and see if it is higher that 
  // highest we have seen

  int maxSample = 0
  for ( int i = 0; i< 50 ; i++ ) {
    // print reading current_sample
    // now read current_sample and test if it is the new maximum
    if( current_sample > maxSample ) maxSample = current_sample ;
    delay( 100 ) ;
  }

  // we are at the end so print the value of maxSample
}

void loop() {
  // do nothing here
}

Detecting the maximum is not a separate function. As soon as you have a sample, you compare it with the previous value of maximum and update maximum if required.

1 Like

How can i treat the (i) as a second ?

Something like:

for ( int i = 0; i < 10 ; i++ ) { 
  Serial.print("This is a countdown and the current second is:") ;
  Serial.println( i ) ;
  delay(1000) ;
}
1 Like

but this is using delay,
i cannot use delay in my code because i will add more functions to it while the countdown is working

Then maybe something like this :

void loop(){ 
. . . 
  static uint32_t countdownNext = millis() ;
  static int8_t count = 10 ;
  if ( count > 0 &&  millis() - countdownNext > 1000UL ) {
     count = count - 1 ;
     Serial.println( count ) ;
     countdownNext = millis() ; 
  }
. . .
}
1 Like

Ok thank you, this part worked well, Now how can i do the second timer? I tried many codes so far and nothing worked, can you help with that too?

Here it is a simple state machine (untested). You'll have to add print (debug) statements to see how it works, then add your own logic.

const uint8_t START = 0 ;
const uint8_t WARMING = 1 ;
const uint8_t BLOWING = 2 ;
const uint8_t END = 3 ;

uint8_t state ;
uint32_t countdownNext ;
uint32_t blowingStartMs ;
int8_t count ;

void setup() {
  state = START ;
}

void loop() {

  if ( state == START ) {
    // print starting
    count = 9 ;
    countdownNext = millis() ;
    state = WARMING ;
  }
  else if ( state == WARMING ) {
    if ( millis() - countdownNext > 1000UL ) {
      count = count - 1 ;
      countdownNext = millis() ;
      // print count
    }
    if ( count <= 0 ) {
      state = BLOWING ;
      blowingStartMs = millis() ;
      // print start blowing
    }
  }
  else if ( state == BLOWING ) {
    // print collect samples and check for maximum here
    delay(100 ) ;  // to limit debug printing
    if ( millis() - blowingStartMs > 5000UL ) {
      // print results here
      state = END ;
    }
  }
  else if ( state == END ) {
    // do nothing or go back to start
    // state = START 
  }

}
1 Like

Great Idea !!

I used this method and it is working so nicely with me...

Thank you so much