chiehmin / Xposed-NetworkSpeedIndicator

Network Speed Indicator
66 stars 29 forks source link

D858HK (G3 Variant) not working for some reason #97

Open prettyvoid opened 9 years ago

prettyvoid commented 9 years ago

Hello,

Loved this module on my G2 but I recently got a D858HK and the network info is not appearing any where.

Here is the log: http://pastebin.com/mzqRSEDS

I tried all placement positions.

The log looks lackluster, only one line about network indicator module being loaded. Does that look right? Can you give some insight? I've read a lot of people with G3 that says it's working fine.

Thank you :)

ADTC commented 9 years ago

The one line in this Xposed log indicates the module is loading correctly. Look for logs in sdcard/NetworkSpeedIndicator.

prettyvoid commented 9 years ago

screenshot_2015-03-13-21-35-44 Thanks for the clarification

http://pastebin.com/rJTPa2PA

Everything seems happy in the log, no?

ADTC commented 9 years ago

No, the log indicates you are changing the settings, but the actual module itself is not running. If you compare the log from G2, you will know what I mean.

This happens a lot, actually, and I don't know why. Xposed successfully loads the module. The app can change settings, but the module has crashed or got stuck somewhere and is not running.

It's a common issue that I currently don't have a solution for. You're welcome to help debug!

prettyvoid commented 9 years ago

Do you reckon that's related only to Xposed, or could it be related to the system image/rom?

I'll give it some more time soon and if I figure out anything I'll let you know. Thanks for the hard work :+1:

ADTC commented 9 years ago

There were reports from users of other ROMs, so probably not a ROM issue.

prettyvoid commented 9 years ago

It's just a bit weird that the problem seems to only occur with Network Speed Indicator, I've tried several other modules and they seemed to function. Let's hope we'll find a solution for this!

prettyvoid commented 9 years ago

Maybe this is not the smartest debugging approach but it might tell you something, I've tried many versions and everything above 0.5 doesn't work. 0.5 works but it comes with problems, like placement have a large offset.

See these screenshots: http://i.imgur.com/KrJm7gy.png http://i.imgur.com/bu5IJjz.png http://i.imgur.com/sEs0PKm.png

So I suppose at this step (0.6) "Compatibility: Should work for all the devices higher than Android 2.3 now" something back-fired (at least for some devices).

edit: I flashed a new stock rom and the same problem still occurs. The placement offset I mentioned above only happens if the stock DPI is changed.

prettyvoid commented 9 years ago

@ADTC I'd love to debug this but I'm not being able to get a working copy of the project. I understand Eclipse was used to build this project but when I try to import this repo from Eclipse, I get 3 projects (Network Speed, Color Picker, Multi list) and I get a whole lot of problems. (The problems are mainly due to the 2 external libraries included in this repo)

I had my fair share of Android development I'm just not experienced a lot with Git and Eclipse. I know it's not your responsibility but if you can help with getting the project working on my machine I'd pretty much appreciate it, I do have Eclipse/Android Studio/android sdk installed. I just can't seem to import this repo correctly to Eclipse.

Thanks,

ADTC commented 9 years ago

I had made a guide here to get the project to compile in Eclipse. It is in the wiki. Have you taken a look? Please look at it first and let me know if you still face problems.

Also, yes there will be three projects. Two of them are libraries used by the app. One library is for multiselect and the other is for color picker.

ADTC commented 9 years ago

This is it, BTW:

https://github.com/chiehmin/Xposed-NetworkSpeedIndicator/wiki/Libraries-Used

Thank you for helping us out! I will definitely help you get it set up, as we need more contributors like you who can debug the code and provide fixes!

prettyvoid commented 9 years ago

I tried to clone the repo recursively and I got the whole folder with the libraries intact, then I tried to add it into eclipse, I got 1 project with all the folders. Then the ide complained about the imports for the Color picker and the multiselect list and I can't seem to fix them. I'm not really familiar with eclipse much.

I understand from that guide if I clone recursively I don't have to do the steps below. Is 1 project right? Or should I have them as 3 separate projects? Mind telling me what options to choose after recursive clone?

ADTC commented 9 years ago

Actually if you clone recursively, you only save the download step (since the recursive command automatically downloads everything). You still need to import the projects in Eclipse. I hope the following will make it clearer.

If you right-click an empty space in the Project Explorer, you can find the Import command in the menu. Use it to import the two projects as follows.

For ColorPickerPreference: Import the Android project from the NetworkSpeedIndicator\android-ColorPickerPreference folder (disable Copy projects into workspace; change the name from Test to ColorPickerLibrary [exactly this, no spaces, not colour] in the Import dialog). Note: After import, it may not compile as it requires API Level 16. You may either install Level 16 SDK or simply change the project properties to use Level 19.

For mulsellistprefcompat: Import the Android project in NetworkSpeedIndicator\mulsellistprefcompat\lib (disable Copy projects into workspace).

In the end, there should be three projects. The above two library projects, and the app itself. Note that everything will be inside one folder in your Eclipse workspace (since you disabled the Copy projects into workspace option).

Let me know if you have figured it out or still facing problem :smile:

ADTC commented 9 years ago

I need to write a proper import guide. That wiki is severely lacking, as I realize now :frowning:

PS: Please be sure not to commit changes you make in the project to make the project compile on your computer. You can configure your local Git to ignore them or simply ignore them manually when staging the actual code changes you want to push.

prettyvoid commented 9 years ago

Ok I finally managed to compile it and run it successfully on my device. Here is some notes to anyone that reads this in the future:

It can be done with only one 'Import' command, after recursively cloning the repo, go into eclipse, import existing android code into workspace, choose the clone folder, you'll get 4 projects in the list box (Network Speed, Multi select list, color picker and color picker example), you can un-tick the example project, rename (within the import dialog) the Test project to ColorPickerLibrary, un-tick Copy into workspace and finish the import process.

After this you'll end up with 3 projects inside your workspace, do a clean and a build, the IDE might be complaining about the 'switch' case inside MultiSelectListPreferenceCompat.java @ Line 67 (case expressions must be constant expression), an easy fix is to change the switch statement into if-else (for more details about this and why it happens click).

Now you should have a solution that compiles and runs happily.

One last question ADTC, is there an easy way to reflect code changes on device while working with xposed modules, or do I have to restart my phone every time I want to see the changes I've done (oh the nightmare)?

Thanks for the help ADTC :+1:

ADTC commented 9 years ago

I'm glad it worked out for you. Did you follow my import instructions, or did you figure it out on your own (before I even wrote them)?

I don't remember having to change the switch-case to if-else, but maybe I did. It was a long time ago. OR maybe the project is compiled in API level 11, which supported switch-case properly.

If you make changes in the Settings section of the app (the list of settings shown when you launch the app icon), you don't need to restart your phone. If you make changes to the actual module (the parts of the app that control what is shown in the notification bar at the top), then you need to restart the phone after update.

Sadly this is a requirement for any Xposed module and there's no way around it, as the framework only loads the module at boot time, and cannot re-load a changed module. I try to make it less painful by pushing the test build through ADB, but that's a whole other discussion.

prettyvoid commented 9 years ago

I've read your earlier post before I tried to do the import and it helped but I agree that wiki is a bit confusing for someone that's unfamiliar. I was and still confused about having the files of Multiselect list + ColorPicker inside SpeedIndicator project. Because they're already 2 separate projects inside the workspace, they only need to be referenced in the build path (not physically reside inside NetworkSpeed project). But anyway, I'm happy as long as it's working correctly.

Here is how a screenshot of the package explorer: http://i.imgur.com/82FtyUV.png

Regarding code modifications, ya I was concerned about code directly related to the module and not the options ui.. I imagine how can this be very painful to someone that just began experimenting/developing an xposed module.

I guess it's time to dive into this. I'm thinking about beginning with some code reading (which should make life easier), specifically comparing v0.5 to 1.0 b2. This is because as I stated before, 0.5 works fine.

Regards,

ADTC commented 9 years ago

You may want to focus on the changes between 0.5 and 0.6. All versions are tagged, and you can see with git tags. You can also check out the tags to poke around. There are some "binary searching" ways in git to narrow down the exact commit that broke the app for you. I forgot the exact git command for it but I'm sure you can Google it up.

prettyvoid commented 9 years ago

Yeah alright, sound idea. 0.6 is not tagged by the way, I guess 0.7.2 will do.

prettyvoid commented 9 years ago

I still haven't reached a solution but at least I know the cause of the problem now.

handleLayoutInflated for status_bar never gets called on my G3 for some reason, it gets called on my G2. Do you reckon there would be differences between both devices SystemUI package? I highly doubt that!

XResources res is never null on G3, so that's not the cause of handleLayoutInflated not being fired.

0.5 works on G3 because TrafficView is created inside handleLoadPackage makeStatusBarView afterHooked, which gets fired properly.

ADTC commented 9 years ago

0.5 works on G3 because TrafficView is created inside handleLoadPackage makeStatusBarView afterHooked, which gets fired properly.

Can you point me what changed in later version that broke the app in G3?

Do you reckon there would be differences between both devices SystemUI package? I highly doubt that!

Actually it is highly likely that the differences in SystemUI package are breaking the app. That's why you have different implementations for Gingerbread, KitKat, etc. I have digged a little into the actual source code of various ROM bases (from Google's Android repositories) and they are already very different from version to version and model to model.

