Enable arbitrary euler angle for Mag rotation #20247
Conversation
junwoo091400
force-pushed
the
pr_mag_arbitruary_rotation
branch
2 times, most recently
from
a2dcb66
to
0dc47a9
Compare
|
Review addressed 😉 |
|
Any extra comments about this? |
|
FYI, betaflight has it as well |
|
Related: betaflight/betaflight@980df15 |
|
Maybe having a QGC UI would unblock this from being merged? Something like this: https://www.youtube.com/watch?v=mENP56CmeVw. But it requires OpenGL & would be quite a bit of an effort! Or, alternatively, we can also just update the doc & with mention a pre-existing online tool: https://danceswithcode.net/engineeringnotes/rotations_in_3d/demo3D/rotations_in_3d_tool.html |
|
What's blocking this PR? The users probably don't need a tool to represent the orientations if they're already supposed to be able to select the orientation from a list. |
Nothing is really blocking it. Got it, then I think we can get it in 🤔, with doc changes. I could maybe add a unit test for this? |
junwoo091400
force-pushed
the
pr_mag_arbitruary_rotation
branch
from
ad74858
to
7603600
Compare
|
Rebased on |
- Move CUSTOM rotation enum out of the normal enum range - Handle Sensor Level Adjustment as well
junwoo091400
force-pushed
the
pr_mag_arbitruary_rotation
branch
from
7603600
to
4523028
Compare
|
FMI is this a real problem? I mean how often does someone actually feel limited by the inability to set a 37 degree angle on the magnetometer? W.r.t. MAVLink if we were to publish or set these other than through parameters then this would take rather more than the 8 bit number used to send the current info; you're talking 3 floats - that's 12 times more information - with all the corresponding changes to the messages etc. Is it worth it/has anyone thought this through? I'm not opposed - but if you put a PR in to MAVLink (and hence to QGC) to support this more deeply in the UI it needs more justification than the statement that it's a problem. |
Here I showcase one example, my X500 V2. This model is *supposed to be built by mounting the GPS on it's dedicated mount on the panel (flat horizontal plate sticking out), but due to the concern of having too much vibration induced on that panel, we mounted the GPS on the rigid frame. And since I already had taped the GPS on the mount before attaching it to the frame, and the yaw 23 degrees or so (that's all the adjustment we needed) wasn't supported in mag rotation, I had to un-tape the whole GPS and tape it again, which was quite ironic since taping it back on isn't even more accurate (It will never be really ROTATION_NONE). This wasn't however the only example we had, but other products even needed a hardware mount to orient the mag in a *supported rotation enum, which isn't scalable for a mass produced vehicles, so that's why this project was initiated. Hope this answers your question! Image1: How the GPS was re-taped to be oriented forward (ROTATION_NONE) Image2: How the GPS was mounted (it's a non-standard mounting method, hence it resulted in yaw not being 0 degrees) |
I don't think this needs to be related to MAVLink, this is just about supporting the custom rotation in PX4 🤔. Or am I missing something? |
|
I don't think anything is preventing this from being merged. And we discussed in the dev call as well, where general consensus that this is a good addition to solve a pain of having to mount everything as specified via enum (or implementing custom mavlink enum for custom rotation): https://discuss.px4.io/t/px4-dev-call-november-09-2022/29606#p3-arbitrary-mag-rotation-junwoo-12 @hamishwillee I hope this is clear for you! |
|
Additional discussion from dev call:
|
@dagar @bresch - is this a blocker for the pr? if cal and sens are used as airframe vs board, probably @junwoo091400 it would make sense to change, but i defer to @bresch and @dagar |
This is external compass, so it's airframe specifc and makes sense to have the Maybe @bresch you could confirm whether |
|
Any ETA on this? |
junwoo091400
added
the
Maintainers call
|
Note: I need to rebase to take into consideration of #20723 |
|
@junwoo091400 will you have time to wrap this up this week? if not - we may be able to put someone else on it. what else is remaining beside rebase? |
Just the renaming of the parameters prefix, I can get it settled certainly by 22nd of June (end of Exams), would that be too late? |
| min: -1 | ||
| max: 40 | ||
| max: 100 |
There was a problem hiding this comment.
This is probably enough spacing, but just to be safe maybe we should set this to a much larger magic number.
How about either UINT8_MAX (255) or even INT32_MAX? Currently the underlying parameter is actually an int32_t, but could be optimized in the future.
There was a problem hiding this comment.
I think we should eventually only use the "custom rotation", so this parameter will be completely removed
|
replaced by #22000 |
Describe problem solved by this pull request
Previously, the rotations for the magnetometer was confined to MAVLink defined MAV_SENSOR_ORIENTATION, which doesn't allow setting non 45-degree multiple sensor orientation.
This was a limiting factor for drones with magnetometer that has Magnetometer installed in Euler angles of something like 37 degrees, 58 degrees, etc.
Describe your solution
Enables setting arbitrary euler angle for magnetometer orientation, which would allow users to set angles like 37 degrees in yaw, which is useful for Alta X for example
Added parameters
Parameters that defines the Euler angle for the orientation the Magnetometer is at were added:
Describe possible alternatives
Quaternion?
Test data / coverage
TODO
TODO
Discussion
Currently, due to bringing in the MAV_SENSOR_ROTATION_CUSTOM enum, which breakes the sequence of numbers, and bumps up the value to 100, the
ROTATION_MAXcan no longer be used to calculate the number of rotations we support in PX4.You can read more about enum members number count issue here.
Would there be a good solution for this?