Im trying to write some code to make a robot move without using they delay function()
the code for the timer is working well but im having trouble with getting the movement not to run on top of each other while the loop is running
ive set some constants to be placeholders for what item is running at this time
//movement timer
int inUse = 0;
const int forwardUse = 1;
const int reverseUse = 2;
const int rightUse = 3;
const int leftUse = 4;
long previousMillis = 0;
and my movement code checks to see if the servos are not in use or if they are in use by the movement that is running
here is an example of my forward function
void forward(long runTime)
{
if(inUse == 0 || inUse == forwardUse) // check if servos are in use by other movement
{
unsigned long currentMillis = millis();
right_servo.write(0);
left_servo.write(180);
inUse = forwardUse;
if(currentMillis - previousMillis > runTime) // if timer has run out stop movement reset inUse and return to loop
{
previousMillis = currentMillis;
halt();
inUse = 0;
return;
}
}
}
it all works as it should, but if in the loop i have something like:
I get random movements, i think the problem is there is no way of checking if the previous steps have been completed, i thought about putting in a step counter and checking to see if its on the right step of the loop before implementing the code, but that seems really clumsy, Im very new to coding and the thought that goes into writing the code, so any help would be great
here is my entire code
// robotMovement 1.0
//servo
#include <Servo.h>
Servo right_servo;
Servo left_servo;
//speaker
const int speaker = 5;
//movement timer
int inUse = 0;
const int forwardUse = 1;
const int reverseUse = 2;
const int rightUse = 3;
const int leftUse = 4;
long previousMillis = 0;
void setup()
{
right_servo.attach(11);
left_servo.attach(10);
pinMode(speaker, OUTPUT);
Serial.begin(9600);
}
void loop()
{
forward(1000);
turnLeft(600);
forward(1000);
turnRight(600);
}
void halt()
{
right_servo.write(90);
left_servo.write(90);
}
void forward(long runTime)
{
if(inUse == 0 || inUse == forwardUse) // check if servos are in use by other movement
{
unsigned long currentMillis = millis();
right_servo.write(0);
left_servo.write(180);
inUse = forwardUse;
if(currentMillis - previousMillis > runTime) // if timer has run out stop movement reset inUse and return to loop
{
previousMillis = currentMillis;
halt();
inUse = 0;
return;
}
}
}
void reverse(long runTime)
{
if(inUse == 0 || inUse == reverseUse) // check if servos are in use by other movement
{
unsigned long currentMillis = millis();
right_servo.write(180);
left_servo.write(0);
inUse = reverseUse;
if(currentMillis - previousMillis > runTime) // if timer has run out stop movement reset inUse and return to loop
{
previousMillis = currentMillis;
halt();
inUse = 0; //reset inUse to not in use
return;
}
}
}
void turnRight(long runTime)
{
if(inUse == 0 || inUse == rightUse) // check if servos are in use by other movement
{
unsigned long currentMillis = millis();
right_servo.write(180);
left_servo.write(180);
inUse = rightUse;
if(currentMillis - previousMillis > runTime) // if timer has run out stop movement reset inUse and return to loop
{
previousMillis = currentMillis;
halt();
inUse = 0; //reset inUse to not in use
return;
}
}
}
void turnLeft(long runTime)
{
if(inUse == 0 || inUse == leftUse) // check if servos are is in use by other movement
{
unsigned long currentMillis = millis();
right_servo.write(0);
left_servo.write(0);
inUse = leftUse;
if(currentMillis - previousMillis > runTime) // if timer has run out stop movement reset inUse and return to loop
{
previousMillis = currentMillis;
halt();
inUse = 0; //reset inUse to not in use
return;
}
}
}
Put some Serial.prints() in so that you can see the value of variables such as inUse at various point, and which functions the code is entering. Print some identifying text so that you know what you are seeing.
Do you really need several values of inUse ? true or false would seem to be enough unless each movement function needs to know not only that some other movement is going on but also which one.
i put some Serial.println()'s in the code, and it works as long as each step is only taken once in the loop.
for example i can:
forward(500);
turnLeft(500);
reverse(500);
turnRight(500);
but if i put in
forward(500);
turnLeft(500);
forward(600);
turnLeft(500);
forward(700);
turnLeft(500);
it rotates through the timers until the first one runs out I think
how do i keep one move forward with a set time to not interfere with the others in the loop?
The way your code is written it seems to call servo.write() lots of times with the same value while a movement is happening (i.e. until the time expires). I think if you rewrite the code so it only calls servo.write() once for each movement you will have code that is easier to follow.
And checking inUse ==0 and inUse == forwardUse allows two forward movements to overlap whereas the simpler approach of just checking inUse == 0 would eliminate that risk.
I did some very basic modifications to the movement code. Im about to leave for work and haven't had time to actually change all the movement functions or to compile it but Im hoping someone can take a look at the code and tell me if im moving in the right direction at least, Im just learning all of this and I appreciate the time that people have taken to criticize my code.
updated move forward function
void forward(long runTime)
{
if(inUse == 0) // check if servos are in use by other movement
{
Serial.println("moving forward");
unsigned long currentMillis = millis();
right_servo.write(0);
left_servo.write(180);
inUse = 1;
}
if(currentMillis - previousMillis > runTime) // if timer has run out stop movement reset inUse and return to loop
{
previousMillis = currentMillis;
halt();
inUse = 0;
return;
}
}
so i modified my variable to make them more clear, i can see how that would make a large difference when the code grows to many lines or when you have to look back at it after some time. i changed my movement functions around and tried move the current_movement_millis = millis(); to the loop(), I made current_movement_millis a global variable along with previous_movement_millis = millis();
and**......**
when my loop contains multiple movement commands**:**
it runs the first command, in the instance above turnLeft for the correct amount of time, then it runs turnRight over and over and over, and just gets stuck there. It never touches the forward function, or any other function after the second one. when i look at it in the serial monitor i get and instance of the first function, then the second repeats over and over, so its running multiple times, not just writing the servo values then getting stuck
I know there must be some stupid flaw to the structure of my program, but i can not for the life of me figure it out
my entire code now
// robotMovement 1.0
//servo
#include <Servo.h>
Servo right_servo;
Servo left_servo;
//speaker
const int speaker = 5;
//movement timer
int movement_in_use = 0;
unsigned long previous_movement_millis = 0;
unsigned long current_movement_millis = millis();
void setup()
{
right_servo.attach(11);
left_servo.attach(10);
pinMode(speaker, OUTPUT);
Serial.begin(9600);
}
void loop()
{
turnLeft(1000);
turnRight(1000);
forward(1000);
current_movement_millis = millis();
}
void halt()
{
right_servo.write(90);
left_servo.write(90);
}
void forward(long runTime)
{
if(movement_in_use == 0) // check if servos are in use by other movement
{
Serial.println("moving forward");
right_servo.write(0);
left_servo.write(180);
movement_in_use = 1;
}
if(current_movement_millis - previous_movement_millis > runTime) // if timer has run out stop movement reset movement_in_use and return to loop
{
previous_movement_millis = current_movement_millis;
halt();
movement_in_use = 0;
return;
}
}
void reverse(long runTime)
{
if(movement_in_use == 0) // check if servos are in use by other movement
{
Serial.println("moving reverse");
right_servo.write(180);
left_servo.write(0);
movement_in_use = 1;
}
if(current_movement_millis - previous_movement_millis > runTime) // if timer has run out stop movement reset movement_in_use and return to loop
{
previous_movement_millis = current_movement_millis;
halt();
movement_in_use = 0;
return;
}
}
void turnLeft(long runTime)
{
if(movement_in_use == 0) // check if servos are in use by other movement
{
Serial.println("turning left");
right_servo.write(0);
left_servo.write(0);
movement_in_use = 1;
}
if(current_movement_millis - previous_movement_millis > runTime) // if timer has run out stop movement reset movement_in_use and return to loop
{
previous_movement_millis = current_movement_millis;
halt();
movement_in_use = 0;
return;
}
}
void turnRight(long runTime)
{
if(movement_in_use == 0) // check if servos are in use by other movement
{
Serial.println("turning right");
right_servo.write(180);
left_servo.write(180);
movement_in_use = 1;
}
if(current_movement_millis - previous_movement_millis > runTime) // if timer has run out stop movement reset movement_in_use and return to loop
{
previous_movement_millis = current_movement_millis;
halt();
movement_in_use = 0;
return;
}
}
loop() gets called. movement_in_use is 0 when turnLeft() is called.
So, turnLeft() assigns values to the servos, and sets movement_in_use to 1. Then, it ends. turnRight gets called.
movement_in_use is 1, so time is checked. current_millis is 0, and previous_millis is 0. The difference is not greater than runTime, so the function ends, and forward() is called.
movement_in_use is 1, so time is checked. current_millis is 0, and previous_millis is 0. The difference is not greater than runTime, so the function ends, and current_millis is updated, and the process begins again.
The real question, in my mind, is why you have a variable called current_millis, since the value stored in that variable is NOT the current time.
Get rid of current_millis. Everywhere where it is currently used, call millis(), instead.
alright i got rid of current_movement_millis, I dont really know why it was in there, I put it in there because it was in the blinkWithoutDelay sketch, like i said im not very good at this whole coding thing. is there a reason its in that sketch?
it doesn't just stay stuck in the second command now it switches back and forth but not in the order that you would expect it to
i tried two things at first, one time as the loop you see below
the code with the delay was more consistant than the one without it
here are the first 11 movements of each from the serial output:
movement# with delay(1) between functions w/o delay between calls
1 turning right turning right
2 turning left turning left
3 turning right turning left
4 turning left turning right
5 turning right turning left
6 turning left turning left
7 turning right turning left
8 turning right turning left
9 turning right turning left
10 turning right turning right
11 turning left turning left
sorry about the formatting, hopefully its pretty easy to see the three columns, on another note, whats the formatting for the forum table tool
and once again my updated code:
// robotMovement 1.0
//servo
#include <Servo.h>
Servo right_servo;
Servo left_servo;
//speaker
const int speaker = 5;
//movement timer
int movement_in_use = 0;
unsigned long previous_movement_millis = 0;
void setup()
{
right_servo.attach(11);
left_servo.attach(10);
pinMode(speaker, OUTPUT);
Serial.begin(9600);
}
void loop()
{
turnRight(1000);
turnLeft(1000);
}
void halt()
{
right_servo.write(90);
left_servo.write(90);
}
void forward(long runTime)
{
if(movement_in_use == 0) // check if servos are in use by other movement
{
Serial.println("moving forward");
right_servo.write(0);
left_servo.write(180);
movement_in_use = 1;
}
if(millis() - previous_movement_millis > runTime) // if timer has run out stop movement reset movement_in_use and return to loop
{
previous_movement_millis = millis();
halt();
movement_in_use = 0;
return;
}
}
void reverse(long runTime)
{
if(movement_in_use == 0) // check if servos are in use by other movement
{
Serial.println("moving reverse");
right_servo.write(180);
left_servo.write(0);
movement_in_use = 1;
}
if(millis() - previous_movement_millis > runTime) // if timer has run out stop movement reset movement_in_use and return to loop
{
previous_movement_millis = millis();
halt();
movement_in_use = 0;
return;
}
}
void turnLeft(long runTime)
{
if(movement_in_use == 0) // check if servos are in use by other movement
{
Serial.println("turning left");
right_servo.write(0);
left_servo.write(0);
movement_in_use = 1;
}
if(millis() - previous_movement_millis > runTime) // if timer has run out stop movement reset movement_in_use and return to loop
{
previous_movement_millis = millis();
halt();
movement_in_use = 0;
return;
}
}
void turnRight(long runTime)
{
if(movement_in_use == 0) // check if servos are in use by other movement
{
Serial.println("turning right");
right_servo.write(180);
left_servo.write(180);
movement_in_use = 1;
}
if(millis() - previous_movement_millis > runTime) // if timer has run out stop movement reset movement_in_use and return to loop
{
previous_movement_millis = millis();
halt();
movement_in_use = 0;
return;
}
}
Instead of the movement_in_use variable, you really should be looking at testing time in loop(), and calling the next function if it is time. turnLeft() shouldn't give a damn what time some other event occurs. loop() should, and it should call the correct function at the correct time.
I guess I dont, maybe one of these days I'll be able to give you a perfectly literal answer, but thank you for the help
i was hoping to get the functions to regulate themselves so that I can insert them in various different things as I learn more about programming, to keep them as flexible bits that stand alone
While there may be all sorts of good reasons for reorganizing your code - perhaps to facilitate future developments - I think it should work the way it is currently organized.
As I see it the overall concept is as follows ...
loop() calls a series of functions (fwd, reverse etc) in a given order and each with a given duration.
when (for example) the fwd function is called
it checks to see if another function is in progress
if there is another function in progress it should return without doing anything
actually, it checks the time and may do something silly which should have been left
to another function
if there is not another function in progress it starts the servos
it should record the time when the servos are started - but I suspect it doesn't
it sets a flag to say the servos are inUse
You should think carefully about the bits I have in bold.
Since the only thing that happens when the time expires is to call halt() that part of the code should be in a function of its own called from loop() and should not be in the direction functions. The direction functions should just start things. If the same code is called several times it's generally a good idea to put it in a function so there is only one copy of it and no chance of the code in the copies getting out of step.
That would mean that on each iteration of loop it would try halt and all of the directions and would act on the appropriate one.
