Needing Help with Code Algorithm

Hi,

My team and I are in the process of finalizing our "voice-controlled electric-powered wheelchair". Basically, we are using two wiper motors to rotate the rear wheels. The wheelchair's steering and movement only depend on the rear wheels, for example, when moving to the left, only the right rear wheel will rotate; and since the left rear wheel remains stationary, the tendency for the wheelchair is to move towards the left. We haven't included the front wheels for the directional control.

Our wheelchair is also equipped with ultrasonic sensors which are used for obstacle detection. The wheelchair should stop whenever an object within 1 foot range is detected. Furthermore, we are using a voice recognition module that is responsible for controlling the wheelchair's direction. We have "trained" the module to follow five commands, namely, "left", "right", "forward", "stop" and "reverse".

So far we were able to build a somehow working prototype, but we have encountered some problems which may be caused by the algorithm of our code:

  1. The ultrasonic sensors return distance values accurately but they fail to let the main microcontroller module to immediately stop the wheelchair (all motor drivers won't switch to LOW quickly) once an object is detected within 1 foot range.
  2. The recognition rate of the module varies. Sometimes it will catch your voice fast enough, then there are times when it needs about 5 repetitions before it catches the spoken command. We have already used a noise-cancelling mic to reduce the noise from the environment and improve the recognition rate, yet the same thing happens.
  3. Let's say an object is already detected by a front-side sensor while the wheelchair is stationary, the wheelchair should not follow a "forward" command. The problem is that when commanding a "reverse", the wheelchair will also not follow.

Please let us know what modifications we should do with the code. Your help is much appreciated. Thanks and more power.

**Attached below is the source code

Final Protoype Code V2.txt (8.03 KB)

My first thought was "get rid of the delay()s".

Then I read your code, and my thought was "get rid of the delay()s".

Subsequent thoughts included "format your code", "make constants constant", "use arrays", and "time for a cup of tea".

AWOL:
My first thought was "get rid of the delay()s".

Then I read your code, and my thought was "get rid of the delay()s".

Hi sir, thanks for the response. Is it the delay(500) code used for the sensors? If you are also referring to the delay()s in the switch statement, does that mean I should revise my algorithm for switching off the relays for the motors?

I mean, you should revise your code so that you get rid of all the delays.

It takes you three seconds just to read the sensors.
How far can it travel in three seconds? (answer: probably too far)

There is a technique to get rid of the delay's and that is shown in the "blink without delay" example that comes with the IDE.

It uses timestamps relative to millis() (most of the time) to check if it is time to do some action.
And when the action is done the timestamp is updated.

in pseudocode

if (actionTime == timeontheclock)
{
actionTime += interval // or newtime
action();
}

(sorry the above code won't compile :wink:

Thanks for the advice. Can you please at least give me an idea on how should I solve my problem #3? Here's my explanation in detail:

Let's say a "forward" command is spoken, the code flow will proceed to the case "goForward" from the switch statement.

case goForward:
       if(firstSensorDistance <= 31 || secondSensorDistance <= 31){
          break;
        }
 else{
          digitalWrite(leftMotorRelay1, LOW);
          digitalWrite(leftMotorRelay2, LOW);
          digitalWrite(rightMotorRelay1, LOW);
          digitalWrite(rightMotorRelay2, LOW);
          delay(500);
          digitalWrite(leftMotorRelay1, HIGH);
          digitalWrite(rightMotorRelay1, HIGH);
          break;
        }

My problem is I can't think of a way of making the relays LOW if an object is detected while leftMotorRelay1 and rightMotorRelay1 are HIGH. I tried putting an if statement below the

digitalWrite(leftMotorRelay1, HIGH);
digitalWrite(rightMotorRelay1, HIGH);

but it doesn't seem to work. I also tried making a sub that can be called if an object is detected but it won't work also.

Hence, I put

  if(firstSensorDistance <=31 || secondSensorDistance <=31){	
	digitalWrite(leftMotorRelay1, LOW);
        digitalWrite(leftMotorRelay2, LOW);
        digitalWrite(rightMotorRelay1, LOW);
        digitalWrite(rightMotorRelay2, LOW);
   }
  if(fourthSensorDistance <=31 || fifthSensorDistance <=31){	
	digitalWrite(leftMotorRelay1, LOW);
        digitalWrite(leftMotorRelay2, LOW);
        digitalWrite(rightMotorRelay1, LOW);
        digitalWrite(rightMotorRelay2, LOW);
   }

before the switch statement to make the wheelchair stop when an object is detected, but it's a bad solution since it will be impossible for the wheelchair to move if an object remains detected within 1 foot while the wheelchair is stationary.

OK, let me explain. Forget about the wheelchair for a moment and think about breakfast.

Say it takes 2 minutes to cook your bacon, and 3 minutes to cook your eggs.

Do you put the bacon on, stare at the frying-pan for 2 minutes until they bacon is cooked, and then put the eggs on? So, total time is 5 minutes?

Or do you organize yourself so that the eggs are put on first, and then the bacon a minute later?

You can't use delay in this situation.

You have to monitor the clock and see what is happening, when.

See: How to do multiple things at once ... like cook bacon and eggs

A function like delay() is usually called "sleep", and the analogy is a good one - would you fall
asleep making breakfast, or would you constantly turn your attention to what needs doing next,
or what might need doing next?

loop() is the conscious mind (as it were) of your code, it needs to keep turning its attention to
any and everything that might need handling, and doing relevant actions when they need doing.

Any action that takes time needs to be broken down into pieces, such as putting the frying pan
on the stove and taking it off again (rather than "cooking bacon" as a single action)

To keep track of such actions in pieces you have to maintain "state" - variable(s) that explain
where you are in a sequence of pieces of an action. The state tells loop() what might happen
next, and would be updated after each piece of action (pan on, pan off, etc).

The demo several things at a time illustrates the use of millis() to manage timing

And the Thread planning and implementing a program shows how to organize the code into several functions which greatly simplifies the sort of problem you are struggling with by separating the "data collection" (reading inputs) from the "action".

...R

It will also help to modularize your code. Instead of

          digitalWrite(leftMotorRelay1, LOW);
          digitalWrite(leftMotorRelay2, LOW);
          digitalWrite(rightMotorRelay1, LOW);
          digitalWrite(rightMotorRelay2, LOW);

You should put all of this into functions like:

  void startForwards();
  void stop();

That way your main loop becomes much easier to read. You should not have to remember which number relay makes it go forwards when you are working at the logical level of "user asked for forward motion."

This code:

  if(userAskedForForwards() && forwardsIsClear()) {
    startForwards();
  }

might seem redundant and silly but it's actually the best way to seperate the logic from the mechanics of turning relays on and off.

Hi,

I have been researching about timing without the use of delay(). I tried to learn using millis() and now, I'm creating another code revision for our project. I would like to ask for further help of what I am doing right now since I'm still pretty confused.

I am focusing first on my algorithm in controlling the motors. What I have done before, for the "turnLeft" command, is this:

case turnLeft:

digitalWrite(leftMotorRelay1, LOW);
digitalWrite(leftMotorRelay2, LOW);
digitalWrite(rightMotorRelay1, LOW);
digitalWrite(rightMotorRelay2, LOW);
delay(500);
digitalWrite(rightMotorRelay1, HIGH);
delay(2000);
digitalWrite(rightMotorRelay1, LOW);
break;

I am first switching all relays to LOW (for what it's worth) then after 0.5 seconds of doing nothing, rightMotorRelay1 is set to HIGH for the wheelchair to turn left. It will keep on turning left for 2 seconds, then switch to LOW.

But like what you guys said about eliminating delay, I'm trying to make a similar algorithm using millis() instead.

unsigned long Timer;
unsigned long offTimeCounter;

 offTimeCounter = millis();
 if((millis() - offTimeCounter >= 5UL){
 digitalWrite(leftRelay1, LOW);
 digitalWrite(leftRelay1, LOW);
 digitalWrite(leftRelay1, LOW);
 digitalWrite(leftRelay1, LOW);
 }

 Timer = millis();
 digitalWrite(leftRelay1, LOW);
 digitalWrite(rightRelay1, HIGH);

 if((millis() - Timer) >= 1500UL){
 digitalWrite(leftRelay1, LOW);
 digitalWrite(rightRelay1, LOW);
 break;
 }

Now this is my question:

So the "Timer" gets the current value of millis, then rightRelay1 is HIGH, does the code flow stay on the if((millis() - Timer) >= 1500UL) part until it is satisfied? Thanks. Just looking for clarification.

Code never "stays on" anything. Not for longer than a machine cycle.

So it seems that the millis algorithm I have written is not applicable for switch statements...

So it seems that the millis algorithm I have written is not applicable for switch statements...

The usual advice is to read, understand, and embrace the blink without delay philosophy. You appear to have completed the first step.

You need a state machine. If it is time to start turning, start turning, and record when, so you can stop turning at the appropriate time.

How long do you expect it to take between these 2 lines of code ?

offTimeCounter = millis();
 if((millis() - offTimeCounter >= 5UL){

It will only take a few machine cycles - perhaps less that 1 microsecond. It could NEVER take 5 millisecs.

Spend some more time studying several things at a time. The use of millis() for timing is an essential part of Arduino programming.

...R

Hi everyone,

I have written another code which I thought would finally work but ... I was wrong. The voice module would catch my voice command, but the relays would not respond.

This time, there are no delays. Only boolean flags and millis() stuff. I still can't fully absorb the concept of state machines, but those boolean flags were used as an attempt.

Please point out the faults and help me know what should I do. I'm getting desperate here since I'm trying to beat a deadline.

Here's some of my code:


Variables and assignments. Everything should be set to false since the device is initially at rest

boolean leftRelay1State = false;
boolean rightRelay1State = false;
boolean leftRelay2State = false;
boolean rightRelay2State = false;

boolean turnLeft = false;
boolean turnRight = false;
boolean goForward = false;
boolean reverse = false;
boolean stopMotors = false;

The next piece of code is located inside void loop, then inside a switch statement .. code flow proceeds to this case named "turnLeft" when a "LEFT" voice command is spoken.

I need a timer to make "left turning" only take 1.5 seconds and I used millis() for this. Current millis() is stored in "TimerForLeft" variable, then after switching turnLeftFlag and leftRelay1State to true (opposite of false), the code flow should exit from "turnLeft" because of "break" ...

case turnLeft:

TimerForLeft = millis();

turnLeftFlag = !turnLeftFlag;
leftRelay1State = !leftRelay1State;

break;

then will try to find the if statement that matches the condition "turnLeftFlag == true", which is this one...

if(turnLeftFlag == true){	

	digitalWrite(leftRelay1, leftRelay1State);  //should switch leftRelay1 to HIGH
                                                   //since leftRelay1State is set to HIGH on the turnLeft case

       //until (millis() - TimerForLeft) >= 1500) is satisfied
       //code flow will just loop

	if((millis() - TimerForLeft) >= 1500){	

		turnLeftFlag = !turnLeftFlag;	 //turnLeftFlag's state reverts to false, 
                                                //so the code flow won't proceed to this part anymore
		leftRelay1State = !leftRelay1State;	//state of leftRelay1 is back to LOW
		digitalWrite(leftRelay1, leftRelay1State);	//leftRelay1 should follow the current given state
                                                              //which is LOW or OFF
	}
}

And yes, it won't work. I hope my elaboration helps.

For complete reference, just look at my new code in an attached text file.

And by the way, thanks for the help along the way :slight_smile:

Prototype Code V4.txt (5.69 KB)

Everything should be set to false since the device is initially at rest

The compiler/crt0 sets anything at global or static scope, that is not otherwise initialized, to zero/false/null, so you don't have to.

Your code is poorly formatted which makes it difficult to see the different sections - for example where a CASE begins and ends. The compiler won't care but it is essential IMHO to make code readable if it is to be maintainable.

You have several sections like this

	if(turnLeftFlag == true){
		digitalWrite(leftRelay1, leftRelay1State);
		if((millis() - TimerForLeft) >= 1500){
			turnLeftFlag = !turnLeftFlag;
			leftRelay1State = !leftRelay1State;
			digitalWrite(leftRelay1, leftRelay1State);
		}
	}

and I think if you put that in a function like this

void turnLeft() {
	if(turnLeftFlag == true){
		digitalWrite(leftRelay1, leftRelay1State);
		if((millis() - TimerForLeft) >= 1500){
			turnLeftFlag = !turnLeftFlag;
			leftRelay1State = !leftRelay1State;
			digitalWrite(leftRelay1, leftRelay1State);
		}
	}
}

you could call it directly from the relevant CASE section like this

			case turnLeft:
				TimerForLeft = millis();
				leftRelay1State = !leftRelay1State;
				turnLeft();              // <=================
				break;

I obviously have not tested this so I am not saying it would definitely work - but I hope it illustrates how the code could be made much easier to manage.

Having said all that, I can't offer any practical advice because you have not told us what that version of your program actually does and what it should do.

...R

Follow up to Reply #17 - I could not edit it without losing all the layout because I was using tabs.

The code in the function should probably be

void turnLeft() {
	digitalWrite(leftRelay1, leftRelay1State);
	if((millis() - TimerForLeft) >= 1500){
		leftRelay1State = !leftRelay1State;
		digitalWrite(leftRelay1, leftRelay1State);
	}
}

...R

boolean leftRelay1State = false;
boolean rightRelay1State = false;
boolean leftRelay2State = false;
boolean rightRelay2State = false;

I don't understand these names. How many right side relays are there? Does that matter? Surely, each relay controls something, and that something would be more meaningful as part of the name than the relative position of the relay on a board.