-
Notifications
You must be signed in to change notification settings - Fork 3
add orientations #121
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
add orientations #121
Conversation
content/api/vision/coordinates.md
Outdated
|
||
## Orientation Coordinates | ||
|
||
Orientation represents the rotation of a marker around the x, y, and z axes. |
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.
Any reason why X, Y, Z are capitalised and x,y,z are not?
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.
Nope, good catch
@trickeydan I've made additions since you reviewed (my fault), is there any chance you can take another look? |
content/api/vision/coordinates.md
Outdated
counter-clockwise about the Cartesian y axis. | ||
- `rot_z_radians` / `rot_z_degrees` - the angle of rotation in radians/degrees | ||
counter-clockwise about the Cartesian z axis. (tip: the z axis typically faces the camera, and thus will seem like a clockwise rotation) | ||
counter-clockwise about the Cartesian z axis. (tip: the z axis typically |
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.
Using the term typically suggests that there is a case where the z axis may not face the camera?
Is it worth mentioning this in the docs?
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.
fixed in c04d06d
content/api/vision/_index.md
Outdated
- `pixel_centre` - returns the location in pixels of the centre of the marker in the captured image. | ||
- `cartesian` - returns details of the position of the marker in the [Cartesian](coordinates/#cartesian-coordinates) coordinate system. | ||
- `spherical` - returns details of the position of the marker in a [spherical](coordinates/#spherical-coordinates) coordinate system. | ||
- `orientation` - returns the rotation of the marker around its centre. See [Orientation](coordinates/#orientation) for its coordinate system. |
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 don't really like the additional sentence just to link, could we word orientation into the initial one. Perhaps:
returns the [Orientation] of the marker around its centre.
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.
fixed in 5566007
content/api/vision/coordinates.md
Outdated
rotations represent. | ||
| | | | ||
|:-:|:-:| | ||
| <img src="/img/api/coordinate-orientations/m45x0y0z.png?width=200px" style="margin:0"/> `rot_x_degrees = 45` | <img src="/img/api/coordinate-orientations/m-45x0y0z.png?width=200px" style="margin:0"/> `rot_x_degrees = -45` | |
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.
Why are we using image tags here rather than the markdown format? I can't see that forcing the margin to 0 is really required here?
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.
the default margin makes them very spaced out
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.
Hmm, you're right, that's really annoying. I'll try and fix it with CSS some time!
content/api/vision/coordinates.md
Outdated
|
||
 | ||
|
||
## Orientation Coordinates |
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.
Orientation isnt a coordinate system. I think this section warrants its own page
content/api/vision/coordinates.md
Outdated
|
||
The following table visually explains what positive and negative | ||
rotations represent. | ||
| | | |
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 table is invalid when rendered with Hugo, see https://deploy-preview-121--sourcebots-docs.netlify.com/api/vision/coordinates/#orientation-coordinates
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.
Good catch, fixed in eb0f59e
Note: I'm holding off reviewing this until sourcebots/sb-vision#34 and sourcebots/robot-api#83 are resolved. |
Add page on orientations and a table of images on what each rotation looks like.