8000 Fix Gnss noise calculation in simulation by pascalzauberzeug · Pull Request #279 · zauberzeug/rosys · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix Gnss noise calculation in simulation #279

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

Merged
merged 2 commits into from
Apr 23, 2025
Merged

Conversation

pascalzauberzeug
Copy link
Contributor

While setting up a test for a new gnss antenna configuration, I noticed that the location noise of the gnss simulation is wrong.
When using _lat_std_dev=1 and _lon_std_dev=0, I got this result which makes sense, because we have two cases:

  1. noise_lat is positive, which leads to both a positive direction and distance for polar, resulting in a spread on positive x
  2. noise_lat is negative, which leads to both negative values canceling each other out, also resulting in a positive spread

Screenshot 2025-04-23 at 05 49 16

After the fix, I recorded both _lat_std_dev and _lon_std_dev alternating between 0 and 1, which led to an expected cross:
Screenshot 2025-04-23 at 05 51 04

Setting both to 1 also shows the expected pattern:
Screenshot 2025-04-23 at 06 00 22

@pascalzauberzeug pascalzauberzeug added the bug Something isn't working label Apr 23, 2025
@pascalzauberzeug pascalzauberzeug added this to the 0.24.0 milestone Apr 23, 2025
@pascalzauberzeug pascalzauberzeug self-assigned this Apr 23, 2025
Copy link
Contributor
@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Oh! Clearly, the original line can't be right:

noise_point = geo_pose.point.polar(noise_lat, direction)

But do we really need to use .polar()? Can't we simply say

geo_pose = GeoPose.from_pose(self.wheels.pose)
noise_pose = GeoPose(lat=np.random.normal(geo_pose.lat, self._lat_std_dev),
                     lon=np.random.normal(geo_pose.lon, self._lon_std_dev),
                     heading=np.random.normal(geo_pose.heading, math.radians(self._heading_std_dev)))

By the way: First I thought converting self._heading_std_dev to radians is wrong. But according to the documentation it is in degrees, so the conversion is necessary. It is just a bit unconventional because we usually keep angles in radians. Anyway, it's probably not worth a breaking change.

@pascalzauberzeug
Copy link
Contributor Author
pascalzauberzeug commented Apr 23, 2025

@falkoschindler _lat_std_dev and _lon_std_dev are in meters that's why we can't use them directly. We also can't use GeoPoint.shift_by because that is relative to the orientation of the GeoReference.

I think we kept them in meters and degrees, because the Septentrio SimpleRTK3b returns them like that.

1. latitude and longitude (parts of a point)
2. magnitude and direction (arguments for polar)
3. point and heading (parts of a pose)
4. pose
Copy link
Contributor
@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Ok, I see. I forgot that lat/lon noise is in meters.

I reordered the lines a tiny bit to get this structure:

  1. latitude and longitude (parts of a point)
  2. magnitude and direction (arguments for polar)
  3. point and heading (parts of a pose)
  4. pose

Let's merge!

@falkoschindler falkoschindler merged commit 02d2c14 into main Apr 23, 2025
5 checks passed
@falkoschindl
6FFA
er falkoschindler deleted the fix_gnss_sim_noise branch April 23, 2025 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0