strands-project / strands_navigation

Higher-level navigation capabilities
41 stars 48 forks source link

Support for multi-robot and different global planners #369

Closed hawesie closed 5 years ago

hawesie commented 5 years ago

This brings in changes we've been using internally for multi-robot support and for running with different navigation planners.

hawesie commented 5 years ago

I'm still testing this locally, but tagging @bfalacerda @Jailander for info

hawesie commented 5 years ago

This works locally, so I'd like it to be merged if possible. I don't know how it effects other open tasks though.

hawesie commented 5 years ago

@marc-hanheide Fixed those issues. Thanks for the review! I will wait for a :+1: from @francesodelduchetto or @Jailander before merging

Jailander commented 5 years ago

Hi yes just checked it I am happy

hawesie commented 5 years ago

Cool, thanks.

marc-hanheide commented 5 years ago

And... as expected there are side effects. We just spent a lot of time drilling down to the problem of localise_by_topic not working anymore. Look at this blame: https://github.com/strands-project/strands_navigation/blame/indigo-devel/topological_navigation/scripts/localisation.py#L30

The problem is that the topological map definition contained

localise_by_topic: '"{\"topic\": \"/battery_state\", \"field\": \"charging\",
      \"val\": true, \"localise_anywhere\": false}"'

So, with the new code this became //battery_state and didn't work anymore. Pinging @Jailander and @hawesie to make aware. Solution is to change it in the definition of the topomap to battery_state.