onnela-lab / beiwe-android

Beiwe is a smartphone-based digital phenotyping research platform. This is the Beiwe Android app code. The Beiwe2 app is also available on the Google Play store to use with open source builds of the Beiwe backend.
https://www.beiwe.org/
BSD 3-Clause "New" or "Revised" License
26 stars 16 forks source link

Having trouble getting call logs #89

Closed enwq closed 1 month ago

enwq commented 2 months ago

Hi there. I'm having trouble collecting call logs from the app. I have call logs enabled in study setting and permission granted on the phone. I received a call after the app has been installed but didn't see any data on the backend. When I connected to the debugger it kept logging error message "Received Broadcast: Intent { act=Check Calls flg=0x10 }". The app didn't crash though. I also noticed this message often got logged in the app log, together with some other messages that look like below: image

biblicabeebli commented 2 months ago

The explanaition about what those messages are is that they are statements of times of events as received by the central "BroadcastReceiver" (it's an Android thing) inside the background process of the app. They indicate, for example, that the app has received a signal to start a gyroscope data recording session has started, or that it should stop. You should (and it sounds like you are) be seeing that "Check Calls" at the same time as a call happens.

Regarding call data - it would help if I knew what version of the app are you are running, and if you are accessing the app through a mechanism other than through the apk download link in the bottom right corner of an up-to-date-ish Beiwe server, what build type of the app you are using.

Oh, and OS version, sometimes the device info too but that shouldn't be relevant here.

Finally, are these "normal" phone calls or is there anything else going on, for example do you have a non-default app that you have set as your phone call app.

enwq commented 2 months ago

It looks like "Check Calls" also gets logged even when a call isn't happening. I'm running Beiwe 3.5.11, cloned from this repo and built with the release variant. I made some changes to the mechanism of recording GPS data but I don't think it matters here. The device is a Oneplus 7 pro running Android 12. For the calls, the device has a sim card with a non-US number and received calls also from a non-US number. I will test with calls between US numbers this afternoon.

enwq commented 2 months ago

Yes I tried calling between US numbers on a pixel running Android 14, still couldn't get the call logs... And I'm pretty sure I was using the default phone app.

biblicabeebli commented 2 months ago

Well, that's a version of the app and OS that really should both work. Buuut it isn't. Looks like that "Check Calls" broadcast is just a trigger to make sure the call logging code is actually enabled.

I suppose there could be a permission management bug - there was some code churn over there for Android 12 and 13 - but you said the permissions are set, and there's no ambiguity when you ~check the box in settings. (and we would have heard about it if this was affecting more people.) So it's proOoOobably a device or manufacturer specific issue?

@enwq One thing for you to confirm, on thing for you to try: 1) Can you confirm that other data streams are available and downloadable from your Beiwe server instance? 2) Based on a very cursory review online, it looks like the Oneplus 7 pro has/supports dual sim. Assuming this means dual physical sim slots, could you try swapping to the other one?

Unfortunately sometimes Android is like this. Manufacturer Y does something slightly weird with Feature X and Thing Z in the Beiwe app magically breaks. Our app waits for the device to tell it that there is an update in the call history database, we run a very broad query to look for new items, but if the query doesn't find anything we are out of luck and without recourse.

I will be swapping to some Android dev for some unrelated updates and if I have time I will try to review the code's behavior more fully and update the developer documentation. The code in there is pretty old. I will also check with my coworker who helps manage studies whether there are any other reports of this kind of problem.

enwq commented 2 months ago

@biblicabeebli Yes I can confirm other data streams are available and downloadable.

I will try swapping the sim on that Oneplus 7 pro. However, this problem also occurred on a Google Pixel so it might not be a manufacturer specific problem.

On the Oneplus 7 pro I got a missed call last night, and when I check the app permission setting I do see it says call log and phone accessed in the past 24 hours. However on the backend I didn't get any data.

enwq commented 2 months ago

@biblicabeebli Okay I did some further investigation. The problem is in PermissionHandler.getNextPermission, the permission CALL_PHONE is only returned when the call clinician button and call research assistant button are enabled. However, in my study I have both buttons disabled, so this permission is never requested.

  if ((PersistentData.getCallClinicianButtonEnabled() || PersistentData.getCallResearchAssistantButtonEnabled())
                && !checkAccessCallPhone(context)) {
            return Manifest.permission.CALL_PHONE
        }

I think I can just move this permission request to the part where all call related permissions are requested.

 if (PersistentData.getCallsEnabled()) {
            if (!checkAccessReadPhoneState(context)) return Manifest.permission.READ_PHONE_STATE
            if (!checkAccessReadCallLog(context)) return Manifest.permission.READ_CALL_LOG
        }
biblicabeebli commented 2 months ago

Good sleuthing, but no the CALL_PHONE permission is about allowing the app to initiate phone calls. https://developer.android.com/reference/android/Manifest.permission#CALL_PHONE The one to access the call history information is READ_CALL_LOG

