-
When loop() finishes, loop() is run by design.
-
The faster you can run loop() the more responsive your program can be.
-
Instead of writing code to do one major task at a time, divide the work into small steps that fit in loop() with steps from other tasks.
Example: -only- [watching serial to read and buffer text until that is done] code vs catching serial available to buffer 1 char at a time as they arrive +and+ allowing other tasks to run and then loop() to end, all quickly if possible. That's why to divide any task to small pieces, so they run quick. You can run the next part in a later execution of loop() when conditions ( if()'s ) are right when everything's ready. -
Every time through loop() is later than the last time, real time code, and loop() has to contain code for what to do every time.
PeterH:
You do not want to have any calls to loop() in your code, ever, under any conceivable circumstances. Is that clear enough?
Apparently it wasn't clear enough. If your code calls loop(), it is wrong. How many ways do we need to say this?
PaulS:
Really. Stop!
Please dont kill innovative ideas....
Khalid:
PaulS:
Really. Stop!Please dont kill innovative ideas....
Nothing innovative about a cruise control. Standard equipment on many motorcycles.
Unless you're referring to the OP's code, it isn't so much innovative as icky and broken. I wouldn't trust it with my life.
PeterH:
PeterH:
You do not want to have any calls to loop() in your code, ever, under any conceivable circumstances. Is that clear enough?Apparently it wasn't clear enough. If your code calls loop(), it is wrong. How many ways do we need to say this?
+1
It's a bad idea to re-enter code on memory-challenged hardware. The stack can grow real fast.
As used in the IDE, setup() and loop() provides a frame for real time code able to handle multiple tasks usually under-1-ms-responsive on simple, limited microcontrollers.
You trim your code and use the right tricks, you can read-and-do a subtask 20+ times per millisecond. And just what subtask(s) to perform each time through loop() can vary completely and independently or by pattern to suit your app.
A state machine in loop() can reduce a whole lot of control code structure to simple state value changes instead. It cuts complexities and dependencies that make debugging headaches. It can exist along with other state machines, timers, and events (RX and sensors) in the Arduino setup() and loop() system.
Not knowing what the bike is I can't be sure, but if something brown DID hit the fan, couldn't you just click off the kill switch and roll yourself to a gentle stop?
Not knowing what the bike is I can't be sure, but if something brown DID hit the fan, couldn't you just click off the kill switch and roll yourself to a gentle stop?
On my bikes, and my cars, stepping on (or pulling) the brakes (or levers) automatically disables the cruise control. No need to take drastic action.
OPs code had a delay() in it. I certainly don't want to wait 150 milliseconds after hitting the brakes for the cruise control to let go of the throttle. While you might argue that at 60 miles an hour, and just over an 1/8th of a second delay, that's only 11 feet. That 11 feet might well be the difference between getting broadsided and stopping in time.
Cruise control is not terribly complicated, but it is not a beginner project. OPs code showed no sign of being professional code.
It is this type of thread that Grumpy_Mike was talking about here:
http://arduino.cc/forum/index.php/topic,69491.0.html
BulletMagnet83:
Not knowing what the bike is I can't be sure, but if something brown DID hit the fan, couldn't you just click off the kill switch and roll yourself to a gentle stop?
That's fine if you're riding down a nice long, straight, lonely Saskatchewan highway but what if you are in rush hour freeway traffic? Losing power can be very dangerous in that situation.
What if the unit decides to suddenly open the throttle? On my DRZ that isn't a big issue on my buddy's GSX1300R that could get serious real quick. It accelerates quite smartly.
but what if you are in rush hour freeway traffic? Losing power can be very dangerous in that situation.
Well, if you are using cruise control in this situation, you need to have your head examined. 8)
But, I presume that your point is that care must be taken to ensure that the cruise control is engaged only when the user wants it engaged. At least, that's what I hope you meant.
I agree, you shouldn't use cruise in heavy traffic. People do though.
That's part of the point. That and the fact that a kill switch doesn't suddenly make this safe.
How about this? i got some help to remove the loop()s
#include <Servo.h>
Servo myservo; // create servo object to control a servo
int potpin = 0;
int val;
// set pin numbers:
const int buttonPin = 10; // the number of the pushbutton pin
const int buttonPin2 = 8;
const int buttonPin3 = 4;
const int buttonPin4 = 2;
void setup()
{
myservo.attach(12); // attaches the servo on pin 12 to the servo object
// initialize the pushbutton pin as an input:
pinMode(buttonPin, INPUT);
pinMode(buttonPin2, INPUT);
pinMode(buttonPin3, INPUT);
pinMode(buttonPin4, INPUT);
}
void loop()
{
val = analogRead(potpin); //takes reading from pot current position
val = map(val, 0, 1083, 0, 180); //maps pot inputs to servo outputmyservo.write(val); //writes current position to servo to move it
myservo.write(val); //writes current position to servo to move it
if (digitalRead(buttonPin) == HIGH)
serloop1(); //sends command to SerLoop1
if (digitalRead(buttonPin2) == HIGH)
serloop1(); //sends command to SerLoop1
}
void serloop1()
{
int val1 = myservo.read(); //reads current servo location
if (digitalRead(buttonPin) == HIGH)
{
myservo.write(val1 + 2); //SUBTRACT 2 degrees to servo position for increased motor rpm
delay(100); //allows time for switch ro reset
}
if (digitalRead(buttonPin2) == HIGH)
{
myservo.write(val1 - 2); //ADDS 2 degrees to servo position for decreased motor rpm
delay(100); //allows time for switch ro reset
}
if (digitalRead(buttonPin3) == HIGH)
{
//Nothing, just exit
}
if (digitalRead(buttonPin4) == HIGH)
{
//Nothing, just exit
}
else
{
serloop1(); //returns to SerLoop1 to run again
}
}
In serloop1() you seem to be modifying the servo position but you've already written the unmodified value to the servo. I suggest you work out what position you want the servo in, and only then move the servo to that position.
This code in serloop1() is completely wrong:
serloop1(); //returns to SerLoop1 to run again
You have misunderstood how the control passes between functions in 'C' and C++. If you want to stay in serloop1() then you need to either not return from it, or arrange that the code you're returning to will call it again. Since the calls to serloop1() are controlled by the switch input, it seems that it would do what you need anyway. But regardless of whether that does what you want, you need to get rid of that recursive call to serloop1().
Just to be clear about the kill switch method, it does definitely electrically and immediately isolate any part of your control circuit from the existing bike throttle control?
I mean, when I'm zooming speedily on those nice twisty corners around where I live, I will want to be 100% sure that even if not activated in cruise mode, that it can not suddenly just jump into cruise mode. I want my mind to be focused on riding, not 'ooops, I hope line 10 of my code is ok'.
Or, if I were using such a control system as you are attempting to build, that if I were zooming down a nice straight road with a good view of the road ahead, giving me assurance that all is clear, that again, suddenly, a rockwallaby jumps out from the side of the road and to avoid such a beautiful creature I need to make quick steering adjustment and maybe some throttle adjustment as well. Be 100% certain you understand the dynamics of any situation before hitting the road, or should I say laying tracks with this gadget of yours!
I really do need to agree with PaulS and others that it appears you are starting out with understanding the fundamentals of programing and that is not a good mix with motorbikes. It seems you are determined to persist. If we don't hear from you in the coming weeks, I guess we may fear what happened.
Also, quiet possibly regardless if the cruise control was even activated or turned on, be aware that such modifications may and most probably will void any and all insurance in case of a crash, needing you to pay all medical and other costs incurred as well as possibly having a fine, depending on your local road traffic regulations.
Keep the rubber side down.
Paul
If you are going to persist in this endeavor, might I suggest some changes.
const int buttonPin = 10; // the number of the pushbutton pin
const int buttonPin2 = 8;
const int buttonPin3 = 4;
const int buttonPin4 = 2;
Rubbish. Meaningless names. How about some meaningful names, like killSwitch, brakeSwitch, onOffSwitch, setSwitch, etc., so we have some clue what you are talking about.
int potpin = 0;
throttlePin?
Servo myservo; // create servo object to control a servo
cruiseServo?
int val;
throttlePos?
val = map(val, 0, 1083, 0, 180);
Create a new variable, controlPos, rather than overwriting the poorly named val.
This is in addition to the other comments that have been posted.
Get rid of the recursive calls to any/all functions. Nothing in a cruise control setup requires recursion.
Also, quiet possibly regardless if the cruise control was even activated or turned on, be aware that such modifications may and most probably will void any and all insurance in case of a crash, needing you to pay all medical and other costs incurred as well as possibly having a fine, depending on your local road traffic regulations.
Good point!
I have been working with arduino for 2 years. I wouldnt even imagine of putting an arduino in my gsxr 750.
Remember the amount of crashes Carlos Checa had when Ducati was tunning the traction control? Imagine the brains working into this, imagine the money and technology!
I have an Arduino in my car but it does not mess around any throttle or brakes. There are many projects you can do with an arduino and a vehicle, but throttle and brakes in a bike are most surely the worst idea for a novice like you.
Be smart and start another interesting project.
While I appreciate all of your concerns, this is 99% bench project right now. It is only for my potential use. It will only go into actual use with extensive bench testing. I have limited knowledge on Arduino programming but extensive knowledge in design and build applications. I fully understand the difference between safe and unsafe. I have built from scratch, my own single side swing arm, handle bars and most of the rest of the bike. People often praise and criticize for those endeavors also because they lack the knowledge and understanding of technical design.
I would appreciate the continued support on programming help on this project and would again emphasize that at this stage it is a bench project.
For all the comments on here, it is interesting that everyone is quick to understand that I am lacking in programming skill and is more interested in telling me what is wrong with my code in ways that are over my head and never offers code snippets as examples to allow me to move forward.
Here is my latest code.
I am trying to incorporate debounce. I have probably done this in the worst way but with everything I have added, the program still works. I will condense it down later but for now I just would like to know how to add the final part that actually initializes the button push for my application.
Reference http://arduino.cc/en/Tutorial/Debounce
// set the LED using the state of the button:
digitalWrite(ledPin, buttonState);
#include <Servo.h>
Servo throttleServo; // create servo object to control a servo
// set pin numbers:
const int throttleSetPlusThrottleUp = 10; // the number of the pushbutton pin
const int throttleSetPlusThrottleDn = 8;
const int ReturnToPot = 4;
const int ReturnToPot2 = 2;
// constants won't change. They're used here to
// set pin numbers:
int throttlePositionPot = 0;
int val;
// Variables will change:
int debounceThrottleSetPlusThrottleUp; // the current reading from the input pin
int debounceThrottleSetPlusThrottleDn; // the current reading from the input pin
int debounceReturnToPot; // the current reading from the input pin
int debounceReturnToPot2; // the current reading from the input pin
int lastdebounceThrottleSetPlusThrottleUp = LOW; // the previous reading from the input pin
int lastdebounceThrottleSetPlusThrottleDn = LOW; // the previous reading from the input pin
int lastdebounceReturnToPot = LOW; // the previous reading from the input pin
int lastdebounceReturnToPot2 = LOW; // the previous reading from the input pin
// the following variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long lastDebounceTime = 0; // the last time the output pin was toggled
long lastDebounceTime2 = 0; // the last time the output pin was toggled
long lastDebounceTime3 = 0; // the last time the output pin was toggled
long lastDebounceTime4 = 0; // the last time the output pin was toggled
long debounceDelay = 50; // the debounce time; increase if the output flickers
long debounceDelay2 = 50; // the debounce time; increase if the output flickers
long debounceDelay3 = 50; // the debounce time; increase if the output flickers
long debounceDelay4 = 50; // the debounce time; increase if the output flickers
int servoPosition = 0; //Old val1, used to store servo position. Allows use by whole program
void setup()
{
throttleServo.attach(12); // attaches the servo on pin 12 to the servo object
// initialize the pushbutton pin as an input:
pinMode(throttleSetPlusThrottleUp, INPUT);
pinMode(throttleSetPlusThrottleDn, INPUT);
pinMode(ReturnToPot, INPUT);
pinMode(ReturnToPot2, INPUT);
}
void loop()
{
val = analogRead(throttlePositionPot); //takes reading from pot current position
val = map(val, 0, 1083, 0, 180); //maps pot inputs to servo output
throttleServo.write(val); //writes current position to servo to move it
if (digitalRead(throttleSetPlusThrottleUp) == HIGH)
{
serloop1(); //Enters button control loop
}
if (digitalRead(throttleSetPlusThrottleDn) == HIGH)
{
serloop1(); //Enters button control loop
}
}
/**
* If button control is enabled, loop and handle control buttons
* If exit buttons (To return to pot control) are pressed, exit loop and return
* to pot control
*/
void serloop1()
{
servoPosition = throttleServo.read(); //reads current servo location
// read the state of the switch into a local variable:
int reading = digitalRead(throttleSetPlusThrottleUp);
int reading2 = digitalRead(throttleSetPlusThrottleDn);
int reading3 = digitalRead(ReturnToPot);
int reading4 = digitalRead(ReturnToPot2);
// If the switch changed, due to noise or pressing:
if (reading != lastdebounceThrottleSetPlusThrottleUp)
// reset the debouncing timer
lastdebounceThrottleSetPlusThrottleUp = millis();
// If the switch changed, due to noise or pressing:
if (reading2 != lastdebounceThrottleSetPlusThrottleDn)
// reset the debouncing timer
lastDebounceTime2 = millis();
// If the switch changed, due to noise or pressing:
if (reading3 != lastdebounceReturnToPot)
// reset the debouncing timer
lastDebounceTime3 = millis();
// If the switch changed, due to noise or pressing:
if (reading4 != lastdebounceReturnToPot2)
// reset the debouncing timer
lastDebounceTime4 = millis();
if ((millis() - lastDebounceTime) > debounceDelay) {
// whatever the reading is at, it's been there for longer
// than the debounce delay, so take it as the actual current state:
debounceThrottleSetPlusThrottleUp = reading;
}
if ((millis() - lastDebounceTime2) > debounceDelay2) {
// whatever the reading is at, it's been there for longer
// than the debounce delay, so take it as the actual current state:
debounceThrottleSetPlusThrottleDn = reading2;
}
if ((millis() - lastDebounceTime3) > debounceDelay3) {
// whatever the reading is at, it's been there for longer
// than the debounce delay, so take it as the actual current state:
debounceReturnToPot = reading3;
}
if ((millis() - lastDebounceTime4) > debounceDelay4) {
// whatever the reading is at, it's been there for longer
// than the debounce delay, so take it as the actual current state:
debounceReturnToPot2 = reading4;
}
int btnControlEnabled = 1; //If button control is enabled this equals 1, else it equals 0
while(btnControlEnabled == 1)
{
if (digitalRead(throttleSetPlusThrottleUp) == HIGH)
{
throttleServo.write(servoPosition + 2); //SUBTRACT 2 degrees to servo position for increased motor rpm
servoPosition = throttleServo.read(); //Read new servo position
delay(100); //allows time for switch ro reset
}
//If first button not pressed, check the next...
else if (digitalRead(throttleSetPlusThrottleDn) == HIGH)
{
throttleServo.write(servoPosition - 2); //ADDS 2 degrees to servo position for decreased motor rpm
servoPosition = throttleServo.read(); //Read new servo position
delay(100); //allows time for switch ro reset
}
else if (digitalRead(ReturnToPot) == HIGH)
{
btnControlEnabled = 0; //Set loop exit condition
}
else if (digitalRead(ReturnToPot2) == HIGH)
{
btnControlEnabled = 0; //Set loop exit condition
}
//If nothing pressed...
else
{
//...do nothing at all, go back to start of loop
}
}
}
Sorr for the multiple posts here. I changed the debounce instructions to the loop() instead of the serloop1() and added the
// save the reading. Next time through the loop,
// it'll be the lastButtonState:
lastButtonState = reading;
into the code. Just need to figure out how to translate and use
// set the LED using the state of the button:
digitalWrite(ledPin, buttonState);
for my buttons use.
#include <Servo.h>
Servo throttleServo; // create servo object to control a servo
// set pin numbers:
const int throttleSetPlusThrottleUp = 10; // the number of the pushbutton pin
const int throttleSetPlusThrottleDn = 8;
const int ReturnToPot = 4;
const int ReturnToPot2 = 2;
// constants won't change. They're used here to
// set pin numbers:
int throttlePositionPot = 0;
int val;
// Variables will change:
int debounceThrottleSetPlusThrottleUp; // the current reading from the input pin
int debounceThrottleSetPlusThrottleDn; // the current reading from the input pin
int debounceReturnToPot; // the current reading from the input pin
int debounceReturnToPot2; // the current reading from the input pin
int lastdebounceThrottleSetPlusThrottleUp = LOW; // the previous reading from the input pin
int lastdebounceThrottleSetPlusThrottleDn = LOW; // the previous reading from the input pin
int lastdebounceReturnToPot = LOW; // the previous reading from the input pin
int lastdebounceReturnToPot2 = LOW; // the previous reading from the input pin
// the following variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long lastDebounceTime = 0; // the last time the output pin was toggled
long lastDebounceTime2 = 0; // the last time the output pin was toggled
long lastDebounceTime3 = 0; // the last time the output pin was toggled
long lastDebounceTime4 = 0; // the last time the output pin was toggled
long debounceDelay = 50; // the debounce time; increase if the output flickers
long debounceDelay2 = 50; // the debounce time; increase if the output flickers
long debounceDelay3 = 50; // the debounce time; increase if the output flickers
long debounceDelay4 = 50; // the debounce time; increase if the output flickers
int servoPosition = 0; //Old val1, used to store servo position. Allows use by whole program
void setup()
{
throttleServo.attach(12); // attaches the servo on pin 12 to the servo object
// initialize the pushbutton pin as an input:
pinMode(throttleSetPlusThrottleUp, INPUT);
pinMode(throttleSetPlusThrottleDn, INPUT);
pinMode(ReturnToPot, INPUT);
pinMode(ReturnToPot2, INPUT);
}
void loop()
{
// read the state of the switch into a local variable:
int reading = digitalRead(throttleSetPlusThrottleUp);
int reading2 = digitalRead(throttleSetPlusThrottleDn);
int reading3 = digitalRead(ReturnToPot);
int reading4 = digitalRead(ReturnToPot2);
// If the switch changed, due to noise or pressing:
if (reading != lastdebounceThrottleSetPlusThrottleUp)
// reset the debouncing timer
lastdebounceThrottleSetPlusThrottleUp = millis();
// If the switch changed, due to noise or pressing:
if (reading2 != lastdebounceThrottleSetPlusThrottleDn)
// reset the debouncing timer
lastDebounceTime2 = millis();
// If the switch changed, due to noise or pressing:
if (reading3 != lastdebounceReturnToPot)
// reset the debouncing timer
lastDebounceTime3 = millis();
// If the switch changed, due to noise or pressing:
if (reading4 != lastdebounceReturnToPot2)
// reset the debouncing timer
lastDebounceTime4 = millis();
if ((millis() - lastDebounceTime) > debounceDelay) {
// whatever the reading is at, it's been there for longer
// than the debounce delay, so take it as the actual current state:
debounceThrottleSetPlusThrottleUp = reading;
}
if ((millis() - lastDebounceTime2) > debounceDelay2) {
// whatever the reading is at, it's been there for longer
// than the debounce delay, so take it as the actual current state:
debounceThrottleSetPlusThrottleDn = reading2;
}
if ((millis() - lastDebounceTime3) > debounceDelay3) {
// whatever the reading is at, it's been there for longer
// than the debounce delay, so take it as the actual current state:
debounceReturnToPot = reading3;
}
if ((millis() - lastDebounceTime4) > debounceDelay4) {
// whatever the reading is at, it's been there for longer
// than the debounce delay, so take it as the actual current state:
debounceReturnToPot2 = reading4;
}
// save the reading. Next time through the loop,
// it'll be the lastButtonState:
lastdebounceThrottleSetPlusThrottleUp = reading;
// save the reading. Next time through the loop,
// it'll be the lastButtonState:
lastdebounceThrottleSetPlusThrottleDn = reading2;
// save the reading. Next time through the loop,
// it'll be the lastButtonState:
lastdebounceReturnToPot = reading3;
// save the reading. Next time through the loop,
// it'll be the lastButtonState:
lastdebounceReturnToPot2 = reading4;
val = analogRead(throttlePositionPot); //takes reading from pot current position
val = map(val, 0, 1083, 0, 180); //maps pot inputs to servo output
throttleServo.write(val); //writes current position to servo to move it
if (digitalRead(throttleSetPlusThrottleUp) == HIGH)
{
serloop1(); //Enters button control loop
}
if (digitalRead(throttleSetPlusThrottleDn) == HIGH)
{
serloop1(); //Enters button control loop
}
}
/**
* If button control is enabled, loop and handle control buttons
* If exit buttons (To return to pot control) are pressed, exit loop and return
* to pot control
*/
void serloop1()
{
servoPosition = throttleServo.read(); //reads current servo location
int btnControlEnabled = 1; //If button control is enabled this equals 1, else it equals 0
while(btnControlEnabled == 1)
{
if (digitalRead(throttleSetPlusThrottleUp) == HIGH)
{
throttleServo.write(servoPosition + 2); //SUBTRACT 2 degrees to servo position for increased motor rpm
servoPosition = throttleServo.read(); //Read new servo position
delay(100); //allows time for switch ro reset
}
//If first button not pressed, check the next...
else if (digitalRead(throttleSetPlusThrottleDn) == HIGH)
{
throttleServo.write(servoPosition - 2); //ADDS 2 degrees to servo position for decreased motor rpm
servoPosition = throttleServo.read(); //Read new servo position
delay(100); //allows time for switch ro reset
}
else if (digitalRead(ReturnToPot) == HIGH)
{
btnControlEnabled = 0; //Set loop exit condition
}
else if (digitalRead(ReturnToPot2) == HIGH)
{
btnControlEnabled = 0; //Set loop exit condition
}
//If nothing pressed...
else
{
//...do nothing at all, go back to start of loop
}
}
}
// read the state of the switch into a local variable:
int reading = digitalRead(throttleSetPlusThrottleUp);
int reading2 = digitalRead(throttleSetPlusThrottleDn);
int reading3 = digitalRead(ReturnToPot);
int reading4 = digitalRead(ReturnToPot2);
You now have some good names for some of the pins. It's a shame that the same can not be said for the states of those pins.
What do ReturnToPot and ReturnToPot2 mean?
Personally, I think that if find yourself in a position where some of the variables need numbers, all the related ones need numbers (ReturnToPot1 and reading1).
if (reading != lastdebounceThrottleSetPlusThrottleUp)
I also think that variables being compared like this should have names that indicate that they go together. I can see that currDebounceThrottleSetPlusThrottleUp and lastDebounceThrottleSetPlusThrottleUp go together. I can't see that reading and lastdebounceThrottleSetPlusThrottleUp go together.
Using camel case notation requires capital letters on ALL words, except the first one, not capital letters on some of the words.
Debounce and state change detection are two separate things. It looks, from the names, that you are trying to mix them.
Time works seamlessly with unsigned longs, no problem with roll over.
Time is a fudge-up with longs but you got almost 25 days find that bug, and if you kludge your way around it then it will never get you except that you won't get why unsigned's are right for handling time.
Time is a fudge-up with longs but you got almost 25 days find that bug, and if you kludge your way around it then it will never get you except that you won't get why unsigned's are right for handling time.
Not too many people can ride for 24 plus days at a time, without gas or other stops, so this isn't likely to be a real problem.
Edit: fixed typo.