BlinkWithoutDelay question

So I have created this project to have an LED blinking at a constant speed whenever the program is running. The first LED (green) will blink faster and faster the closer an object gets to the sensor. The second LED (Blue) should be blinking constantly. The LED (white (is referenced in code as blue)) is only blinking when my ultrasonic sensor has something in range. I would like to make it so it blinks no matter the circumstances. This is probably something obvious Im missing - excuse my noob-ness NOTE: if the picture of the circuit isnt correct (pins not being correct) know that it is correct in real life, this is what I get for doing a quick mock up and post at 1:00 AM

#include "DHT.h";


#define DHTTYPE DHT22   // DHT Type is DHT 22 (AM2302)
#define DHTPIN 7       // DHT-22 Output Pin connection
#define trigPin 5
#define echoPin 6
#define beepPin 12
#define scalingfactor 10
#define blueLed 11

#define ledPin2 10

float hum;    // Stores humidity value in percent
float temp;
float soundsp;
float soundum;
 DHT dht(DHTPIN, DHTTYPE);  // Initialize DHT sensor for normal 16mhz Arduino

int centi = 0;

const long erval = 500;

unsigned long previous = 0;
unsigned long lastduration = 0;
unsigned long duration = 0; //time it takes for sound to return
unsigned long maxtime = 2000;
unsigned long i = 1;  //variable to determine actual distance
unsigned long time2 = 0;
unsigned long time1 = 0;



void setup() {
  Serial.begin (9600);  //open monirot channel
  dht.begin();  //start temp and humidity sensor
  pinMode(trigPin, OUTPUT);
  pinMode(echoPin, INPUT);
  pinMode(beepPin, OUTPUT);
  pinMode(blueLed, OUTPUT);
}


void calculate(){
      // Calculate the Speed of Sound in M/S in accordance with temp
    soundsp = 331.4 + (0.606 * temp) + (0.0124 * hum);
    
    // Convert to cm/us
    soundum = soundsp / 10000;
}

void logdistance() {
  digitalWrite(trigPin, HIGH); // send sound signal
  delayMicroseconds(10);
  digitalWrite(trigPin, LOW);  // stop sound signal
  duration = pulseIn(echoPin, HIGH);  // listen for sound signal
  centi = (duration* soundum)/2;   // determine delay of sound signal in one direction
  maxtime = scalingfactor * centi;  // change max time in accordance to the distance of the previous object
}


void beep() {
  digitalWrite(beepPin, HIGH);   // turn on the beep pin (or LED pin)
  delay(60);  
  logdistance();   // run the log distance function again so we get accurate data
  if (time2 - time1 < 30) {  // delay based on how close an object is
    delay((60 - (time2 - time1)));
  }
  digitalWrite(beepPin, LOW);  // turn off beeper (or LED)
  i = 1;
    while (1) {
    if (i % 60 == 0) { 
      logdistance();  // run my log distance funciton
    } 
    delay(1);
    ++i;  // add one to i to report that we have recorded the distance
    if (i >= maxtime) { // dont report distanc if sound took longer than max time to return
      break;
      } 
    }
}


void BlinkBlue (int interval){
   static long prevMill = 0;
   if (((long)millis() - prevMill) >= interval){ 
    prevMill = millis(); 
    digitalWrite(blueLed, !digitalRead(blueLed));
   }
}

void temps(){
  hum = dht.readHumidity();  // Get Humidity value
  temp= dht.readTemperature();  // Get Temperature value
}



void loop() {
  BlinkBlue(500); 
  beep();

  temps();
  calculate();




  //print results
  Serial.print (duration);
  Serial.print("c. ");
  Serial.print(centi);
  Serial.print(" ");
  Serial.print(temp);
  Serial.println(" ");
}

BlinkBlue(500); //<---- your trying to blink the LED with 0.5s period

pulseIn(echoPin, HIGH); //<---- the default (blocking) timeout for the pulseIn function is 1s

can you see the problem here?

IMHO you may want to review your code flow especially since you are considering having multiple blinking LEDs…

hope that helps…

It’s your beep() function. If you want to use non-blocking techniques (Which is what BlinkWithoutDelay is) in your code, you have to structure your entire program to be non-blocking. Your beep() function on the other hand spends significant amounts of time looping and waiting on the ultrasonic sensor, and is looping THOUSANDS of times. pulseIn has a default timeout of 1 second, meaning the processor will sit there and wait for 1 full second, doing nothing, wasting literally millions of processor cycles that could have been doing something else. And you stay in that while(1) loop (and that’s another problem in and of itself) for 2,000 iterations. 2,000 seconds is over half an hour. That’s how long your program would spend in that one function with nothing in front of the ultrasonic sensor.

if (i >= maxtime) { // dont report distanc if sound took longer than max time to return

This comment does not make any sense. i is not storing a pulse length value, it is a loop counter.

You need to fundamentally restructure how your program handles the ultrasonic sensor so that it doesn’t spend too much time babysitting one thing. You want your processor to be as ADHD as possible. Many processes in a microcontroller might take significant amounts of time to do, so maybe you split them into multiple steps and keep track of where you are in the sequence so you can check other things in the middle of the task. Some things might not change all that often. Do you really need to check temp and humidity and recalculate the speed of sound on every pass through the loop? You can put that one a timer too and only do it every 5 seconds or minute or whatever you want.

The ultrasonic sensor sounds ripe for interrupts to handle it in the background. You can set it up on Timer1 so the PWM hardware sends out the pulses and the Input Capture input records the time it takes for the pulse to get back. Set it up right and it’ll all happen automatically in the background.

What Jiggy-Ninja said.

In addition:
Well done for breaking your code into functions rather than trying to cram everything into loop().
The code for BlinkBlue() is as it should be, your other code should be modelled on that. No delays, not even delay(1). No while loops.

   static long prevMill = 0;

Should be:

   static unsigned long prevMill = 0;

Or

   static uint32_t prevMill = 0;
   if (((long)millis() - prevMill) >= interval){

Should be:

   if (millis() - prevMill) >= interval){

Why did you think you needed a signed integer for millis() when millis() is unsigned?

++Karma; // For using functions.

The first LED (green) will blink faster and faster the closer an object gets to the sensor. - ok
The second LED (Blue) should be blinking constantly. - ok
The third LED (white) blinks no matter the circumstances - The same as blue?

-jim lee

The default timeout on pulseIn() is 1,000,000 microseconds (1 second). That is WAY more than you need for an HS-SR04 ultrasonic range finder. I have found that 30,000 microseconds is a good value:

  duration = pulseIn(echoPin, HIGH, 30000);  // listen for sound signal

Listening for an echo pulse shouldn't really require any blocking at all. Use a pin change interrupt and separate millis()-based timeout in a non-blocking method.

Jiggy-Ninja:
And you stay in that while(1) loop (and that's another problem in and of itself) for 2,000 iterations. 2,000 seconds is over half an hour. That's how long your program would spend in that one function with nothing in front of the ultrasonic sensor.

how should I replace the while loop? Im not sure of an alternative to while().
Im writing out all of my delays now with non blocking delays. After I do that I will be re timing the puleIn(echoPin, HIGH) with a 30000 time rather than the 1 second time. hopefully that will help speed things up.
is there anything else I need to do in order to get this working?
Thank you all so much for your help- this alone has given me alot of knowledge ill use in the future

Sort out your delays and other problems then post what you have. The general answer is you let loop do the looping and usually use if to see if something needs doing.

Im not sure of an alternative to while().

If you want a loop that does something for a period then look no further than the loop() function. The clue is in the name. Using loop() allows you to do other things at more or less the same time, such as controlling LEDs, making beeping noises and responding to user input if required, but of course you must eliminate all blocking code such as delay(), which you say you are doing

okay, heres what I have so far:

//#include "DHT.h";


//#define DHTTYPE DHT22   // DHT Type is DHT 22 (AM2302)
//#define DHTPIN 7       // DHT-22 Output Pin connection
#define trigPin 5
#define echoPin 6
#define beepPin 12
#define scalingfactor 10
#define blueLed 11

#define ledPin2 10

int hum = 25;    // Stores humidity value in percent
int temp = 25;
float soundsp;
float soundum;
// DHT dht(DHTPIN, DHTTYPE);  // Initialize DHT sensor for normal 16mhz Arduino

int centi = 0;

const long erval = 500;
unsigned long delayStart = 0;
bool delayRunning = false;
unsigned long previous = 0;
unsigned long lastduration = 0;
unsigned long duration = 0; //time it takes for sound to return
unsigned long maxtime = 500;
unsigned long i = 1;  //variable to determine actual distance
unsigned long time2 = 0;
unsigned long time1 = 0;



void setup() {
  Serial.begin (9600);  //open monirot channel
  //dht.begin();  //start temp and humidity sensor
  pinMode(trigPin, OUTPUT);
  pinMode(echoPin, INPUT);
  pinMode(beepPin, OUTPUT);
  pinMode(blueLed, OUTPUT);
  delayStart = millis(); //start delay timer
  delayRunning = true;

  
}


void calculate(){
      // Calculate the Speed of Sound in M/S in accordance with temp
    soundsp = 331.4 + (0.606 * temp) + (0.0124 * hum);
    
    // Convert to cm/us
    soundum = soundsp / 10000;
}

void logdistance() {
  digitalWrite(trigPin, HIGH); // send sound signal
  delayMicroseconds(10);
  digitalWrite(trigPin, LOW);  // stop sound signal
  duration = pulseIn(echoPin, HIGH);  // listen for sound signal
  centi = (duration* soundum)/2;   // determine delay of sound signal in one direction
  maxtime = scalingfactor * centi;  // change max time in accordance to the distance of the previous object
}


void beep() {
  digitalWrite(beepPin, HIGH);   // turn on the beep pin (or LED pin)
  logdistance();   // run the log distance function again so we get accurate data
  if (time2 - time1 < 30) {  // delay based on how close an object is
    if (delayRunning && ((millis() - delayStart >= ((60 - (time2 - time1))))));
    delayRunning = false;
  }
  digitalWrite(beepPin, LOW);  // turn off beeper (or LED)
  i = 1;
    while (1) {
    if (i % 60 == 0) { 
      logdistance();  // run my log distance funciton
    } 
    delay(1);
    ++i;
    if (i >= maxtime) { 
      break;
      } 
    }
}


void BlinkBlue (int interval){
   static unsigned long prevMill = 0;
   if ((millis() - prevMill) >= interval){ 
    prevMill = millis(); 
    digitalWrite(blueLed, !digitalRead(blueLed));
   }
}

//void temps(){
//  hum = dht.readHumidity();  // Get Humidity value
//  temp= dht.readTemperature();  // Get Temperature value
//}



void loop() {
  BlinkBlue(500); 
  beep();

//  temps();
  calculate();


}

No matter what I try Im unable to replace the delay(1); in the beep() function. Ive tried replicating the non blocking delay I used at the beginning of the beep() (with the same and different time variables) function however that only keep returning errors

Two of my tutorials may help
How to write Timers and Delays in Arduino introduces the millisDelay class that lets you start, stop restart multiple timers/delays
and
Multi-tasking in Arduino which shows you how to do multiple tasks at once. Using millisDelay instead of delay()s insures that the tasks return quickly if there is nothing to do. There are numerous example sketches in the tutorial

If not mistaken, NewPing is a library that allows you to do non-blocking readings with the SR04.

i rewrote my beep() function as follows:

void beep() {
  digitalWrite(beepPin, HIGH);   // turn on the beep pin (or LED pin)
  logdistance();   // run the log distance function again so we get accurate data
  if (time2 - time1 < 30) {  // delay based on how close an object is
    if (delayRunning && ((millis() - delayStart >= ((60 - (time2 - time1))))));
    delayRunning = false;
  }
  digitalWrite(beepPin, LOW);  // turn off beeper (or LED)
  i = 1;
    while (1) {
    if (i % 60 == 0) { 
      logdistance();  // run my log distance funciton
    } 
    //delay(1);
    if (delay2Running && ((millis() - delay2Start) >= DELAY_TIME)) {
      delay2Start += DELAY_TIME;
        }
    ++i;
    if (i >= maxtime) { 
      break;
      } 
    }
}

As you see I replaced the one second dealy with a non blocking technique however it is behaving as if there is not a delay there.
Thanks again for your help- certainly amazing to see how kind this community is to noobs

You need to break your beep() method into to part,
i) calculate the beep() delay
ii) execute the beep depending on the current delay

try this type of code

#include "millisDelay.h"
// install SafeString library from Arduino manager to get millisDelay class 
// or see the tutorial https://www.forward.com.au/pfod/ArduinoProgramming/TimingDelaysInArduino.html
unsigned long interBeepDelay_mS;
unsigned long beepDelay_mS = 1000;
millisDelay beepDelay;
millisDelay interBeepDelay;
boolean shouldBeBeeping;

void calculateBeepDelay() {
  // cacluate delay to next beep here
  // add logic to set/ clear shouldBeBeeping flag here
  shouldBeBeeping = true; // some distance calc
  if (!shouldBeBeeping) {
    return;
  }
  // calculate actual interBeepDelay hre
  interBeepDelay_mS = 1000; // <<< add some real cacluation
  if (!interBeepDelay.isRunning()) {
    // not already in an interdelay start one now
    interBeepDelay.start(interBeepDelay_mS);
  } // else just use interBeepDelay for next restart
}

void checkInterBeepDelay() {
  if (interBeepDelay.justFinished()) {
    if (shouldBeBeeping) {
    // start beep
     digitalWrite(beepPin, HIGH);   // turn on the beep pin (or LED pin)
     beepDelay.start(beepDelay_mS); 
    }    
  }
}

void checkStopBeep() {
  if (beepDelay.justFinished()) {
    digitalWrite(beepPin, LOW);  // turn off beeper (or LED)    
  }
}

void loop() { 
  //  other stuff
  calculateBeepDelay();  // <<< these 3 methods just return quickly if there is nothing to do
  checkInterBeepDelay();
  checkStopBeep();
}

willbill26:
i rewrote my beep() function as follows:

void beep() {

digitalWrite(beepPin, HIGH);  // turn on the beep pin (or LED pin)
  logdistance();  // run the log distance function again so we get accurate data
  if (time2 - time1 < 30) {  // delay based on how close an object is
    if (delayRunning && ((millis() - delayStart >= ((60 - (time2 - time1))))));
    delayRunning = false;
  }
  digitalWrite(beepPin, LOW);  // turn off beeper (or LED)
  i = 1;
    while (1) {
    if (i % 60 == 0) {
      logdistance();  // run my log distance funciton
    }
    //delay(1);
    if (delay2Running && ((millis() - delay2Start) >= DELAY_TIME)) {
      delay2Start += DELAY_TIME;
        }
    ++i;
    if (i >= maxtime) {
      break;
      }
    }
}



As you see I replaced the one second dealy with a non blocking technique however it is behaving as if there is not a delay there.
Thanks again for your help- certainly amazing to see how kind this community is to noobs

You still have the while loop. Why are you looping 2,000 times?

Remember the point of non-blocking code techniques is hyper ADHD. Get into the function, check if you need to do anything, do something if you do, then get out quickly so you can check on other tasks. A long loop that you are stuck inside is BAD. Get rid of it.

Why are you looping 2,000 times? Why are you only logging distance every 60th time through that loop, and doing nothing the other 59 times?

drmpf:

//calculate delay to next beep here

//calculate actual interbeepdelay

Whats the difference between delay to next beep and interbeepdealy?

Jiggy-Ninja:
You still have the while loop. Why are you looping 2,000 times?

Remember the point of non-blocking code techniques is hyper ADHD. Get into the function, check if you need to do anything, do something if you do, then get out quickly so you can check on other tasks. A long loop that you are stuck inside is BAD. Get rid of it.

Why are you looping 2,000 times? Why are you only logging distance every 60th time through that loop, and doing nothing the other 59 times?

jiggy-ninja: I had the while loop there because this is as follows- sorry, should have explained this better earlier

i=1  //time keeping variable
while(1){   //excecutes logdistance(); every 60 miliseconds
  if (i%60 = 0){
   logdistance();
}
delay(1);  // delay by 1 millisecond
++i;  // adds to the time keeper
if(i>=maxtime){   //once it has run through all this 2000 times exit this loop and sequetially run 
// the other functions
  break;
}
}
}

willbill26:
Whats the difference between delay to next beep and interbeepdealy?

Confusion on my part. When I started the post I imagined he wanted variable beeps, but later realised he wanted variable interbeep intervals (at least I think that’s what he wants)
so should say something more like what is in the loop() code

  calculateBeepDelay();  //  next interbeep delay calc
  checkInterBeepDelay();  // check if interbeep delay just finished
  checkStopBeep();    // check if beep delay should stop

Actually calculateBeepDelay() should be more accurately named calculateInterBeepDelay() :slight_smile: