Skip to content

Conversation

@boldtrn
Copy link
Member

@boldtrn boldtrn commented Aug 22, 2017

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
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?

Copy link
Member

@karussell karussell Aug 22, 2017

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?

Copy link
Member

@karussell karussell left a comment

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
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
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
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