search

yujinrobot / kobuki_desktop

Visualisation and simulation tools for Kobuki
http://www.ros.org/wiki/kobuki_desktop
37 stars 58 forks source link

Publish sensor state with bumper and cliff readings so we can integrate them on the costmaps #66

Closed corot closed 3 years ago

corot commented 3 years ago

I was going to suggest that the sensor state be stored be more rigorously maintained as state here, i.e. as a private member variable that is kept up to date with the proper state at any time instead of spinning up a temporary version each update step.

Then I looked at the rest of the class here. There are private variables that look like state, but are just temporary and would have been better not enumerated as private member variables. Member methods to update<Foo> also look like they're updating some proper notion of internal state, but actually just computation engines for publishing.

So, reforming this class to do it right is a larger effort and out of scope for this PR.

As it is, LGTM. At least it will do the job.

yep, limited scope: just make bumpers and cliff sensors appear in the costmap no intention to rewrite the plugin, but unerring comments are always appreciated!