search

Field-Robotics-Lab / nps_uw_multibeam_sonar

Multibeam sonar plugin with NVIDIA Cuda library
Apache License 2.0
35 stars 20 forks source link

fix false artifact with max_distance cutoff #23

Closed woensug-choi closed 2 years ago

woensug-choi commented 2 years ago

Fix for the https://github.com/Field-Robotics-Lab/nps_uw_multibeam_sonar/issues/22

The max_distance tag defined at sonar SDF cut-off the max range it is going to calculate by limiting the time domain range. This fix not only limits the domain range but also cut-off the data calculated if the cloud point is out of the max_distance range.

A newly added simple if condition does this https://github.com/Field-Robotics-Lab/nps_uw_multibeam_sonar/blob/1706d633c425f9ea791035fddc41632965564045/src/sonar_calculation_cuda.cu#L231-L233

Note that the cloud point is generated from the depth_camera dataset defined at clip. In this case, up to 10 meters. https://github.com/Field-Robotics-Lab/nps_uw_multibeam_sonar/blob/00a5ab7f88a6453be51569b528adb6d3d683d907/models/oculus_m1200d_nps_multibeam/model.sdf#L46-L49 The sonar calculation has this max_distance tag that limits the calculation range on top of the clip to make the refresh rate higher. In this case, up to 2meters. https://github.com/Field-Robotics-Lab/nps_uw_multibeam_sonar/blob/00a5ab7f88a6453be51569b528adb6d3d683d907/models/oculus_m1200d_nps_multibeam/model.sdf#L64

image

woensug-choi commented 2 years ago

@amarburg, if you are available, could you check this PR works for you to fix the false artifacts?

lauralindzey commented 2 years ago

Hi @woensug-choi -- sorry, the last few days I've been working on my laptop which can't run the simulator.

I just tried with your branch, and it seems to have fixed the issue. Thanks!!!

(However, I'm not running Aaron's simplified test case, so it would be worth double-checking that this fixed it for him as well.)

lauralindzey commented 2 years ago

@woensug-choi -- I see that you requested a review from me ... but I'm not familiar enough with the internals of your simulation to be comfortable commenting on the code itself.

woensug-choi commented 2 years ago

@lauralindzey No worries! As long as it fixes your problem for false artifacts. I agree on double-checking with Aaron's work too. After that I will merge this :D

amarburg commented 2 years ago

I will definitely be testing the new code. Given field trials it will take a few days…

amarburg commented 2 years ago

I was able to test this code last week and this seems to have resolved the artifacts issue...