🐛[BUG] - motor `.get_direction()` does not work

[ssejrog]

Describe the bug
This outputs 2147483647

To Reproduce
Steps to reproduce the behavior:
The example project uses motor = 127 instead of motor.move(), so with that this is the example code.

void opcontrol() {
  pros::MotorGroup mg({1, 3});
  pros::Controller master(E_CONTROLLER_MASTER);
  while (true) {
    mg.move(master.get_analog(E_CONTROLLER_ANALOG_LEFT_Y));
    // Print the motor direction for the motor at index 1. (port 3)
    std::cout << "Motor Direction: " << mg.get_direction();
    pros::delay(2);
  }
}

Expected behavior
Because the motor isn't reversed, I should see 1.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: [e.g. iOS, Windows, Linux]
• Version [e.g. 22]

Additional context
Add any other context about the problem here.

[WillXuCodes]

are the motors plugged in

[ssejrog]

are the motors plugged in

it turns out I was confusing two issues-

I'm having a constructor take a motor group instead of a vector of ints. I was trying to check the direction of the motors so I could copy/paste it to the "motor groups" that are used internally in the code. This didn't work, and then the example code didn't work (when I don't have motors in port 1 or 3), and I made this issue.

I've done more testing now and it seems like something still isn't quite right.

No matter what I do with the directions of 11 and 15, the only thing I see printed is 0. I tried{11, 15}, {-11, 15}, {-11, -15}, {11, -15}.

pros::MotorGroup mg({11, 15});
int dir = mg.get_direction();
using namespace pros;
void opcontrol() {
  std::cout << "Motor Direction: " << dir << "\n";
  printf("\n\n");
  pros::Controller master(E_CONTROLLER_MASTER);

  while (true) {
    mg.move(master.get_analog(E_CONTROLLER_ANALOG_LEFT_Y));
    dir = mg.get_direction();
    std::cout << "Motor Direction: " << dir << "\n";
    pros::delay(2);
  }
}

This is also true of 11 and -11 with a motor and not a motor group, only 0 is printed.

pros::Motor mg(11);
int dir = mg.get_direction();
using namespace pros;
void opcontrol() {
  std::cout << "Motor Direction: " << dir << "\n";
  printf("\n\n");
  pros::Controller master(E_CONTROLLER_MASTER);

  while (true) {
    mg.move(master.get_analog(E_CONTROLLER_ANALOG_LEFT_Y));
    // Print the motor direction for the motor at index 1. (port 3)
    dir = mg.get_direction();
    std::cout << "Motor Direction: " << dir << "\n";
    pros::delay(2);
  }
}

and it is also true of the C functions, only 0 is printed.

void opcontrol() {
  while (true) {
    pros::c::motor_move(-11, pros::c::controller_get_analog(pros::E_CONTROLLER_MASTER, pros::E_CONTROLLER_ANALOG_LEFT_Y));
    printf("Motor Direction: %d\n", pros::c::motor_get_direction(11));
    pros::delay(2);
  }
}

In all of these examples, I confirmed the motors were moving with the joystick. Sorry for not being as thorough initially!

[thiccaxe]

We can't reproduce this. "2147483647" is when the motor is not plugged in, which you seem to have resolved. But if the motor is moving, it should output -1 or 1. Are you running the latest version of the kernel?

[ion098]

@thiccaxe @ssejrog
I think the issue here is that negative port numbers do NOT call pros::c::motor_set_reversed, they simply reverse the command. It also looks like the C and C++ API differ here: pros::c::motor_set_reversed is setting a VEXos flag (which is read then by pros::c::motor_get_direction), whereas Motor::set_reversed is simply flipping the sign of the Motor object's port member variable. I think the best solution would be to deprecate pros::c::motor_set_reversed and pros::c::motor_get_reversed, since there's not a good way of getting the C API to match the C++ API.

[djava]

I think the best solution would be to deprecate pros::c::motor_set_reversed and pros::c::motor_get_reversed

But then you wouldn't be able to reverse the motor from C? This might be okay if the C functions (i.e. pros::c::motor_move and pros::c::motor_get_position) support negative port numbers (which I'm unsure of), but I don't think we're going to want to break that API in a minor update.

[ion098]

re @djava

I think the best solution would be to deprecate pros::c::motor_set_reversed and pros::c::motor_get_reversed

But then you wouldn't be able to reverse the motor from C? This might be okay if the C functions (i.e. pros::c::motor_move and pros::c::motor_get_position) support negative port numbers (which I'm unsure of), but I don't think we're going to want to break that API in a minor update.

The C functions do indeed support negative port numbers, so there's not too much of an issue. As far as breaking API goes, pros::c::motor_get_reversed is already arguably broken, so making it a deprecated function that always returns false isn't really breaking anything further imo.

[djava]

@ion098

As far as breaking API goes, pros::c::motor_get_reversed is already arguably broken, so making it a deprecated function that always returns false isn't really breaking anything further imo.

Deprecating motor_set_reversed is much more of a concern to me than get.

[ion098]

@djava

As far as breaking API goes, pros::c::motor_get_reversed is already arguably broken, so making it a deprecated function that always returns false isn't really breaking anything further imo.

Deprecating motor_set_reversed is much more of a concern to me than get.

I think the C API is infrequently used to the extent that this shouldn't be too much of an issue.

Also, it turns out there's even more API madness: the C++ API has a method get_direction which uses both VEXos's flag and the sign of the port number (???), but there's no C++ API equivalent to set_reversed. I'm not sure if this is intentional or just a big oversight, but it's definitely very confusing behavior.

[djava]

@ion098
I'm honestly not super opposed to having an array of "reversed" values for each port number that is toggled by pros::c::motor_set_reversed so that we can recreate the functionality (though this is still a bitttt of an API break because now it's persistent across disconnects, but that seems closer to okay). I don't want to just kill the function entirely in a minor update though.

there's no C++ API equivalent to set_reversed

false?

[ion098]

@djava

there's no C++ API equivalent to set_reversed

false?

I was referring to implementation, the C++ API doesn't set the VEXos flag like the C API does, rather it merely flips the sign of the port stored by Motor.