schildbach / public-transport-enabler

Unleash public transport data in your Java project.
https://groups.google.com/forum/#!forum/public-transport-enabler-discuss
GNU General Public License v3.0
390 stars 133 forks source link

Navitia: queryNearbyLocations requires location.isIdentified() #26

Closed grote closed 9 years ago

grote commented 9 years ago

@aelkhour In AbstractNavitiaProvider.java:763 you are using location.isIdentified() instead of location.hasLocation() which breaks querying for locations with LocationType.ANY that only have GPS coordinates.

Other AbstractProviders use location.hasLocation() as well, so I suggest using this here, too. But @schildbach would know that better than me.

schildbach commented 9 years ago

Yes and no. The code after line 763 seems to have both ways to query for nearby stations: 1) by coordinate 2) by stop or poi id of an already identified location

For the first, you should indeed use hasLocation(). For the second, hasId() would be appropriate.

@grote Generally it's a good idea to supply a testcase that shows the error (and has asserts to show how you would expect the API to answer).

aelkhour commented 9 years ago

What is a Location of type LocationType.ANY supposed to represent? I could for instance use the id first then the latlon info if there is no id in order to interpret the location, but what if both members are filled?

schildbach commented 9 years ago

ANY means "we don't know". It's mainly meant for textual user input without the user selecting any type.

It's up to you how you set the priorities. I suggest (in that order)

grote commented 9 years ago

Here's a simple test case:

    @Test
    public void nearbyStationsAny() throws Exception
    {
        final NearbyLocationsResult result = queryNearbyLocations(EnumSet.of(LocationType.STATION), new Location(LocationType.ANY, null, 48877095, 2378431), 700, 10);
        assertEquals(NearbyLocationsResult.Status.OK, result.status);
        print(result);
    }
grote commented 9 years ago

Tested and verified to work. Thanks @aelkhour!