Skip to content
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

Not showing journey path on map for Nicaragua network #585

Closed
ialokim opened this issue Feb 14, 2019 · 21 comments
Closed

Not showing journey path on map for Nicaragua network #585

ialokim opened this issue Feb 14, 2019 · 21 comments
Assignees
Labels
bug 🐞 A functional defect or unexpected behavior. needs info ℹ️ More information needed,

Comments

@ialokim
Copy link
Collaborator

ialokim commented Feb 14, 2019

Describe the bug
Not sure since when this is happening, but Transportr apparently does not show the journey paths following the roads for Nicaragua anymore, although the data is provided by Navitia.

To Reproduce
Steps to reproduce the behavior:

  1. Select Nicaragua as network provider
  2. Search for a trip from COTRAN Sur to Mercado Mayoreo
  3. Only see a path linking the two stops together instead of following the road.

Expected behavior
The journey path should follow the road as provided by Navitia. I know this has worked some time before in Transportr, not sure when it stopped and what changed back then.

Screenshots
As shown in Transportr:
transportr_1
As provided by Navitia (see Navitia playground):
transportr_2

Versions (please complete the following information):

  • Transportr Version: 2.0.5
@grote
Copy link
Owner

grote commented Feb 14, 2019

Very strange. You could try the first version when Nicaragua was added and see how it behaves there. If the line information is missing there as well, this might be a Navitia issue which is what I suspect, because IMHO the AbstractNavitiaProvider didn't change recently, did it?

@grote grote added bug 🐞 A functional defect or unexpected behavior. needs info ℹ️ More information needed, labels Feb 14, 2019
@ialokim
Copy link
Collaborator Author

ialokim commented Feb 15, 2019

So with version 2.0.0 this still looks like it should:

transportr

@grote
Copy link
Owner

grote commented Feb 15, 2019

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 15, 2019

It was still working with 2.0.4, so if it is was a change on PTE's side, it would have happened between Dec 9, 2018 and Jan 10, 2019.

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 15, 2019

Now that is really strange: It still shows the paths as expected in 2.0.5-debug, but doesn't so in 2.0.5 from your testing repo?

Could there be some setting influencing this?

transportr_2

@grote
Copy link
Owner

grote commented Feb 15, 2019

No, there shouldn't be. If you build a release build from 2.0.5 source, does this change the picture?

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 15, 2019

Yes, indeed. When building a release for 2.0.5, it behaves as the officially shipped version.

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 15, 2019

And when adding

shrinkResources false
minifyEnabled false
multiDexEnabled true

to the release buildType it works again.

@grote
Copy link
Owner

grote commented Feb 15, 2019

Wow crazy s**t! Can you narrow it down to one of this option?

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 15, 2019

Hum, was quite difficult... It works too with

//shrinkResources false
//minifyEnabled false
multiDexEnabled true

but it didn't only with minifyEnabled true. Anyways, it seems shrinkResources true requires minifyEnabled true which in turn requires multiDexEnabled true to build successfully.

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 15, 2019

The release build for 2.0.0 still worked as expected.

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 15, 2019

I have been checking it on another phone with the apks from F-Droid and it worked as expected until 2.0.3, but didn't so starting with 2.0.4. Probably you changed something in the proguard configuration between these versions?

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 17, 2019

It is happening exactly after the PTE version switch from 4354c4a8 to c1c2b1e1 in d2b7d5e.

Not sure if that has something to do with the problem, but there happened this change in the checksums file:

com.github.tony19:logback-android:1.1.1-12:logback-android-1.1.1-12.aar:3102228f0e408e3c003b34e96a604e9b9f59d314dcf8f03aa78d9d3648198932',
-        'com.gitlab.opentransitmap.public-transport-enabler:enabler:4354c4a8:enabler-4354c4a8.jar:1db5cfd805caad04c9728ea9456ac720ba1392a5b435e443e4f2dd65be70997b',
-        'com.gitlab.opentransitmap:public-transport-enabler:4354c4a8:public-transport-enabler-4354c4a8.jar:aab28b1a9798cb2c01a6ca2484949885a9653a068868d727e6d2f293cbbef32a',
+        'com.gitlab.opentransitmap:public-transport-enabler:c1c2b1e1:public-transport-enabler-c1c2b1e1.jar:fdcc25f2c8f6100e4e2bd7040130b427b893e29c0c2cf05c8c909a650ada269c',
         'com.google.android.apps.common.testing.accessibility.framework:accessibility-test-framework:2.0:accessibility-test-framework-2.0.jar:cdf16ef8f5b8023d003ce3cc1b0d51bda737762e2dab2fedf43d1c4292353f7f',

Perhaps PTE has been delivered in another fashion so that it didn't download two JARs anymore and perhaps that could have had some impact on the proguard configuration?

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 17, 2019

Perhaps it could be linked to this change introducing a custom serialization for Trip.path?

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 17, 2019

Indeed, it works now after adding

-keepclassmembers class * implements java.io.Serializable {
	private void writeObject(java.io.ObjectOutputStream);
	private void readObject(java.io.ObjectInputStream);
}

to the proguard rules. See https://www.guardsquare.com/en/products/proguard/manual/examples#serializable

@grote
Copy link
Owner

grote commented Feb 17, 2019

Wow, thanks a lot for this amazing detective work!

Crazy how throwing these methods out doesn't trigger any warning. Do you think we should restrict the proguard rule to the class in question or keep *?

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 17, 2019

Hmmm I was wondering the same, but I think keeping * makes sense as these methods should never been excluded when they are overridden (as there is certainly a reason to implement them). Not sure though if keeping the rule like this or adding all methods proposed by proguard:

     private static final java.io.ObjectStreamField[] serialPersistentFields; 
     private void writeObject(java.io.ObjectOutputStream); 
     private void readObject(java.io.ObjectInputStream); 
     java.lang.Object writeReplace(); 
     java.lang.Object readResolve();

What's your opinion?

@ialokim ialokim self-assigned this Feb 17, 2019
@grote
Copy link
Owner

grote commented Feb 17, 2019

Maybe they are overridden by a class that we don't need and can be purged?

@grote
Copy link
Owner

grote commented Feb 17, 2019

Reading the documentation properly, it seems we can just use the proguard ruled you propose.

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 17, 2019

we can just use the proguard ruled you propose.

Okay, which one do you refer to? The first, minimal one or the second, complete one from the docs?

@grote
Copy link
Owner

grote commented Feb 18, 2019

If the minimal one fixes it, let's use this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 A functional defect or unexpected behavior. needs info ℹ️ More information needed,
Projects
None yet
Development

No branches or pull requests

2 participants