(WIP) Add AbstractOpenTripPlannerProvider #211
Conversation
| productsToModeMap.put(Product.SUBURBAN_TRAIN, OTPMode.RAIL.toString()); | ||
| productsToModeMap.put(Product.SUBWAY, OTPMode.SUBWAY.toString()); | ||
| productsToModeMap.put(Product.TRAM, OTPMode.TRAM.toString()); | ||
| // GONDOLA and FUNICULAR are no PTE Products, but OTP modes. Associate them with TRAM (?) |
There was a problem hiding this comment.
What's the difference to Product.CABLECAR?
There was a problem hiding this comment.
Ok, I agree, FUNICULAR might be seen as a specialization of cable car. But for gondolas PTE has no corresponding product. For OTP, gondolas correspond to telecabin services, for which PTE has currently no product.
For lines, the product is not mandatory and may be null when parsing an OTP response in such a case. I guess I should really set it to null for gondolas.
For requests, I think it should be subsumed under a more or less resembling code, so it can be (de)selected when filtering products.
| @@ -101,7 +101,7 @@ public void log(final String message) { | |||
| OKHTTP_CLIENT = builder.build(); | |||
| } | |||
|
|
|||
| private static final String SCRAPE_ACCEPT = "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"; | |||
| private static final String DEFAULT_ACCEPT = "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"; | |||
There was a problem hiding this comment.
See comment below, it‘s now a default that can be overridden
| @@ -173,10 +173,10 @@ public void getInputStream(final Callback callback, final HttpUrl url, final Str | |||
| while (true) { | |||
| final Request.Builder request = new Request.Builder(); | |||
| request.url(url); | |||
| request.header("Accept", DEFAULT_ACCEPT); | |||
There was a problem hiding this comment.
OTP‘s responds with xml or json, depending on The accept header. To have it return json, I need to set it to json, which was always overridden by the SCRAPE_ACCEPT header. Hope just using this as default but not mandatory will not break anything for other providers. Or was there an explicit reason why it was necessary to override it?
| import de.schildbach.pte.dto.Trip.Public; | ||
| import okhttp3.HttpUrl; | ||
|
|
||
| @RunWith(MockitoJUnitRunner.class) |
There was a problem hiding this comment.
Not sure that you really need Mockito. Any reason you can not use assertEquals() or assertNotNull()?
There was a problem hiding this comment.
I introduced mockito to stub out the AOTPP.request() method to do some unit testing. Current provider tests seemed to be all live provider tests, which mostly print the results and do only a few plausibility checks. I‘m still not happy having large json snippets in the test class. Still thinking about this... I‘ll rewrite the asserts to match the current pte style, but still think mockito would be useful
| print(result); | ||
| assertEquals(QueryTripsResult.Status.OK, result.status); | ||
| assertTrue(result.trips.size() > 0); | ||
| } |
There was a problem hiding this comment.
How is this testing "onlyBike"?
|
Just saw that opentransitmap has an incubation repository. Should I request a pull there first? |
|
That's up to you. Shouldn't be necessary. Ideally, things get merged here. I don't think they do much code review there. If your PR gets ignored for too long, I'd consider it. |
schildbach
force-pushed
the
master
branch
2 times, most recently
from
63d8043
to
2e5f16d
Compare
schildbach
force-pushed
the
master
branch
3 times, most recently
from
b017ce1
to
28dd2c8
Compare
|
Is there any progress on this issue? I'd really like to use transportr in Norway |
|
I see no progress the past few years. Can I post a 20 Euro feature bounty? |
The AbstractOpenTripPlannerProvider enables the integration of OTP based journey planners like e.g. Mitfahren-BW, plannerstack, TriMet…
There'll still be some issues but I'd be glad to already get some feedback.
@schildbach could you have a look into it? Thx.