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

Requests for cell tower location are always blocked #1

Open
wsot opened this issue Jan 15, 2013 · 42 comments
Open

Requests for cell tower location are always blocked #1

wsot opened this issue Jan 15, 2013 · 42 comments
Labels

Comments

@wsot
Copy link
Owner

wsot commented Jan 15, 2013

"Issue with pd2.0 1.52.

It is preventing Llama from getting my cell location:
08:55:24.146 PrivacyTelephonyRegistry( 347)
package: com.kebab.Llama blocked for CellLocation

This prevents llama from triggering events because it does not know which area i am in. I have NOT changed any permissions in pd2.0 for the llama app, everything is allowed. Llama stopped working as soon as I installed pd2.0."

It is likely this problem also exists in openpdroid

@oneacl
Copy link

oneacl commented Jan 17, 2013

I can confirm Llama is broken with OpenPDroid.
CollegeDev advised that he fixed the problem in PD2.0 but he never made the changes public.

@CollegeDev
Copy link

Yes, I'm new to github and now learning how to handle with it 👍
As far as I know a good solution how to sync my sources I will offer new fixes and many other things...

@mateor
Copy link
Collaborator

mateor commented Jan 20, 2013

I am not a professional, but I have gotten relatively good with git. Anything you need to know you can ask me, through pm or whatever else. I am going to update those headers with your preferred name today.

We can merge anything you push upwards and thereby preserve your authorship as well. But no matter how it gets to github, we will find a way to preserve your credits for all of your contributions. Let me know if you would like any help.

@oneacl
Copy link

oneacl commented Jan 21, 2013

Any chance this will get pushed into the autopatcher? I'm currently using Llama with android location and it's heavily eating the battery life, previously (with cell towers) it wasn't using any battery at all. Thanks.

@mateor
Copy link
Collaborator

mateor commented Jan 21, 2013

It will, we just need to rebuild. The plan is to keep the master branches of all the OpenPDroid repos, the OpenPDroidPatches and the Auto-Patcher 'latest' smali patches in sync.

So we will be pushing these changes out, and hopefully pretty soon. There have been some upstream changes which are putting a little pressure on us as well. It just takes some time to put together, as we have to build several times over for each romtype everytime we update the smali patches.

edit: and anything specific that the PDroid2.0 application might need can be worked at as well, just with the same limitations in time and the need for all the builds increasing in constant time if PDroid2.0 is not compatible with OpenPdroid. Hopefully we can get to just one core patch and obviate the need for double build requirements.

@wesnm
Copy link

wesnm commented Jan 23, 2013

We're talking about pushing patches, but I don't think this bug has actually been fixed. Has it?

@mateor
Copy link
Collaborator

mateor commented Jan 23, 2013

This is wsot, right?

