Make the Spherical coordinates useful #302
Conversation
|
I wasn't expecting the reference positions for each of the 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). |
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.
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.
PeterJCLaw
force-pushed
the
fix-spherical-coordinates
branch
from
8a6efea to
8047c67
Compare
Oh yay, looks like CI has picked up that broken mypy version. |
#306 fixes that. |
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.
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.
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.
PeterJCLaw
force-pushed
the
fix-spherical-coordinates
branch
from
df2527b to
f4a337a
Compare
|
Have rebased on #312. |
|
|
||
| Zero values for April Tags markers have the marker reference point at | ||
| the top left. | ||
| """ | ||
| return self.yaw | ||
|
|
||
| @property |
There was a problem hiding this comment.
These don't really make sense with the current coordinate system.
There was a problem hiding this comment.
Could you elaborate on what you mean? Both in terms of which version you're referring to as "current" and which of the properties you're referring to here.
There was a problem hiding this comment.
Yaw, pitch and roll do not line up with their standard definition https://en.wikipedia.org/wiki/Aircraft_principal_axes?wprov=sfla1
For instance yaw is a rotation about the vertical axis which is not the case here.
There was a problem hiding this comment.
I would probably say that the correct fix here is to fix the incorrect convention, as we discussed in the kit team meeting last night.
There was a problem hiding this comment.
Unfortunately changing the mappings of which axes are which has the potential to be severely breaking to competitors code.
Even the changes which are in this branch as it stands are technically breaking, though relatively unlikely to affect most cases (and have the advantage of making this logic match what's actually in the SR docs).
|
@PeterJCLaw What do we need to do to move this forward? |
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. |
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.
This changes the Spherical coordinates to (almost) following the ISO convention. Consequently there are a number of changes to the values, notably: - the reference positions for each angle changes - the interpretation of the angles changes See inline documentation for the interpretation of the values now. Fixes RealOrangeOne#300
PeterJCLaw
force-pushed
the
fix-spherical-coordinates
branch
from
f4a337a to
98dd517
Compare
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 #300
Builds on #312