Go Down

Topic: I'm somewhat new would like expert to critique my sketch (Read 1 time) previous topic - next topic

alfiesty

I have not programmed since playing with the original IBM basic. I trying to write a sketch to control the rolling roof on my observatory. I have a motor on both ends and reverse voltage to the motor at the opposite end to the direction of travel to minimize drag. I'm using one of those 8 relay boards you can get on ebay, two switch bricks, three pushbutton bricks and three LED bricks. Voltage reversal is done in how the relays are wired. I'd just like some expert to critique my work. The sketch is at www.alfiesty.com/sketch. Thanks

Jim
8000ft above the average

Simpson_Jr

Unfortunately I get a 403  (forbidden, no permission) - error while trying to get access.


alfiesty

Sorry, I did not post the entire URL. It is www.alfiesty.com/sketch/sketch_roof_control.

Jim
8000ft above the average

Graynomad

Rob Gray aka the GRAYnomad www.robgray.com

alfiesty

Sorry, This is the first time I have put a file up on the Internet without help and I screwed it up! Try www.alfiesty.com/sketch/sketch_roof_control/sketch_roof_control.pde
8000ft above the average

AWOL

#5
Apr 28, 2011, 07:02 pm Last Edit: Apr 28, 2011, 07:12 pm by AWOL Reason: 1
Code: [Select]
if (bstop = HIGH)
This is an assignment, not a comparison; it will always be true.
Code: [Select]
if (bismoving = true)//
Ditto.

I'd say if anything, the program is over-commented - this:
Quote
Set up Constants
    Digital Pins 0-8 are outputs for relay drivers
    Pin 0 = Out motor 1 out plus
    Pin 1 = Out motor 1 out minus
    Pin 2 = Out motor 2 in plus
    Pin 3 = Out motor 2 in minus

could just as well be put in the declarations of the pins, whilst this:
Code: [Select]
digitalWrite(openmotoropenplus, LOW);      // sets the digital pin as output

is simply stating the obvious incorrectly.

Generally, it is better to qualify things like pin numbers with "const".
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

alfiesty

Code:

if (bstop = HIGH)

This is an assignment, not a comparison; it will always be true.
Code:

if (bismoving = true)//

Ditto.

I assume it should be
if (bbstop == HIGH)

I'd say if anything, the program is over-commented - this:

All the comments is a matter of style!

Quote
Set up Constants
     Digital Pins 0-8 are outputs for relay drivers
     Pin 0 = Out motor 1 out plus
     Pin 1 = Out motor 1 out minus
     Pin 2 = Out motor 2 in plus
     Pin 3 = Out motor 2 in minus
could just as well be put in the declarations of the pins, whilst this:
Code:

digitalWrite(openmotoropenplus, LOW);      // sets the digital pin as output

is simply stating the obvious incorrectly.

How? Example showing correct would help.
Generally, it is better to qualify things like pin numbers with "const".
done

Thank for looking, this is my first programming in close to 25 years., I have to say the help provided by the arduino site is great!

Jim (8000 feet above the average)
8000ft above the average

AWOL

Quote
digitalWrite(openmotoropenplus, LOW);      // sets the digital pin as output

is simply stating the obvious incorrectly.

How? Example showing correct would help.


Well, for a start, it doesn't make the pin an output, it was made an output with a "pinMode".
Your comment might have read "Set the pin low." or "write zero to the pin", but that doesn't describe what that actually achieves, which is what should be commented, if a comment is needed.
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

Graynomad

Just a stylistic thing, I find long variable names like this

openmotoropenplus

very hard to read, especially when they are in large blocks. I would suggest camel case

OpenMotorOpenPlus

or maybe underscores

open_motor_open_plus

______
Rob
Rob Gray aka the GRAYnomad www.robgray.com

alfiesty

Thanks Rob, thats a very valid observation. I'll do immediately!

Jim
8000ft above the average

Go Up