add isochrone snapped center coordinates to response

[nilsnolde]

#1961

Adds the snapped center point of an isochrone to the response.

This is my first ever C++ code 🍾 Wasn't the most difficult task though. Still not sure if this is properly done:

• I'm not sure what the valhalla::Location::path_edges represent before the start of the isochrone computation. I'd guess all edges found near the input location?! Does it then expand the search from all these edges it found nearby the input location? In which case there actually isn't smth like a single snapped point?!

• Which property of the valhalla::Location_PathEdge::Location_PathEdge is giving me the actual snapped point for a given edge in valhalla::Location::path_edges? I can see percent_along() as a function, suggesting it does know the snapped origin. The only attribute I found is ll_, but whether that's the edge geometric center, one of it's start/end nodes or the actual snapped origin, I'm not sure..

I'll amend if you'd be so nice to quickly point me in the right direction. And also apply this to the other Compute functions for

• multi-modal.
• reverse

Tasklist

• Add tests
• Review - you must request approval to merge any PR to master
• Add #fixes with the issue number that this PR addresses
• Generally use squash merge to rebase and clean comments before merging
• Update the changelog
• Update relevant documentation

[nilsnolde]

Ok, I think I got it now.. I misunderstood some of the code in the json serializer and I actually found a bug (#2115) concerning multiple locations for isochrones. Also, I added the same logic to reverse search and multimodal.

Should we address #2115 before finishing off this PR?

And my first question is answered I think: the path_edges for each origin are (I think) all edges connected to the node it snapped to and the ll() function for each edge gives the same coordinate, so I assume that's the starting node for isochrone expansion. That'd be nice and would also answer my second question:)

The build for OSX is failing with

In file included from /Users/distiller/project/src/midgard/gridded_data.cc:1:
/Users/distiller/project/valhalla/midgard/gridded_data.h:4:10: fatal error: 'valhalla/proto/tripcommon.pb.h' file not found
#include "valhalla/proto/tripcommon.pb.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Not sure, what's wrong, but looks like a rookie mistake. The Azure Win build failures look like the same issue:

D:\a\1\s\valhalla\midgard/gridded_data.h(4): fatal error C1083: Cannot open include file: 'valhalla/proto/tripcommon.pb.h': No such file or directory [D:\a\build\src\midgard\valhalla-midgard.vcxproj]
  polyline2.cc

Any advice on those?

[nilsnolde]

I reverted back to master and did what you suggested @kevinkreiser. I just changed the whole location response to MultiPoint and each, input & snapped, are one feature with a property indicating which is which.

[nilsnolde]

Small bump 😇

[nilsnolde]

ok, I think now it could be good to go @kevinkreiser :) Each origin gets one input Point feature and multiple aggregated snapped points as MultiPoint feature and both have location_index as additional property.

[nilsnolde]

Damn, smth went wrong with merging master.. Might be a good idea to squash the commits, should be ok other than that. Next time will be smoother!

[nilsnolde]

What do you think @kevinkreiser ? Can we merge this?

[kevinkreiser]

@nilsnolde thanks for updating the PR. when the builds pass we'll merge it