simondlevy / BreezySLAM

Simple, efficient, open-source package for Simultaneous Localization and Mapping
GNU Lesser General Public License v3.0
758 stars 251 forks source link

Odometry doesn't improve SLAM. #19

Closed epinal closed 6 years ago

epinal commented 6 years ago

I've been using breezySLAM for over a year and I've always had the issue that when the robot is moving between long hallways the SLAM gets messed up, the robot position doesn't change but the map moves instead. I suppose that since the laser scans (the slam image) are almost the same the algorithm doesn't know that the robot is moving. So I thought that this will improve using the odometry data. Based on your WheeledRobot and the C++ source code I implemented a class which I already tested and works like a charm, meaning that if I move forward 10 feet I get dxy_millimeters = 3040 and if I rotate to the right 90 degrees I get dtheta_degrees = 90 to 94 so I know I'm getting the right dxy_millimeters, dtheta_degrees, dt_seconds values. Im calling RMHC_SLAM.update(scan_mm, velocities) and Im getting the same messed up SLAM like if it wasn't taking into account that the robot is moving as shown in the velocity. Let me know what can I do.

simondlevy commented 6 years ago

I can't follow what you're doing here. You don't say anything about the kind of vehicle you're using, what Lidar you're using, etc. Also, why did you look at the C++ code, instead of just sub-classing the existing Python WheeledVehicle class?

epinal commented 6 years ago

Thanks for closing the issue without giving me the chance to answer your questions. So helpful... I'll figure it out.

simondlevy commented 6 years ago

Sorry, Ernesto -- I didn't mean to close the issue; I must've accidentally hit the wrong button!

On Fri, Jan 5, 2018 at 8:40 PM, Ernesto notifications@github.com wrote:

Thanks for closing the issue without giving me the chance to answer your questions. So helpful... I'll figure it out.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/simondlevy/BreezySLAM/issues/19#issuecomment-355713263, or mute the thread https://github.com/notifications/unsubscribe-auth/AGc_yYfPdZbWnuhCzOoYtNeFuqzmG0QZks5tHs8BgaJpZM4RVDqy .

-- Simon D. Levy Professor, Computer Science Department Chair, Hillel Advisory Board

407 Parmly Hall Washington & Lee University Lexington, VA 24450

540-458-8419 (voice) 540-458-8255 (fax)

epinal commented 6 years ago

LOL, Ok no problem. Im using a SweepLidar http://scanse.io/ on a SuperDroidLT2 https://www.superdroidrobots.com/shop/item.aspx/lt2-tracked-atr-robot-platform/1513/ The lidar and the encoders are working as expected. I've been able to navigate buildings using the slam map and position. I debugged the code and slam.update(scan, velocities) is being called with the right data and Im getting the map but without any improvement. What do you think that I can check to make sure the velocities are being used by the SLAM algorithm?

simondlevy commented 6 years ago

Your best bet is probably to look at the C code internals, specifically, the functions scan_update and scan_update_xy. If you can put some debugging printf() statements in there, you may be able to see what's going on.

epinal commented 6 years ago

Ok. I’ll take a look on Monday and I’ll let you know my findings. Have a good weekend. Thanks.

epinal commented 6 years ago

Hi Simon, I took a look into the C code and it seems to be fine. I got a question here CoreSlam update

simondlevy commented 6 years ago

I wrote this code in a few years ago, so I can't be very confident in answering :^( But I believe that the #Update velocities section is how new odometry evidence gets introduced into the algorithm, AFTER the scan has been computed based on the existing evidence.

Specifically, the dt seconds in velocities[2] (time elapsed since previous reading) is turned into a frequency (velocity_factor), which then converts the linear (xy) and angular (theta) differences into rates (mm/sec, degrees/sec) Then the new velocity evidence is available to the point-cloud computation. Something like:

  1. Update scan based on current odometry.
  2. Update odometry.
  3. Update point-cloud based on scan from (1) and updated odometry from (2)
  4. Goto 1.

I'm pretty sure I based this on the approach in the original article, but again, it's been a long time!

epinal commented 6 years ago

Ok, So following your logic:

We should change this to: self._updateMapAndPointcloud(self.velocities, should_update_map) Am I right?

simondlevy commented 6 years ago

Yeah, I see your point: what I've implemented in the code doesn't follow the logic I just laid out.

Looking at the C++ code, I see that it's consistent with the Python code, but not with the logic. So there must've been some reason why I wrote the code that way, but at this point I can't even recall. I just spent some time looking at the core C code as well, and still can't make sense of why I used the two different velocity computations as I did.

Unfortunately our winter term has just started, and I'm buried in teaching and other projects. Rather than confusing things further, I'll suggest you try making that change and seeing whether it improves things. I wish I could be more helpful ....

epinal commented 6 years ago

LOL, I agree, I checked the C and C++ code too and I couldn't figure out why you did it that way that's why I asked. I'll make the change and test again. I know you are busy so thanks for helping me.

epinal commented 6 years ago

Hi Simon, It did improve changing this to: self._updateMapAndPointcloud(self.velocities, should_update_map) You can close the issue. Thanks for your help.

simondlevy commented 6 years ago

Cool, thanks, I've changed it as you suggest.