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.
I think we're talking this frame right?
I don't see the current text as particular confusing if you know what the "aeronautical frame" is. You don't need to know that it is right handled, or that there is an order.
I think @olliw42 suggestion is technically correct, but it isn't as easy to understand as a reader - you have to unpack what is meant by intrinsic etc.
How about we just get rid of the "right hand rule" so the order is irrelevent, and add extra axis info to make it more intuitive what we mean by the aeronautical frame?
@dakejahl @peterbarker Do you have an opinion?
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.
the problem is that this does not specify the order of the rotations/angles, it just associates them to some axes
so it could be Rz(yaw)Ry(pitch)Rx(roll) or Rz(yaw)Rx(roll)Ry(pitch) or Rx(roll)Ry(pitch)Rz(yaw) or ...
so, both the assignment of anlges to axes AND the order of the sequence matter, but the assignment to the axes does NOT imply the order of the sequence of rotations! (and it also matters in which "direction" the transformation shall be applied). That's just the mathematical situation.
so, as sad as it is, your suggestion does nothing as regards clarifying the order of angles
One can try to be "clearer" in a sense some may think is clearer, but when someone else will come by and stumple across another inclarity because the mathematical details are not fully speficied.
one really would have hoped that specifing "aeronautical" should have been fully sufficient. Probably it could be better to just strip of all except "aeronautical" and have a link to an authorative reference.
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.
@peterbarker I don't want to have this discussion - I don't understand (or care to understand) the implications of the change, or why the order of rotations matters.
Can you work with @olliw42 and @arikrupnik to come up with something mutally satisfying and accurate? If not then I will reject this PR as incorrect and not worth my time to resolve.
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.
@hamishwillee like you're saying, it's not important why the order of rotations matters, only that it does. Right now, the description lists axes in a different order than the order in which the rotation applies them. Anything that fixes that order is an improvement. Anything that includes "ZYX" or "3-2-1" would help. "Intrinsic" would be a welcome addition.
@olliw42 If you find a stable, freely available reference that is accessible to people who need an explanation for "aeronautical frame" and say "the angles have the same meaning as in this text," that would definitively solve the problem. I'm sure it's been a long time since you had to struggle with this, but have mercy on us who have not taken aerospace as undergrads. Everyone I know makes the same assumption coming in, first that the angles are independent, then when they realize the order matters, they assume it's roll-pitch-yaw, or XYZ. Everyone. And their math produces nonsense results, and they question everything from
<math.h>
to MAVLink documentation. Have mercy and let them narrow their search to<math.h>
.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 think this change is a good one. If we're going to list a set of axes - even if they're not in the same "units" as OlliW points out - they should be in the order in which they're applied. It would have saved one person some pain, and I can't see it causing anyone else pain.
FWIW, I hadn't heard of the term "aeronautical frame". We tend to use "Tait-Bryan" or "3-2-1" in ArduPilot.
I think "intrinsic" I think should address @olliw42 's concern?
I think this should go in.
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 think you don't yet fully comprehended the difference between angles and axis relation ship and sequence of angles.
frankly, "(right-handed, Z-down, Y-right, X-front, ZYX, intrinsic)" is as cryptic as it possibly can get ... it's a mystery to me how this possibly can help anyone who doesn't already know what these cryptic symbols exactly mean.
anyway, it's just my one cent, if you feel it's an improvement, I'm happy to hear that. :)
EDIT: if you want to go with cryptic symbols, I think 3-2-1 would be much better than ZYX
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.
@olliw42 one way this can help is by telling users what arguments to pass into third-party libraries. There are many libraries that work with Euler/Tait/Bryan angles. They need to caller to pass in rotation order and whether the rotations are intrinsic or extrinsic. If MAVLink documentation tells the user which of the 24 possible combinations are correct for the data in this message, that user can pass in the correct arguments into a math library even if the user doesn't understand the intricacies of the nomenclature.