-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarify Rotation Sensor Angle/Position Docs #341
base: master
Are you sure you want to change the base?
Conversation
@@ -149,7 +149,9 @@ Analogous to `pros::Rotation::reset_position <../cpp/rotation.html#reset-positio | |||
rotation_get_position | |||
--------------------- | |||
|
|||
Get the Rotation Sensor's current position in centidegrees | |||
Get the Rotation Sensor's current angle in centidegrees (0-36000). This means the absolute position |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As someone who has not worked with this sensor before, the following strikes me as a bit of a clearer wording of the same concept:
Get the Rotation Sensor's angle _relative to its starting position_ in centidegrees (1/100th of a degree, or 1/36000th of a rotation). This angle is measured in relation to its 0 centidegree position when the sensor was last initialized or reset. This reading will _not_ roll over when crossing a full rotation, but will continue to increment or decrement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. The current wording implies that the value will roll over, when this is not actually the case.
@@ -234,7 +236,9 @@ Analogous to `pros::Rotation::get_velocity <../cpp/rotation.html#get-velocity>`_ | |||
rotation_get_angle | |||
------------------ | |||
|
|||
Get the Rotation Sensor's current angle in centidegrees (0-36000) | |||
Get the Rotation Sensor's current position in centidegrees. This means the absolute position |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar request for a tweak to the wording:
Get the Rotation Sensor's _absolute_ angle in centidegrees (0-36000). This angle is measured in relation to its 0 centidegree position when the sensor was last initialized or reset. The angle will roll over to 0 as the sensor rotates past +-36000 centidegrees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it last reset or is it some global 0 angle on the sensor? Not sure how reset survives restarts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it survives restarts I'd say that explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I think Jonathan's "rotates past +-36000" is not accurate. As much as I would love it to be a zero centered range I think it's (0,36000).
So that part could be,
"As it rotates below 0 it wraps around to 36000 and vice versa for above 36000; always returning the distance from 0 point irrespective of the path the sensor took to get there"
A bit wordy but hopefully more explicit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the reason why this is a little tricky to deal with wording is because since the rotation sensor is a Hall effect sensor, angle is maintained between resets. Position's initial value after reset is set to angle as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh so it's technically distance from reset but not for the reason you might think.
My opinion is if the 2 "modes" are so confusing to need a paragraph to explain it properly take a paragraph. This is the source for someone to try to understand what the hell it does. I wouldn't have guessed get_position reset to get_angle instead of 0. Now that you say that I can think why, get_position is likely defined as get_angle + full_rotations * 36000,with reset only wiping full rotations, but that wasn't my educated guess.
Get the Rotation Sensor's current angle in centidegrees (0-36000) | ||
Get the Rotation Sensor's current position in centidegrees. This means the absolute position | ||
of the sensor in relation to its position when initialized or reset. The value is continuous, | ||
meaning it will wrap around to zero after it passes 36000 centidegrees. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unclear about the exact behavior around 3600/0. It implies that the value can be both 0 and 3600 at different times, even though they are the exact same angle. Is this actually the case? If so, should we consider cutting off one of those values so that the range is instead [0, 3600), not [0, 3600]?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, given that this is an integer value, express the range as [0, 3599]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I remember it being correct as [0,36000) on the Kernel doxygen docs, but ig I messed up here. Will fix!
Discussion that needs to be had: how relevant is updating the PROS 3 docs when PROS 4 exists now? |
Does what it says on the tin. This was brought up on VF and VTOW.