How about this- recent commits to cm, aokp and pa have broken our build patches as well as the auto-patcher, starting with the SMSDispatcher and continuing to spread to several other classes. The build patches probably no longer apply cleanly (haven't checked recently but if broken it should be the telephony patch) and the auyipatcher patches are 100% broken. They are only still working because of my trickery, but I am getting four-seven questions about this a day.

So we need to rebuild one way or another. I thought that we may as well include the fixes that are already done and counter known issues that generate regular bug reports. If we want to wait a little longer we can. If you have an idea about the SMS bugs or cell tower bug that could be a valid reason, although I wouldn't wait long. Soon my workarounds will fail too. I am also talking with CollegeDev about his sources so a merge with his latest contributions may be soon. But that might be something we do more meticulously, as an official major update as opposed to what I am talking about, which is a bug fix.

Welcome to chasing live development in the upstream branches. They are impossible to predict and break without remorse.

@wsot
Copy link
Owner Author

wsot commented Jan 23, 2013

Sorry all; busy days.
I haven't fixed (read: looked at yet) this issue, because I was working on all the exception handling code, which is now 'done' pending bugs.
I'll get onto this one, and specifically test using Llama.

@mateor
Copy link
Collaborator

mateor commented Jan 23, 2013

I like what you have done with the recent commits, makes a lot of sense.

We should think about (and I don't really know how) migrating these issues over to the OpenPDroid repo. Maybe just go ahead and add the stub to the organization after all. It won't be completely out of place when/if I ever add the autopatcher stub, as a way to keep you informed of what we have going on at ground level.

@phillipberndt
Copy link

On my phone, this bug has worse implications than just Llama not working. I find it also very hard to get a GPS lock when no WIFIs are near by, and I guess this is because the requests by com.google.android.location / com.google.android.apps.maps are also blocked.

Can you estimate when you will find the time to look into this? I am a complete novice in Android core development, but I've tried to do some research nonetheless. If you won't have time to fix this in the foreseeable feature, I will try to setup a build environment and try to fix this on my own.

Here's what I've got so far: Logcat reports

I/PrivacyGSMPhone( 6899): Package: com.android.phone asked for getCellLocation()
[..]
E/PrivacySettingsManager( 2674): getSettings - PrivacySettingsManagerService is null
I/PrivacyTelephonyRegistry( 2674): package: com.google.android.location blocked for CellLocation

As far as I understood the patch, the explaination for the above message must be that in the PrivacyTelephonyRegistry class, the pSetMan member is NULL. Which means that for some reason in the constructor ServiceManager.getService("privacy") returns NULL.

@mateor
Copy link
Collaborator

mateor commented Apr 8, 2013

There is the possibility that this has been fixed but not merged into the main branch. I am working on that update as I can. If you want to be of some assistance, since you reliably have the issue, you could build and test. The jb-mr1-release-pdroid2.0 branch would be the best place to start, and I may even have a second commit to apply to that should it still exist after the test.

@phillipberndt
Copy link

I tried to do that, but it seems that integrating other repositories into Android mods is beyond my current abilities. Sorry!

What I managed to do is to confirm that my above speculation is correct: I applied the current 4.2 patches from OpenPDroid/OpenPDroidPatches to the cm10.1 trunk and then redacted the patch such that in PrivacyTelephonyRegistry the PrivacySettingsManager member pSetMan is initialized in isPackageAllowed rather than in the constructor. (See https://gist.github.com/phillipberndt/5339943) This fixed the bug for me - apps I've authorized have access to CellLocation, others don't. So the problem indeed seems to be that at the time the constructor is called, the privacy service is not available yet. I hope that this information is still of some use to you.

@CollegeDev
Copy link

Hey phillip,

your thoughts are good, but not right at all. The only reason why your code is working is because you're always check for every data access if the pSetMan is null or not and if it is null you're going to connect to the service again. But, it is a "fix" and I'm happy to see it :-). The check for

if (ServiceManager.getService("privacy") != null)

is unnecessary, because the service will be added in system server on boot just before other applications or whatever can access these methods. Furthermore, all privacy classes are preloaded too.

It is an general "android" and PDroid problem with the services. They deconnect (local instance of the service == null) after a while if they're idling or whatever (I've found some hints on the inline documentation from the framework-devs). Furthermore: I've implemented a general fix for that few month ago, but no one took it over to OPD btw (like a lot of other fixes I made)

@mateor
Copy link
Collaborator

mateor commented Apr 8, 2013

Thanks for the gist phillip, and for the review CollegeDev, I agree that all contributions are welcome, even an unfinished fix is better than an untouched problem!

You can see my current progress on the merge at the openpdroid-pdroid2.0-merge branch. If you see above, my first request was for him to build and see if the bug was still present in the pdroid2,0 branch, mostly because I haven't had any testers report on that yet but I remembered you telling me it was resolved.

Send me a line when you have time, okay?

/mateor

@phillipberndt
Copy link

I'm glad to hear that a solution to this bug is known. Thank you guys! I will keep trying to compile a version from that branch and check back on you if I succeed. Still, out of academic interest:

CollegeDev, I believe that your explanation for why my patch is working is not correct. The service reference returned by getService is passed to a newly initiated PrivacySettingsManager and is stored in its service member. It is this locally initiated PrivacySettingsManager which is then stored in pSetMan, not the reference to the service. So my check if pSetMan is null ensures that the member is only initialized once, but that should be about it. Also, If in the version without my patch pSetMan was null, the line

PrivacySettings settings = pSetMan.getSettings(packageName, Process.myUid());

in PrivacyTelephonyRegistry.isPackageAllowed should raise a NullPointerException, but instead logcat shows the error message dropped by pSetMan.getSettings,

if (service != null) {
  return service.getSettings(packageName);
} else {
  Log.e(TAG, "getSettings - PrivacySettingsManagerService is null");
  return null;
}

About that check for getService returning null.. this could be interesting. On my phone, getService does actually return null most times when called in the constructor and only occasionally gets it right. (I tested that by Log.d'ing getService's output, but only with apps which likely start early like Llama and com.google.android.location.) If I get you right, this should not be happening?!

@mateor
Copy link
Collaborator

mateor commented Apr 10, 2013

I am currently going through the entirety of the patches pretty thoroughly as I merge the two forks. Your find seems like something fairly straight forward to check. When I get to that section I will look for your suspicions and see.

Thanks for taking the time, not only for the original gist, but also to explain your thinking. I will report back here later this week.

@CollegeDev
Copy link

Hell yeah, I should have gone to bed instead of writing thi comment... I'm sorry.
I will explain you what I mean:
In this case the pSetMan is a private globale class variable and only init one time. The constructor of the pSetMan is following:

 PrivacySettingsManager(Context context, IPrivacySettingsMangerService service)

Inside the pSetMan we hold an reference to the service with an private globale class variable of IPrivacySettingsManagerService. The problem of all that is (based on experience in ICS) that the reference get lost after a while (thats the pretty logcat message getSettings - PrivacySettingsManagerService is null). For this "problem" I implemented a central reconnector which fixes this bug in a more general way. As I can read in some inline comments inside the framework classses, the framework devs sometimes have the same problem.

In this case, your "bugfix" should work some hours, but after a while it doesn't work anymore until you will restart your phone. If you have time, can you test if I'm right or if android changed something to that in JB?

@phillipberndt
Copy link

Sorry it took me a while, but I wanted to wait a few days to be sure. My SGS2 has been up for 4 days now with my patch in place and the bug did not return: Llama still reacts on cell location changes and also freshly started apps can still get the CellLocation. I can't tell if this is due to a change in JB or solely due to my patch though.

I wonder if those are actually two different bugs? Anyway, even if they are, from the sounds of it your solution will likely resolve the problem I described above as well.

@mateor
Copy link
Collaborator

mateor commented Apr 18, 2013

Thanks phillip. I am going to give your fix a full test myself this week.

@kimboinatl
Copy link

I am also noticing this problem with Tasker (AOKP MR1 build 6 on my VZW Gnex). I did a logcat and I am seeing the following:

04-18 10:50:35.794 V/PDroidAlternative(2224): NotificationHandler: Notification for: com.google.android.location:locationNetwork:0
04-18 10:50:36.223 V/PDroidAlternative(2224): NotificationHandler: Notification for: net.dinglisch.android.taskerm:locationNetwork:0
04-18 10:50:41.270 I/PrivacyCDMALTEPhone(638): Package: com.android.phone asked for getCellLocation()
04-18 10:50:41.270 I/PrivacyTelephonyRegistry(381): package: com.google.android.location blocked for CellLocation

I granted all permissions to both Tasker and Phone.

I guess at this point y'all know about the problem, but I wanted to add this in case it helps at all. I've just started playing around with PDroid so I'm not too familiar with its inner workings, but let me know if there is anything I can test out or do to help.

@mateor
Copy link
Collaborator

mateor commented Apr 18, 2013

I think that this issue has exposed up to three bugs, all of which have fixes in the pipeline. We are working on the update now and are appreciative of anyone who takes the time to file a bug report, especially one with logs.

I will try and remember to post to this issue when I push the update so that you guys can test the fix if able.

@kimboinatl
Copy link

Good deal, I'd be happy to help test.

@uniquePWD
Copy link

Any news on getting this bug fixed? It also causes Foursquare to crash.

@mateor
Copy link
Collaborator

mateor commented May 23, 2013

the fix is in scattered places around the project...it has not yet been merged into even the -devel branches yet. I would expect the upcoming release to have a fix for this issue.

@uniquePWD
Copy link

Awesome, thanks for the update. Is there a workaround that could be done on the end-user end in the meantime? I understand that new versions don't roll out overnight and it would be cool if there was less pressure on you to fix this bug and enable you to have as much time as you require to get the next version ready.

@phillipberndt
Copy link

mateor, towards that workaround: Is PrivacyTelephonyRegistry.smali the same for all mods? (I believe it should, because the class is introduced by the openpdroid patch, but I might misunderstand the inner workings of Dalvik and/or auto_patcher)

If it is, I could use your diff_tools to create an auto_patcher patch to be applied after the openpdroid patch (i.e. I'd diff a mod with openpdroid against the same mod with openpdrod + my patch) which would work for all users, regardless of their mod (version), only depending on them using the same set of openpdroid patches.

Am I right? I'd provide that patch here..

@mateor
Copy link
Collaborator

mateor commented May 24, 2013

Yes it is the same.

I have been looking at that fix and debating this for awhile. My main reluctance to simply shipping it was twofold...CollegeDev's reluctance and then the fact that I wanted any updates to be making progress on supporting PDroid2.0 as well as PDroid Manager. But even though I am getting close to finishing the merge, upon second thought those reasons don't really hold water.

Let's add it...the autopatcher can do this pretty easily I expect. I was about to suggest exactly what you outlined before I went back and read your exact proposal. I will just add that small diff to every shipment.

Thanks for suggesting this, I think it is the right thing to do and probably should have been done earlier.

Also, if anyone is interested, the merge's staging area is here

@phillipberndt
Copy link

sabret00the, if you'd like to test: Have a look at my fork of auto_patcher. (If you haven't used the git version of auto_patcher before note that you'll have to run ./batch.sh and use the auto_patcher from within the archive created by that script.)

I have so far tested the patch with today's nightly of cyanogenmod on my i9100 (sgs 2). The best way to see if it's working IMHO is Llama's history panel: If the patch works, it should show cell numbers. If it did not, you'll see 1-2 correct cell id's and then lots of -1 : -1 : -1 there)

mateor, here's the smali patch: phillipberndt/auto-patcher@db33df8. Worked like a charm. I did not find a mechanism for adding a second patch against the same jar file, so I added one in phillipberndt/auto-patcher@89dc51b. I also fixed what I think were two typos in the diff_tools.

@mateor
Copy link
Collaborator

mateor commented May 26, 2013

Thanks a lot...I will not be able to look at this until tonight, but I will put some time in then.

I have been planning on adding an "incremental" patch set/support to the autopatcher and will probably do so with your patches. When the next OpenPdroid version goes out, I anticipate several waves of bug fixes and updates...hopefully I can use that to avoid rebuilding for every romtype. Your bugfix patch is a good opportunity to soft launch that. I will pull your changes over to a test branch and see what works the best in the short/long term.

Oh, and yeah, the diff_tools are simply not quite up to snuff. The are remnants from ICS, and are out of date in several ways. I spent maybe 15 mins trying to bring them forward for some collaborators, but the collaborators moved on and so i never finished. They are a valuable idea and tool, I appreciate the time you spent looking at them and this patch in general. I will respond again when I have given it a look.

@uniquePWD
Copy link

@phillipberndt there was no AutoPatcher/APG.exe created after running the batch.sh from your branch

@phillipberndt
Copy link

@sabret00the AFAIK, APG is a frontend to auto_patcher and not part of it. Its XDA thread mentions that it's able to use the latest git version of auto_patcher though. Since mateor has already merged my changes into his repo, simply rebuilding might work?!

If you want to try using auto_patcher without APG, try to run the auto_patcher script (within the zip file created by batch.sh) through bash, for example as

bash ./auto_patcher cm-daily.zip openpdroid cm

@lukash-master
Copy link

I've tried to apply patch via APG to my Motorola Milestone 2 CM10.1 rom, but experienced some problems. Please find log here: http://codepad.org/Eq2OdcRJ
Could you please share some information if I am doing anything wrong and what to do to apply this patch to my rom?

@JayXon
Copy link

JayXon commented May 29, 2013

I tried autopatcher 2.9.7 with pa_maguro-3.56-28MAY2013-135145.zip, but still got fc in foursquare.

@mateor
Copy link
Collaborator

mateor commented May 31, 2013

Thanks for the report...I have had the foursquare problems reported. It looks like the GPS problems are maybe half fixed. I have another set of GPS fixes in the queue, I hope to get them out in the auto-patcher soon.

@lukash-master
Copy link

Any news regarding the cell tower location issue? I would love to upgrade my CM rom to the newest version (which includes PDroid) but the cell tower location is crucial for me :).

@phillipberndt
Copy link

With one of the latest versions (I did the last update around end of july, so I can't tell exactly) the cell tower problem resurfaced on my SGS2 for cm-10.1.3-rc2. From a brief look into auto_patcher I think the problem is that while you define $INCREMENTAL, you then do not ever actually use it to apply the patches. Instead, there only is a work-in-progress comment:

# Incremental--use patch_matcher? (how, b/c dry-run...should INCREMENTAL be optional or fail-possible?

I added the incremental patching by copying the Actual application of patches code above the comment and replacing $PATCH_QUEUE by $INCREMENTAL. After building a new patch and installing it onto my phone, the problems were gone again.

One interesting observation is that when I created my patch half a year ago, Llama actually noticed cell changes, but got -1:-1:-1 as the cell id instead of the correct one. With current builds, it does not show those ids in its log anymore. I can't tell whether this is due to a change in Llamas front end or due to changes in Openpdroid though..

(On another note, in the current git version of auto_patcher you execute git log HEAD^1..HEAD >> "$LOG" after everything is finished and abort when this command fails. It actually does fail for shallow repositories cloned with --depth 1, because there is no HEAD^1 there...)

Edit: See phillipberndt/auto-patcher@eb841e8 and phillipberndt/auto-patcher@a190aba

@mateor
Copy link
Collaborator

mateor commented Sep 12, 2013

Thanks for the debugging...there is a lot about the new implementation that is basically temporary...figuring out how to fit it all together took most of the time. I have cherry-picked both your contributions, although as my comment you quoted above indicates, I haven't decided on a long-term strategy with the Incremental fixes. Thanks.

@oneacl
Copy link

oneacl commented Oct 16, 2013

It seems that Tasker is also affected (no cellid is being received) and also the networklocation package from NOGAPPS (http://forum.xda-developers.com/showthread.php?t=1715375). With wi-fi disabled the network location fails, when I enable wi-fi I get a location in a few seconds based on the wi-fi hotspots nearby.

Thanks.

@flimflammachine
Copy link

The lack of location by cell tower is a real problem for me. Is there anyway a noob like me can patch my ROM with one of these experimental patches using the autopatcher?

Massive credit to everybody involved in this project, but this has been going on for ages now, does anybody know when it will be fixed (even using a workaround)?

If I can help using a patch , then please point me in the right direction.

Ta.

@mateor
Copy link
Collaborator

mateor commented Oct 27, 2013

I don't own a device that can test this feature, so the fix will not be coming from me. Pull requests are welcome.

@oneacl
Copy link

oneacl commented Oct 29, 2013

There seems to be a hit and miss in getting this to work by forcing phone.apk and your application by trusted. I've managed to get it to work on my phone (mako running cm10.2) but I had issues in the past with this method.
Others have also reported various success by forcing phone.apk to be trusted (as opposed to "no settings").

Mateor - I thought this applies to all phones/roms?

@Ajex
Copy link

Ajex commented Jan 16, 2014

I have tryed last version of patch, problem is still alive ((( foursquare isn't working (crashing) after patching (cm 10.2 jb 4.3.1). Phone - SII 9100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

13 participants