Simplifying code trouble

Hello there!

I have some code that reads from two LDRs and averages the last 10 readings and compares them to the current reading. The point of the sensors is to detect the color green on a white surface.

However, i know my code is overly complicated but i am struggling to simplify it at the moment.

The code:

#include <Servo.h>

Servo Lmotor, Rmotor;
int total = 0;
int cnt = 0;
int avgColor = 0;
int total2 = 0;
int cnt2 = 0;
int avgColor2 = 0;




void setup(){
  Serial.begin(9600);
  Lmotor.attach(10);
  Rmotor.attach(11);

}

void loop(){
  green();
}
void green(){

  int color = analogRead(A4);
  total = total + color;
  cnt = cnt + 1;  
  avgColor = (total/cnt);

  int color2 = analogRead(A3);
  total2 = total2 + color2;
  cnt2 = cnt2 + 1;  
  avgColor2 = (total2/cnt2);


  if(color < avgColor - 30){
    Lmotor.write(90);
    Rmotor.write(90);
    delay(1000);
  }
  else if(color2 < avgColor2 - 30){
    Lmotor.write(90);
    Rmotor.write(90);
    delay(1000);
  }
  else{
    Lmotor.write(75);
    Rmotor.write(75);
  }

  Serial.print(avgColor2);
  Serial.print("  ");
  Serial.println(color2);

  if(cnt > 10){
    total = 0;
    cnt = 0;
  }

  if(cnt2 > 10){
    total2 = 0;
    cnt2= 0;
  }
delay(60);
}

I tried making a function out of it but i could only read from one LDR at a time successfully. When i tried reading from both through the same function i would get weird readings from the sensors.

Function code:

void loop(){
 green(3);
 green(4);
}

void green(int pin){

  int color = analogRead(pin);
  total = total + color;
  cnt = cnt + 1;  
  avgColor = (total/cnt);

  if(color < (avgColor - 30)){
   
   Stop();
  }
  /*Serial.print(avgColor);
   Serial.print("  ");
   Serial.println(color);*/

  if(cnt > 10){
    total = 0;
    cnt = 0;
  }
  delay(60);
}

So i was just hoping someone could guide me into writing this better or something.

Thank you!

So i was just hoping someone could guide me into writing this better or something.

You need to look at the first code. What is it doing? It reads from one sensor, and updates the average fro that sensor. Independently, it reads from the other sensor and updates the average for that sensor. Independently, it uses the two readings to determine which way to steer the servos.

How can you break three independent actions up into two functions?

Maybe something like this? I can’t test this code right now so i am not sure…

#include <Servo.h>

Servo Lmotor, Rmotor;


void setup(){
  Serial.begin(9600);
  Lmotor.attach(10);
  Rmotor.attach(11);

}

void loop(){
adjustServo(avgColor(4), 4);
adjustServo(avgColor(3), 3);

}


void adjustServo(int avg, int pin){

  if(analogRead(pin) < avg-30){
    if(pin==4)
      turnRight();
    else
      turnLeft();
  }

}

int avgColor(int pin){

  int color=0;

  for(i=0; i<=10; i++)
    color =+ analogRead(pin);

  return (color/10);
}

You probably need to have arrays to store the colour values and the pin numbers so your function code could be something like this (assumes green has the value 0 and red has the value 1)

void updateAverage(colour) {
   colourVal[colour] = analogRead(ldrPin[colour]);
   totalVal[colour] += colourVal[colour];
   // etc
}

and the function would be called with

updateAverag(green);

or

updateAverage(red);

or

for (byte n = 0; n < 2; n ++) {
   updateAverage(n);
}

…R

Hi, If you look at your averaging code lines you are using int type variables, when you divide an int by an int the result will be an int. If you answer is trying to be say 0.25, it will be stored as 0.

You need to go through your code and decide what has to be int and what has to be float.

Tom..... :)