Add a GPS + IMU Fusion Part#1237
Conversation
|
Are the x and y GPS positions in degrees or in meters from the Greenwich Meridian and the Equator? Regards, |
I believe that the Fusion part outputs the position in UTM coordinates (same as the gps part as far as I know). So yes it give the distance from the equator in meters but for easting it outputs the number of meters from its specific zone's central meridian. Is this something we should describe in the fusion class? |
DocGarbanzo
left a comment
There was a problem hiding this comment.
Thank you @Phill310 - this is good PR that we would like to merge. I have two general feedback points:
- We should not have to call
pip install some_packageto make use of the BNO085 or other sensors. The relevant python packages should be added tosetup.cfgso everything works out of the box. - The IMU interfaces use scalars instead of vectors for acceleration, gyro, quaternions, etc. That is bad, but it is of course not your fault as such code was there before. It would be great if instead of making this even worse, we would clean this up as part of this PR as for the moment, no-one is really using IMU data yet. - Let me know if this exceeds your intended contribution and we can do a refactoring PR afterwards.
| cosy_cosp = 1 - 2 * (qj * qj + qk * qk) | ||
| return math.atan2(siny_cosp, cosy_cosp) | ||
|
|
||
| def run(self, gps_x, gps_y, ax, ay, az, gx, gy, gz, qi, qj, qk, qr): |
There was a problem hiding this comment.
Can you change the interface to pass vectors instead of single coordinates:
[gps_x, gpy_y] -> gps
[ax, ay, az] -> a or accel?
[gx, gy, gz] -> g or gyro?
[qi, qj, qk, qr] -> q or quarternion?
|
|
||
| - MPU9250 | ||
| pip install mpu9250-jmdev |
There was a problem hiding this comment.
These install commands are partially wrong (sudo...). can you add the python dependencies into setup.cfg so they get installed as part of the standard donkey install?
| self.gyro = { 'x' : 0., 'y' : 0., 'z' : 0. } | ||
| self.accel = {'x': 0., 'y': 0., 'z': 0.} | ||
| self.gyro = {'x': 0., 'y': 0., 'z': 0.} | ||
| self.mag = {'x': 0., 'y': 0., 'z': 0.} |
There was a problem hiding this comment.
Dictionaries are unnecessary here, please just use vectors (or lists, if you prefer python native over numpy. Both types would be fine).
| self.mag = { 'x' : ret[13], 'y' : ret[14], 'z' : ret[15] } | ||
| self.accel = {'x': ret[1] * GRAVITY, 'y': ret[2] * GRAVITY, 'z': ret[3] * GRAVITY} | ||
| self.gyro = {'x': ret[4], 'y': ret[5], 'z': ret[6]} | ||
| self.mag = {'x': ret[13], 'y': ret[14], 'z': ret[15]} |
There was a problem hiding this comment.
Same here, vectors are better than dicts.
|
|
||
|
|
||
| def run_threaded(self): | ||
| return self.accel['x'], self.accel['y'], self.accel['z'], self.gyro['x'], self.gyro['y'], self.gyro[ |
There was a problem hiding this comment.
It would also be much cleaner to return a tuple of two vectors (accel and gyro) and a float (temp), instead of unravelling the vectors.
| if cfg.SIM_RECORD_LOCATION: | ||
| inputs += ['pos/pos_x', 'pos/pos_y', 'pos/pos_z', 'pos/speed', 'pos/cte'] | ||
| types += ['float', 'float', 'float', 'float', 'float'] | ||
| types += ['float', 'float', 'float', 'float', 'float'] |
| types += ['float', 'float', 'float', 'float', 'float'] | ||
| types += ['float', 'float', 'float', 'float', 'float'] | ||
| if cfg.SIM_RECORD_GYROACCEL: | ||
| inputs += ['gyro/gyro_x', 'gyro/gyro_y', 'gyro/gyro_z', 'accel/accel_x', 'accel/accel_y', 'accel/accel_z'] |
| types += ['float', 'float', 'float', 'float', 'float', 'float'] | ||
| types += ['float', 'float', 'float', 'float', 'float', 'float'] | ||
| if cfg.SIM_RECORD_VELOCITY: | ||
| inputs += ['vel/vel_x', 'vel/vel_y', 'vel/vel_z'] |
| @@ -877,7 +873,7 @@ def add_camera(V, cfg, camera_type): | |||
| 'imu/acl_x', 'imu/acl_y', 'imu/acl_z', | |||
There was a problem hiding this comment.
If you don't want to touch this class, but want to work with vectors, then you can write a one-liner class that combines x, y, z values into a vector, like:
class Vectoriser:
def run(x, y, z):
return [x, y, z]| V.add(fusion, | ||
| inputs=[ | ||
| 'gps/utm/longitude', 'gps/utm/latitude', # From GPS | ||
| 'imu/acl_x', 'imu/acl_y', 'imu/acl_z', # From IMU |
There was a problem hiding this comment.
Also, use vectors here.
|
We are working on refactoring our code to use vectors instead of scalars, and we added the package install to |
Refactored IMU code and all its dependencies to use vectors, in the context of our fusion module
|
I've refactored the IMU code to use vectors and changed all the dependencies for the IMU to do the same, as well as verified that it works properly on path following. Is there anything else that needs fixing in this code? |
This is the addition mentioned in #1236.
This fusion part uses a 5 state Kalman filter to predict your position based on your velocity and acceleration data. The 5th variable is an acceleration bias to prevent drift when the car isn't moving. There is also a low pass EMA filter to clean out noise. Finally we added a friction factor to slowly bleed velocity.
We tested with a SparkFun NEO-F10N GPS which had a frequency of 10Hz. I believe the BNO08x IMU has a frequency of 50 Hz so in theory the fusion part can go up to 50 Hz. The main limiting factor is the drive loop frequency since the fusion part recalculates position once every drive loop.
This should support any GPS since the fusion part only takes the x and y position data from the gps.
Config Options added:
I hope this explains everything we did. If not please ask questions so I can expand on our thinking.
Should I also make a pr in the docs repo to add a section for the fusion part and update the IMU section?