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

Odometry topic contains duplicate messages #156

Closed
ll-nick opened this issue Jul 17, 2020 · 9 comments · Fixed by #163
Closed

Odometry topic contains duplicate messages #156

ll-nick opened this issue Jul 17, 2020 · 9 comments · Fixed by #163
Labels
bug Something isn't working

Comments

@ll-nick
Copy link

ll-nick commented Jul 17, 2020

We're currently testing our SLAM algorithm using the given ground truth odometry data. We noticed that the messages received via this topic (fsds/testing_only/odom) contain the same data over a few time steps. I've attached some data showing the issue here : GroundTruth_PoseVelocity_Data.zip. The data was generated by driving with constant throttle and a constant steering angle.

This is an issue, because the velocity does not change while the position data is also constant. But the vehicle position must of course change, if the velocity is != 0. So when computing the vehicle pose from the given velocities, the car will obviously move further compared to the ground truth position as you can see in this image (the black arrow is the ground truth pose and the red arrow is the pose computed from ground truth velocity data):
Issue_WrongVelocityData

I'm assuming that the ground speed sensor you're planning to implement (see this issue) is going to rely on the same data, so issue should be looked at.

@wahllca
Copy link

wahllca commented Jul 17, 2020

I also noticed the same issue in the /fsds/imu topic (Orientation).
I would assume that the Unreal simulation calculates the new information at a lower frequency than the publisher sends it out.
I can't imagine that the simulation is capable of simulating a vehicle with a frequency of 250Hz, considering the cost of computation.
It doesn't make sense to publish the same information multiple times, because there is no benefit of it.
Therefore, I would suggest to run the publisher at a lower frequency than the Simulation to ensure plausible values.
But I'm not sure if frequency is dependent on the device which runs the Unreal Sim.

@SijmenHuizenga SijmenHuizenga added the bug Something isn't working label Jul 18, 2020
@SijmenHuizenga
Copy link
Member

SijmenHuizenga commented Jul 18, 2020

Thanks for taking the time to file this issue!

So the problem is that if an odom message has a non-0 velocity, the next odom message should have a position that is different position the first message? Please correct me if I'm misinterpreting this.

This is a bug and should be solved 👍 (because it is a testing_only topic, solving this does not have the highest priority for the dev team. However PR's are more then welcome ❤️)

As @wahllca points out correctly, the simulation is not capable of simulating a vehicle with a frequency of 250Hz. In reality it is much lower, but I have to check at which rate it is. I expect it to be dependent on the device and framerate of the simulator.

@davidoort
Copy link
Member

If there isn't a straightforward way of capping the publishing frequency, or of somehow creating a sensor callback in the ROS wrapper that is triggered when the CarState changes, we could also cache the previously published groundtruth CarState for each relevant publisher (aka, prioproceptive sensor publishers) and check if it has changed. If it hasn't then we simply don't publish anything.

@SijmenHuizenga
Copy link
Member

Agreed. At the moment it there is no way to do an unreal-to-ros push system so let's implement the check in the wrapper

@ll-nick
Copy link
Author

ll-nick commented Jul 20, 2020

Thank you for your answers.
@SijmenHuizenga yes, that's what I meant. And it is true that this is on the testing_only topic. However, as I mentioned, if you are planning on using the same signal to simulate the ground truth sensor, this is very relevant. In this case, sending the signal out only if it changes could solve the problem, but the frequency needs to still be high enough to use it for localization.

@SijmenHuizenga
Copy link
Member

In this case, sending the signal out only if it changes could solve the problem, but the frequency needs to still be high enough to use it for localization.

Yes, the problem is that I don't know if we can update the sensordata more often then we do at this moment. Unreal engine runs at a certain tickrate at which all sensors are updated. The ros wrapper requests the data from unreal. So I think we are limited by the tickrate of unreal.

To improve data frequency, we need to improve tickrate. Maybe we can increase tickrate or maybe sub-stepping can help. I will get back to this when I understand it better.

@SijmenHuizenga
Copy link
Member

Yea... So only publishing messages if the the new message differs is a problem when the car is standstill. Because then it won't send a message at all.

@SijmenHuizenga
Copy link
Member

In #163 I added that zero-velocity messages should always be sent.

@SijmenHuizenga
Copy link
Member

Closing this issue because #163 fixed this specific bug and opening #166 to figure out what we are going to do with the low tickrate.

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 a pull request may close this issue.

4 participants