uwreact / uwreact_robot

Software behind our fully autonomous FIRST robots
BSD 3-Clause "New" or "Revised" License
8 stars 6 forks source link

Move ZED config parameters to a config file #78

Open wraftus opened 5 years ago

wraftus commented 5 years ago

🚀 Feature Request

To make the zeds.launch file less cluttered and to make changing parameters easier in the future, we want to have a yaml file describe the params instead of within the launch file.

wmmc88 commented 5 years ago

As discussed here, i think we should have the yml cfg serve as defaults, but still have cl args for the camera properties. (all cameras would have the same resolution, framerate, etc)

wmmc88 commented 5 years ago

this would allow for on-the-fly tuning of those params without needing to edit files between launches. The preferable alternative would be to fork the zed ros wrapper and add a dynamic reconfigure server (and pr it maybe lol).

matthew-reynolds commented 5 years ago

^ I would argue that the command-line args don't add on-the-fly tuning of those parameters, as you mentioned the only true on-the-fly would be via dynamic reconfigure.

I see no advantage to command-line over editing the files. Editing the files doesn't really take longer and is clearer as the number of parameters gets large (See ros_control's use of yaml). However, if we only have a small number of command-line arguments, there isn't a significant disadvantage either, as long as you remember to commit the new "defaults" you chose.

The main reason I prefer yaml here over command-line is perhaps just the way I'm looking at these options. I see them as static robot configuration parameters, once they're determined they will be (relatively) set in stone. I like the idea of treating them the same as we treat urdfs, the ros_control controller config files, etc.

As I said in the other chat though, if we are enforcing that all the cameras use the same frame rate and resolution (Which seems super restrictive to me but you guys know a lot more about it than I do), then there is very little cost to adding the launchfile arguments