Setting a custom API URL doesn't work

[barbeau]

If you perform these steps:

1. Go to Settings and enter a custom API URL http://rtp.trimet.org/opentripplanner-api-webapp/ws
2. Check the “Custom OTP Server” checkbox
3. Hit back to go to the map
4. Long press on the screen and try to set a origin or destination

You get the message Location outside of network boundaries. Marker not set.

I got this after the app auto-detected Tampa, and it seems to be retaining the bounding box info from the Tampa server and enforcing it on the custom API URL trip requests.

[barbeau]

@vreixo do you think this is a quick fix you could implement and we could get out in a v2.0.1 release in the next day or two? If not, we can wait.

[barbeau]

Custom server API issue always happens. I enter the Trimet URL, then I get the Toast Ok: Server is working. Then after I check the "Custom OTP Server" checkbox, I get the Toast Metadata downloaded correctly. When I hit back to go to the map it is centered on Portland, but I get the error when trying to set the markers.

[barbeau]

Here's my LogCat output, althought I don't see any obvious issues:

09-19 17:34:18.599: D/SensorManager(4767): unregisterListener::  Listener= android.view.OrientationEventListener$SensorEventListenerImpl@426a7ee0
09-19 17:34:18.599: I/SensorManager(4767): Set normal delay = true
09-19 17:34:18.619: V/OTP(4767): A preference was changed: selected_custom_server
09-19 17:34:18.879: V/OTP(4767): res= org.apache.http.message.BasicHttpResponse@4289d060
09-19 17:35:07.751: D/dalvikvm(4767): GC_FOR_ALLOC freed 4752K, 28% free 21798K/29959K, paused 59ms, total 60ms
09-19 17:35:07.751: D/AbsListView(4767): [unregisterDoubleTapMotionListener]
09-19 17:35:07.751: I/MotionRecognitionManager(4767):   .unregisterListener : / listener count = 0->0, listener=android.widget.AbsListView$4@42a61c88
09-19 17:35:07.992: D/dalvikvm(4767): GC_FOR_ALLOC freed 370K, 27% free 22585K/30727K, paused 48ms, total 48ms
09-19 17:35:09.693: V/OTP(4767): Custom server Button clicked
09-19 17:35:09.693: V/OTP(4767): A preference was changed: selected_custom_server
09-19 17:35:09.834: V/OTP(4767): Now using custom OTP server: http://rtp.trimet.org/opentripplanner-api-webapp/ws
09-19 17:35:09.874: D/OTP(4767): URL: http://rtp.trimet.org/opentripplanner-api-webapp/ws/metadata
09-19 17:35:10.344: V/OTP(4767): LowerLeft: 44.761538,-123.5271731
09-19 17:35:10.344: V/OTP(4767): UpperRight45.901268,-121.121062
09-19 17:35:30.726: V/OTP(4767): Custom server Button clicked
09-19 17:35:30.736: V/OTP(4767): A preference was changed: selected_custom_server
09-19 17:35:31.337: V/OTP(4767): Custom server Button clicked
09-19 17:35:31.337: V/OTP(4767): A preference was changed: selected_custom_server
09-19 17:35:31.357: V/OTP(4767): Now using custom OTP server: http://rtp.trimet.org/opentripplanner-api-webapp/ws
09-19 17:35:31.377: D/OTP(4767): URL: http://rtp.trimet.org/opentripplanner-api-webapp/ws/metadata
09-19 17:35:31.897: V/OTP(4767): LowerLeft: 44.761538,-123.5271731
09-19 17:35:31.907: V/OTP(4767): UpperRight45.901268,-121.121062
09-19 17:35:41.277: V/OTP(4767): MainFragment onResume
09-19 17:35:41.277: W/ResourceType(4767): getEntry failing because entryIndex 939 is beyond type entryCount 1
09-19 17:35:41.277: W/ResourceType(4767): Failure getting entry for 0x7f0b03ab (t=10 e=939) in package 0 (error -2147483647)
09-19 17:35:41.277: E/GooglePlayServicesUtil(4767): The Google Play services resources were not found. Check your project configuration to ensure that the resources are included.
09-19 17:35:41.297: I/Adreno200-EGLSUB(4767): <ConfigWindowMatch:2098>: Format RGBX_8888.
09-19 17:35:41.497: I/Adreno200-EGLSUB(4767): <ConfigWindowMatch:2098>: Format RGBX_8888.
09-19 17:35:41.507: D/SensorManager(4767): registerListener :: handle = 4  name= MPL accel delay= 200000 Listener= android.view.OrientationEventListener$SensorEventListenerImpl@42829da8

[barbeau]

I've having the issue on a Samsung Galaxy S3 with Android 4.1.2.

[vreixo]

I have discovered that this is not a problem of how a custom server is selected, is a problem of how is made the calculation to check if a marker is inside the bounds. This is a fairly complex calculation that calculates the distance in the map between coordinates several times, which produces some inexactness which added one to the other overpass the acceptable error (not by far).

I have changed the function to check if a point is inside of the bounding box to a much simpler one that just checks if the latitude and longitude are bigger and smaller than the server limits (within an acceptable error of 0.005 degrees) and it just works OK. As I don't have an important background in GeoLocation please tell me if you think this approach is correct.

Also I have detected a related error: the app crashes if the marker is dragged directly outside the boundaries (if it was dragged before it does not crash). For solve this I have changed the way to restore the previous location after an invalid drag.

[vreixo]

I have found another erratic behavior for setting a custom server but is not very related to this so I have created Issue #43

[barbeau]

@vreixo on changing the bounding box calculations - the more complex calculations are probably to handle the situation where the bounding box crosses the International Date Line. This is more complex since you can't assume that the minimum longitude is the left side of the bounding box (e.g., if the box crosses the IDL, then the minLong is actually the right side of the box, and the maxLong is the left side - hence why OTP includes the "lowerleft" and "upperright" coordinates, since there is no abiguity over whether or not the box crosses the IDL).

I'd like to keep this calculation if possible - or, correct it if its producing wrong results.

In my tests, it didn't seem to be an error in accuracy, since I was trying to plan a trip in downtown Portland (which I believe is well within the box) and it was giving me this error. Also, are we using the same bounds calculation for server auto-detection? If so, that seems to work ok.

Let me know what you think.

@mentaer - thoughts?

[vreixo]

I will make some tests to see if my theory of the problems with the accuracy of the function is right. I understand that for the situation that you said are necessary this "complex" calculations, so I wont change them, I will just check if they're correct or augment the acceptable error if that is the problem.

[barbeau]

@vreixo Ok, thanks! fyi, here's a sample map I put together a while back explaining why lowerLeft/upperRight coordinates are preferred to minLat/minLong.

[vreixo]

@barbeau thanks for the link! Now I fully understand why are needed UpperRight and LowerLeft and why in OTP min and max have been deprecated (it was a question that I have asked myself some time ago).

Regarding the issue I'm now more convinced that is an accuracy problem. For check this I have used the test google doc https://docs.google.com/spreadsheet/ccc?key=0AsoU647elPShdDlCUVNKS0JQUXprX3pQd0ZENW5JMGc&single=true&gid=0#gid=0 and I have set the same bounds for the Portland server that are in the metadata http://rtp.trimet.org/opentripplanner-api-webapp/ws/metadata and make the custom server fail. With these bounds the error is the same when you try to put a marker that is with the custom URL. I have also made the test with Tampa putting huge bounds and also an error is obtained placing the marker.

One important point is that the calculations don't fail in all the area of the server, as it possible to set a marker close to the borders, as you can see in the image (this is also possible when setting a custom server. If the bounds where the same used for the previous server this won't be possible at all (I have also checked that they are actually not the same in the code of the app).

To be sure I have made some more tests with incremental size for the bounds and checked the errors. The error is proportional to how far is the point from the center and how big are the bounds, values of max errors in meters (horizontal or vertical), in the center from smaller to bigger bounds:
Coruña: 19
Portland (custom server bounds): 1904
Tampa: 4760
Calgary: 436802
Greenville: 9918163
Porto: huge, I didn't noted it

Also for Tampa I have checked:
Center: 4760
Center, closer to up edge: 3000
Very close to up edge: 500

With this context, I think that is safe to set a bigger acceptable error to solve the problem or use a better function that the provided by the API to calculate the distance in the map. With an error of 5 km we will cover approximately a region with the size of the state of Florida, so maybe is OK as a quick solution.

The message is because I have tried to put after a marker in the middle.

[vreixo]

I have remembered now that @mentaer said that there is a class in OTP to calculate this, org.opentripplanner.common.geometry.SphericalDistanceLibrary, maybe is more accurate. Although, this won't be a quick fix.

[barbeau]

Oh - so the error is in our measurement of distance between two points? Can you share a link to this section of the code?

If this is the case, there is a built in Android class (Location.distanceTo())to measure distance between two points. We should use this, as it uses Vincenty's formula, which is accurate to within millimeters even accounting for error when measuring distance between two points on WGS84.

There are also some utilities related to bounds measurements in OBA Android - we can use code from that project too, since its also licensed under Apache 2.0.

Also, please beware of your GSoC 2013 deadline - I don't recall exactly when it is, but this can all wait until after the deadline if you still need to create documentation, etc.

[barbeau]

@vreixo I should note that the OBA Android isLocationWithinRegion() method doesn't handle the IDL issue properly, so just beware of that. And thanks for all your work on this!

[vreixo]

I have just made a quick test converting the latitudes and longitudes to Locations and using Location.distanceTo and the errors are the same. We are now already using an Android function, Location.distanceBetween(). Here is the code https://github.com/CUTR-at-USF/OpenTripPlanner-for-Android/blob/master/src/edu/usf/cutr/opentripplanner/android/util/LocationUtil.java

You're right about the deadline, so I will organize the fixes that I have already made to the most important bugs and after I will continue with documentation work.

[vreixo]

@mentaer I have tried with spherical distance from OTP https://github.com/openplans/OpenTripPlanner/blob/c9f4184263c740f886384afded3399f58aa91288/otp-core/src/main/java/org/opentripplanner/common/geometry/SphericalDistanceLibrary.java and for large bounds as Portland metadata bounds still does not work. For smaller ones is OK.

[barbeau]

@vreixo could you list the set of lat/long points and the erroneous distance that Location.distanceBetween() returned for each?

This method has always worked correctly for me in other projects, so I'd like to try and dig deeper to figure out why its returning an incorrect distance.

[barbeau]

@vreixo I created a unit test (and test project for OTP Android) to help troubleshoot the LocationUtil.checkPointInBoundingBox() method - its in the boundsTest branch on the main repo.

Main test code is in the BoundsTest class.

Important code looks like this:

      @Override
    protected void setUp() throws Exception {
        // Tampa server
        serverTampa.setRegion("Tampa");
        serverTampa.setBaseURL("http://opentripplanner.usf.edu/opentripplanner-api-webapp/ws");
        serverTampa.setBounds("27.6236434,-82.8511308,28.3251809,-82.0559399");
        serverTampa.setLanguage("en_US");
        serverTampa.setContactName("Sean Barbeau");
        serverTampa.setContactEmail("opentripplanner@cutr.usf.edu");

        super.setUp();
    }

    public void testBoundsEvaluation(){
        // Tampa server
        LatLng pointInTampa1 = new LatLng(27.9710, -82.4650);       
        Assert.assertTrue(LocationUtil.checkPointInBoundingBox(pointInTampa1, serverTampa, ACCEPTABLE_ERROR));

        LatLng pointOutOfTampa1 = new LatLng(45.416, -122.839);     
        Assert.assertFalse(LocationUtil.checkPointInBoundingBox(pointOutOfTampa1, serverTampa, ACCEPTABLE_ERROR));
    }

Please add additional points and servers here, and submit them via a pull request to this branch (I'll merge the test project into the main repo once we're done, since we should really have one anyway).

To use this, you'll need to import the new OpenTripPlannerAndroidTests project (off the root folder) into Eclipse using "File->Import->Existing Android code". Then, right-click on project and click "Run As->Android JUnit Test". Note that you'll need an emulator or device connected to execute the test. After it runs, the JUnit windows should pop up in Ecilpse and the two assertion tests should currently fail (since the method isn't working correctly).

After the method is fixed, these tests (and any you add) should pass.

Hopefully this should be a lot faster than editing the code manually with different test cases.

More on unit testing on Android here.

[vreixo]

I have expanded the test class to check all the "official" servers and I have added also Portland with the bounds from metadata that are bigger.

I have check the calculations and repeated them step-by-step with the help of this tool http://www.daftlogic.com/projects-google-maps-distance-calculator.htm and the results are slightly different but there are also with very big errors.

Checking these things I have thought why the algorithm does not measure the total size horizontal and vertical at the latitude or longitude of the point, if it would be more accurate. Now, for measure horizontal distance it uses the upper right latitude of the server bounds instead of the point latitude and for vertical distance it uses the lower right longitude instead of the one of the point. I have made some tests in the online tool and use part of the original point seems to work better.

I have looked around for another source of this bounds algorithm used and I didn't found anything, so, I have changed the algorithm myself following previous point and now it works perfect! All tests pass with an accepted error of 10 meters.

Here is the change (locationLat and locationLong refer to the chosen point):

New:

    Location.distanceBetween(locationLat, leftLon, locationLat, rightLon, resultHorizontal);
    Location.distanceBetween(upLat, locationLon, downLat, locationLon, resultVertical);

Previous:

    Location.distanceBetween(upLat, leftLon, upLat, rightLon, resultHorizontal);
    Location.distanceBetween(upLat, leftLon, downLat, leftLon, resultVertical);

If you agree with me and you think that this is the correct solution I will made a pull request with the solution (and I will also make another request to update the tests with all the servers to your branch).

[barbeau]

@vreixo Great! Glad you were able to come up with a fix.

Before submitting a pull request, I'd ask that you add a few test cases with bounds that cross the International Date Line (IDL), since that is the reason for the more complicated method. So, I'd test points within the bounds to the west of the IDL, points within the bounds to the east of the IDL, and points to outside the box with both negative and positive longitudes. These tests will also make it clear to future developers why we're using more complex methods (if you could add more comments in the code itself as well, that would be great too). If these all pass, then please submit the pull request.