Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Clarify Rotation Sensor Angle/Position Docs #341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions v5/api/c/rotation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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.

Copy link
Contributor

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.

of the sensor in relation to its position when initialized or reset. The value will continue to increment or decrement
as the sensor rotates past 36000 centidegrees.

This function uses the following values of ``errno`` when an error state is reached:

Expand Down Expand Up @@ -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
Copy link
Contributor

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.

Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Member Author

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.

Copy link
Contributor

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.

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.
Copy link
Contributor

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]?

Copy link
Contributor

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]

Copy link
Member Author

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!


This function uses the following values of ``errno`` when an error state is reached:

Expand Down
8 changes: 6 additions & 2 deletions v5/api/cpp/rotation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,9 @@ This function uses the following values of ``errno`` when an error state is reac
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
of the sensor in relation to its position when initialized or reset. The value will continue to increment or decrement
as the sensor rotates past 36000 centidegrees.

This function uses the following values of ``errno`` when an error state is reached:

Expand Down Expand Up @@ -323,7 +325,9 @@ This function uses the following values of ``errno`` when an error state is reac
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
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.

This function uses the following values of ``errno`` when an error state is reached:

Expand Down