This causes two problems: When the app is not designed to handle a specific implementation of SystemUI, the meter will never appear. Secondly, the app can simply error out (or rather crash with a cast exception) because none of the implementations known to the app as of now match the SystemUI implementation used in your phone. The settings panel of the app will continue working because it's actually a completely separate part of the app (the module part runs separately like a service and simply picks up the changes made in the settings).

This is a mind-numbing task, especially if you want to support all the different Android-based ROM implementations out there. Every manufacturer or ROM designer wants to add his own tweaks on top of vanilla Android, and to achieve this, they may modify the layout of SystemUI to their liking, while (mostly) still keeping the general look and feel as shown to the end user to be the same. In the end, developers decide to support only the most popular devices to cover as much market share as they can - it's the 80/20 rule.

PS: This is not to discourage you or to say it won't be fixed. The good thing about open source is that if you have a device where it doesn't work, you can debug and propose a fix. As explained below...

I still haven't reached a solution but at least I know the cause of the problem now.

Thank you for your findings, it's good that you can do this and help figure out what's wrong and more importantly, you can test it in your devices!!! (something I could never do). I will take your hints and look over the code once again when I get time (having some personal matters and other work stuff so time is quite tight right now). But you can try to fix it yourself. If you can get the source code base of your G3 ROM, you can dig into SystemUI and figure out why the module is failing.

ADTC commented 9 years ago

Btw, I took a quick look and this looks more like an Xposed version conflict. Do you have the same Xposed library version installed in both phones?

prettyvoid commented 9 years ago

Yeah I agree with all of what you said.

Can you point me what changed in later version that broke the app in G3?

As I already mentioned, in 0.5 TrafficView was getting initialized (and configured) inside XposedHelpers.findAndHookMethod("com.android.systemui.statusbar.phone.PhoneStatusBar", lpparam.classLoader, "makeStatusBarView", new XC_MethodHook() afterHookedMethod

This method properly fires on G3. In later versions the initialization and configuration of TrafficView is moved (and the implementation got changed too) to handleLayoutInflated, which doesn't even fire on G3.

I've tried to decompile LGSystemUI.apk but it's not working, the apk is very different than all other apks i've looked at.

Regarding xposed versions, I had this in my mind but I ignored it. Both devices have different versions, I can't put the same version on both devices now because 2 days ago I updated my G3 to Lollipop and the Xposed Lollipop version is different from Kitkat one. However Lollipop is not the reason this is not working, because the module didn't work on KitKat too... but ya it could be because of the xposed version, just can't verify that after updating to L.

If it's really related to the xposed version then maybe I should report this on xposed page, but now I'm starting to believe that the cause is like you said, differences in SystemUI.

ADTC commented 9 years ago

About the Xposed version, it may not be a bug, but simply incompatibility with the version used. Since 0.5 used an old API and that was removed in 0.7.2. So, I have an idea - bring back the removed old API use. You can try it and see if it works.

Move the code that creates TrafficView into its own method (taking care of dependencies required). Then re-instate the deleted handleLoadPackage but replace the legacy code creating TrafficView with a call to the new method.

The new method will be called by both handleLoadPackage and handleLayoutInflated. You may also need to update the rest of the legacy code to reflect the new code in handleLayoutInflated or move them to similar shared methods.

If you don't get what I mean, just give me some time and I'll do it myself to give you a test version.

PS: The change should also be tested in 1.0b2+ over latest commit.

prettyvoid commented 9 years ago

I tried to replace Module.java with the one from 0.5 and I also brought back LegacyPositionCallbackImpl.java but the module still didn't work. I know what I did doesn't seem proper, if you get the time and do the modifications I'd be happy to test it.

I'm still interested why the current commit doesn't work on G3, if you know of a way to properly decompile my SystemUI please let me know.

I mean of course we can do specific modifications so the module works on G3, but it would be better if we can figure the reason why handleLayoutInflated is not being triggered as it might help a lot of other people too.

Thanks for all the help, this have been a fun and an informative ride

prettyvoid commented 9 years ago

Managed to get the latest version working by doing something similar to what you described. I copied Module.java code from 0.5 into latest version and I also brought back the Legacy positioning apis, plugin is working, however with the positioning woes that I mentioned way up in this thread. Now maybe if somehow the old TrafficView creation method that was used in 0.5 is ported to 1.02 (of course without sacrificing any of the latest functionality) then a lot of other people that complained about this module is not working for them, will be happy again.

ADTC commented 9 years ago

Please commit your changes into a new branch but don't open a PR. I'll take a look in your fork. Thanks.

git stash
git checkout -b issue-97
git stash pop
git add --all
git commit
# Enter commit message, save and close
git push
ADTC commented 9 years ago

Please write a good commit message. Wrapping the paragraphs at 72 chars is standard practice. You can also use the GitHub app.

Short summary in first line
<blank second line>
Longer descriptions and paragraphs from third line.
prettyvoid commented 9 years ago

Ok done, sorry for any hiccups, I'm rusty with Git.