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

Ability to specify snapping radius in app.config for each profile individually #418

Closed
TimMcCauley opened this issue Feb 22, 2019 · 10 comments

Comments

@TimMcCauley
Copy link
Contributor

Currently the snapping radius from the coordinates input of the user to the nearest street segment is hard coded in kilometers, it would be great if this could be controlled via the app.config

@MichaelsJP
Copy link
Member

MichaelsJP commented Mar 4, 2019

@TimMcCauley For API v2 only? It can be easily set when the RouteRequest is converted to the RoutingRequest object. But that would only affect the new API. For the old API I would have to change a few things more.

@TimMcCauley
Copy link
Contributor Author

@MichaelsJP I'd suggest to set this once when the app.config is read initially and set globally. Would that work?

@MichaelsJP
Copy link
Member

@TimMcCauley Should be possible. I'll find a way that doesn't affect too much other things. Should it be set per service and per profile or only per service? The first option wouldn't be a minor change anymore, I would have to change the config readers and a lot of different things. But I guess that is what is needed right?

@TimMcCauley
Copy link
Contributor Author

TimMcCauley commented Mar 4, 2019

So something like

...
"profile-bike-road": {
     "profiles": "cycling-road",
     "parameters": {
         "encoder_options":
             "consider_elevation=false|turn_costs=true|block_fords=false",
             "elevation": true,
             "snapping_distance": 50,
             "preparation": {
                 "min_network_size": 200,
                 "min_one_way_ne...

@rabidllama what'ya think?

@MichaelsJP
Copy link
Member

MichaelsJP commented Mar 4, 2019

ok. I my opinion it would be intuitive to define the radii at one place directly under the profiles. That would make the option more accessible. e.g. :

...   
"profiles": {
          "active": ["vehicles-car", "vehicles-hgv", "bike", "bike-mtb", "bike-road", "bike-e", "pedestrian-walk", "pedestrian-hike", "wheelchair"],
          "profiles_radii":[1,2,3,4,5,6,7,8,9]

One thing though, in order to work every radii have to be set. Imo this is not a real downside that speaks against it...
@TimMcCauley @rabidllama Any thoughts on that?

@TimMcCauley
Copy link
Contributor Author

I think it should be an optional setting in general, defaults to 50km or w/e - just my opinion

@rabidllama
Copy link
Contributor

I would go for the option of setting it inside of the profile settings else we come across the issue of posting null values in the array. Plus when we debug and turn off profiles, in the array method we then have to remove the items from there aswell.

So from my pov, I would have the global snapping_distance: 50 inside the default_params, and then we can override it in the individual profiles.

A programmed default should remain though if there have been no parameters entered in the app.config.

"profiles": {
  "active": ["car", "hgv", "bike-regular"],
  "default_params": {
    "snapping_distance": 50,
    ...
  },
  "profile-car": {
    "parameters": {
      "snapping_distance": 100,
      ...
    },
    ...
  },
  "profile-wheelchair": {
    "parameters": {
      "snapping_distance": 20,
      ...
    },
    ...
  },
  ...
}

@TimMcCauley
Copy link
Contributor Author

Perfect solution @rabidllama , agreed.

@rabidllama rabidllama added this to the 5.1 milestone Mar 4, 2019
MichaelsJP pushed a commit that referenced this issue Mar 8, 2019
@nilsnolde
Copy link
Contributor

what's the status on this @MichaelsJP? Working as expected? Here adding a few tests would be beneficial;)

@MichaelsJP
Copy link
Member

In the making ;)... I had to figure out and understand a few things as it affects the initial config reading and object generation at ors start.

MichaelsJP pushed a commit that referenced this issue Mar 21, 2019
MichaelsJP pushed a commit that referenced this issue Mar 21, 2019
MichaelsJP pushed a commit that referenced this issue Mar 21, 2019
MichaelsJP pushed a commit that referenced this issue Mar 21, 2019
MichaelsJP pushed a commit that referenced this issue Mar 25, 2019
MichaelsJP pushed a commit that referenced this issue Mar 25, 2019
…into feature-#418-snapping-distance

# Conflicts:
#	openrouteservice-api-tests/src/test/java/heigit/ors/v2/services/routing/ParamsTest.java
MichaelsJP pushed a commit that referenced this issue Mar 25, 2019
MichaelsJP pushed a commit that referenced this issue Mar 25, 2019
MichaelsJP pushed a commit that referenced this issue Mar 25, 2019
MichaelsJP pushed a commit that referenced this issue Mar 26, 2019
MichaelsJP pushed a commit that referenced this issue Mar 26, 2019
MichaelsJP pushed a commit that referenced this issue Mar 26, 2019
MichaelsJP pushed a commit that referenced this issue Mar 27, 2019
MichaelsJP pushed a commit that referenced this issue Mar 27, 2019
rabidllama pushed a commit that referenced this issue Apr 1, 2019
MichaelsJP pushed a commit that referenced this issue May 20, 2019
MichaelsJP pushed a commit that referenced this issue May 20, 2019
TimMcCauley pushed a commit that referenced this issue May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants