pick holes in my code...

Projects General Guidance
1

Guys,

Please find errors and help me improvise my code. I have tried to comment to the best extent. However, I putting up the project description below.

I am using 2 LDRs to sense the first axis (East to West). I had the following issue. The LDRs does not give equal outputs and hence I had to subtract 50 to ensure that the outputs are close to each other (line 94 of the code). I am certain that this is NOT the best approach and might not be reliable.

The system uses RESET system to bring back the panel to East after sunset. I have planned to put across two Limit switches, one at East and other at West. As soon at the West limit switch is triggered, a relay (DPDT), is switched ON. The first NO of the relay is used as holding contact and second goes to Arduino. Similarly the NC of the East Limit switch is in series with the NO of the West limit switch and the relay.

/*
  MADHAV
  04122015  0954
  Ver 01

MAJOR NOTE:  THIS VERSION USES SMOOTHING 
             INCORPORATING AUTO SET / RESET FACILITY

 *  Read Analog Voltage of East_Sens and West_Sens on pins A0 and A1
  

LCD Conn:

 * LCD is from DFROBOT

 * LCD RS pin to digital pin D8
 * LCD Enable pin to digital D9
 * LCD D4 pin to digital pin 4
 * LCD D5 pin to digital pin 5
 * LCD D6 pin to digital pin 6
 * LCD D7 pin to digital pin 7
 * LCD R/W pin to ground
 * 10K resistor:
 * ends to +5V and ground
 * wiper to LCD VO pin (pin 3)
 */


// Variables

#include <LiquidCrystal.h>
LiquidCrystal lcd(8,9,4,5,6,7);



int diff1 = 0;                                          // difference between east and west sensor inputs
int diff2 = 0;                                          // difference betweeen west and east sensor inputs

int East_drive = 10;                                    // CW output to motor H bridge circuit
int West_drive = 11;                                    // CCW output to motor H bridge circuit

int EastLS = 2;                                        // Limitswitch on east end of travel for stopping the end 
of travel from
                                                        // west to east and be ready for next day.
int WestLS = 3;                                        // Limit swtich on west end of travel to sense end of day 
                                                        // a relay (DPDT) will switch on the motor in CW (East) direction and is triggered by 
                                                        // West Limit switch for ON.  The other NO will work as holding contact and shall be 
                                                        // switched OFF by NC of East end Limit switch.
int tol = 5;                                            // dead band between East sensor and west sensor
int ton = 500;                                         // motor's time ON duration.

int var = 0;                                            // for future PWM output
                                                        // where we have the PWM (to) in proportion to error
                                                        // larger error = larger ton
                                                        // this PWM signal will to to fifth Transistor (MOSFET later) to be placed
                                                        // below the lower two transistors of the H bridge.  
int PWM = 13;

void setup()

{Serial.begin(9600);
Serial.println("start");

lcd.clear();
lcd.setCursor (0,0);
lcd.print("SOLAR TRACKER");

lcd.setCursor (0,2);
lcd.print("Ver.4 Madhav");
delay (1000);

  lcd.begin (16, 2);
  lcd.clear();
 
  pinMode(10, OUTPUT);                                                      // East_drive motor input to H bridge
  pinMode(11, OUTPUT);                                                      // West_drive motor input to H bridge
  pinMode(13, OUTPUT);
  
  pinMode(2, INPUT);                                                        // Limitswitch on East side to stop panel 
  pinMode(3, INPUT);                                                        // Limitswitch on West side to sense sun set and start to East
    
}


void loop() 
{



  int East_Sens = analogRead (A5);                                          // read the East_Sens on analog pin A0:
  int West_Sens = analogRead (A4);                                          // read the West_Sens on analog pin A1:
 
  
  East_Sens = map (East_Sens, 0, 1023, 0, 255);                             // restrict the value between 0 and 255
  West_Sens = map (West_Sens, 0, 1023, 0, 200);                             // restrict the West_Sensor between 0 and 255
  
  diff1 = East_Sens - West_Sens;
  diff2 = West_Sens - East_Sens;
 
 if(abs (diff1) <= tol)

 {
    digitalWrite (10 & 11, LOW);
     
  } 
  
  else  
  {    
    if( (diff1 > tol))
    {
      digitalWrite (East_drive, LOW);
      digitalWrite (East_drive, HIGH);
      delay (ton);
      digitalWrite (East_drive, LOW);
    }

    
if( (diff2 > tol) )

    {
      digitalWrite (West_drive, LOW);
      digitalWrite (West_drive, HIGH);
      delay (ton);
      digitalWrite (West_drive, LOW);
    }
  }
  delay (10);


digitalRead (EastLS);
digitalRead (WestLS);

if (WestLS == HIGH && East_drive == LOW)                               // Sunset is detected as West side limit swtich goes high, this is one end of the tracking 
                                                                        // and the panel starts back journey back to East through East drive and until the East side
                                                                        // limit switch goes high.But the limit switch immediately bounces to close condition once the
                                                                        // journey starts.  To prevent that a holding contact has to be provided.  This is done by
                                                                        // external hardwiring.
{  
  int Var=255;                                                          // go back from West to East at full speed.
  
  digitalWrite (East_drive, HIGH );                                     
                                                                        
}

if (WestLS == HIGH && East_drive == HIGH)                                   // Sunset is detected as West side limit swtich goes high, this is one end of the tracking 
                                                                        // and the panel starts back journey back to East through East drive and until the East side
                                                                        // limit switch goes high.But the limit switch immediately bounces to close condition once the
                                                                        // journey starts.  To prevent that a holding contact has to be provided.  This is done by
                                                                        // external hardwiring.
{  
                                                          
  
  digitalWrite (East_drive, LOW ); 

                                                                        
}

Serial.print ("East_Sens =");
Serial.println (East_Sens);
Serial.print ("West_Sens = ");
Serial.println (West_Sens);
Serial.print ("diff1 = ");
Serial.println (diff1);
Serial.print ("diff2 = ");
Serial.println (diff2);

delay(750);

}
2 
digitalWrite (10 & 11, LOW);

What do you imagine that line does?

3 
{ 
  int Var=255;                                                          // go back from West to East at full speed.
 
  digitalWrite (East_drive, HIGH );                                     
                                                                       
}

Create a variable, assign a value to it, then let it go out of scope without ever using it.

4 
  int East_Sens = analogRead (A5);                                          // read the East_Sens on analog pin A0:
  int West_Sens = analogRead (A4);                                          // read the West_Sens on analog pin A1:

When the code and the comment don't match, guess which one the compiler believes?

5 
    digitalWrite (10 & 11, LOW);

Please explain what you think this is doing. It isn't doing that, but without knowing what you think it is doing, it is not possible to tell you how to fix it.

digitalRead (EastLS);
digitalRead (WestLS);

Like reading a book with your eyes closed.

if (WestLS == HIGH && East_drive == LOW)

Ummm. 3 is not HIGH, so this statement will never be true. 10 is not LOW, either.

{ 
  int Var=255;                                                          // go back from West to East at full speed.
 
  digitalWrite (East_drive, HIGH );                                     
                                                                       
}

The local variable (Var) goes out of scope without ever being used. Why bother declaring it?

6

Hi,

Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?

Thanks... Tom..... :slight_smile:

7 
int PWM = 13;

That's going to cause confusion at some point.

8 
  pinMode(10, OUTPUT);                                                      // East_drive motor input to H bridge
  pinMode(11, OUTPUT);                                                      // West_drive motor input to H bridge
  pinMode(13, OUTPUT);
 
  pinMode(2, INPUT);                                                        // Limitswitch on East side to stop panel
  pinMode(3, INPUT);                                                        // Limitswitch on West side to sense sun set and start to East

You already gave them nice (but too big) names - why not use them?

9 
  East_Sens = map (East_Sens, 0, 1023, 0, 255);                             // restrict the value between 0 and 255

That's an expensive "divide by four" operation.

10

AWOL:

  East_Sens = map (East_Sens, 0, 1023, 0, 255);                             // restrict the value between 0 and 255

That's an expensive "divide by four" operation.

Hello, what do you mean by expensive?
Do you mean that it uses a lot of processing power per say? Or it could be done easier to consume less resources? Would it be more efficient to do something like: East_Sens=((East_Sens*255)/1023);

Thanks!

11

Please use spaces and indents and brackets always in the same way. That makes it (a lot) easier to read the code. In the Arduino IDE menu is a auto-text-format option (or Ctrl+T).

12

Hello, what do you mean by expensive?

Look at the source code for map. See how many multiplication and divisions are performed.

Would it be more efficient to do something like

vs:
East_Sens /= 4;
?
Which looks faster/easier to you?

13

Pringles:
Hello, what do you mean by expensive?
Do you mean that it uses a lot of processing power per say? Or it could be done easier to consume less resources? Would it be more efficient to do something like:

East_Sens=((East_Sens*255)/1023);

Thanks!

Here's the source of "map":

long map(long x, long in_min, long in_max, long out_min, long out_max)
{
  return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
}

I don't know what "per say" means - maybe you meant "per se"

I think you'll agree that on an eight-bit micro, converting all those variables to thirty-two bits and doing all that arithmetic is going to be a lot more time consuming than simply shifting a sixteen bit value two places to the right (which is what the compiler will reduce a "/= 4" to)

14

Ok - who is @Pringles, and what is their relationship (if any) to @madhavdivya?

I mean - @Pringles is asking a question about map, in a way which seems to suggest a familiarity or some form of relationship to @madhavdivya's project...

I'm probably reading too much into the whole thing, and maybe @Pringles is just looking for some clarification (while doing a slight bit of thread derail)...

AWOL:
I think you'll agree that on an eight-bit micro, converting all those variables to thirty-two bits and doing all that arithmetic is going to be a lot more time consuming than simply shifting a sixteen bit value two places to the right (which is what the compiler will reduce a "/= 4" to)

@AWOL - while ultimately correct, and perhaps @Pringles will understand it - I have my doubts that @madhavdivya will have any chance of understanding it - since they already seem confused as to what variables are for and how they work (let alone anything else). Hopefully they will respond to some of the questions and constructive criticism in here, in order to enlighten everyone further, and allow us (well, you guys - I'm in the peanut gallery for now) to help them understand.

:smiley:

15

Ok - who is @Pringles, and what is their relationship (if any) to @madhavdivya?

Different continents.

16 
  East_Sens = map (East_Sens, 0, 1023, 0, 255);

Maybe I did the OP a disservice - this isn't the same as East_Sens /= 4;, but I'll bet that's what was intended.

17

PaulS:
Look at the source code for map. See how many multiplication and divisions are performed.
vs:
East_Sens /= 4;
?
Which looks faster/easier to you?

I didn't know this was possible. Where is the documentation and all of the variations you can do for this?
I've seen the

Var += something

but not ever this

East_Sens /= 4

Where can I find what else can I do with the operators before the = ? Thank you! EDIT: PM me if you can, because @cr0sh

@AWOL, thanks for your reply too.

cr0sh:
Ok - who is @Pringles, and what is their relationship (if any) to @madhavdivya?

Why do you ask? None, I just taught map was very efficient, turns out there is easier ways to do it, so I asked a question. Hope it makes sense to you.

Anyhow, I just learned something and I'll stop posting now, @madhavdivya, don't pay attention to my post if it confuses you.

18

@Pringles: Really useful page - you should read it sometime.

19

Where can I find what else can I do with the operators before the = ?

Bottom of the left hand column.

20 
  East_Sens = map (East_Sens, 0, 1023, 0, 255);

Or just East_Sens = East_Sens>>2;
xx yyyy yyyy becomes xxyy yyyy