LimHyungTae / patchwork

SOTA fast and robust ground segmentation using 3D point cloud (accepted in RA-L'21 w/ IROS'21)
MIT License
481 stars 75 forks source link

fixed lots of warnings with gcc 9.4.0 #31

Closed frankhoeller closed 6 months ago

frankhoeller commented 8 months ago

Hi! I fixed all warning shown on my system. Next pull request will be the changes I had to do to make it work with out robot. Cheers Frank

frankhoeller commented 8 months ago

Hi!

Apparently GitHub already added my second commit to the pull request. In this commit I made parameter loading compatible with your and our parameter files. I also added a parameter for the data output frame. And I've set the online ros publishers not to be latched and they only publish when there are subscribed nodes. I'll explain in detail now, but I hope you like it and everything works. If it does, you don't need to change anything in your launch files. I tested it with kitti sequence 00 and 04.

Cheers Frank

Details:

output frame: We already have a frame named "map" so patchwork cannot use that name. "map" is normally a frame from a mapper (SLAM/LIO) and is a global reference, so "map" was confusing to me. Anyway, it's a parameter now.

publishers: On a live system, collision relevant topics must not be latched, so I had to change this. With latched disabled, it makes sense, to check it anyone is listening on the topics. If not, we can save resources by not preparing and sending the point cloud.

parmeters: This is the big one. Your parameter system is somewhat unusual. I assume, you have other launch files and/or systems, where this needs to function as it does. (If not, tell me and I clean it up some more.) Biggest problem was, that the params.yaml contains a global patchwork section. Because of this, the ros param function calls always had to have the static prefix "/patchwork". This causes a lot of issues, e.g. in multi-robot environments or if two instances of patchwork are needed. Now, when reading a parameter, it is loaded from "/patchwork". If it doesn't exist, "/[node_name]" is tried as prefix, which is the best practice behavior. Special cases for parameters from just "/" also work.

LimHyungTae commented 8 months ago

Hello @frankhoeller I really appreciate your contributions. I'll read the commit thoroughly, and add some reviews soon (because in South Korea, these days are long holidays)! At a glance, it's very looks good to me

frankhoeller commented 6 months ago

Hi @LimHyungTae, any updated yet? Cheers