The permissions for these phone related options is merged together into one UI checkbox in app permissions. Checking the phone call permissions in app settings (should) grant the all the permissions stated in the app manifest in app/src/main/AndroidManifest.xml.


starts typing some a comment about how you should ignore app/src/textsAndCallsStats/AndroidManifest.xml because it is irrelevant


The problem is that the contents of /app/src/textsAndCallsStats/AndroidManifest.xml never got merged into app/src/main/AndroidManifest.xml

Oops.

Working on a fix.

@hydawo We seem to have a big bug, happy monday!

biblicabeebli commented 2 months ago

@enwq could you try this build of the app? it should install just fine. Beiwe-3.5.12-commStatsCustomUrl-release.zip

All I've done is added the permissions from the old build variant to the general manifest. I have done zero testing, I'm over in iOS App dev land and don't have my android test device available at the moment, this is a quick and dirty change to see if it solves the bug or not. We will have to test a clean install and registration before we can push this to the target of the download link.

But also I suspect the bug here is actually a missed detail in our deploy script after removing the app store build?, maybe I removed the directive to use the commStatsCustomUrl build assuming I had correctly transferred permissions? Looks like my ide was set to commStatsCustomUrl mode, but there was also an ide update so who know.

biblicabeebli commented 2 months ago

that version number is wrong, ignore the version number.

@enwq could you provide me with the exact Beiwe website you downloaded the app from? you should have gotten 3.5.12, not 3.5.11. I'm wondering how that happened.

enwq commented 2 months ago

@biblicabeebli I cloned this repo when 3.5.12 wasn't out yet, and made some changes in the 3.5.11 code to fit our study. Then we will distribute this customized version as an apk file to our participants.

Regarding the permission issue, in MainService.kt a CallLogger is only initiated when confirmCalls(applicationContext) returns true https://github.com/onnela-lab/beiwe-android/blob/dd6e7c36ab5edf888ca8bd1a2c7e1847442b3c24/app/src/main/java/org/beiwe/app/MainService.kt#L148C8-L149C30

https://github.com/onnela-lab/beiwe-android/blob/dd6e7c36ab5edf888ca8bd1a2c7e1847442b3c24/app/src/main/java/org/beiwe/app/MainService.kt#L396C16-L397C38

Then in PermissionHandler.kt, confirmCalls checks PersistentData.getCallsEnabled() && checkCallsPermissions(context). https://github.com/onnela-lab/beiwe-android/blob/dd6e7c36ab5edf888ca8bd1a2c7e1847442b3c24/app/src/main/java/org/beiwe/app/PermissionHandler.kt#L226C5-L228C6

checkCallsPermissions checks 3 things: READ_PHONE_STATE, CALL_PHONE, and READ_CALL_LOG. https://github.com/onnela-lab/beiwe-android/blob/dd6e7c36ab5edf888ca8bd1a2c7e1847442b3c24/app/src/main/java/org/beiwe/app/PermissionHandler.kt#L197C4-L199C6

These permissions are requested in getNextPermission. READ_PHONE_STATE and READ_CALL_LOG are requested as long as we are requesting call data. https://github.com/onnela-lab/beiwe-android/blob/dd6e7c36ab5edf888ca8bd1a2c7e1847442b3c24/app/src/main/java/org/beiwe/app/PermissionHandler.kt#L283C8-L286C10

However, CALL_PHONE is requested when the call clinician or call research button is enabled. https://github.com/onnela-lab/beiwe-android/blob/dd6e7c36ab5edf888ca8bd1a2c7e1847442b3c24/app/src/main/java/org/beiwe/app/PermissionHandler.kt#L298C1-L301C10

So in my case when I disable both buttons, CALL_PHONE permission is never requested. As a result checkCallsPermissions returns false and CallLogger is never initiated.

So to fix this I just requested CALL_PHONEpermission if we want to get call data. I tested on my device and the app did collect call logs.

        if (PersistentData.getCallsEnabled()) {
            if (!checkAccessReadPhoneState(context)) return Manifest.permission.READ_PHONE_STATE
            if (!checkAccessReadCallLog(context)) return Manifest.permission.READ_CALL_LOG
            if (!checkAccessCallPhone(context)) return Manifest.permission.CALL_PHONE
        }
biblicabeebli commented 2 months ago

Apologies for the delay in responding, your comment was very thorough and I needed to go set up a device to manually run through the full scenario.

I was incorrect about the source of the bug being the location of the app build permissions - I tested that and it leads to the expected behavior of the app always pushing you back to the app permissions.

