Can this be optimized anymore?

Programming Support
1

I’m just trying to improve my sloppy programming and I came up with this statement to power the lift.
Is there a way to do this with just one statement, or any other suggestions?

bool R1 = Controller1.ButtonR1.pressing();
bool R2 = Controller1.ButtonR2.pressing()*-1;

if(R1 + R2 == 0){
backMogo.stop(brakeType::brake);
}
else{
backMogo.spin(directionType::fwd, (R1+R2)*100 ,velocityUnits::pct);
}

2

soooo first [code} [/ code] or these ````

Second and far more importantly you are adding booleans? I mean it does work but what you are really trying to do is boolean algebra. what you seem to be wanting to do is:

 if ( R1 == false && R2 == false) {
// stop motor
} else {
  // run motor 
  // the numerical val of R1 + R2 is 2 because its true + true which is equal to 1 + 1
  // so your saying you want to run the motor at 2*100 = 200 with percent as your units
  // your telling your motor to run at 200% velocity
  // if you want your motor to run at max power then you should use voltage units.
  backMogo.spin(directionType::fwd, 12 ,voltageUnits::volt);
}

Now I don’t know if your code will work but I hope it this makes your programing practice better.

1 Like
3

I think running the motor at 200% velocity wouldn’t happen due to:
bool R2 = Controller1.ButtonR2.pressing()*-1;

The R2 is either -1 or 0? (I hope, that’s the intention: maybe I shouldn’t use a bool 0_0)

That means that the motor would then be in reverse as (0 + -1)*100=-100 which lets the R2 button reverse the direction of the lift to go down.

Is that right?

4

If you really wanted, you could use a nested ternary statement, as follows:

while(true) {
    bool R1 = Controller.ButtonR1.pressing();
    bool R2 = Controller.ButtonR2.pressing();

    R1 ? TestMotor.spin(forward, 100, pct)  : ( R2 ?  TestMotor.spin(reverse, 100, pct) : TestMotor.stop());
  }

In general though, it’s much more concise and better style to simply write it as:

while(true) {
    bool R1 = Controller.ButtonR1.pressing();
    bool R2 = Controller.ButtonR2.pressing();

    if(R1) { 
       backMogo.spin(forward, 100, pct); 
    } else if (R2) { 
          backMogo.spin(reverse, 100, pct); 
     } else { 
        bacMogo.stop(); 

      }
  }

There’s no need to do any unnecessary optimization. In fact, it’s not really optimizing anything, more obfuscation.

4 Likes
5

If you really want to, you u can do something like this:

backMogo.stop(brakeType::brake);

while(true){
    bool R1 = Controller1.ButtonR1.Pressing();
    bool R2 = Controller1.ButtonR2.Pressing();
    // When only R1 is pressed, R1 - R2 = 1, making the lift go up
    // When only R2 is pressed, R1 - R2 = -1, making the lift go down
    // When both R1 and R2 are pressed / unpressed, R1 - R2 = 0, making the lift stay
    backMogo.spin(directionType::fwd, (R1 - R2) * 100, velocityUnits::pct);
    delay(10);
}

You can also do the entire lift control in a statement if you just use Controller.Pressing() as the parameter like this (code written for pros):

image
image1321×50 10.5 KB

However, building on to what Oscar said earlier, the compiler is actually already good at optimizing code, so these tiny changes won’t really affect anything in the long run. There really isn’t too much of a reason to try and make the code look “clean” and write a bunch of boolean manipulation magic one-liner that you won’t understand after a year.

For example, here is an excerpt of my PID class I wrote over a year ago.

  integral += error * (error < minDist);
  integral *= (((int)error ^ (int)prevError) >= 0);
  integral = fmin(integral, maxIntegral);

As you can see, it is very hard to understand, and I probably couldn’t figure out what it does without looking at the comments I wrote in the past. You’ll be better off just writing some if statements and make the code as readable as possible.

6 Likes
6

Here’s some rough pseudocode of a much easier to understand version. Apologies if my syntax is a bit off, I pretty much only program in Javascript these days.

bool r1 = getR1();
bool r2 = getR2();

if(r1 == r2){ // If either both or neither buttons are pressed, brake
	brake();
} else if (r1){
	fullFoward();
} else if (r2){
	fullBackward();
}

Some general advice about optimization: if you don’t actually really really need the performance, don’t sacrifice readability for it. Readability and maintainability is vastly more important than slight performance improvements for almost any code you’re likely to work on in the real world.

I also suspect that using if statements like this is also going to be way more performant than doing addition and multiplication to generate a value anyway. People who know more about the V5 architecture, please correct me if I’m wrong, not sure how expensive multiplication is on this architecture.

6 Likes
7

oops sorry misread that but other posts here help just as well.

I hope you take a way from this threads is that:

8

Your first code is absolutely beautiful. No need for unnecessary complexities. Just simple variable arithmetics. Good practice to not go to the extremes with code compressing (unless some low storage device needs it, which is not the case) while at the same time keeping it simple.

1 Like