TorryDo / Floating-Bubble-View

πŸ€an Android library that adds floating views on top of your screen🎨, supports both XML and Jetpack Compose
Apache License 2.0
204 stars 34 forks source link

Unable to pass parameters to `setupBubble()`/`setupNotificationBuilder()` from `onStartCommand()` #28

Closed moazelsawaf closed 1 year ago

moazelsawaf commented 1 year ago

Hello Dear,

I was using version 0.5.0 and everything was working fine, but I wanted to upgrade to the latest version (v0.5.1) to have the new updates, unfortunlly, my class which implements the FloatingBubbleService() has an override for onStartCommand() method, once I upgraded the dependency, this overridden method stopped working, it is not called at all.

NOTE: I have made sure that I am registering the service in the <application> tag in the Manifest file and starting the service with an intent.

Also I would like to mention that I am using this method to initialize a lateinit variable that holds a value passed from the intent that starts the service to be used inside setupBubble() method, so if you have a better way to do that please suggest.

I would be glad if you could help me with this issue πŸ™πŸ»

Thanks in Advance πŸ™πŸ»β€

TorryDo commented 1 year ago

Hi @moazelsawaf,

In v0.5.1, I removed showBubbles() from onStartCommand() (which I should have done a long time ago) and moved it to the onCreate() method.

The reason for that is, if you pass the intent to the service multiple times throughout the service lifecycle, you must override the onStartCommand() method first. Otherwise, the bubble will appear overlapped each time you send the intent.

Therefore, in this version, onStartCommand() is not being touched, and it should behave normally. The reason why the overridden method is not working may be something else.

EDIT: The reason is onStartCommand() initializes the lateinit variables correctly, but the setupBubble() method is called before that occurs, resulting in the bubble displaying old or uninitialized data.

Thank you!

TorryDo commented 1 year ago

Looks like a bug!.

After a short investigation, I found out that it's hard to pass bubble parameters via onStartCommand() to setupBubble(). This is a silly bug that can be fixed in a blink of an eye. For now, please stay with v0.5.0 and I'll fix this issue in the next version.

Thank you for pointing it out πŸ’–.

moazelsawaf commented 1 year ago

Great ❀

Thank you so much @TorryDo for caring πŸ™πŸ»β€

Waiting for the next release Insha'Allah πŸ’ͺ🏻❀

TorryDo commented 1 year ago

The issue is now gone in v0.5.2

If you have any further concerns, don't hesitate to reopen the issue.

Cheers 🍺🍻

moazelsawaf commented 1 year ago

Unfortunately, the issue still exists, setupBubble() & setupNotificationBuilder() are being called before onStartCommand() so I am still unable to initialize the variables.

πŸ“ƒ Here are the logs of printing a statement in the first line of each method:

2023-04-17 04:49:05.936 13718-13718 BubbleService           dev.moaz.dash_bubble_example         D  setupNotificationBuilder
2023-04-17 04:49:05.939 13718-13718 BubbleService           dev.moaz.dash_bubble_example         D  setupBubble
2023-04-17 04:49:05.961 13718-13718 BubbleService           dev.moaz.dash_bubble_example         D  onStartCommand
TorryDo commented 1 year ago

That's weirdπŸ€”. Did you override initialRoute() and return Route.Empty?. After that, in onStartCommand(), you can show the bubble manually by calling showBubbles()

moazelsawaf commented 1 year ago

No, I am not overriding initialRoute() ...

I tried now and overridden it to return Route.Empty, but setupBubble() method stopped being called at all ... πŸ˜”

πŸ“ƒ Here are the new logs:

2023-04-17 05:04:33.646 18188-18188 BubbleService           dev.moaz.dash_bubble_example         D  initialRoute
2023-04-17 05:04:33.648 18188-18188 BubbleService           dev.moaz.dash_bubble_example         D  setupNotificationBuilder
2023-04-17 05:04:33.652 18188-18188 BubbleService           dev.moaz.dash_bubble_example         D  onStartCommand
TorryDo commented 1 year ago

I'm confused now πŸ˜…. Please take a quick look at Sample (line 41 -> 60)

Is your code somthing similar to this?


    override fun initialRoute(): Route {
        return Route.Empty
    }

    private var size = 0

    override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {

        val _size = intent?.getIntExtra("size", 0)  // assume that _size = 60

        size = _size ?: 0

        showBubbles()          // At this point, the bubble's size parameter = 60

        return START_STICKY
    }
    ...
moazelsawaf commented 1 year ago

Hmm, sorry I was not calling showBubbles() method, now after calling it the issue should be solved for setupBubble() because I have the control to decide when to call it, but the problem persists for setupNotificationBuilder() because it is still being called before onStartCommand() method.

πŸ“ƒ Here are the new logs:

2023-04-17 05:15:29.895 21779-21779 BubbleService           dev.moaz.dash_bubble_example         D  initialRoute
2023-04-17 05:15:29.897 21779-21779 BubbleService           dev.moaz.dash_bubble_example         D  setupNotificationBuilder
2023-04-17 05:15:29.900 21779-21779 BubbleService           dev.moaz.dash_bubble_example         D  onStartCommand
2023-04-17 05:15:29.900 21779-21779 BubbleService           dev.moaz.dash_bubble_example         D  setupBubble
TorryDo commented 1 year ago

So you want to pass parameters to setupNotificationBuilder() too?

I have a plan with this stuff in the future so for now, you can do that by using the code below, which will update the notification. the setupNotificationBuilder() is basically just a method to init the notification.

        val notification = // setupNotificationBuilder(channelId())   // your notification
        NotificationManagerCompat.from(this).notify(notificationId(), notification)
moazelsawaf commented 1 year ago

Do you mean I should not override setupNotificationBuilder() method for now and use your idea of updating the notification in onStartCommand() after initializing my variables?

TorryDo commented 1 year ago

Nope, you should override setupNotificationBuilder(). After that, simply using the code above in onStartCommand(). It'll update the current notification.

moazelsawaf commented 1 year ago

That is what I meant πŸ‘ŒπŸ»

It worked, but unfortunately, I have to request a new unnecessary permission from the user which is POST_NOTIFICATIONS, that would complicate the process.

So please any other suggestion? πŸ™πŸ»

TorryDo commented 1 year ago

Maybe I have to revamp setupNotificationBuilder() soon.

Thank you!

TorryDo commented 1 year ago

hi @moazelsawaf , πŸ‘‹ You can pass parameters to the notification now. Simply by following these steps:

In Android 13+, the notifications are only visible if "POST_NOTIFICATIONS" permission is granted. However, the service will still run fine without the notification.

If you have any problems, don't hesitate to re-open the issue.

Thank you!

moazelsawaf commented 1 year ago

Thanks for your concerns ❀

Should I handle the notification my self or it is already handled inside the library? Should I add it to the Manifest file or do anything else?

TorryDo commented 1 year ago

If you want the notification visible, first add "POST_NOTIFICATIONS" to your manifest file. Then ask for the permission. If permission is not granted, the notification won't show up. However, initialNotification and notify() will still be able to call, but without the notification.

If you don't want to ask for the permission, the service will still run fine. πŸ€

Feel free to ask if you have any further concerns. Thank you.

moazelsawaf commented 1 year ago

Thank you so much ❀

Now everything is clear, I will add the permission request for Android 13, but I wanted to ask a question, in the old logic in the earlier versions, would it be also a must to request the POST_NOTIFICATIONS permission in Android 13?

TorryDo commented 1 year ago

No, it's not required. In Android 12 and earlier versions, the system automatically granted apps the ability to post notifications. However, in Android 13, the system no longer automatically grants this permission. That's why POST_NOTIFICATIONS come in. You only need to request this permission in android 13+ if you want to show the notification.

You can find more info here: Android Service. Thank you.

moazelsawaf commented 1 year ago

Thank you so much for the clarification πŸ™πŸ»β€οΈ