Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osrm-datastore command line improvements #951

Closed
springmeyer opened this issue Mar 13, 2014 · 6 comments
Closed

osrm-datastore command line improvements #951

springmeyer opened this issue Mar 13, 2014 · 6 comments

Comments

@springmeyer
Copy link
Contributor

In addition to #890 here are a few improvements that ideally could be made to the osrm-datastore tool to avoid pitfalls/confusion for users.

First, it seems like there are two modes: you can either pass one argument to the base.osrm file or you can pass multiple options to point to each bit of pre-processed data, but using the flags of --hsgrdata and --nodesdata, etc. If my understanding here is correct then the output of osrm-datastore -h is confusing because at first I thought you could combine them by doing osrm-datastore base.osrm --hsgrdata base.osrm.hsgrdata .... etc. But this leads to very odd errors that are hard to interpret:

~/projects/node-osrm[shared-memory]$ osrm-datastore berlin-latest.osrm --hsgrdata berlin-latest.osrm.hsgr --nodesdata berlin-latest.osrm.nodes --edgesdata berlin-latest.osrm.edges --ramindex berlin-latest.osrm.ramIndex --fileindex berlin-latest.osrm.fileIndex --namesdata berlin-latest.osrm.names
[info] validator called for berlin-latest.osrm
[info] validator called for berlin-latest.osrm.hsgr
[info] validator called for berlin-latest.osrm.nodes
[info] validator called for berlin-latest.osrm.edges
[info] validator called for berlin-latest.osrm.ramIndex
[info] validator called for berlin-latest.osrm.fileIndex
[info] validator called for berlin-latest.osrm.names
[warn] caught exception: berlin-latest.osrm.hsgr not found

Next, it appears that the format of --option=value is not supported and only --option value is supported. Ideally an error could be thrown up front if --option=value syntax is used saying it is invalid, otherwise understanding the user error is difficult. For example passing --sharedmemory=on leads to:

[warn] caught exception: the argument ('=on') for option '--sharedmemory' is invalid. Valid choices are 'on|off', 'yes|no', '1|0' and 'true|false'
DennisOSRM added a commit that referenced this issue Mar 13, 2014
@DennisOSRM
Copy link
Collaborator

Note: previous commit (6b69d6d) implements #890 not this one.

@DennisOSRM
Copy link
Collaborator

Behavior now is much nicer, i.e.:

$ ./osrm-datastore 
[info] osrm-datastore <base.osrm> [<options>]:

Options:
  -v [ --version ]                    Show version
  -h [ --help ]                       Show this help message
  -c [ --config ] arg (="server.ini") Path to a configuration file

Configuration:
  --hsgrdata arg        .hsgr file
  --nodesdata arg       .nodes file
  --edgesdata arg       .edges file
  --geometry arg        .geometry file
  --ramindex arg        .ramIndex file
  --fileindex arg       File index file
  --namesdata arg       .names file
  --timestamp arg       .timestamp file

@springmeyer
Copy link
Contributor Author

good improvements, but you missed one. I'll clarify. This command:

$ osrm-datastore berlin-latest.osrm --hsgrdata berlin-latest.osrm.hsgr --nodesdata berlin-latest.osrm.nodes --edgesdata berlin-latest.osrm.edges --ramindex berlin-latest.osrm.ramIndex --fileindex berlin-latest.osrm.fileIndex --namesdata berlin-latest.osrm.names

Leads to this error:

[warn] caught exception: berlin-latest.osrm.hsgr not found

It should instead say something like:

Error: incorrect usage - please either pass only <base.osrm> or configure options to point at pre-processed data, but not both.

Otherwise it is baffling why the berlin-latest.osrm.hsgr not found error is returned because it was found and it does exist and the option --hsgrdata was passed.

@DennisOSRM DennisOSRM reopened this Mar 15, 2014
@DennisOSRM
Copy link
Collaborator

@springmeyer This is now (hopefully) fixed with 823e8d2.

@DennisOSRM
Copy link
Collaborator

@springmeyer are we good here?

@springmeyer
Copy link
Contributor Author

I assume so, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants