Can't create a reversed Rotation sensor using the constructor

[maxgreen01]

Expected Behavior:

Just like motors, users should be able to construct a Rotation sensor object using a signed port number, and the sensor would be reversed if the port is negative. Alternatively, users should be able to pass in a signed port number along with a bool to determine if the sensor is reversed, where the bool overrides the sign of the port number.

Existing pros::Motor equivalent constructors:

Motor::Motor(const std::int8_t port, const bool reverse) : _port(abs(port)) {
	set_reversed(reverse);
	if (port < 0) 
		set_reversed(true);
}

Motor::Motor(const std::int8_t port) : _port(abs(port)) {
	if (port < 0) 
		set_reversed(true);
}

 

Actual Behavior:

The Rotation constructor that takes an int uses a uint8_t, and passing a signed int into this function means the sensor is constructed using a completely different port than intended because of the conversion from int to uint. When I try to get data from a sensor initialized like this, error values are returned.

Also, in one of my projects, using the constructor that takes a uint8_t and a bool for reversed immediately causes a Data Abort Exception with the error code 3437384. This happens when the bool is either true or false, but only when the sensor is defined outside of a function scope. I'm not sure if this is caused by something in PROS or by how I have it set up in my project, and I've been able to recreate it in a blank project but with slightly different results.

 

Steps to reproduce:

Try to construct a Rotation sensor using a negative port number.

Example setup in a default project:

void opcontrol() {
	pros::Rotation r1(14); //works as expected
	r1.reverse(); //also works as expected
	pros::Rotation r2(-14); //runs but isn't usable - I think tries to use port 242 because of int to uint conversion
	pros::Rotation r3(14, true); //data abort exception in some scenarios
	pros::Rotation r4(14, false); //same as above
	pros::Rotation r5(-14, true); //same as r2, and data abort exception in some scenarios
	pros::Rotation r6(-14, false); //same as above

	while (true) {
		printf("r1:   reversed = %i   pos = %i\n", r1.get_reversed(), r1.get_position()); //prints normally as expected
		printf("r2:   reversed = %i   pos = %i\n", r2.get_reversed(), r2.get_position()); //prints error codes for both
		printf("r3:   reversed = %i   pos = %i\n", r3.get_reversed(), r3.get_position()); //works normally most of the time
		printf("r4:   reversed = %i   pos = %i\n", r4.get_reversed(), r4.get_position()); //same as r3
		printf("r5:   reversed = %i   pos = %i\n", r5.get_reversed(), r5.get_position()); //same as r2
		printf("r6:   reversed = %i   pos = %i\n", r6.get_reversed(), r6.get_position()); //same as r2

		pros::delay(20);
	}
}

As for my weird issue with the Data Abort Exceptions, I was able to replicate it using these files in a default project:

chassis.hpp:

#pragma once

#include "api.h"

class Chassis {
    public:
        Chassis(pros::Rotation rot);
        pros::Rotation track;
        void run();
};

chassis.cpp:

#include "api.h"
#include "chassis.hpp"

Chassis::Chassis(pros::Rotation rot):
    track(rot) {
        run();
    }

void Chassis::run() {
    while (true) {
        printf("rot: %i\n", track.get_position());

        pros::delay(20);
    }
}

main.cpp:

#include "main.h"
#include "chassis.hpp"

inline Chassis chassis (
	pros::Rotation(14, true)
);

void initialize() {}
void disabled() {}
void competition_initialize() {}
void autonomous() {}
void opcontrol() {}

 

System information:

Platform: v5
PROS-CLI Version: 3.4.3
PROS-Kernel Version: 3.7.2

Additional Information

The main takeaway from what I've found is that the rotation sensor constructors don't work with negative port numbers in the same way that motors do, and there may be a separate problem issue with the constructors with the bool parameters causing Data Abort Exceptions.

Screenshots/Output Dumps/Stack Traces

Brain screen output for Data Abort Exception example code above:

DATA ABORT EXCEPTION
PC: 3437384
CURRENT TASK: PROS System Daemon.

[maxgreen01]

Also I’m not sure what the implications might be, but it seems like the issue of negative port numbers causing problems could be fixed by replacing the uint8_t in the constructor with int8_t, then assigning the port itself using the absolute value of the int. This concept is almost identical to how the motor constructors already work.

[WillXuCodes]

In PROS 3 this would be a breaking change, there is a possibility of adding this into PROS 4 if we want.

[maxgreen01]

Ok, cool. Any ideas why the data abort exceptions might be happening? I'm not sure if it's something related to the rotation sensor constructor, it being declared outside a function scope, inside an inline variable, or something else entirely, but it's causing some problems in my code and I'm not sure how to fix it. I could send more code snippets in DMs if that helps.

[nickmertin]

The error is likely caused by run (and consequently get_position and pros::delay) being called from the global variable initializer. In C++, initialization order is undefined, which means that it's possible that run is called before the PROS kernel is fully initialized. Overall, it's generally best to avoid constructing these as global variables, or at the very least don't invoke procedural code from constructors of global variables (rather, just initialize things, and start run on a new task from initialize).

[maxgreen01]

Good insight. Adding a new start() function that starts run() in a Task solves the issue in my test project, but this same idea of initializing the object first and calling the function later is already present in my other project where I first noticed this issue.

// in chassis.cpp
void Chassis::start() {
    pros::Task task([this] { this->run(); });
}

// in main.cpp
void initialize() {
	chassis.start();
}

Strangely, in my original project, removing the equivalent of chassis.start() from initialize doesn't fix the error, which makes me think the problem lies elsewhere. I suspect it might be caused by using the rotation sensor object in other functions (like std::bind) in the global variable initializer. My idea is to delay the execution of these functions using a Task to ensure that the rotation sensors are initialized before the functions run, but I have a feeling this could cause unintended side effects. Do you have any other thoughts on this?

[maxgreen01]

Now that I think about it, using the same code structure in my main project with a rotation sensor inside a global variable works with no issues when the sensor uses the 1-param constructor. But, using the 2-param constructor immediately causes the problems. Although you might be on to something with the initialization order idea, I'm not sure why my code would only throw errors when using the 2-param constructor but not the 1-param one. It makes me think there's an issue with the construction of the sensor itself, maybe where parts of the sensor's constructor are being called before they should be. But, this problem doesn't occur using similar objects like Motors in the same context.