search

iris-ua / iris_lama_ros

LaMa on ROS
BSD 3-Clause "New" or "Revised" License
208 stars 59 forks source link

Let's merge master into noetic-devel #35

Closed cosmicog closed 3 years ago

cosmicog commented 3 years ago

I've tested the loc2d node, and about other nodes, there seems to be no bad change, actually a good changes made that will be good for Noetic too as we discussed earlier.

eupedrosa commented 3 years ago

Hi @cosmicog. I exposed the localization covariance and it is now being published. I would appreciate if you can test it with your data.

My objective is to do a new release in the upcoming days.

cosmicog commented 3 years ago

I tested and it works, CIs are failing because the iris_lama package is not updated there I think. I'm currently not using the covariance values but I've just compared with the amcl's output. In the beginning, amcl doesn't publish the initial covariances but iris_loc2d published the identity matrix. I'm not sure what difference it would make. Except the beginning it looks pretty much same.

eupedrosa commented 3 years ago

It publishes the identiy matrix because it has to publish something and all zeros is not a good idea. Usually, a high value (such 999999) can be used, but this usually mean that the robot can be anywhere, but that is not the case because it starts in a known pose. Using the identity was just "easy".

Note that the covariance here will underestimate the true error. This happens because the localization uses an optimizer that will always try to find a local minimum.

CIs are failing because the iris_lama package is not updated there I think.

This is correct. We can ignore the CI for now. Once the iris_lama package is released and a sync happens the error will go away.

cosmicog commented 3 years ago

I deleted frame_id line back, along with other lines while trying to fix the structure here https://github.com/iris-ua/iris_lama_ros/pull/35/commits/afcbbdc3801bc41857e9e5ed972d9b677554b0c7 And I didn't notice... :man_facepalming: