pixix4 / ev3dev-lang-rust

Rust language bindings for http://ev3dev.org
MIT License
71 stars 14 forks source link

Adding get methods to gyro sensor #6

Closed MassiminoilTrace closed 3 years ago

MassiminoilTrace commented 3 years ago

Hi! This is my first contribution in Rust language.

I noticed that gyro_sensors.rs didn't have any get method. I added two methods:

These methods willingly fail if the sensor is in the wrong mode, for instance, if we try to access rotational speed while we're in angle-only mode. I found the details about this here

The code compiles successfully, but I'm still quite new to Rust language and I didn't test it. Do you have any preferred test procedure I can follow in this repo? Also, according to the docs, my file still lacks getter methods while in TILT-RATE, TILT-ANGLE, GYRO-CAL(no details in docs) and GYRO-FAS modes.

pixix4 commented 3 years ago

Thanks for the work. Currently there are not yet very many help functions. This will make it easier to work with the library.

For testing I have created the example programs. If you want you can create one for the GyroSensor.

I would ignore the GYRO-CAL and GYRO-FAS modes as they are not properly documented anyway. The TILT modes I haven't considered yet. If you want, you can include the constants and setters. Then that would be also complete.

For the performance it would be good if you could reduce the number of self.get_mode() calls. Cause each call is an I/O call to the kernel driver and takes a while to complete. This could be done with a match statement like:

match self.get_mode()?.as_ref() {
    GyroSensor::MODE_GYRO_RATE => self.get_value0(),
    other_modes => // Return error 
}
MassiminoilTrace commented 3 years ago

Thank you, I'll follow your instructions!

I have a question: I noticed that the method to set MODE_GYRO_ANG is called set_mode_col_ang instead of the simpler set_mode_gyro_ang. Same for the two methods below it. Is there a naming reason I'm missing or it's just a copypaste oversight from the color sensor file? If that's the case, I can fix it

pixix4 commented 3 years ago

Oh, that's copy paste error. It would be great if you can fix that as well