strands-project / strands_movebase

A repository for all the STRANDS-augmented movebase, including 3D obstacle avoidance, etc.
MIT License
10 stars 20 forks source link

strands_movebase params vs scitos_2d_nav_params #66

Closed bfalacerda closed 8 years ago

bfalacerda commented 9 years ago

We are maintaining two sets of params, and even loading stuff from scitos_2d_nav for strands_movebase (see https://github.com/strands-project/strands_movebase/issues/54, which I forgot about meanwhile)

Also, we have stuff that was pushed into one and not into the other. I think we should try to unify this, or at least have a version of simulation that loads the strands_movebase params, to make sure that we are testing the right ones.

Any opinion on this?

marc-hanheide commented 9 years ago

Opinion: :+1:

I think we should put those params in strands_movebase for all our systems.

nilsbore commented 9 years ago

Also :+1: , we used to have everything in strands_movebase but decided to use the parameters that we could from scitos_2d_navigation. I'm considering adding a simplified chest camera for the simulation also. Basically we could have a camera with almost the same resolution as the subsampled cloud used for navigation. This would make simulation navigation more similar to what we're using.

Jailander commented 9 years ago

I think I'm having a deja vu :P https://github.com/strands-project/scitos_2d_navigation/issues/55

bfalacerda commented 9 years ago

is it ok to have scitos_2d_nav depending on strands_movebase?

marc-hanheide commented 9 years ago

Nope, that wouldn't work.

A quick search in strands_movebase reveals that it depends on scitos_2d_navigation: https://github.com/strands-project/strands_movebase/search?utf8=%E2%9C%93&q=scitos_2d_navigation

So, the original idea always was that those 2D nav parameters like DWA etc are in scitos_2d_navigation and not in strands_movebase... We can change that policy, but we certainly should not depend scitos_2d_navigation on strands_movebase.

marc-hanheide commented 9 years ago

BTW, all the dependencies are shown in this: here

So there you see you'd introduce a forbidden cycle.

bfalacerda commented 8 years ago

https://github.com/bfalacerda/strands_movebase/commit/dd2f53eb10f18f40021099dd56c172549bf10277 makes strands_movebase not load from scitos_2d_nav.

For the time being we'll stop updating scitos_2d_nav params, and once the testing is done we'll replicate the final params there