Open christianrauch opened 6 years ago
Thanks for bringing up these points. I'll try to give explanations.
The encoder position values are not accurate enough to require the precision offered by floating point values. To demonstrate the encoder position accuracy, grab a wheel connected to a motor, and mildly rotate back and forth without actually turning the motor/encoder. Due to the gearbox backlash, the wheel will rotate maybe 1 degree or so. Providing a value that is more precise than it is accurate is not necessary in my opinion.
Regarding the motor speed feedback (in degrees per second), due to timing tolerances and refresh rates in the firmware calculating the value, I can't guarantee better than 1 DPS accuracy. From a practical standpoint, 1 DPS resolution is equivalent to about 0.16 RPM resolution, which in my opinion is precise enough for anything people will be doing with the GPG3 drive motors.
In PR #161 I added reset_motor_encoder() which is a simpler way to reset the motor encoder values.
I want to avoid adding encoder reset to reset_all() for several reasons. Since reset_all is typically called at the termination of a program, if the wheels continue to rotate a little (e.g. the motors were running at 100% power when reset_all() was called), the encoders will be reset, but then they will count e.g. another 90 degrees as the motor is coming to a stop, making the reset pointless. Another reason is that sometimes it's handy to have the encoder values remembered between programs. For example, if you have a robot start at a specific location (0cm) on a measured line, drive 25cm and then terminate the program, if you then run a program to tell it to drive to the 15cm mark, it will know that it needs to drive backwards 10cm (15cm - 25cm = -10cm) rather than driving forward 15cm (which would put it at 40cm).
I would agree though, that based on the name, reset_all should indeed reset the encoder values as well. Perhaps I should go ahead and add encoder resets in reset_all, and then create another method that just stops everything (float motors and servos, turn off LEDs, and unconfigure the Grove ports).
Thanks for going into that detail.
I am still wondering if the rounding when getting the current encoder values and setting the offset
offset = int(int(encoder / self.MOTOR_TICKS_PER_DEGREE) * self.MOTOR_TICKS_PER_DEGREE)
doesn't add up after many calls to reset the encoder.
Wouldn't it be simpler to just set the encoder values on the controller to a given value? E.g. a method like set_encoder(port, value)
could be used to reset the encoder or to restore a previous encoder state.
I just thought about the use case of setting the encoder values after starting another program:
For example, if you have a robot start at a specific location (0cm) on a measured line, drive 25cm and then terminate the program, if you then run a program to tell it to drive to the 15cm mark, it will know that it needs to drive backwards 10cm (15cm - 25cm = -10cm) rather than driving forward 15cm (which would put it at 40cm).
This really only makes sense when using either linear (move forward/backward) or angular (turn left/right) movements only. If you translate and rotate arbitrarily, restoring the encoder values does not make sense at all. Consider the two cases: (1) driving forward 10cm and then rotate 90 degrees, and (2) rotate 90 degrees and then driver forward by 10cm. In both cases, your both wheel encoders will provide the same values, but the pose (position and orientation) of the robot in the world will be different. Restoring the encoder values after a program start does not provide any useful information in these cases.
So in the end, I do not see a benefit of setting the encoder values other than resetting them to 0, if you want to compute your position (based on wheel encoders) from a new starting point. I propose to remove the method offset_motor_encoder
(because I do not see a valid use case) and include a method for resetting the encoder values to 0.
Currently the encoder and speed values are rounded to integers which means that there is a loss in precision for
get_motor_status
: https://github.com/DexterInd/GoPiGo3/blob/b489bbd6d796c017df280f510e7a5eca9476c491/Software/Python/gopigo3.py#L545 andget_motor_encoder
: https://github.com/DexterInd/GoPiGo3/blob/b489bbd6d796c017df280f510e7a5eca9476c491/Software/Python/gopigo3.py#L569.The use for this rounding is probably only for resetting the encoders via
offset_motor_encoder(MOTOR_RIGHT, get_motor_encoder(MOTOR_RIGHT))
. However, this rounding can be done manually andget_motor_status
andget_motor_encoder
should provide values as floating points.In addition to this, there should be a more direct way for setting the encoders to 0, without calculations that depend on other values. This resetting should then also be done in
reset_all
, since old encoder values become meaningless when starting a new program.