Skip to content
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

Merged
merged 2 commits into from
Jan 22, 2018
Merged

REP: 145 #95

merged 2 commits into from
Jan 22, 2018

Conversation

paulbovbel
Copy link
Contributor

Standard Conventions for IMU Sensor Drivers

  1. Pending Discussion on https://groups.google.com/forum/#!topic/ros-sig-drivers/Fb4cxdRqjlU
  2. ROS-Users Discussion: http://lists.ros.org/lurker/message/20150217.230638.6f94450a.en.html

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.
Copy link

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.

Copy link
Contributor Author

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.

@mintar
Copy link

mintar commented Feb 19, 2015

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 orientation published in the Imu msg and the tf transform from base_footprint --> imu_link[_ned]. I can't come up with a succinct formulation, but what I basically want to say is that no matter how the IMU is mounted on the robot, the base_footprint --> imu_link[_ned] transform has to be set up such that when the IMU message is transformed into base_footprint, the "z axis" you get from the rotation is "up" when the robot is stationary, no matter whether it uses NED or ENU.

This implies that the rotation between the imu_link that would be used by an ENU IMU to the imu_link_ned used by an NED IMU should be like this:

- Rotation: in Quaternion [0.707, 0.707, 0.000, 0.000]
            in RPY [3.142, 0.000, 1.571]

imu_links

I'm aware that base_footprint is is nothing that the IMU driver should know about, but I think this is still an important thing that has to be specified in order to ensure interoperability between modules, and it's what mirrors the assumptions already made by widely deployed packages such as robot_pose_ekf. My heuristic of when an implementation is correct would be that if you put the Imu message and the base_footprint --> imu_link[_ned] tf into robot_pose_ekf, the z axes of odom and base_footprint will be roughly aligned afterwards (unless the robot is really upside down).

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.
Copy link

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.

Copy link
Contributor Author

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.

Copy link

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.

Copy link
Contributor Author

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.

Copy link

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.

@paulbovbel
Copy link
Contributor Author

From the response on the SIG discussion, I gathered that that kind of guidance may be better suited for a tutorial than then REP.

@mintar
Copy link

mintar commented Feb 19, 2015

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 robot_pose_ekf don't work with all IMUs.

Perhaps it would really be cleaner to simply mandate the use of ENU everywhere.

@kmhallen
Copy link

Can we include the 'imu/is_calibrated' topic and 'imu/calibrate' service from the imu_drivers wiki? http://wiki.ros.org/imu_drivers

@paulbovbel
Copy link
Contributor Author

@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.

@paulbovbel
Copy link
Contributor Author

@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.

@PaulBouchier
Copy link

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>

@paulbovbel
Copy link
Contributor Author

@PaulBouchier +1, body frame is a little ambiguous

@rbirac
Copy link

rbirac commented Feb 25, 2015

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.”.
As far as I can see, both NED and ENU axis conventions are right handed. (they differ by just a 180 degree rotation about the X axis), so I assume you aren’t talking about NED to ENU conversion. If there is truly left handed data (with x pointed forward), then either the Y or Z axis might need to be reversed to achieve a right handed system in NED or ENU. It is also possible that a device might report the Y axis as vertical and Z as lateral which would also require driver conversion.
It might be better to specify exactly what axis system(s) is to be reported, as is done for the definition of the NED and ENU axes described earlier, and leave the appropriate changes up to the driver developer.

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.
It seems that the most likely transforms to IMU device data would be from NED to ENU and from base_link to a non-standard IMU mounting configuration (including both static and dynamic orientations). Both of these transforms could be done in the driver hence providing standard topics. If both NED and/or ENU convention data is necessary, perhaps separate messages could be provided or the message frame selected by a parameter.

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 :-)

@paulbovbel
Copy link
Contributor Author

@tfoote
Copy link
Member

tfoote commented Feb 27, 2015

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.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@paulbovbel paulbovbel closed this Jun 23, 2015
@tfoote
Copy link
Member

tfoote commented Jun 30, 2015

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 tfoote reopened this Jun 30, 2015
@paulbovbel
Copy link
Contributor Author

@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?

@tfoote
Copy link
Member

tfoote commented Jun 30, 2015 via email

@TobiSchluter
Copy link

TobiSchluter commented Mar 3, 2017

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 ;)

@TobiSchluter
Copy link

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

@mintar
Copy link

mintar commented Mar 6, 2017

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.

@paulbovbel
Copy link
Contributor Author

paulbovbel commented Mar 6, 2017

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.

@TobiSchluter
Copy link

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.

@paulbovbel
Copy link
Contributor Author

@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.

@TobiSchluter
Copy link

TobiSchluter commented Mar 8, 2017

@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.

@tfoote
Copy link
Member

tfoote commented Mar 11, 2017

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.

@paulbovbel
Copy link
Contributor Author

@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?

@mintar
Copy link

mintar commented Jan 22, 2018

+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!

@tfoote
Copy link
Member

tfoote commented Jan 22, 2018

@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.

@paulbovbel
Copy link
Contributor Author

paulbovbel commented Jan 22, 2018

@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.

@tfoote
Copy link
Member

tfoote commented Jan 22, 2018

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.

@tfoote tfoote merged commit 9d0a19c into ros-infrastructure:master Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants