Skip to content

Add April Cal Grid Cailbration#62

Merged
dehann merged 12 commits intomasterfrom
feat/4Q20/gridcalib
Dec 9, 2020
Merged

Add April Cal Grid Cailbration#62
dehann merged 12 commits intomasterfrom
feat/4Q20/gridcalib

Conversation

@dehann
Copy link
Copy Markdown
Member

@dehann dehann commented Dec 8, 2020

Must first finish #63 since current API conventions are all mixed up.

@dehann dehann added this to the v0.8.0 milestone Dec 8, 2020
@dehann dehann added caiibration compat enhancement New feature or request labels Dec 8, 2020
@dehann dehann marked this pull request as draft December 8, 2020 03:26
@dehann
Copy link
Copy Markdown
Member Author

dehann commented Dec 8, 2020

HI @Affie , thanks FYI similar renaming for f_width and c_height to overcome much confusion.

@Affie
Copy link
Copy Markdown
Member

Affie commented Dec 8, 2020

Hi @dehann, my preference is still to follow the apriltag library and perhaps just document it clearly here also. If you use a library such as openCV or even ROS to calibrate your camera you get fx,fy,cx,cy.
For me, width and height make it less clear if you work with the tag pose, x-y-z follows the right-hand rule and is standard.

Alternatively, we can have a diagram in the docs that show that fx is f_width and so on.

@dehann
Copy link
Copy Markdown
Member Author

dehann commented Dec 8, 2020

i think the x is the confusing part because it is used by both conventions. right hand rule should always be followed. I'm a bit frustrated with fx, cx because it clashes with Array convention and right hand rule. I want hard verbose so the user knows exactly what is going on. there is only one width and one height, and that overcomes the (x,y) vs (y,x) that gets smeared under (X,Y)

@dehann
Copy link
Copy Markdown
Member Author

dehann commented Dec 8, 2020

we can document that fx, cx in the library is f_width, and c_width. User is responsible for choosing whether they want to use (y,x), (u,v), (i,j). But it must always be right hand rule. Never add anything that is left hand rule.

i tried reading the current documentation but was not clear. So how about keeping the low level '_verb...' wrappers as cx and fy, and high level functions as c_width f_height.

I've been battling with the convention for days now and want a permanent fix for the problem

@dehann
Copy link
Copy Markdown
Member Author

dehann commented Dec 8, 2020

we should definitely do a diagram

@Affie
Copy link
Copy Markdown
Member

Affie commented Dec 8, 2020

We should maybe repeat some of the apriltag c library docs. I just documented the julia interface part. There is a bunch of people that also get confused over there.
Also the tag size.

@dehann
Copy link
Copy Markdown
Member Author

dehann commented Dec 8, 2020

I definitely want to find a solution that works for both of us, because then it should be good for wider audience. I'll update here and call again

@dehann dehann marked this pull request as ready for review December 9, 2020 00:41
@dehann dehann merged commit 246ba1b into master Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants