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

[Bug]: Implementing Android INotificationServiceExtension #92

Open
1 task done
aalsamoht opened this issue Jan 25, 2024 · 5 comments · May be fixed by #111
Open
1 task done

[Bug]: Implementing Android INotificationServiceExtension #92

aalsamoht opened this issue Jan 25, 2024 · 5 comments · May be fixed by #111

Comments

@aalsamoht
Copy link

What happened?

I’m trying to implement Android INotificationServiceExtension in a .NET MAUI application using OneSignalSDK.DotNet 5.0.2
I have followed the instructions from https://documentation.onesignal.com/docs/service-extensions#android-notification-service-extension

The DotNet SDK does not wrap the Android code, so I must implement the interface from the Android binding project.

public class NotificationServiceExtension : Java.Lang.Object, Com.OneSignal.Android.Notifications.INotificationServiceExtension
{
    public void OnNotificationReceived(INotificationReceivedEvent e)
    {
        Console.WriteLine($"{LogHelper.Tag} OnNotificationReceived START");

        // Customizing the notification alternative one:
        e.PreventDefault();
        var notification = OneSignalSDK.DotNet.Android.Utilities.FromNativeConversion.ToNotification(e.Notification);
        Console.WriteLine($"{LogHelper.Tag} Title: {notification.Title} Body: {notification.Body}");
        // TODO: Create a local notification with the data from the notification.
		
        // Customizing the notification alternative two:
        // IMutableNotification is an Empty interface, can't modify the notification.
        IMutableNotification mutableNotification = e.Notification;

        Console.WriteLine($"{LogHelper.Tag} OnNotificationReceived END");
    }
}

I have successfully received the event in both foreground and background and created my own push message using Notification.AdditionalData.

I call e.PreventDefault() to suppress the push message created by the OneSignalSDK, however after approximately 30 seconds the following log message is written and there is no "Confirmed" callback to OneSignal.

remoteNotificationReceived timed out, continuing with wantsToDisplay=false.
kotlinx.coroutines.TimeoutCancellationException: Timed out waiting for 30000 ms
   at  kotlinx.coroutines.TimeoutKt.TimeoutCancellationException(Timeout.kt:191)
   at kotlinx.coroutines.TimeoutCoroutine.run(Timeout.kt:159)
   at kotlinx.coroutines.EventLoopImplBase$DelayedRunnableTask.run(EventLoop.common.kt:501)
   at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:280)
   at kotlinx.coroutines.DefaultExecutor.run(DefaultExecutor.kt:108)
   at java.lang.Thread.run(Thread.java:1012)

However there is a greater problem with calling e.PreventDefault(), push notifications are sent multiple times to INotificationServiceExtension.OnNotificationReceived().

onesignal_logcat_duplicate_notification.csv

MauiApp1.zip

Steps to reproduce?

One way to reproduce this is using the attached sample app.

After setting `OneSignalAppId` in `App.xaml.cs`, start the application, allow push, send a push from OneSignal and see that _Title_ and _Message_ are logged.

Restart the application, and the same _Title_ and _Message_ are logged again. See the attached log file `onesignal_logcat_duplicate_notification.csv`.

When running the application without calling `e.PreventDefault()` push notifications are delivered once to `INotificationServiceExtension.OnNotificationReceived()`.


logcat filter: (OneSignal | ************************************) -message:open_from_bundles

What did you expect to happen?

Alternative one e.PreventDefault()

Q1. How can I use e.PreventDefault() and set the event as successfully completed so that:

  • INotificationServiceExtension.OnNotificationReceived() is only called once for each push notification, avoid Restoring notifications. (Critical)
  • remoteNotificationReceived timed out exception is not logged. (Optional)
  • Confirmed status callback is completed to OneSignal. (Optional)

Q2. Are there any plans to implement INotificationServiceExtension in SDK so that I don’t need to use the Android.Binding project.

Alternative two notification.setExtender(...)

The OneSignal Android SDK has an interface IMutableNotification with a setExtender method, https://documentation.onesignal.com/docs/service-extensions#step-1-create-a-class-for-the-service-extension

notification.setExtender(builder -> builder.setColor(0xFF0000FF));

There is no such method available in the OneSignalSDK.DotNet IMutableNotification interface, the interface is empty.

Q3. How can i modify the notification before it is displayed by the OneSignalSDK.DotNet?

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@snice1981
Copy link

Hi

I have the same problem. It seems to work fine except the timeout error that is logged. Is there any update?

@aalsamoht
Copy link
Author

The last update I have is from OneSignal support on the 9th of February.

Hi there,

Just to update you on this, we have shared the feedback with our team and they have tested and determined the following :

Potential Android SDK Bug
I do see this which indicates it should be marked as dismissed when the event handler calls e.preventDefault() , and not processed again on app open. We can look into this further for a future release.

Potential .NET SDK Enhancement
Calling event.Notification.Display() with an extender should work, and they are right it does not exist in our SDK. However the PR I referenced earlier shows how we can update the SDK to enable that, but that is work we will need to do I'm not sure if the customer can create that binding on their own without forking our SDK

Any further updates will be shared on your Github issue as you have opted to reach out to the team directly.

Thank you

@snice1981
Copy link

thank you for your feedback, so if I understand you still have the problem.

@tmijieux
Copy link

tmijieux commented Jun 16, 2024

I had come up with a pull request on the previous version (for xamarin). at the time the team member who responded me sayed there was an interest but they had no time to do it themselves, eventually the PR never got merged and xamarin went out of support in the meantime.
OneSignal/OneSignal-Xamarin-SDK#381

I will try to look at this problem for this version and come up with a pull request. I have already a good hunch what might be happening . many APIs are not kept in the binding library for the notifications module because they reference type in the other (core) module.

@tmijieux tmijieux linked a pull request Jun 18, 2024 that will close this issue
18 tasks
@mehmetcankamar
Copy link

mehmetcankamar commented Jul 10, 2024

We are experiencing this problem too.

It has nothing to do with the topic, but I can provide the namespace in android:value with ridiculous values ​​like crc64c75c38fa2186e567. Do you know an easy way to do this?

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 a pull request may close this issue.

4 participants