-
Notifications
You must be signed in to change notification settings - Fork 137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
REP: 145 #95
REP: 145 #95
Conversation
rep-0145.rst
Outdated
Transformation | ||
-------------- | ||
|
||
Applying a transformation to IMU data requires applying an identical rotation to both the body and the world frames. After a transformation from one body frame to another, the output data represents the output of a simulated IMU having the new body and world frames. |
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.
This is wrong for the accelerations if the angular velocity and translation vectors are not zero.
An ideal IMU in the new frame b would output the acceleration vector
b.linear_acceleration
= R_ab * (a.linear_acceleration + (a.angular_velocity x linear_velocity_of_b_given_in_a))
= R_ab * (a.linear_acceleration + (a.angular_velocity x (a.angular_velocity x T_ab)))
compared to an IMU in frame a, where T_ab
and R_ab
are the translational and rotational parts of the transformation and x
is the vector cross product. If the transformation is not static, this would require additional correction terms for angular velocity and linear acceleration. For most slow-moving ground robots these inertial terms are negligible, but for aerial robots or when transforming accelerations between frames of fast moving robotic arms they could become very important.
Obviously, the transformation introduces couplings between the bias of the angular velocity vector in frame a and the acceleration error in frame b and is therefore of limited use for uncalibrated/unfiltered IMU data.
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.
Thanks, will test and add to ros/geometry2#78. Considering taking the whole transformation section out, as it seems out of scope for the REP.
ENU vs NED Sorry to bring this up again, but I still believe this REP should include some clarification on how to properly use NED or ENU. It's a tricky question, because it depends on both the This implies that the rotation between the
I'm aware that If we don't specify this, I'm afraid we'll end up with two different sets of ROS packages that make two different assumptions of how the orientation from an IMU message is to be interpreted. |
|
||
* If the device does not have an absolute yaw reference (magnetometer), the world frame will only be aligned in roll and pitch (due to gravity), while the orientation around the z axis can be arbitrary. | ||
|
||
* If the device does not have an absolute gravity reference (accelerometer), the world frame is not aligned to any external reference and aligned entirely to the power-on orientation of the sensor. |
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'm having second thoughts about this bullet point, and now I think it should be removed (although I know I put it in there). To recap why I put it in: On our old robots, we only had a single gyro that measured the yaw axis, so we put only that into the orientation, and it worked with robot_pose_ekf
. But what we have actually been representing there is not an arbitrary orientation; instead, we've been faking a gravity-referenced orientation.
@paulbovbel : Please wait a bit before making this change in case somebody else has a different 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.
If I understand you correctly, were you integrating the gyro output, relative to the power on position of the sensor? If so, then this stanza still makes sense, even for that context.
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.
We were integrating the gyro output to get the yaw of the orientation, but that case is already covered by the previous bullet point. We left roll and pitch at 0, which means that we didn't output an orientation that reflects an arbitrary power-on position; instead, this "fakes" a gravity-referenced orientation (under the assumption that our robot is always upright). This was always a hack, so I think we don't need to mention it in this REP.
Publishing an orientation relative to an arbitrary and unknown power-on orientation is utterly useless. This is why I vote for removing this bullet point.
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'm ambivalent about this. I think my thinking here was to describe what if anything should be written to orientation
if the IMU has neither an accelerometer nor a magnetometer.
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.
Ok, I'm fine with it. We can leave it as it is.
From the response on the SIG discussion, I gathered that that kind of guidance may be better suited for a tutorial than then REP. |
I don't want detailed tutorial-style "how to set up your TF tree" instructions here. What I'm saying is that the REP should make absolutely clear that when transformed into any frame that is expected to have "z up" according to ROS conventions, the orientation from the IMU message should also point up (unless the device is upside down, of course). The current wording regarding NED and ENU seems to imply that there is an ambiguity how the orientation is to be interpreted. This would mean that standard modules like Perhaps it would really be cleaner to simply mandate the use of ENU everywhere. |
Can we include the 'imu/is_calibrated' topic and 'imu/calibrate' service from the imu_drivers wiki? http://wiki.ros.org/imu_drivers |
@kmhallen, I would consider that to be a little too device-specific. A service interface for this isn't the greatest since calibration may take quite some time, so device may choose to expose an action as well. And some devices don't offer a straightforward 'calibration' routine. I would also expect that calibration would either occur offline (during integration), or on driver startup. Information regarding device status would probably be better suited for /diagnostics output as well. |
@mintar, I don't think there's ambiguity, there is a straightforward explanation of what ENU and NED are, and plenty of references to REP 103. I would consider https://github.com/ros-infrastructure/rep/pull/95/files#diff-03867c82717cfc2167b91e1cdefe2306R67 a good compromise. |
Hi Paul. Very nice REP - thank you for writing it, and the discussion in the thread was informative too. I propose a small change in terminology, drawn from where I work, that I think will improve it by removing ambiguity. Use the term "sensor frame" instead of "device body frame", or in some places, just "body frame". "Body frame" is less specific, and easily confused with the robot body, which is also very relevant but provides the context (base_link) within which the sensor frame exists. <--soapbox> It's too bad REP-105 stopped at three frames and didn't define a sensor frame, because it's a concept that's broadly applicable, not just for IMUs, where the orientation of the sensor frame can be different based on sensor mounting, but for cameras, especially those on pan-tilt units, where the sensor frame can change dynamically relative to the robot body (base_link). The sensor frame is an important reason why tf exists. <--/soapbox> |
@PaulBouchier +1, body frame is a little ambiguous |
Well, I finally managed to get my new (perhaps soon to be obsolete) um7 driver released. A real challenge for a newbie. So, now I can return to the proposed spec. The data reporting section says “all frames must be right handed. If any data is left handed, it should be converted by… inverting the y axis.”. I assume that the driver should also convert to SI units where necessary; and that rotations should be converted to right handed if necessary? I still have problems with the general rule that “Otherwise, all data should be published by the driver as it is reported by the device”. This places the burden of transforming the data to (usually) the ros standard on the users for every application created. It also means that reviewing test data will be much less intuitive since the IMU topic data will not be consistent with other (transformed) data. If we go with the “user does all the transforms” approach, I suspect that I will add a new node just to convert the IMU data to the ENU REP-103 standard. Just seems a little inefficient. OR, maybe I’ll become comfortable enough using transforms to agree with you guys :-) |
For reference, if you're not on the ros-sig-drivers mailing list, I've started a new discussion there relevant to this: https://groups.google.com/forum/?hl=en#!topic/ros-sig-drivers/2NvgNBOjcFQ |
rep-0145.rst
Outdated
|
||
* Magnetometers | ||
|
||
- The magnetometers report magnetic field strength in the sensor frame of the device. This data is output from the driver as a 3D vector, with the components representing magnetic field strength in each direction. This data must be in Tesla. |
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.
Possibly clarify that the maximum magnetic field reading is when the axis is pointing to North and minimum reading when point South?
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.
+1
I'd like to keep this open and roll in the changes from the drivers sig. https://groups.google.com/forum/?hl=en#!topic/ros-sig-drivers/2NvgNBOjcFQ |
@tfoote, I was cleaning out old pulls, but if there's still interest I can adapt the original REP proposal. The tooling is what I'm unsure about getting done in the near future. What comes first, the REP or the tooling? |
I think we can define the REP and leave it in draft status until we write
the conversion tools. There may be some complications when writing the
conversion tools, but I don't forsee major design oversights in specifying
the REP first.
|
Any news on this? I've been looking for this kind of information recently in the context of developing a ROS driver for our LPMS IMUs. A driver exists at https://github.com/larics/lpms_imu and it works nicely, but it doesn't follow REP-145 to the letter. Before spending time on this, I would like to know the timeline for this to become official. "Waiting for conversion tools" sounds to me like "ain't gonna happen (without pushing)" from sad experience ;) |
ps G van der Hoorn pointed me here when I asked about this on ROS Answers, see the thread here: http://answers.ros.org/question/255867/sensor_msgimu-format-specification/?comment=256072 |
So, what's blocking the release of this REP? Also, it would be really nice to have @tfoote and @wjwwood 's proposal for the new AHRS.msg and InertialMeasurements.msg be made official and integrated here as preferred alternatives to the current Imu.msg. It's been almost two years since that proposal, and the old Imu.msg has even been copied to ROS2 already. If we don't do anything we'll be stuck with the current situation for another 10 years. |
My motivation for this PR lost steam as I haven't worked hands-on with IMUs (or physical robots) for some time. I'll have the opportunity to be touch some sensors in the next month or so, and I can pick the PR up again since there's interest. Alternatively, if anyone else has the motivation to jump in and make the sig's proposed changes, I would greatly welcome it. |
Even if the imu message is clearly deficient in that it mixes up orientations (defined in some --perhaps drifting-- global frame) and the actual imu measurements (defined in some body frame), having a definition to work against still beats all the guesswork and searching through mailing list discussions and GitHub threads that is necessary otherwise. I don't know the sociology of ROS well enough to understand whether standardizing the IMU message along this REP will lead to accelerating or decelerating the development of its replacement itself. On the other hand, standardizing the imu_msg will certainly make it easier to harmonize the assumptions and expectations of existing code, which will in turn make it easier also for non-experts to port existing code to whatever new convention is established in the future. It also benefits the opposite direction: once the new convention is established, it documents how to interface existing code from new code. |
@TobiSchluter, I think we're sort of on one page there, because defining the current Imu standard is what drove this PR for me. At the time, I was working on aligning and standardizing several non-conforming robot implementations. |
@paulbovbel that was my impression as well, I just wanted to give some outside input. From how I see it motivation to finish this REP is a bit low, as everybody involved understands that it is deficient because it tries to nail down something pre-existing that is vaguely defined, and that it should be replaced by something that is thought through from the outset. OTOH I think we would also agree that it's useful to have something that documents the (deficient) status quo. So maybe it's sensible to make this a REP, but to be clear in it that there are things that it cannot address, and that interested parties should support the development of its replacement. Stepping a bit back, to give you my perspective (only read if you're bored): I'm working at an IMU vendor, and I'm fairly new to ROS. I have used it in internal projects, but so far purely as a message passing library with no deeper insight into the system than that. I was trying to find out what ROS wants from our sensors, and of course I was happy when I found sensor_msg/imu. The message type is fairly easy to find, being a standard message and all. I was less happy when I was trying to find out what to put into them and couldn't find any documentation (as we all agree, the comment in the source doesn't cut it, and as a neophyte I didn't know that ROS documentation is spread out over GitHub ;) ;) A google search led me to a 3rd party driver for our IMUs, and when I asked the authors what they based the conventions they used on, they pointed me to a loooong mailing list thread archived on google. IOW they also weren't aware of this discussion, in spite of them being fairly well-versed with ROS. So, the status quo is not good, it prevents people from contributing to ROS, and it prevents people from using the work of others, as it's not clear what that work tries to do. |
Thanks for the follow ups. It sounds like merging this in draft format would be good for visibility. It would be great to pick this up again. I just opened a PR based on the drivers thread: https://groups.google.com/forum/#!topic/ros-sig-drivers/2NvgNBOjcFQ here: ros/common_msgs#101 There's still a few things todo but I think they're relatively well defined if someone wants to pick them up. |
@tfoote I'd like to pick this PR back up. Should I iterate on the comments/review and then we can proceed with the existing message format in draft state? Or do you want to push ahead with the new message definitions? |
+1 for picking this up again, @paulbovbel ! I've been referring people to this draft PR for years, since it's the best reference on IMU messages available. Would be good to iterate on the comments and then merge it! |
@paulbovbel With the proposed new message could you integrate that here and reviewing the discussions we just need to make sure we also provide the conversion methods and then we can put it all together. Is there someone willing to volunteer to do that? Otherwise I think we should merge this as is and do a 2nd iteration as it's clearly been a while and having this merged would be better if there might be a long implementation lag for the new messages. |
@tfoote, I would prefer to merge as is in 'Draft' form, then iterate on the message definitions and conversion tooling, if that's acceptable to you. |
Sounds good. I'll merge this as it is for a draft and we can start another round of reviews with the message definitions and conversion tools. |
Standard Conventions for IMU Sensor Drivers