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

Implement missed call notifications #253

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Grafcube
Copy link

@Grafcube Grafcube commented Oct 1, 2024

What is it?

  • Bugfix
  • Feature
  • Codebase improvement

Description of the changes in your PR

  • Implement BroadcastReceiver on a new class MissedCallReceiver. This way, the system still manages missed calls but Fossify handles the broadcasts instead of the pre-installed dialer app.
  • Additional actions include "call back" and "message".
  • Clicking the notification will open the app.

Fixes the following issue(s)

Acknowledgement

Copy link
Member

@Aga-C Aga-C left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! It's an important feature, and I'm happy you did it.

I have two comments regarding how the notification works:

  1. When I tap "Call back" or "Message", the notification doesn't disappear. Tapping these buttons should also remove notification, the same as entering it. It should be fixed before merging.
  2. A nice to have feature would be, if we could merge all notifications into one, so there's less clutter in notification space. But if you don't know how to do it, that's not a problem.

Ideally, the notifications should be working in the same way as in Google's Phone, as it's a reference app for many users.

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
@Grafcube
Copy link
Author

Grafcube commented Oct 2, 2024

I've pushed all the requested changes. Let me know if everything works!

@Aga-C
Copy link
Member

Aga-C commented Oct 2, 2024

There are few bugs:

  • Notification is set as silent and there's no way to change it. Because of it, the notification is not visible on the lock screen. Google Phone's missed call notification isn't a silent one, and it's visible on a lock screen.
  • When there are multiple missed calls, you can't call to a specific number. It calls a random one from the list. The same is for messages.
  • When you want to call or send a message, notification drawer doesn't hide after tapping the button
  • After calling or sending a message, the empty group stays in the notification drawer.

@Grafcube
Copy link
Author

Grafcube commented Oct 2, 2024

* Notification is set as silent and there's no way to change it. Because of it, the notification is not visible on the lock screen. Google Phone's missed call notification isn't a silent one, and it's visible on a lock screen.

For some reason, I'm not able to fix this. I reinstalled the app, changed the importance and tried deleting and recreating the channel but alert/silent and show as popup stayed greyed out (for all channels, not just missed calls).

* When there are multiple missed calls, you can't call to a specific number. It calls a random one from the list. The same is for messages.

It seems to be using the most recent missed call. I tried to fix it by creating an activity but unfortunately I couldn't figure out how to make this work properly. I don't have too much experience with Android dev.

@Grafcube
Copy link
Author

Grafcube commented Oct 5, 2024

When there are multiple missed calls, you can't call to a specific number. It calls a random one from the list. The same is for messages.

I've fixed this in the latest commit. I had misunderstood how PendingIntent works.

When you want to call or send a message, notification drawer doesn't hide after tapping the button

After looking at a lot of documentation and forum threads, I don't think this is possible to do via code. The system is supposed to automatically close the panel when another activity starts but it doesn't for some reason and the older methods are deprecated. I think this is a separate issue and if there is a solution, I couldn't find one.

After calling or sending a message, the empty group stays in the notification drawer.

I couldn't reproduce this. The system properly clears the empty group automatically.

Notification is set as silent and there's no way to change it. Because of it, the notification is not visible on the lock screen. Google Phone's missed call notification isn't a silent one, and it's visible on a lock screen.

I think this is a separate issue because the same thing appears to happen for the other channels (call_notification_channel) on the master branch. The stable release doesn't seem to have this issue.

Please let me know if there are any other bugs I missed!

@Goodwy
Copy link

Goodwy commented Oct 17, 2024

I was just trying to figure out how to implement missed call notifications so that the standard application doesn't duplicate this notification. It turned out that you just need to intercept SHOW_MISSED_CALLS_NOTIFICATION. Thanks.

After looking at a lot of documentation and forum threads, I don't think this is possible to do via code. The system is supposed to automatically close the panel when another activity starts but it doesn't for some reason and the older methods are deprecated. I think this is a separate issue and if there is a solution, I couldn't find one.

I found a solution to this problem.
It turns out that if you use PendingIntent.getBroadcast(...) in addAction() then notifications are not closed, but if you use PendingIntent.getActivity(...) then they are closed.

@Grafcube
Copy link
Author

Grafcube commented Nov 1, 2024

I found a solution to this problem.
It turns out that if you use PendingIntent.getBroadcast(...) in addAction() then notifications are not closed, but if you use PendingIntent.getActivity(...) then they are closed.

Thanks for the tip! It's fixed in the latest commit on this branch.

This PR is pretty much complete. The notification channels in the latest stable release are also greyed out so it wasn't because of this PR. I'll make a separate issue on that if it doesn't already exist.

@Goodwy
Copy link

Goodwy commented Nov 1, 2024

I failed to bind the call back button and the message button to the same activation, I had to make a separate activation for each action. Otherwise the first action from the list will always be executed, in this case MISSED_CALL_BACK.
The code PendingIntent.getActivity(...) is executed when the notification is created, not when the button is clicked. And it turns out that when an activity is launched, it is passed both the callback intention and the message intention, and the first one is executed accordingly.

