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

Review InertialUnit with respect to REP 145 #5197

Closed
omichel opened this issue Sep 9, 2022 · 1 comment
Closed

Review InertialUnit with respect to REP 145 #5197

omichel opened this issue Sep 9, 2022 · 1 comment
Assignees
Labels
enhancement Implementation of a minor feature
Milestone

Comments

@omichel
Copy link
Member

omichel commented Sep 9, 2022

We should review our InertialUnit implementation and interface with respect to REP-0145, Conventions for IMU Sensor Drivers and corresponding PR.

@omichel omichel added the enhancement Implementation of a minor feature label Sep 9, 2022
@omichel omichel added this to the R2023a milestone Sep 9, 2022
@ygoumaz
Copy link
Contributor

ygoumaz commented Sep 12, 2022

After reading the REP in question and putting it in perspective with the sensors available in Webots here is what I think is the best to do in our case:

  • Implementing a PROTO representing a real IMU product combining several key sensors would allow the user to access a generic IMU that directly combines the essential sensors. This has already been started in PR New ROSbot PROTO with associated devices PROTOs #5168, with the implementation of the MPU-9250. However, the product datasheet does not mention an overlay for attitude output (roll, pitch, yaw). So I suggest not to add an InertialUnit in this PROTO. MPU-9250 PROTO would contain only accelerometer, magnetometer and gyroscope. In addition, the REP clearly states that the attitude output is optional on the IMUs.
  • The documentation of the InertialUnit must be updated to clearly indicate that it is a device allowing to obtain the attitude of the robot. It must also be clearly specified that this data is the ground truth and not the result of the combination of several sensors. It makes life easier for the user who does not want to use a library or a sensor fusion algorithm to combine the data from an IMU and calculate the attitude.
    EDIT: this is achieved in Improve InertialUnit documentation #5234.
  • The sample world named inertial_unit.wbt can be left as it is. It might be interesting to add a new sample world which could be titled imu.wbt which would use the MPU-9250 to compute the attitude (by merging its sensor data) and indicate the difference with the exact values provided by the InertialUnit.
    I feel that separating the example world for the InertialUnit with the one that highlights the fusion of sensors makes sense. But this is an open question and it is always possible to simply add the attitude calculation part to the existing world.
    EDIT: this is achieved in Add new IMU sample world for attitude computation #5256.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Implementation of a minor feature
Development

No branches or pull requests

2 participants