search

itinero / routing

The routing core of itinero.
Apache License 2.0
221 stars 69 forks source link

Error in TryResolve #75

Closed risinghero closed 7 years ago

risinghero commented 7 years ago

File RouterExtensions. Branch develop line 81. Should be: return router.TryResolve(profiles, (float)coordinate.X, (float)coordinate.Y, searchDistanceInMeter); While there is: return router.TryResolve(profiles, (float)coordinate.Y, (float)coordinate.X, searchDistanceInMeter);

xivk commented 7 years ago

Is this in Itinero.Geo? Because coordinate.Y = latitude and coordinate.X = longitude, makes the original line correct.

risinghero commented 7 years ago

Hi, Ben.

Is this in Itinero.Geo

Yes

Because coordinate.Y = latitude and coordinate.X = longitude, makes the original line correct.

Ok. Let's assume coordinate.X=longitude. And go a little deeper into TryResolve implementation. It references another TryResolve like so: return router.TryResolve(profiles, (float)coordinate.X, (float)coordinate.Y, searchDistanceInMeter); So second parameter is coordinate.X which is in our assumption a longitude. Looking at RouterBaseExtensions method implementation: return router.TryResolve(profiles, latitude, longitude, null, searchDistanceInMeter); Which assumes that second parameter is latitude. But we have longitude. So we came to contradiction. It looks like parameters in router.TryResolve were just a little confused.

xivk commented 7 years ago

Ok, thanks! 👍 Will check!

xivk commented 7 years ago

There are four methods in Itinero.Geo that use the .X and .Y properties of the GeoAPI coordinate:

https://github.com/itinero/routing/blob/develop/src/Itinero.Geo/RouterExtensions.cs#L81 https://github.com/itinero/routing/blob/develop/src/Itinero.Geo/RouterExtensions.cs#L91 https://github.com/itinero/routing/blob/develop/src/Itinero.Geo/RouterExtensions.cs#L160 https://github.com/itinero/routing/blob/develop/src/Itinero.Geo/RouterExtensions.cs#L196

These all use methods that are lat-lon, so use Y-X. Where did you find this line?

return router.TryResolve(profiles, (float)coordinate.X, (float)coordinate.Y, searchDistanceInMeter);

I can't find it anywhere in the code...

risinghero commented 7 years ago

Hi Ben. I can't find this line. Looks like It was my mistake. Now I see that there are two Coordinate classes one with X and Y and one with Latitude and Longitude. I mixed up them with help of intellisense. Maybe it will be good to rename one of them. For example, to GeometryCoordinate.

xivk commented 7 years ago

Ok, I'll see what I can do about the names, I'm not going to change this now because it would be a pretty bit breaking change.

When working with NTS I would recommend not using the namespace Itinero.LocalGeo for now.