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

Enable GPS-precise map matching #51

Closed
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@MK-42

MK-42 commented May 13, 2016

Added Fetching of wayGeometry on the virtual edges at start and end of the gpxTrack. Closes #50

I don't know if you have any special code-formatting constraints as line length. Could change that if needed.

What do you think about that approach

@karussell

This comment has been minimized.

Show comment
Hide comment
@karussell

karussell May 13, 2016

Member

Thanks & Looks good.

Now only the web module needs to implement and use this too. Also I would like to hide this somehow from public usage in MatchResult, hmmh, not sure how.

Member

karussell commented May 13, 2016

Thanks & Looks good.

Now only the web module needs to implement and use this too. Also I would like to hide this somehow from public usage in MatchResult, hmmh, not sure how.

@karussell

This comment has been minimized.

Show comment
Hide comment
@karussell

karussell May 13, 2016

Member

Ah, maybe we add a 'getWayGeometry' method to EdgeMatch and do a fallback if non-existing?

PointList getWayGeometry() {
   if(geometry != null)
      return geometry;
   else
      return edgeState.fetchWaygeometry(0);
}

Then we can change the web API from:

PointList pointList = edgeMatch.getEdgeState().fetchWayGeometry(emIndex == 0 ? 3 : 2);

to

PointList pointList = edgeMatch.getWayGeometry();
Member

karussell commented May 13, 2016

Ah, maybe we add a 'getWayGeometry' method to EdgeMatch and do a fallback if non-existing?

PointList getWayGeometry() {
   if(geometry != null)
      return geometry;
   else
      return edgeState.fetchWaygeometry(0);
}

Then we can change the web API from:

PointList pointList = edgeMatch.getEdgeState().fetchWayGeometry(emIndex == 0 ? 3 : 2);

to

PointList pointList = edgeMatch.getWayGeometry();
@MK-42

This comment has been minimized.

Show comment
Hide comment
@MK-42

MK-42 May 13, 2016

done i think. Much smoother to use now. good idea!

MK-42 commented May 13, 2016

done i think. Much smoother to use now. good idea!

@MK-42

This comment has been minimized.

Show comment
Hide comment
@MK-42

MK-42 May 13, 2016

but i forgot the change in the web service. will push that next
btw: doesn't the Console-Version need that change too to be gps precise?

MK-42 commented May 13, 2016

but i forgot the change in the web service. will push that next
btw: doesn't the Console-Version need that change too to be gps precise?

@MK-42

This comment has been minimized.

Show comment
Hide comment
@MK-42

MK-42 May 17, 2016

did some more changes to make using easier. What do you think @karussell ?

MK-42 commented May 17, 2016

did some more changes to make using easier. What do you think @karussell ?

}
assertEquals(mr.getGpxEntriesLength(), mr.getMatchLength(), 1);
// TODO why is there such a big difference for millis?

This comment has been minimized.

@karussell

karussell May 17, 2016

Member

what do you mean with this TODO and compared to which value this is a 'big' difference?

@karussell

karussell May 17, 2016

Member

what do you mean with this TODO and compared to which value this is a 'big' difference?

This comment has been minimized.

@MK-42

MK-42 May 17, 2016

the Mapmatched paths is 24sec different in length (I don't remember if shorter or longer) -> see the third param of the assertEquals. Acctually I don't know why that is the case. But the other tests also have such a difference (and such a ToDo-comment, that I copied from there).

@MK-42

MK-42 May 17, 2016

the Mapmatched paths is 24sec different in length (I don't remember if shorter or longer) -> see the third param of the assertEquals. Acctually I don't know why that is the case. But the other tests also have such a difference (and such a ToDo-comment, that I copied from there).

renamed EdgeMatch::getWayGeometry to EdgeMatch::fetchWayGeometry due …
…to code-review and set wayGeometry via setter
@MK-42

This comment has been minimized.

Show comment
Hide comment
@MK-42

MK-42 May 17, 2016

sorry, had wrong indentation settings and messed it up a little bit.

I think I've adressed your points. The setWayGeometry part is not that nice in the phase 4, because we need it two times and i didn't want to duplicate the conditions... but extracting that is also not that nice because of the index-params.

What do you think?

MK-42 commented May 17, 2016

sorry, had wrong indentation settings and messed it up a little bit.

I think I've adressed your points. The setWayGeometry part is not that nice in the phase 4, because we need it two times and i didn't want to duplicate the conditions... but extracting that is also not that nice because of the index-params.

What do you think?

Added another edge-case. if the path consists of only one edge and th…
…e end point is matched to a tower node, the wayGeometry got overridden. Added also a testcase for that.
@karussell

This comment has been minimized.

Show comment
Hide comment
@karussell

karussell May 20, 2016

Member

I think I've adressed your points.

Thanks!

Looks now indeed better. Will investigate (after next week) why there is the time difference and how to improve this "The setWayGeometry part is not that nice in the phase 4"

sorry, had wrong indentation settings and messed it up a little bit.

No problem

Member

karussell commented May 20, 2016

I think I've adressed your points.

Thanks!

Looks now indeed better. Will investigate (after next week) why there is the time difference and how to improve this "The setWayGeometry part is not that nice in the phase 4"

sorry, had wrong indentation settings and messed it up a little bit.

No problem

@karussell

This comment has been minimized.

Show comment
Hide comment
@karussell

karussell May 27, 2016

Member

These differences in time should come from the edge vs. virtual edge and normally should be fixed with these changes here. A bit strange.

Now after a bit thinking and struggling I think it is more valuable to merge #49 before or merge both at the same time. I'll let you know once this is done (hopefully in the next 2 weeks) - thanks a lot for this change!

Member

karussell commented May 27, 2016

These differences in time should come from the edge vs. virtual edge and normally should be fixed with these changes here. A bit strange.

Now after a bit thinking and struggling I think it is more valuable to merge #49 before or merge both at the same time. I'll let you know once this is done (hopefully in the next 2 weeks) - thanks a lot for this change!

@MK-42

This comment has been minimized.

Show comment
Hide comment
@MK-42

MK-42 Jun 13, 2016

Anything i can do to get that merged an in the next Release (i think there is an upcoming. Right?)
Then I could also Test the hmm stuff.

MK-42 commented Jun 13, 2016

Anything i can do to get that merged an in the next Release (i think there is an upcoming. Right?)
Then I could also Test the hmm stuff.

@karussell

This comment has been minimized.

Show comment
Hide comment
@karussell

karussell Jun 13, 2016

Member

Anything i can do to get that merged an in the next Release (i think there is an upcoming. Right?)

Sorry, the release is already kind of out. But will make a merge afterwards and hopefully the next release won't last long. Currently we plan to release more often and delay the features instead of delaying the release with planned features.

I also hoped that I could this working within the release but it would have taken too long. The hmm stuff needs some more tuning and will include it but make it optional to allow a fallback to the old algorithm ... this all makes it a bit more work than currently done.

Member

karussell commented Jun 13, 2016

Anything i can do to get that merged an in the next Release (i think there is an upcoming. Right?)

Sorry, the release is already kind of out. But will make a merge afterwards and hopefully the next release won't last long. Currently we plan to release more often and delay the features instead of delaying the release with planned features.

I also hoped that I could this working within the release but it would have taken too long. The hmm stuff needs some more tuning and will include it but make it optional to allow a fallback to the old algorithm ... this all makes it a bit more work than currently done.

@karussell

This comment has been minimized.

Show comment
Hide comment
@karussell

karussell Jul 16, 2016

Member

I stumbled over a new problem where also in-between edges needs this GPS precision, not only start and end. E.g. a tracked truck delivers a dead end road and needs to go forth&back.

Member

karussell commented Jul 16, 2016

I stumbled over a new problem where also in-between edges needs this GPS precision, not only start and end. E.g. a tracked truck delivers a dead end road and needs to go forth&back.

@karussell karussell changed the title from Added Fetching of wayGeometry on the virtual edges at start and end o… to Enable GPS-precise map matching Oct 10, 2016

@karussell

This comment has been minimized.

Show comment
Hide comment
@karussell

karussell Oct 10, 2016

Member

The "hmm" work is now better integrated and I would like to push this issue forward now. @MK-42 Would you have some time to invest in this issue soonish? Otherwise I'll have to move this to another version/release.

Member

karussell commented Oct 10, 2016

The "hmm" work is now better integrated and I would like to push this issue forward now. @MK-42 Would you have some time to invest in this issue soonish? Otherwise I'll have to move this to another version/release.

@LothbrokR

This comment has been minimized.

Show comment
Hide comment
@LothbrokR

LothbrokR Oct 5, 2017

Hello guys, I would like to use this precise map matching feature. I thus downloaded different versions that correspond to the changes you are talking about here. To be precise, I downloaded and tested the following versions:

https://github.com/graphhopper/map-matching/tree/9606f2fc28f7b0d801e97a5e36f830014a147d85
https://github.com/graphhopper/map-matching/tree/7005e9c901788e7aed862e78cb6499946daddf58
https://github.com/graphhopper/map-matching/tree/2cef7f298c9f44f1a294a91eb3d5b7dc283fe009

All three versions returned the same result (which isn't precise), that can be seen here:
screenshot

The gpx file I am using as input is this one: 120+00085_13643.zip

I am probably doing something wrong, can you please explain what I should do to get it working ?

LothbrokR commented Oct 5, 2017

Hello guys, I would like to use this precise map matching feature. I thus downloaded different versions that correspond to the changes you are talking about here. To be precise, I downloaded and tested the following versions:

https://github.com/graphhopper/map-matching/tree/9606f2fc28f7b0d801e97a5e36f830014a147d85
https://github.com/graphhopper/map-matching/tree/7005e9c901788e7aed862e78cb6499946daddf58
https://github.com/graphhopper/map-matching/tree/2cef7f298c9f44f1a294a91eb3d5b7dc283fe009

All three versions returned the same result (which isn't precise), that can be seen here:
screenshot

The gpx file I am using as input is this one: 120+00085_13643.zip

I am probably doing something wrong, can you please explain what I should do to get it working ?

@michaz

This comment has been minimized.

Show comment
Hide comment
@michaz

michaz May 20, 2018

Member

Fixed this in e380cef (by doing less work: concatenating the Paths returned from map-matching instead of reconstructing a new one (where the begin/end-splits are missing))

Member

michaz commented May 20, 2018

Fixed this in e380cef (by doing less work: concatenating the Paths returned from map-matching instead of reconstructing a new one (where the begin/end-splits are missing))

@michaz michaz closed this May 20, 2018

@karussell

This comment has been minimized.

Show comment
Hide comment
@karussell

karussell May 20, 2018

Member

Really cool - thanks a bunch!

Member

karussell commented May 20, 2018

Really cool - thanks a bunch!

@karussell karussell added this to the 0.11 milestone Jun 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment