Introducing engineMode for future use #549
Conversation
There was a problem hiding this comment.
I'm not really sure how this PR is related to the linked issues, because the modes are called "seed" and "other" but the issues request automatic route suggestion and map matching.
As with your last huge PR I would suggest first adding the feature and afterwards exposing the API because it allows you to freely change the implementation without modifying a public API.
docs/revisions.md
Outdated
| ### New last version | ||
|
|
||
| Android | ||
|
|
||
| - Add parameter dialog for profile | ||
|
|
||
| Library |
There was a problem hiding this comment.
Unrelated to this PR. I think we should stick to a common format like keepachangelog which suggests a unreleased section instead of "new last version" (which should be "new since last version"?)
There was a problem hiding this comment.
Yes, you are right, bad wording.
docs/revisions.md
Outdated
|
|
||
| Library | ||
|
|
||
| - Add engineMode for future use |
There was a problem hiding this comment.
I don't know if that would be helpful to anyone. People would care about the new feature, not some API extension
There was a problem hiding this comment.
For me, users also include developers who want to adapt their app accordingly.
The idea is to open the engine to more functionality than standard routing.
May be. For me, this way is the better way. This offers you and others the chance to place new features on these terms. Otherwise we develop nice new features and when the first one comes out with it, the others notice that the api implementation is different. |
|
Here a first new function that uses this new entry:
Returns a wpt gpx with elevation when found. |
|
I think it's not sensible to extend the Shouldn't we just extend the AIDL with a new method to retrieve elevation? |
|
@zod |
|
We could also add an API endpoint for the server to provide elevation which could be used e.g. by brouter-web to show elevation at POIs. I think brouter-web would rather consume a simple JSON instead of parsing a GPX file with waypoints just for elevation. For the CLI it should be possible to provide a different command. I just skimmed through the code and if we would pursue the |
What do you mean be that? And yes, for the server I also thought on json export as well. As usual initialized by parameter. |
| public RoutingEngine(String outfileBase, String logfileBase, File segmentDir, | ||
| List<OsmNodeNamed> waypoints, RoutingContext rc) { | ||
| this(0, outfileBase, logfileBase, segmentDir, | ||
| waypoints, rc); | ||
| } | ||
|
|
||
| public RoutingEngine(int engineMode, String outfileBase, String logfileBase, File segmentDir, | ||
| List<OsmNodeNamed> waypoints, RoutingContext rc) { |
There was a problem hiding this comment.
I think the first parameters of the constructors should be the same and engineMode should be appended.
|
I'm quite surprised that it was merged. I still fail to understand why it's a good idea to add reading points elevation data to What's your reason to not provide a new API endpoint & Android service which provides just elevation data without the GPX stuff? |
As discussed before the server also could use this call. |
To build a base for future an engine mode is added.
This is related to #460 or #222 or other things that will not working with 'normal' routing mode.