Could I get some advice on Improving this coding please?

Hello

Grumpy_Mike:
Ditch the idea of using delay and fading by using a loop. The whole thing becomes on continuous loop saying
{
what time is it now?
is it time to check the sensor -> ping it
is it time to look at the result of the ping
is it time to change the LED brightness a notch
}

This helped alot! Thankyou

I had a think and a play and came up with this simple test, which works grand :slight_smile:

const int Trigger = 8;
const int Receiver = 7;
const int ROne = 11;
const int GOne = 10;
const int BOne = 9;

unsigned long CurrentMillis = 0;
long PreviousMillis = 0;
int SonicPeriod = 250;
int ContentPeriod = 30;

int LEDBrightness = 0;

int RBrightness = 0;
int GBrightness = 0;
int BBrightness = 0;

int Distance = 0;
int Responce = 0;

void setup () {
  Serial.begin(9600);
  pinMode (Trigger, OUTPUT);
  pinMode (Receiver, INPUT);
  pinMode (ROne, OUTPUT);
  pinMode (GOne, OUTPUT);
  pinMode (BOne, OUTPUT);
  
}

void loop () {
  CurrentMillis = millis();
  
  if (CurrentMillis - PreviousMillis > SonicPeriod) {
     PreviousMillis = CurrentMillis; 
    digitalWrite(Trigger, LOW);
    delayMicroseconds(5);
    digitalWrite(Trigger, HIGH);
    delayMicroseconds(5);
    digitalWrite(Trigger, LOW);
    
    Responce = pulseIn(Receiver,HIGH);
    Distance = (Responce/29);
    Serial.print(Distance);
    Serial.print("cm");  
    
  } 
  
  
    if (Distance > 100) {
      digitalWrite (ROne, HIGH);
      digitalWrite (GOne, LOW);
      digitalWrite (BOne, HIGH);
    }
    
    else if (Distance > 50) {
      digitalWrite (ROne, HIGH);
      digitalWrite (GOne, HIGH);
      digitalWrite (BOne, LOW);
    }
    
    else if (Distance <= 50) {
      digitalWrite (ROne, LOW);
      digitalWrite (GOne, HIGH);
      digitalWrite (BOne, HIGH);
    }
    
 
}

Until I tried to introduce a second millis check:

const int Trigger = 8;
const int Receiver = 7;
const int ROne = 11;
const int GOne = 10;
const int BOne = 9;
const int Brightness = 3;

unsigned long CurrentMillis = 0;
long PreviousMillis = 0;
int SonicPeriod = 250;
int ContentPeriod = 30;
int ContentBrightnessPeriod = 50;

int LEDBrightness = 255;
int FadeAmount = 1;

int RBrightness = 0;
int GBrightness = 0;
int BBrightness = 0;

int Distance = 0;
int Responce = 0;

void setup () {
  Serial.begin(9600);
  pinMode (Trigger, OUTPUT);
  pinMode (Receiver, INPUT);
  pinMode (ROne, OUTPUT);
  pinMode (GOne, OUTPUT);
  pinMode (BOne, OUTPUT);
  pinMode (Brightness, OUTPUT);
  
}

void loop () {
  CurrentMillis = millis();
  
  if (CurrentMillis - PreviousMillis > SonicPeriod) {
    PreviousMillis = CurrentMillis; 
    digitalWrite(Trigger, LOW);
    delayMicroseconds(5);
    digitalWrite(Trigger, HIGH);
    delayMicroseconds(5);
    digitalWrite(Trigger, LOW);
    
    Responce = pulseIn(Receiver,HIGH);
    Distance = (Responce/29);
    Serial.print(Distance);
    Serial.print("cm");  
    
  } 
  
  if (CurrentMillis - PreviousMillis > ContentBrightnessPeriod) {
        
    analogWrite (Brightness, LEDBrightness);
    LEDBrightness = LEDBrightness + FadeAmount;
    
    if (LEDBrightness == 220 || LEDBrightness == 255) {
      FadeAmount = -FadeAmount;
    }
  }  
  

  
  
    if (Distance > 100) {
      digitalWrite (ROne, HIGH);
      digitalWrite (GOne, LOW);
      digitalWrite (BOne, HIGH);
    }
    
    else if (Distance > 50) {
      digitalWrite (ROne, HIGH);
      digitalWrite (GOne, HIGH);
      digitalWrite (BOne, LOW);
    }
    
    else if (Distance <= 50) {
      digitalWrite (ROne, LOW);
      digitalWrite (GOne, HIGH);
      digitalWrite (BOne, HIGH);
    }
    
 
}

The issue is I dont know where best to put the PreviousMillis = CurrentMillis; line. If I put it with the most frequent check, then the sonic will never get up a long enough interval to be checked, and if I put it with the sonic, the Brightness will only get check every 500milliseconds, instead of 30. Do I need two seperate intergers for each Current/Previous Millis?

Cheers guys :slight_smile: