Hi, I am looking for some advice on this sketch I wrote. I am wondering how "clean" it is.
I basically combined 2 different demo sketches: an ultrasonic sensor sketch using newping.h and a motor shield demo.
I am using an Arduino Motor Shield and have connected the motor via channel A + and -. I also added code I found for changing PWM frequencies because the motor was making a high-pitched noise--the line I left active actually resolved it, though I have no clue how or why.
As an FYI, this sketch currently DOES work: a motor turns at a very low speed by default, then if something is within 120cm of the sensor, the motor turns at full speed.
My questions are:
How would I change this to have the motor not running by default and then run at full speed when something is within 120cm? Would I just change the analogWrite command (that I have in the else portion of my if/else statement) to 0 instead of 120?
What can I "clean up" in this sketch? I know this is probably pretty messy/incorrectly written, but it is working so I don't know if I am doing something wrong.
Is there anything wrong with the pins I've set? I was worried that I might be doubling up the use of pin 12 since it's being used by the sensor, but since it's working I am assuming they are 2 different things?
Code:
/*-----( Import needed libraries )-----*/
#include <NewPing.h>
/*-----( Declare Constants and Pin Numbers )-----*/
#define TRIGGER_PIN 12
#define ECHO_PIN 11
#define MAX_DISTANCE 500 // Maximum distance we want to ping for (in centimeters).
//Maximum sensor distance is rated at 400-500cm.
/*-----( Declare objects )-----*/
NewPing sonar(TRIGGER_PIN, ECHO_PIN, MAX_DISTANCE); // NewPing setup of pins and maximum distance.
/*-----( Declare Variables )-----*/
int DistanceIn;
int DistanceCm;
void setup() /****** SETUP: RUNS ONCE ******/
{
//Setup Channel A
pinMode(12, OUTPUT); //Initiates Motor Channel A pin
pinMode(9, OUTPUT); //Initiates Brake Channel A pin
Serial.begin(9600);
//TCCR2B = TCCR2B & B11111000 | B00000001; // set timer 2 divisor to 1 for PWM frequency of 31372.55 Hz
//TCCR2B = TCCR2B & B11111000 | B00000010; // set timer 2 divisor to 8 for PWM frequency of 3921.16 Hz
//TCCR2B = TCCR2B & B11111000 | B00000011; // set timer 2 divisor to 32 for PWM frequency of 980.39 Hz
//TCCR2B = TCCR2B & B11111000 | B00000100; // set timer 2 divisor to 64 for PWM frequency of 490.20 Hz (The DEFAULT)
//TCCR2B = TCCR2B & B11111000 | B00000101; // set timer 2 divisor to 128 for PWM frequency of 245.10 Hz
TCCR2B = TCCR2B & B11111000 | B00000110; // set timer 2 divisor to 256 for PWM frequency of 122.55 Hz
//TCCR2B = TCCR2B & B11111000 | B00000111; // set timer 2 divisor to 1024 for PWM frequency of 30.64 Hz
}//--(end setup )---
void loop() /****** LOOP: RUNS CONSTANTLY ******/
{
delay(100);// Wait 100ms between pings (about 10 pings/sec). 29ms should be the shortest delay between pings.
DistanceIn = sonar.ping_in();
//Serial.print("Ping: ");
//Serial.print(DistanceIn); // Convert ping time to distance and print result
//Serial.print(" in ");
delay(100);// Wait 100ms between pings (about 10 pings/sec). 29ms should be the shortest delay between pings.
DistanceCm = sonar.ping_cm();
//Serial.print("Ping: ");
//Serial.print(DistanceCm);
//Serial.println(" cm");
if ( (DistanceCm <= 120) && (DistanceCm != 0) )
{
//Serial.println("120 Cm or closer! ");
analogWrite(3, 255); //Spins the motor on Channel A at full speed
}
else
{
analogWrite(3, 120); //Spins the motor on Channel A at low speed
}
}//--(end main loop )---
Would I just change the analogWrite command (that I have in the else portion of my if/else statement) to 0 instead of 120?
Yes.
Is there anything wrong with the pins I've set? I was worried that I might be doubling up the use of pin 12 since it's being used by the sensor, but since it's working I am assuming they are 2 different things?
It is not two different things it is the same pin, so yes make them different. While it might appear to work that is only because you have not tested it sufficiently.
You don't use DistanceIn so there is no need for that
Your motor is on pin3, not 11 and 12 which are for the ultrasound sensor so no conflict. Because your motor is on pin3 and you play around with timer2 which Drives Pins 11 and 3 on a UNO, you are indeed influencing the duty cycle
Have you tried to see what happens if you put 0 instead of 120 in analogWrite(3, 120);? Give it a try.
Grumpy_Mike: The code is being used in an art project which did run successfully for over a month straight. It was my understanding that by setting pin 12 in the setup function (which runs once), I was just telling the motor what direction to go in: forward or backward (the writing next to pin 12 says "Dir A" on the arduino motor shield I am using). Is that not necessary?
J-M-L: Thanks for seeing that--I didn't realize I had both DistanceIn and DistanceCm there. Changing the value does seem to work!
/*-----( Declare Constants and Pin Numbers )-----*/
#define TRIGGER_PIN 12
And
pinMode(12, OUTPUT); //Initiates Motor Channel A pin
These references the same physical pin, so the same pin can be controlled by two different ways in the software. This is not how to write code and will result in all sorts of wired behaviour. The fact that you did not spot this is probably down to the fact that you have no idea what you are doing and once you got the "expected" behaviour you stopped looking. Code always does what it does, whether this makes sense to anyone reading the code is another matter.
I did read and responded with questions to which I received an unnecessarily condescending answer. That kind of behavior is not in the maker spirit this community is meant to embrace.
That kind of behavior is not in the maker spirit this community is meant to embrace.
The maker community is about helping you learn. You asked why something was important if it "worked". I told you it was because you didn't understand what you were doing.
OK, so you do know what you are doing and therefore there is no need to post any questions. So why are you here?
You are on your own buddy as far as I am concerned.
Goodbye, and the best of luck trying to learn anything.
in lines 5 and 6 you define the pins (12,11) that are used for the sonar. This is good practice. I would add a define statement for the pins used for the motor control. In the "analogWrite" statements near the end of the code you are using pin 3 for motor speed, so something like "#define MOTOR_SPEED 3" would make sense. You'd then replace "3" with "MOTOR_SPEED" in lines 50 and 54.
As "J-M-L" notes in post 2, you don't use the value "DistanceIn" so you can get rid of line 13 and lines 35-40.
In the setup section, pin 12 is actually used for the sonar, so the comment "Initiates Motor Channel A pin" on line 19 is confusing. Pin 9 is not used elsewhere in the code, so you should be able to delete or comment out line 21.
The second argument in "analogWrite" defines the relative speed of your motor. "0" is full off and "255" is full on. The motor will theoretically run faster as you go from 0 to 255. It may not run at all for small values up to some threshold that allows it to overcome it's starting inertia.
On the "Tools" menu pull down, clicking "Auto Format" will indent lines according to broadly accepted practice which makes the code more readable.