I got sidetracked thinking it was a reproduceable version of that-bug-where-android-just-lies-about-the-permissions-you-have-been-granted (it's a thing, it has been around since they introduced this permissions architecture in Android 6) but after poking around and confusing myself I eventually remembered that that fun bug crashes the app because you end up calling illegal code.

Your change does work, but in my opinion the correct change is a modification to either confirmCalls on the code that calls it over in the main service.

// logic for the call (metadata) logger
if (broadcastAction == applicationContext.getString(R.string.check_for_calls_enabled)) {
    if (confirmCalls(applicationContext)) {   // This should return true when  call_phone  is not enabled.
        startCallLogger()
    } else if (PersistentData.getCallsEnabled())
        timer!!.setupExactSingleAlarm(30000L, Timer.checkForCallsEnabledIntent)
}

I suspect the origin of this is 1) almost all of the studies we run use that phone call feature (might be an IRB thing for us) 2) "checkCalls" is ambiguous in it's name about whether that means dispatching calls or tracking calls. This is exactly the kind of mistake I would make - naming something wrong and then 5 years of refactors later a literal reading of a name, or a misread of two similar things as identical and then removing the "duplicate"... - anyway. I follow this now and can make a decision on the best fix.

I will also try to take a look at your fork, maybe there are features or other improvements to merge in, and I don't want to diverge too much in this particular fix for you.

(This is weirdly coming at the end of my day, I'll try to have a fix available on our main branch and beiwe servers.... let's just say maybe friday eastern time and not get ahead of ourselves....)

enwq commented 2 months ago

Yes I agree modifying confirmCalls or checkCallsPermissions is a better solution for the public app. But at least for our study we are pretty sure we won't have the two phone call buttons enabled and we will only distribute the app to our participants so our fix is working in this case.

Regarding our app, we host it in our organization repo so you won't be able to see the changes we made. Specifically we modified the GPS listener. Beiwe is using GPS_PROVIDER plus a duty cycle to sense locations. However we prefer a more continuous sensing scheme. At the same time we don't want it to consume too much power.

So we use the FUSED_PROVIDER (added in Android 12) with QUALITY_BALANCED_POWER_ACCURACY as the location provider and configured the GPS OFF duration to 1. In this case it collects location data more continuously while mostly relying on Wi-Fi and cellular signals. Location accuracy is usually 10-100 meters which is okay for our study since we don't really need location samples with GPS-level accuracy.

I believe this FUSED_PROVIDER is mostly the same as Fused Location Provider API from Google Play Services, which can be used for devices earlier than Android 12. Looks like Google decided to add some contents from this API to the native Android locationManager for Android 12. In our study we simply limit participants to those running Android 12 and above.

biblicabeebli commented 2 months ago

I would be very interested to see that code, as you can probably tell it has been a while since the android app had a tech update, and that sounds like it would save on battery life.

enwq commented 2 months ago

I would be very interested to see that code, as you can probably tell it has been a while since the android app had a tech update, and that sounds like it would save on battery life.

Sure here's the code.

        // Create location request
        val locationRequestBuilder = LocationRequest.Builder(10000)
        locationRequestBuilder.setQuality(QUALITY_BALANCED_POWER_ACCURACY)
        locationRequestBuilder.setMinUpdateIntervalMillis(1000)
        val locationRequest = locationRequestBuilder.build()

        // Initialize a custom executor
        val executor = Executors.newSingleThreadExecutor()

        if (finePermissible && coarsePermissible && locationManager.hasProvider("fused")){
            locationManager.requestLocationUpdates(LocationManager.FUSED_PROVIDER,locationRequest,executor,this)
        }
biblicabeebli commented 2 months ago

@enwq I want to bring your attention to this issue I just created, I believe you are missing push notification credentials to allow the ios app to work on your studies (you may just be doing android though, I don't know.

enwq commented 2 months ago

@enwq I want to bring your attention to this issue I just created, I believe you are missing push notification credentials to allow the ios app to work on your studies (you may just be doing android though, I don't know.

Yes we are only using Beiwe for Android users. We built an app ourselves for iOS. I will close this issue for now, and thank you so much for your help on the Android app!

biblicabeebli commented 1 month ago

I'm reopening this issue because I still need to get the change into the "official" Beiwe app

biblicabeebli commented 1 month ago

Issue resolved in commit a874cf3, I renamed the functions so that call logging vs can make calls was no longer ambiguously named, then removed the suddenly very visible bad permissions check inside the newly named checkCallLoggingPermissions function.

@enwq thank you once again for your help. There are several fixes in the version 3.6.0 rc I'm putting up in the 3.6.0 "issue"

The current description of changes in the build.gradle file, if you want to merge them in to your fork, is:

84: 3.6.0 - Adds new message receiving push notification feature
    Adds app-to-server heartbeat.
    Adds several extra datapoints of app activity like last upload attempt.
    Fixes bug where fcm token was gated by the no-cellular-upload flag.
    Adds an real-time-ish check to the main menu that updates the list of surveys to reflect the
      status of active push notifications.
    Fixes bug where the back stack was incorrectly updated after a survey was completed causing the
      Main Menu activity to be on the back stack twice, which also caused the button state to be wrong.
    Fixes a bug where the permissions check for the call log data stream was erroneously checking
      whether the ability to make a phone call was granted.