1254 views

# Going insane with inconsistency...help please!

1. 8 months ago

### choacher

7 Dec 2017

Students created a machine that lifts a weight. It starts when button is pushed, stops when reaching upper limit, reverses direction on next button push, stops when reaches lower limit switch. This is a continuous loop. Problem we are having is direction does not always change. Can you please help me help my students?

{
int countValue=3;

while(1)
{
if(SensorValue[button]==1)////////////////////////////PUSH THE BUTTON IT RUNS ALL MOTORS POSITIVE DIRECTION
{

startMotor(rightMotor,127);
startMotor(leftMotor,127);
startMotor(midMotor,127);

}
else if (countValue %2==0) /////////no remainder in countValue runs motors opposite direction
{
startMotor(rightMotor,-127);
startMotor(leftMotor,-127);
startMotor(midMotor,-127);

countValue=countValue+1;

}
else if(SensorValue[upper]==1 ||
SensorValue[lower]==1)
{
stopMotor(leftMotor);
stopMotor(rightMotor);
stopMotor(midMotor);
}

}

}

See post in context

@choacher it works....i exchanged 128 for 127 maybe a coincidence...but either way..

Oops. That was me rushing through, typing that in 3 minute.

@choacher Now can you tell me why this works. Explain the path thru the code once the button is pushed and how the 2 variables work together. This is a teachable moment my friend!

1. You weren't letting any time pass (well, 1 ms or two) between checks, so you were finding the same button press as multiple button presses.
2. As part of that, your second check wasn't really looking for a button press. It was just firing off any time you didn't have certain button presses.

So, what's different about my code? What is it doing.

First, why goUp? This is the only variable that really matters. Sure, you can keep a count, and that's useful if you want to know how many times you've done something. But if you're just switching between two options, swapping a boolean requires less math. In this case, we want sign changes for the drive power, so we might as well use +/-1 instead of a boolean and apply that to the drive power. It's not really different than what you were doing, but it cuts down on the if-then statements a lot.

Then we set the motors going, just like you planned to.

The next if-then/while loop is really the key. Notice it is inside the if statement that looks for the button press. That means we get stuck here and never cycle around to look for another button press until this process is done. This means you can't turn the thing around in the middle of the process. But it didn't sound like you wanted to, so I figured that was fine and made for shorter code. If you want to be able to reverse in the middle, we have to change things. So, what specifically does this if-then/while loop do?...

Next, why motorPower? Logically, it doesn't matter. You could just multiply 127 by goUp each time you set a motor. I prefer this as a general practice for two reasons. First, it means you're only multiplying once. Three isn't a lot, but if you are doing a lot multiplication, it can slow down a processor. When evaluating processing speed, addition and replacement are essentially free while multiplication is slow. Of course, this is just a single bit swap to change a sign. But I'm talking general practice. Second, a lot of times there may be some varying value instead of 127, and frequently there is no guarantee the value won't change in the middle of setting different motors. That's not the case here, but again I'm talking general practice. So motorPower doesn't do anything really functional here; I just find it better practice so I write that way.

Now, within the while loop is only the one if statement. We'll just keep checking for a button press since that's all we care about discovering from the user.

If the button is pressed, we set the motors to drive the opposite direction from the previous time. The reason it's the opposite direction is from the very last line of the loop. At the end of the loop we swap goUp between +1 and -1 so it's always the opposite of what it had been when we get to the next button press.

We don't want to check both sensors. Theoretically one of them is pressed at the moment. So if we check that one, then we're checking the wrong direction. The if-then statement says that if we're going up we'll only watch the upper limit switch, otherwise we're going down and we'll only watch the lower limit switch. The way we watch the limit switches is by running the while loop, we just keep delaying (letting the motors run and not looking for a button press) until the limit switch is pressed. Once we've hit the limit, we turn off the motors and swap goUp as mentioned above so we're ready to look for a new button press.

If this were running in some other task or you were running some other task, I would have followed the original if-then statement with a delay to give time for other stuff to run while this is doing nothing. But I wasn't concerned with that here since nothing else is happening.

Does that help?

2. ### Mystellianne

7 Dec 2017 Miami, Florida 4411S

Right now, your direction-changing code is inside of an "else if" statement, which will only run if the "if" statement before it is not true. Because the "if" statement before it checks if the button is pressed, the direction will only try to be reversed if that condition is NOT true, so when the button is not pressed.

From what I read it seems that you want it to reverse the direction every other time it's pressed (hence why you're using modulus), so have it check to see if you should reverse INSIDE the actual "if" statement, because you want that to happen when the button is pressed.

3. ### choacher

7 Dec 2017

Thank-you. I will try that out tomorrow when I get to school.

4. ### callen

7 Dec 2017 Braintree, MA, USA

Why not get rid of some of the conditionals? Just put a +/-1 along with countValue. Swap it each button press. Make the motor powers that value multiplied by the desired power amount.

5. ### nenik

7 Dec 2017 V5 Beta Tester

Just to explain clearly what the problem is:
Pressing a button, the way the code is structured, is a state, not an event.
Your loop will run many times before you release the button, thus ticking the counterValue up by pretty much random number every time you press a button. Sometimes this increment ends up odd (behaves as expected), sometimes it's even (no observed change in direction).

You need to recognize the button press event from the button press state. One way to do that is to keep a bool lastButtonPressed and only consider the press if !lastButtonPressed.

I'll give you a chance to figure out the exact code yourself, but feel free to come back for more hints.

6. ### callen

7 Dec 2017 Braintree, MA, USA

Yes, @nenik is right. You need some sort of delay whenever you read buttons or you end up reading them many, many times. But there is a simpler approach that handles a bunch of things together. Here is some pseudocode:

int goUp = 1;

while (true) {
if button is pressed {
set the motors to goUp*127
countValue++;
goUp=-goUp;
while(SensorValue[upper]==0 && SensorValue[lower]==0) {
wait1Msec(5);
}
stop the motors
}
}

Here the delay is forced by running until a limit switch is hit before going back to check the if statement. Now, if you're not using countValue for anything else (like keeping track of the total), it can just be dropped. This has also assumed you don't want to be able to reverse direction in the middle, but it sounded like you did not want that based on the original statement.

7. ### choacher

7 Dec 2017

This is what I attempted and the program is more consistent but not spot on. Still having issues reversing motors each button press.

{
int countValue=3;
int buttonValue=0;
int lastButtonValue=0;

while(1)
{
buttonValue=SensorValue[button];

if(buttonValue==1 && lastButtonValue==0)

{
startMotor(rightMotor,127);
startMotor(leftMotor,127);
startMotor(midMotor,127);
countValue=countValue+1;
}

else if (countValue %2==0)
{
startMotor(rightMotor,-127);
startMotor(leftMotor,-127);
startMotor(midMotor,-127);
countValue=countValue+1;

}
else if(SensorValue[upper]==1 ||
SensorValue[lower]==1)
{
stopMotor(leftMotor);
stopMotor(rightMotor);
stopMotor(midMotor);
}

}

}

8. ### callen

7 Dec 2017 Braintree, MA, USA

You're still flying through this program at high speed. The program will finish its loop several times in the time it takes a person to press and release the button a single time. That means you'll register a whole bunch of different things for one press. Look at what I wrote above about delays for button presses.

And, as mentioned by others, if you're not hitting your button, you'll go to the next option repeatedly.

You really need to change the approach. Look through what I wrote above.

9. ### choacher

8 Dec 2017

Callen
I am truly stumped. I guess I don't understand how to use a delay. I tried using what you shared and the motors would not change direction at all. What is the reason for the line

while(SensorValue[upper]==0 && SensorValue[lower]==0) {
wait1Msec(5);
}

if both limit switches are off why wait?

goUp=-goUp; //////what is this suppose to do? reverse motors? Why did you initially set it equal to 1? Does the 1 mean a value of 1 or does it mean "on"

Is my code close to being functional? I understand that the program loops very quickly when the button is pushed but I truly can't figure out what everyone is telling me here. I asked an IT guy at school today and even he can not help.

10. ### choacher

8 Dec 2017

nenik
bool lastButtonPressed and only consider the press if !lastButtonPressed.

why bool vs int and does that exclamation point in front of lastButtonPressed mean something?

11. ### choacher

8 Dec 2017

Why not get rid of some of the conditionals? Just put a +/-1 along with countValue. Swap it each button press. Make the motor powers that value multiplied by the desired power amount.

How would I replace conditionals with +/- countValue

Thank you

12. ### kypyro

8 Dec 2017 V5 Beta Tester Central Kentucky
Edited 8 months ago by kypyro

@choacher nenik
bool lastButtonPressed and only consider the press if !lastButtonPressed.

why bool vs int and does that exclamation point in front of lastButtonPressed mean something?

bool is a variable type that has a boolean value. Meaning it is either true or false. One typical use for a bool is for flag variables that are used in conditional statements.

The exclamation point is a logical "not" operator. It gives the logical negative of the term which follows the not. "not true" is false; "not false" is true. So, the pseudo code line "if !lastButtonPressed" should be read as "if not lastButtonPressed". Since lastButtonPressed is of type bool, the if condition will only be true when lastButtonPressed is false.

I haven't looked at all at the logic @callen presented, but suggesting a bool flag and testing it the way he/she indicates is a good sign that he/she knows a bit about this sort of thing

13. ### jpearman

8 Dec 2017 Moderator, ROBOTC Tech Support, V5 Beta Moderator Los Angeles 8888

@choacher This is what I attempted and the program is more consistent but not spot on. Still having issues reversing motors each button press.

what happened to these lines

```    lastButtonValue = buttonValue;
wait1Msec(10);```
14. ### nenik

8 Dec 2017 V5 Beta Tester

There are more flaws in the code anyway. For example, the end stop will still be in effect when you release the button (especially since we added the edge-trigger logic).
Let's follow the logic starting with the elevator all the way up and count=3 when you press the button.

• The button press event is (correctly) recognized, so the motors are started. In the "up" direction though.
• The code quickly (think microseconds) skips the else branches and goes into the next loop iteration.
• But even though you're still likely holding the button, the first blocks gets skipped, since the button wasn't released in the mean time.
• As count is 4 by now, the second block gets executed, switching the motors to go down (good, what we wanted) and counting up to 5.
• But the next loop iteration runs quickly again, skipping the first two blocks - button still held, and count is now odd.
• The motors only had a chance to run for microseconds so far (in reality, not at all, there's a delay in tens of milliseconds regardless of any system inertia), so the lift didn't move yet.
• So the last condition is true - the elevator still pressing the endstop and thus stopping the motors again.

I wonder if the elevator ever moved with the new code, or ever moved down with the old code.

Based on the above, you see your conditions need to be structured very differently. For example, you can't check the upper end stop if your intention is to move down (and vice versa). How to write a working code depends on your planned learning outcome, i.e. whether you specifically wanted to showcase the modulo operator, or explicit state variable.

One way to structure the code (while loosing the above mentioned lessons) would be making the state implicit (encoded in the program counter position), like:

```while (true) {
while (...) { // the elevator is down or raising
// handle upper endstop and possibly direction change
}
// fall through when done on or direction change
while (...) { // the evelator is up or going down
// handle lower endstop and possibly direction change
}
// fall through when done on or direction change, wraps around
}```
15. ### choacher

8 Dec 2017

jPearman
I ran the code example you gave me and motors do not change direction at all.

{
int countValue=3;
int buttonValue = 0;
int lastButtonValue = 0;

while(1)
{
buttonValue = SensorValue[button];

if( buttonValue==1 && lastButtonValue==0 )////////////////////////////PUSH THE BUTTON IT RUNS ALL MOTORS POSITIVE DIRECTION
{
startMotor(rightMotor,127);
startMotor(leftMotor,127);
startMotor(midMotor,127);

}
else
if (countValue %2==0)
{
startMotor(rightMotor,-127);
startMotor(leftMotor,-127);
wait1Msec(10);
startMotor(midMotor,-127);

countValue=countValue+1;
}
else
if(SensorValue[upper]==1 || SensorValue[lower]==1)
{
stopMotor(leftMotor);
stopMotor(rightMotor);
stopMotor(midMotor);
}

lastButtonValue = buttonValue;
wait1Msec(10);
}
}

16. ### callen

8 Dec 2017 Braintree, MA, USA

Please try the code I sent you. I'm updating it from pseudocode here. This should make it reverse direction

```int goUp = 1;
int motorValue=0;

while (true) {
if(SensorValue[button]==1) {
motorPower=128*goUp;
startMotor(rightMotor,motorPower);
startMotor(leftMotor,motorPower);
startMotor(midMotor,motorPower);
if(goUp==1) {// I don't know which + v. - is up or which is down for you, but you can swap them
while(SensorValue[upper]==0) {
wait1Msec(5);
}
else {
while(SensorValue[lower]==0) {
wait1Msec(5);
}
stopMotor(leftMotor);
stopMotor(rightMotor);
stopMotor(midMotor);
goUp=-goUp;
}
}```
17. ### choacher

8 Dec 2017

undefined variable motorpower

18. ### choacher

8 Dec 2017

should I

int motorPower=0;

19. ### callen

8 Dec 2017 Braintree, MA, USA

Sorry, I changed motorValue to motorPower

20. ### choacher

8 Dec 2017

ok
unexpected else