Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add more PathDetails#1142

Merged
karussell merged 5 commits into
graphhopper:mastergraphhopper/graphhopper:masterfrom
boldtrn:issue_1140boldtrn/graphhopper:issue_1140Copy head branch name to clipboard
Aug 23, 2017
Merged

Add more PathDetails#1142
karussell merged 5 commits into
graphhopper:mastergraphhopper/graphhopper:masterfrom
boldtrn:issue_1140boldtrn/graphhopper:issue_1140Copy head branch name to clipboard

Conversation

@boldtrn

@boldtrn boldtrn commented Aug 22, 2017

Copy link
Copy Markdown
Member

Fixes #1140.

I added all PathDetails that were requested in #1140. In order to add the time, I used the Weighting. I don't especially like this, as it adds an additional dependency, but I think it's the best solution for this problem.

public boolean isEdgeDifferentToLastEdge(EdgeIteratorState edge) {
if (edge.getEdge() != edgeId) {
edgeId = edge.getEdge();
time = weighting.calcMillis(edge, encoder.isBackward(edge.getFlags()), -1);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't have the reverse Flag here, I used this "hack" to determine the direction of the flag. Not sure if this is valid or if we should do it different?

@karussell karussell Aug 22, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The edges in the Path should be already properly directed so reverse should be always false for the path details?

@karussell karussell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! I really like how simple and clean this looks now :)

assertEquals(3, averageSpeedDetails.get(3).getFirst());
assertEquals(4, averageSpeedDetails.get(3).getLast());

List<PathDetail> streetNameDetails = details.get(STREET_NAME);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have different test methods for the different path details (and maybe reuse the common parts)

else if (value.getValue() instanceof Integer)
gen.writeNumber((Integer) value.getValue());
else if (value.getValue() instanceof Integer)
gen.writeNumber((Integer) value.getValue());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is somehow duplicated?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, you are right :)

@karussell karussell added this to the 0.10 milestone Aug 23, 2017
@karussell
karussell merged commit b82b139 into graphhopper:master Aug 23, 2017
@boldtrn
boldtrn deleted the issue_1140 branch August 23, 2017 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.