search

RealOrangeOne / zoloto

A fiducial marker system powered by OpenCV - Supports ArUco and April
https://zoloto.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
13 stars 7 forks source link

Make the Spherical coordinates useful #302

Open PeterJCLaw opened 1 year ago

PeterJCLaw commented 1 year ago

This changes the Spherical coordinates to (almost) following the ISO convention. Consequently there are a number of changes to the values, notably:

See inline documentation for the interpretation of the values now.

Fixes https://github.com/RealOrangeOne/zoloto/issues/300 Builds on #312

PeterJCLaw commented 1 year ago

I wasn't expecting the reference positions for each of the rot_x and rot_y angles to change, which suggests that the previous values were likely even further from their documented behaviour than had been anticipated.

Still missing from this documentation is explicit notation of which direction on each axis is positive; I'm not sure how to establish that. Consequently, I'm also not sure which way around each axis represents a positive angle for each case. Establishing and documenting that would also be good.

This change is essentially untested in the real world. Unfortunately I don't have any printed markers nor a working webcam right now to test with, so this change is derived entirely from resources about how various coordinate systems, including this one, should behave (rather than how the system employed here actually does).

PeterJCLaw commented 1 year ago

env/lib/python3.10/site-packages/numpy/init.pyi:638: error: Positional-only parameters are only supported in Python 3.8 and greater

Oh yay, looks like CI has picked up that broken mypy version.

PeterJCLaw commented 1 year ago

env/lib/python3.10/site-packages/numpy/init.pyi:638: error: Positional-only parameters are only supported in Python 3.8 and greater

Oh yay, looks like CI has picked up that broken mypy version.

https://github.com/RealOrangeOne/zoloto/pull/306 fixes that.

PeterJCLaw commented 1 year ago

Have rebased on #312.

trickeydan commented 1 year ago

@PeterJCLaw What do we need to do to move this forward?

PeterJCLaw commented 1 year ago

@PeterJCLaw What do we need to do to move this forward?

~In local terms it needs rebasing/merging/something to fix the conflicts with master. I think I have half a branch which does that somewhere, but I'm not sure (don't wait for me to find that if you want to take this on).~ (edit: have done this)

More generally there needs to be a decision made about how happy we are to break competitor code. The changes here are technically breaking, even if they do fix what is arguably a bug in the code to make it match the SR docs.