@Grafcube
Copy link
Author

Grafcube commented Nov 1, 2024

The code PendingIntent.getActivity(...) is executed when the notification is created, not when the button is clicked. And it turns out that when an activity is launched, it is passed both the callback intention and the message intention, and the first one is executed accordingly.

I'm not sure what you mean. It executed the correct intent when I tested it.

@Aga-C
Copy link
Member

Aga-C commented Nov 2, 2024

I've retested it:

  • Empty group still stays in notification drawer. I can always reproduce it - have missed calls from few contacts, call back to each of them, and empty group will stay.
  • If the number is stored in private contacts, the contact name is not shown. It's shown in incoming call notification.
  • If the contact has multiple numbers, the notification should show the number type. It's also shown in incoming call notification.

@Grafcube
Copy link
Author

Grafcube commented Nov 4, 2024

  • Empty group still stays in notification drawer. I can always reproduce it - have missed calls from few contacts, call back to each of them, and empty group will stay.

I made some changes but since I can't reproduce the issue, I don't know if it works.

  • If the contact has multiple numbers, the notification should show the number type. It's also shown in incoming call notification.

Implemented in the latest commit.

  • If the number is stored in private contacts, the contact name is not shown. It's shown in incoming call notification.

I don't know how to do that. I don't use private contacts. I think this should be a separate issue. I have been using the patched version on my phone and so far it's been working well.

@Aga-C
Copy link
Member

Aga-C commented Nov 4, 2024

I made some changes but since I can't reproduce the issue, I don't know if it works.

Now it doesn't work at all. Not only group doesn't hide, but even notification doesn't hide after doing any action. App throws an exception, so it's even impossible to call back. Here's the logcat:

FATAL EXCEPTION: main
Process: org.fossify.phone.debug, PID: 12470
java.lang.RuntimeException: Unable to start activity ComponentInfo{org.fossify.phone.debug/org.fossify.phone.activities.MissedCallNotificationActivity}: android.content.ActivityNotFoundException: Unable to find explicit activity class {org.fossify.phone.debug/org.fossify.phone.receivers.MissedCallReceiver}; have you declared this activity in your AndroidManifest.xml, or does your intent not match its declared ?
	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:4048)
	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:4235)
	at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:112)
	at android.app.servertransaction.TransactionExecutor.executeNonLifecycleItem(TransactionExecutor.java:174)
	at android.app.servertransaction.TransactionExecutor.executeTransactionItems(TransactionExecutor.java:109)
	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:81)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2636)
	at android.os.Handler.dispatchMessage(Handler.java:107)
	at android.os.Looper.loopOnce(Looper.java:232)
	at android.os.Looper.loop(Looper.java:317)
	at android.app.ActivityThread.main(ActivityThread.java:8705)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:580)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:886)
Caused by: android.content.ActivityNotFoundException: Unable to find explicit activity class {org.fossify.phone.debug/org.fossify.phone.receivers.MissedCallReceiver}; have you declared this activity in your AndroidManifest.xml, or does your intent not match its declared ?
	at android.app.Instrumentation.checkStartActivityResult(Instrumentation.java:2427)
	at android.app.Instrumentation.execStartActivity(Instrumentation.java:2005)
	at android.app.Activity.startActivityForResult(Activity.java:5874)
	at android.app.Activity.startActivityForResult(Activity.java:5832)
	at android.app.Activity.startActivity(Activity.java:6329)
	at android.app.Activity.startActivity(Activity.java:6296)
	at org.fossify.phone.activities.MissedCallNotificationActivity.onCreate(MissedCallNotificationActivity.kt:37)
	at android.app.Activity.performCreate(Activity.java:9002)
	at android.app.Activity.performCreate(Activity.java:8980)
	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1526)
	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:4030)
	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:4235) 
	at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:112) 
	at android.app.servertransaction.TransactionExecutor.executeNonLifecycleItem(TransactionExecutor.java:174) 
	at android.app.servertransaction.TransactionExecutor.executeTransactionItems(TransactionExecutor.java:109) 
	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:81) 
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2636) 
	at android.os.Handler.dispatchMessage(Handler.java:107) 
	at android.os.Looper.loopOnce(Looper.java:232) 
	at android.os.Looper.loop(Looper.java:317) 
	at android.app.ActivityThread.main(ActivityThread.java:8705) 
	at java.lang.reflect.Method.invoke(Native Method) 
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:580) 
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:886) 

I don't know how to do that. I don't use private contacts. I think this should be a separate issue. I have been using the patched version on my phone and so far it's been working well.

It's not a separate issue. Using contacts from both Fossify Contacts and phone-wide contacts is the basic and important function of Fossify Phone, and we can't accept a pull request that doesn't handle it. Its support is done literally everywhere in the app where contacts are used.

@Grafcube
Copy link
Author

Grafcube commented Nov 4, 2024

Now it doesn't work at all. Not only group doesn't hide, but even notification doesn't hide after doing any action. App throws an exception, so it's even impossible to call back. Here's the logcat:

I made some changes. It should work this time.

It's not a separate issue. Using contacts from both Fossify Contacts and phone-wide contacts is the basic and important function of Fossify Phone, and we can't accept a pull request that doesn't handle it. Its support is done literally everywhere in the app where contacts are used.

Fair enough. Unfortunately even after looking at other parts of the app, I can't figure out how to do it. Here's what I have so far.

helper.getAvailableContacts(false) { contactList ->
    val privateCursor = context.getMyContactsCursor(favoritesOnly = false, withPhoneNumbersOnly = true)
    val privateContacts = MyContactsContentProvider.getSimpleContacts(context, privateCursor)
    contactList.addAll(privateContacts)
    contactList.sort()
    ...
}

I changed the number I'm using for testing to a private contact but privateContacts still returns an empty list.

@Aga-C
Copy link
Member

Aga-C commented Nov 5, 2024

I made some changes. It should work this time.

Now it works fine, thanks!

I changed the number I'm using for testing to a private contact but privateContacts still returns an empty list.

Try using ContactsHelper and MyContactsContentProvider.getContacts instead of SimpleContacts. It's used in incoming call notification and there it works, so it should also work in your case.

@Grafcube
Copy link
Author

Grafcube commented Nov 5, 2024

It works now 🎉

@Aga-C
Copy link
Member

Aga-C commented Nov 5, 2024

There's an exception when the app gets a call from an unknown number:

FATAL EXCEPTION: main
Process: org.fossify.phone.debug, PID: 4210
java.lang.IllegalArgumentException: Uri must not be null.
	at android.graphics.drawable.Icon.createWithContentUri(Icon.java:870)
	at org.fossify.phone.receivers.MissedCallReceiver$notifyMissedCall$1.invoke(MissedCallReceiver.kt:147)
	at org.fossify.phone.receivers.MissedCallReceiver$notifyMissedCall$1.invoke(MissedCallReceiver.kt:90)
	at org.fossify.commons.helpers.ContactsHelper$getContacts$1.invoke$lambda$11(ContactsHelper.kt:99)
	at org.fossify.commons.helpers.ContactsHelper$getContacts$1.$r8$lambda$hF5JS2E96vAnhALDHVdQwhcZn6k(Unknown Source:0)
	at org.fossify.commons.helpers.ContactsHelper$getContacts$1$$ExternalSyntheticLambda0.run(D8$$SyntheticClass:0)
	at android.os.Handler.handleCallback(Handler.java:959)
	at android.os.Handler.dispatchMessage(Handler.java:100)
	at android.os.Looper.loopOnce(Looper.java:232)
	at android.os.Looper.loop(Looper.java:317)
	at android.app.ActivityThread.main(ActivityThread.java:8705)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:580)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:886)

@Grafcube
Copy link
Author

Grafcube commented Nov 5, 2024

That was because I didn't test if photoUri was null. I've fixed it now.

@Aga-C
Copy link
Member

Aga-C commented Nov 5, 2024

Notification shows now properly.

Unfortunately, I've found a rather major bug while playing around. Missed calls aren't marked as seen. You can test it either by changing phone app to another (e.g. Google's phone displays a badge with number of fresh missed calls) or restarting the phone (you'll get notifications for all missed calls).

Also, a minor thing - now you don't need plural forms for "Missed call" string, you're always using a singular one.

@Grafcube
Copy link
Author

Grafcube commented Nov 5, 2024

Also, a minor thing - now you don't need plural forms for "Missed call" string, you're always using a singular one.

Fixed

Unfortunately, I've found a rather major bug while playing around. Missed calls aren't marked as seen. You can test it either by changing phone app to another (e.g. Google's phone displays a badge with number of fresh missed calls) or restarting the phone (you'll get notifications for all missed calls).

It clears when all notifications are canceled (either with interaction or just clearing it) but I didn't account for switching apps. When I tested marking the missed calls as seen while there were multiple notifications, it clears all of them. I don't know what the correct behaviour should be.

@Aga-C
Copy link
Member

Aga-C commented Nov 5, 2024

It clears when all notifications are canceled (either with interaction or just clearing it) but I didn't account for switching apps. When I tested marking the missed calls as seen while there were multiple notifications, it clears all of them. I don't know what the correct behaviour should be.

You don't even need to change the app. Clear all missed call notifications in any way you like and restart the phone (or do the Cold Boot on an emulator). You'll get all of them once again.

@Grafcube
Copy link
Author

Grafcube commented Nov 5, 2024

I can't reproduce the issue.

I'm guessing it's because I'm only calling context.telecomManager.cancelMissedCallsNotification() when there are no notifications left. Calling it when there are still notifications causes it to clear all missed call notifications, which is the same behaviour as google's app last I checked. I can change the code to match that but that would mean all the notifications would be cleared on cancel (not on action).

@Aga-C Aga-C removed their request for review November 6, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missed call notification
3 participants