This repository was archived by the owner on Jul 26, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1
Add marker orientations #34
Open
Adimote
wants to merge
13
commits into
master
Choose a base branch
from
add-orientation
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
50d38c5
Fix rotation of markers
Adimote b711cbb
add orientation to tokens
Adimote d17aed4
add tests for orientations
Adimote 217b7a3
fix missing cast import
Adimote a34fd37
Merge branch 'master' into add-orientation
Adimote f69d719
make cv3d output Orientation
Adimote db590b6
fix regression caused by merging cv3d
Adimote 0e066c8
isort cv3d
Adimote 843e455
clarify parameters of tests are degrees or radians
Adimote 361ce09
make wrapping checks optional
Adimote 4a79783
fix tests
Adimote 5a8a949
mention Orientation in the readme
Adimote 638580b
fix rot_x description
Adimote File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
"""Tests for marker orientations.""" | ||
|
||
import math | ||
from pathlib import Path | ||
|
||
import pytest | ||
from pytest import approx | ||
|
||
from sb_vision import FileCamera, Orientation, Vision | ||
|
||
ROTATION_TOLERANCE_DEGREES = 6 | ||
|
||
CALIBRATIONS = Path(__file__).parent.parent / 'calibrations' / 'tecknet_rotations' | ||
|
||
TEST_IMAGES = [ | ||
( | ||
'0rot_x-45rot_y1.1z.png', | ||
False, | ||
Orientation(rot_x=0, rot_y=math.radians(-45), rot_z=0), | ||
), | ||
( | ||
'-45rot_x0rot_y1.1z.png', | ||
False, | ||
Orientation(rot_x=math.radians(-45), rot_y=0, rot_z=0), | ||
), | ||
( | ||
'-22.5rot_x0rot_y0.6z.png', | ||
False, | ||
Orientation(rot_x=math.radians(-22.5), rot_y=0, rot_z=0), | ||
), | ||
( | ||
'0rot_x-22.5rot_y0.6z.png', | ||
False, | ||
Orientation(rot_x=0, rot_y=math.radians(-22.5), rot_z=0), | ||
), | ||
( | ||
'0rot_x0rot_y0.55z.png', | ||
False, | ||
Orientation(rot_x=0, rot_y=0, rot_z=0), | ||
), | ||
( | ||
'-90rot_z.png', | ||
False, | ||
Orientation(rot_x=0, rot_y=0, rot_z=math.radians(-90)), | ||
), | ||
( | ||
'90rot_z.png', | ||
False, | ||
Orientation(rot_x=0, rot_y=0, rot_z=math.radians(90)), | ||
), | ||
( | ||
'180rot_z.png', | ||
True, | ||
Orientation(rot_x=0, rot_y=0, rot_z=math.radians(180)), | ||
), | ||
( | ||
'135rot_z.png', | ||
False, | ||
Orientation(rot_x=0, rot_y=0, rot_z=math.radians(135)), | ||
), | ||
( | ||
'45rot_z.png', | ||
False, | ||
Orientation(rot_x=0, rot_y=0, rot_z=math.radians(45)), | ||
), | ||
] | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"photo, allow_wrapping, expected_orientation", | ||
TEST_IMAGES, | ||
) | ||
def test_image_coordinates(photo, allow_wrapping, expected_orientation): | ||
"""Make sure that this particular file gives these particular tokens.""" | ||
camera = FileCamera(CALIBRATIONS / photo, camera_model='C016') | ||
vision = Vision(camera) | ||
token, = vision.snapshot() | ||
|
||
def approx_ang(expected_degrees): | ||
return approx(expected_degrees, abs=ROTATION_TOLERANCE_DEGREES) | ||
|
||
def assert_angle(angle_radians, expected_degrees, message): | ||
expected_degrees = approx_ang(math.degrees(expected_degrees)) | ||
# Check both +0 and +360 so approx can cover the jump between -180 and 180 | ||
if allow_wrapping: | ||
assert \ | ||
math.degrees(angle_radians) == expected_degrees or \ | ||
math.degrees(angle_radians) + 360 == expected_degrees, \ | ||
message | ||
else: | ||
assert math.degrees(angle_radians) == expected_degrees, message | ||
|
||
rot_x, rot_y, rot_z = token.orientation | ||
|
||
assert_angle(rot_x, expected_orientation.rot_x, "Wrong Orientation rot_x") | ||
assert_angle(rot_y, expected_orientation.rot_y, "Wrong Orientation rot_y") | ||
assert_angle(rot_z, expected_orientation.rot_z, "Wrong Orientation rot_z") |
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.
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 do we need to do this?
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 we can allow for a small error on a value which is close to 180 or -180. it means it checks both the 180 version and the -180 version in one check.
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.
Right, but these are tests -- they're dealing with fixed inputs which should mean that they have fixed outputs. We should know which side the result is and just assert that.
Are you saying that we get varying results from
solvePnP
even for constant input data?An alternative way of spelling this, iff it's absolutely needed, might be to declare that the tests operate in a positive-only frame and run all the values through
% 360
first.I'm still not sure I like this approach as it means we're not really checking the direct output from the API. The not-checking-the-real-value aspect could be mitigated by adding a range bounds first, perhaps like this:
While I'll admit there's a bit more processing here, degrees being a modulo thing is fairly obvious to everyone since we're used to the idea that 720° is the direction same as 0°.
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 respect what you're saying but I disagree that we should check exact values. It makes the tests extremely fragile for changes higher up the pipeline (i.e. thresholding). If we want to check that the outputs don't vary, we should have a separate test
Uh oh!
There was an error while loading. Please reload this page.
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.
%360
doesn't solve the problem, it shifts the barrier to be between 0 and 360