Going insane with inconsistency...help please!

Missing some }'s. I was trying to write that in 3 min before running to teach a class. No time left for debugging, let alone for even a little editing. Now I have a few minutes. I think this fixes those bugs. I changed “motorPower” and added } in two spots. I’m really spoiled by editors that do all the bracket checking.

int goUp = 1;
int motorPower=0;

while (true) {
     if(SensorValue[button]==1) {
          motorPower=127*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;
     }
}

runs on 1st button press, stops on 1st limit, runs same direction on next button and then does not stop or change direction anymore…

I feel like I have an unsolvable problem…lol

it works…i exchanged 128 for 127 maybe a coincidence…but either way…

THANK YOU THANK YOU THANK YOU

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!

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

Let me start with the two problems you had before:

  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?

Sure does. I truly appreciate your dedication to my problem and look forward to future conversations. Thank you!

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.

Callen
would you please show me how you would have put the delay in my code if I had other tasks. I was thinking about adding lights to our machine that signal that it is moving…like a safety measure

Look at the last non-bracket line below. That’s the important change. I also increased the delays inside the if statement.

int goUp = 1;
int motorPower=0;

while (true) {
     if(SensorValue[button]==1) {
          motorPower=127*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(10);
               }
          }
          else {
               while(SensorValue[lower]==0) {
                    wait1Msec(10);
               }
          }
          stopMotor(leftMotor);
          stopMotor(rightMotor);
          stopMotor(midMotor);
          goUp=-goUp;
     }
     wait1Msec(20);
}

The big problem before for a task was that if a button wasn’t pressed, this would keep cycling around checking for that button repeatedly, starving other tasks.

But those lights really don’t need another task. You just put them in the prior code where I’ve commented below. You might even put in a sizable delay (like 1000 ms or more) if you want the lights to act as a warning before the motors even start.

int goUp = 1;
int motorPower=0;

while (true) {
     if(SensorValue[button]==1) {
          motorPower=127*goUp;
          // turn on the light here
          // big delay?
          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);
          // turn off the light here
          goUp=-goUp;
     }
}