UKHeliBob:
Thanks for the clarification, but I bet that the OP didn't mean to use that construction as that is not what the comments say.
Oh, I have no doubt the OP didn't know what he was doing with that part of the code.
UKHeliBob:
Thanks for the clarification, but I bet that the OP didn't mean to use that construction as that is not what the comments say.
Oh, I have no doubt the OP didn't know what he was doing with that part of the code.
I am happy with my motor control, it runs, I can controller it's rise and fall with great precision. John thanks you have been most helpfull and have explained the best way to write my else if statements I did pre check the analog values before doing my else if statements 168/339/509/680/852/1023.
Kind regard
Dale
Thank you and I am glad it is running for you "so far". When it comes time to add buttons you are really going to have to take your understanding "to the next level" though!
And the first step of that will be to heed the advice above about getting rid of the gotos. If you need a better hint it would be:
void loop() {
// NO start: LABEL
int s=sensorValue;
if (sensorValue > 100 && sensorValue < 200) {
pos=10;
}
else if (sensorValue > 300 && sensorValue < 400) {
pos=13;
}
...
myservo1.write(pos);
// NO goto: start
} // Arduino does it for you, back to loop
Cheers,
John
Thanks John,
Yes I do need a hint, I think understand the code you posted, I'll do a re write and post it back here.
Kind regards
Dale
A somewhat different multi-press button example -
class push_button_t
{
const uint8_t _pin;
const uint8_t _transition_level;
const uint8_t _number_of_states;
uint8_t _press_counter;
uint8_t _earlier;
public:
push_button_t(uint8_t pin, uint8_t transition_level = LOW, uint8_t number_of_states = 2)
: _pin(pin)
, _transition_level(transition_level)
, _number_of_states(number_of_states)
, _press_counter(0)
, _earlier(0)
{}
void begin()
{
pinMode(_pin, INPUT);
}
operator uint8_t()
{
uint8_t now = digitalRead(_pin);
if ( now != _earlier )
{
if ( now == _transition_level )
{
_press_counter++;
_press_counter %= _number_of_states;
}
}
_earlier = now;
return _press_counter;
}
};
class led_t
{
const uint8_t _pin;
public:
led_t(uint8_t pin) : _pin(pin) {}
void begin() { pinMode(_pin, OUTPUT); }
uint8_t operator = (uint8_t rhs) { digitalWrite(_pin, rhs); return rhs; }
operator uint8_t () { return ((LOW == digitalRead(_pin)) ? LOW : HIGH); }
};
const uint8_t pinLED = 33; // chipKIT Basic I/O Shield LD1
const uint8_t pinBUTTON = 4; // chipKIT Basic I/O Shield BN1
led_t led(pinLED);
push_button_t button(pinBUTTON, LOW, 4);
void loop()
{
switch ( button )
{
case 0: led = 0; break;
case 1: led = !led; delay(125UL); break;
case 2: led = !led; delay(250UL); break;
case 3: led = !led; delay(500UL); break;
}
}
void setup()
{
led.begin();
button.begin();
}
lloyddean:
A somewhat different multi-press button example -led_t led(pinLED);
push_button_t button(pinBUTTON, LOW, 4);
void loop()
{
switch ( button )
{
case 0: led = 0; break;
case 1: led = !led; delay(125UL); break;
case 2: led = !led; delay(250UL); break;
case 3: led = !led; delay(500UL); break;
}
}
void setup()
{
led.begin();
button.begin();
}
Excellent! That's what I love about C++ OO!
Thanks des,
Thats a lot above my ability right now, I think i can make out it's altering the delay of the led based on the the button couter. hard for me to understand without the comment lines.
Kind regards
Dale
The key here is in asking questions of things you don't understand now, and will never understand, until enough questions are asked, and not simply ignoring what is not currently understood.
Object oriented code, even on embedded systems, can simplify code considerably.
Thing to know learn here is classes and what conversion operators do.
I was gratfully for the post but i was beening honest in my knowlege, I will do some research on the posted code after I re write my program.
Regards
Dale.
Not offended. SImply trying to encourage curiosity and exploration.
Given that your project appears to be at its start I'll throw this one out there.
It does make use of a non-standard feature of the GCC/CPP compiler included with the Arduino IDE and thus I feel fair game but it may be non portable to other compilers - the range based 'case' statements associated with the standard 'switch'/'case' pair.
#include <Servo.h>
const uint8_t pinSERVO = 9;
float pos = 0; // variable to store the rotary switch input
Servo servo;
void loop()
{
// ... 'analogRead' will return a value in the range 0 - 1023
switch ( analogRead(A0) )
{
case 0 ... 99: return;
case 200 ... 299: return;
case 400 ... 499: return;
case 700 ... 799: return;
case 100 ... 199: pos = 10; break;
case 300 ... 399: pos = 13; break;
case 500 ... 599: pos = 20; break;
case 600 ... 699: pos = 25; break;
case 800 ... 899: pos = 30; break;
case 900 ... 1023: pos = 120; break;
}
servo.write(pos);
}
void setup()
{
servo.attach(pinSERVO);
servo.write(pos); // arm speed controller
delay(3000);
}
Hi,
I've modified my code without the goto's and the program is working ok
#include <Servo.h>
Servo myservo1; // create servo object to control a servo
// a maximum of eight servo objects can be created
float pos = 0; // variable to store the rotary switch input
void setup() {
myservo1.attach(9); // attaches the servo on pin 9 to the servo object
myservo1.write(pos);
delay(3000);
}
// the loop routine runs over and over again forever:
void loop() {
int sensorValue = analogRead(A0);
if (sensorValue > 100 && sensorValue < 200) {
pos=10;
}
else if (sensorValue > 200 && sensorValue < 400) {
pos=13;
}
else if (sensorValue > 400 && sensorValue < 600) {
pos=14;
}
else if (sensorValue > 600 && sensorValue < 700) {
pos=15;
}
else if (sensorValue > 700 && sensorValue < 900) {
pos=16;
}
else if (sensorValue > 900) {
pos=17;
}
myservo1.write(pos);
}
But then I added a basic rise and fall code for the first rotary switch position but get the error:
_13_prog.cpp: In function 'void loop()':
_13_prog:44: error: 'else' without a previous 'if'
I removed the else and the code compiled, but when I tested the air raid it only ran the rise and fall code and not the other switch positions
#include <Servo.h>
Servo myservo1; // create servo object to control a servo
// a maximum of eight servo objects can be created
float pos = 0; // variable to store the rotary switch input
void setup() {
myservo1.attach(9); // attaches the servo on pin 9 to the servo object
myservo1.write(pos);
delay(3000);
}
// the loop routine runs over and over again forever:
void loop() {
int sensorValue = analogRead(A0);
if (sensorValue > 100 && sensorValue < 200)
{
pos=12.5;
}
// rise and fall code
for(pos = 12.5; pos < 15; pos += .1) // goes from 12.5 to 15
{ // in steps of .1
myservo1.write(pos); // tell servo to go to position in variable 'pos'
delay(25); // waits 15ms for the servo to reach the position
}
delay(5000);
for(pos = 15; pos>=12.5; pos-=.1) // goes from 15 to 12.5
{
myservo1.write(pos); // tell servo to go to position in variable 'pos'
delay(25); // waits 15ms for the servo to reach the position
}
else if (sensorValue > 200 && sensorValue < 400) // error on this line
{
pos=13;
}
else if (sensorValue > 400 && sensorValue < 600) {
pos=14;
}
else if (sensorValue > 600 && sensorValue < 700) {
pos=15;
}
else if (sensorValue > 700 && sensorValue < 900) {
pos=16;
}
else if (sensorValue > 900) {
pos=17;
}
myservo1.write(pos);
}
Where am I going wrong with this piece of code? Any infomation would be gratefully recieved
Regards
Dale
Dale, the simple format of if is
if (condition)
{
code if true
}
In your case you need to put everything you want to happen "if true" within those braces:
...
if (sensorValue > 100 && sensorValue < 200)
{
set position
rise code
delay
fall code
}
else if (sensorValue > 200 && sensorValue < 400)
... etc
Keeping things indented properly helps you keep track of the braces. And the auto-format tool also. (on the Tools Menu)
Cheers,
John
Thanks John,
Code modified and running ok, much appreciated.
Hazzard thanks for the upload, I was thinking more of a count then time based, but thinking more about it, if i did go down the count method I would need to have a way to re-set the counter and time based is maybe the way to go. Hmm my hurts. I need to wire these switches to my breadboard and start experimenting.
Regards
Dale
Hi,
I thought I had it sorted when I testing last night, the wife was giving me the evil eye on testing the siren so late. turns out that it is not rising and falling. I think it's due to it doing everthing between the braces which is increment up .1 then delay and then increment down .1 which keeps the speed the same, which is whats happining. I posted the code below with better coments
void loop() {
int sensorValue = analogRead(A0);
if (sensorValue > 100 && sensorValue < 200)
{
// rise and fall code
for (pos = 12.5; pos < 18; pos += .1) // goes from 12.5 to 12.6
myservo1.write(pos); // tell servo to go to position in variable 'pos'
delay(25); // waits 15ms for the servo to reach the position
// because there is no braces code does not loop and reach pos 18, but moves onto next line
delay(1000);
for (pos = 18; pos>=12.5; pos-=.1) // this one is odd as I should hear it jump from 12.6 to 18
// but it seems a constant speed, no change in sound anyway
myservo1.write(pos); // tell servo to go to position in variable 'pos'
delay(25); // waits 15ms for the servo to reach the position
} // loop back
I went back to I to an old program just to check it worked with the braces and it did code below.
void loop()
{
for(pos = 12.5; pos < 18; pos += .1) // goes from 12.5 degrees to 18 degrees in .1 increments
{
myservo.write(pos); // tell servo to go to position in variable 'pos'
delay(15 ); // waits 15ms for the servo to reach the position
}
// loop until pos = 18
{
delay (5000);
}
for(pos = 18; pos>=12.5; pos-=.1) // goes from 18 to 12.5 in .1 increments
{
myservo.write(pos); // tell servo to go to position in variable 'pos'
delay(15); // waits 15ms for the servo to reach the position
}
// loop until pos = 12.5
}
Am I on the right lines in my thinking, seems it's a formatting error but I don't know the correct format. Any information gratefully recieved
Regards
Dale
What type is pos? If it is an int, than you can't assign 12.5 to it. Writing 12.5 to a servo doesn't make sense, either, since the Servo::write() method expects an int.
// because there is no braces code does not loop and reach pos 18, but moves onto next line
Well, you ought to have them, even if they are not strictly needed. Having them makes them look like you know what you are doing.
If you didn't add this comment, why have you ignored it?
The other code has some braces that you need and some that you do not. You need to learn to tell the difference.
Yes Dale.
"More cowbells! I need more cowbells!"
cowbells == braces
http://www.cplusplus.com/doc/tutorial/control/
And put the delay(25) inside them, so it will be part of your for loop
John
Hi paul
I'll post the entire code below
#include <Servo.h>
Servo myservo1; // create servo object to control a servo
// a maximum of eight servo objects can be created
float pos = 0; // variable to store the rotary switch input
void setup() {
myservo1.attach(9); // attaches the servo on pin 9 to the servo object
myservo1.write(pos);
delay(3000);
}
// the loop routine runs over and over again forever:
void loop() {
int sensorValue = analogRead(A0);
if (sensorValue > 100 && sensorValue < 200)
{
// pos=12.5;
// rise and fall code
for (pos = 12.5; pos < 18; pos += .1) // goes from 12.5 to 18
// in steps of .1
myservo1.write(pos); // tell servo to go to position in variable 'pos'
delay(25); // waits 15ms for the servo to reach the position
delay(1000);
for (pos = 18; pos>=12.5; pos-=.1) // goes from 18 to 12.5
myservo1.write(pos); // tell servo to go to position in variable 'pos'
delay(25); // waits 15ms for the servo to reach the position
}
else if (sensorValue > 200 && sensorValue < 400) // error on this line
{
pos=13;
}
else if (sensorValue > 400 && sensorValue < 600) {
pos=14;
}
else if (sensorValue > 600 && sensorValue < 700) {
pos=15;
}
else if (sensorValue > 700 && sensorValue < 900) {
pos=16;
}
else if (sensorValue > 900) {
pos=17;
}
myservo1.write(pos);
}
Pos is a float as I want more control over the rise and fall of my motor
Well, you ought to have them, even if they are not strictly needed. Having them makes them look like you know what you are doing.
If you didn't add this comment, why have you ignored it?
The other code has some braces that you need and some that you do not. You need to learn to tell the difference.
I don't think I explained it very well, originally I had it with braces but I got an error with my else before if statement, to get the program to compile as john pointed out i need to place all the rise a fall code in between the braces. but this causes the program to loop the rise and fall part between the braces.
Regards
Dale
P.S. John just posted as i was writing this, cheers I'll check those links out right now
If statements need braces
for loops need braces
while loops need braces
... etc
Almost everything almost always needs braces
float pos = 0; // variable to store the rotary switch input
for (pos = 12.5; pos < 18; pos += .1) // goes from 12.5 to 18
// in steps of .1
myservo1.write(pos); // tell servo to go to position in variable 'pos'
I fail to see where any rotary switch is read. So, the initial comment is crap.
The only place that pos is used is to position the servo. The Servo::write() function takes an int.
So, why is pos a float?
The Tools + Auto Format function would do an excellent job of properly indenting your code, so you can see where braces are missing. Indenting a statement doesn't mean squat to the compiler. But, it means a lot to people reading the code. Properly indented code has the same meaning to people and to compilers. The auto format tool makes the indenting match how the compiler will understand the code. It can, then, be quite obvious that how you understand it and how the compiler will understand it are not the same thing.
I can not stress the importance of properly indented code enough. Yours, of course, is not.
johncc's comment regarding braces is nonsense. If statements, and for and while loops, may or may not need braces. It is recommended that they always be used, so that you can add a statement to the block, and be sure that the statement is executed the correct number of times. Switch statements always need braces.
Functions always need curly braces. Almost any other use of curly braces (except in array initializations) is unnecessary.