Document the coordinate system #312
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PeterJCLaw
force-pushed
the
document-coordinate-system
branch
3 times, most recently
from
00a8153 to
80865cd
Compare
PeterJCLaw
force-pushed
the
document-coordinate-system
branch
from
80865cd to
2ed9f0f
Compare
This was referenced Oct 21, 2022
| Positive values indicate a rotation counter-clockwise from the | ||
| perspective of the camera. | ||
|
|
||
| Zero values for April Tags markers have the marker reference point at |
There was a problem hiding this comment.
Nit: Not all markers supported by Zoloto are April.
There was a problem hiding this comment.
Indeed, this is why it's specified which markers this applies to. I haven't tested nor explored the values for the other marker types.
There was a problem hiding this comment.
This portion of the marker logic doesn't change between dictionaries, they're all top left.
Suggested change
| Zero values for April Tags markers have the marker reference point at | |
| Zero values for markers have the marker reference point at |
There was a problem hiding this comment.
Does this hold for all the other places where this PR mentions April Tags, or just this one?
There was a problem hiding this comment.
Ah I hadn't spotted the others. Yes it does hold for those too.
RealOrangeOne
previously approved these changes
Nov 19, 2022
It would be great to document the orientation of the axes etc., however that's already in flight in RealOrangeOne#302 so leaving that for now.
It would be great to document the orientation of the axes etc., however that's already in flight in RealOrangeOne#302 so leaving that for now.
This was determined by experimentation with an old printed marker I had to hand. There's some slight oddness here in that the values suggest the marker I have is upside down, however that seems to disagree with other sources I've checked for the same marker. Also worth noting that I don't have a calibration for the camera I used, so this is based off another calibration I found for the same resolution.
Derived from testing of the current system. This includes moving the construction of the coordinates into the type as that is functionally where the definition of the nature of the spherical coordinates comes from.
This helps understand the relationship between these and ensures that changes aren't accidental.
PeterJCLaw
force-pushed
the
document-coordinate-system
branch
from
2ed9f0f to
6294d47
Compare
This was referenced Jan 28, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The documents each of the coordinate types, establishing the nature of their axes including the respective origins and positive directions.
This is based on local testing with a two cameras, one for which I do not have a valid calibration (though I did find one with the same resolution in
sr-robot3) and the confirmed using a C270.There is definitely some oddness here as the results suggest that positive Y axis values are "down" in the camera's field of view.
Validation of this with another camera would be most welcome.
I've built this on top of #304 in part to avoid conflicts, but also because that PR establishes much more clearly that the types have intended semantics rather than allowing them to be essentially shallow wrappers around whatever data happens to fit their shape.
These changes will inevitably conflict with #302, however I suggest we aim to merge this PR first (as it doesn't change behaviour) and then come back to that one. To that end I've ended up copying over some of the relevant changes in where some of the logic lies as well as the tests, but all keeping the behaviour as it is in
master.