osvr_server support for cmdline options and blank config generation (resolves #228)

[Outurnate]

Patch to resolve #228. Includes options --config= and --help. Will create config with contents "{ }" in the event that the given config doesn't exist

[godbyk]

Instead of osvr_server --config=<config file>, I think osvr_server [config file] would be better (where the config file is optional and defaults to an empty JSON string as you've done).

[Outurnate]

Done! Removed the line from help as well.

Quick error check was added to config file creation

[godbyk]

Running osvr_server --help only shows the --help command-line option. It doesn't show the config file option.

Also, instead of creating a file in case someone happens to misspell it, it would be better to construct the server config from the empty JSON string. (This probably entails refactoring osvr::server::configureServerFromFile() into some smaller helper functions.)

[Outurnate]

I think the chance of misspelling is low - it'll be daemonized by most users. Either way, I agree, and the function should be split up

[godbyk]

👍 Looks good to me. Thanks, @Outurnate!

[araujobsd]

There are some conventions for command line parameters, basically the usage of "-" or "--" or "/" makes more sense then only "osvr_server [config file]"

Personally I have strong objection about the update, and the previous patch was much better.

[godbyk]

@araujobsd There are conventions for command-line parameters and I believe my comments reflect those conventions. See the fourth paragraph under Standards for Command Line Interfaces from the GNU Coding Standards which also generally reflect the POSIX conventions.

[araujobsd]

Yeap, I know that too! Thanks to point it out, my answer was based on POSIX.1-2008 (Utility Conventions).

And my main concern is, if in the future we want to extend the options via CLI such like "example": osvr_server --iface=eth1 --ipv4=0.0.0.0 --config=config_file

... looks for me [-c|--config] is more consistent than "osvr_server config_file"; you can argue that all those options shall be passed via config_file and never via CLI, however file as an operand via CLI for my eyes looks strange. Maybe I'm being pedantic.

[godbyk]

@araujobsd The POSIX doc you linked to agrees with my suggestion. The config filename is an operand, to use their terminology.

I do envision osvr_server taking command-line options—especially for more transient settings such as setting log verbosity levels or to temporarily override a configuration file setting. But I also anticipate it taking multiple configuration files that it would compose together internally. This would allow plugins to ship plugin-specific configuration files and allow the user to tell the server to load a few of them simultaneously.

The syntax for osvr_server might then be something like:

osvr_server [-v|--verbose] [-D|--daemon] [-V|--version] [config files...]

and an example execution might be:

osvr_server --verbose my_custom_hmd.json razer_hydra.json omni_treadmill.json

This usage is much more common than, say, osvr_server -c my_custom_hmd.json -c razer_hydra.json --config=omni_treadmill.json. Options are rarely repeated (though there are some common exceptions like -v, -vv, etc. to increase verbosity).

[araujobsd]

Hey,

OK, I didn't envision multiple configuration files at once in my previous replies, that is why I was pedantic with it. Actually you don't need -c config1 -c config2 -c config3 nobody does that, but instead you can have [-c|--config] config1 config2 config3.

But makes sense why you choose that way after your explanation.

Thanks.