Skip to content

Conversation

JaredBanyard
Copy link

@JaredBanyard JaredBanyard commented Aug 3, 2025

Description

The proper way to implement this sort of scanner is in a foreground service that can be managed by notifications and bound UI. I migrated to this architecture along with implementing proper UI to disconnect to/from the foreground service from the application layer.

Screenshots

image image image

Checklist

@callebtc
Copy link
Collaborator

callebtc commented Aug 5, 2025

that looks like an awesome change, thank you. the foreground service should make it run a lot more reliably over a long time period?

I'm not quite happy with the styling of the notification, it looks unfinished. what do you think of changing it to a vanilla notification that maybe only shows the number of peers?

@callebtc callebtc added enhancement New feature or request needs review Someone should look at it labels Aug 5, 2025
@JaredBanyard
Copy link
Author

JaredBanyard commented Aug 5, 2025

that looks like an awesome change, thank you. the foreground service should make it run a lot more reliably over a long time period?

I'm not quite happy with the styling of the notification, it looks unfinished. what do you think of changing it to a vanilla notification that maybe only shows the number of peers?

Yes, this allows the service to run within a foreground service that does not need UI to run. It will then hook in to the UI when it is launched via the launcher or notification.

Yeah the custom UI is a bit overkill :) If you all are interested in this, I can take some time to clean it up with a vanilla implementation.

@callebtc
Copy link
Collaborator

callebtc commented Aug 5, 2025

Yeah the custom UI is a bit overkill :) If you all are interested in this, I can take some time to clean it up with a vanilla implementation.

yeah I've tested and it and it seems to work. can certainly use more support from experienced android devs like you.

a few things / questions:

  • you've added new hamburger buttons in the UI, I think those should be removed too
  • the buttons allow the user to kill the service, can we do that in the notification? I understand it's a bit tricky since we don't have any settings yet.
  • when I put the app into the background and open it back up from the notification all works but that used to work before too. does the foreground service improve anything about that?
  • when I kill the app, I see the notification, but when I press it, it restarts the app again, the peer list is empty and doesn't fill up anymore
  • which features of the app keep working as a foreground service? all of them? only parts? does routing work for example?
  • the app seems to be able to receive DMs because I can see the read receipts but notifications aren't fired. when I reopen the app, I can't read the DMs because of the fact that the peer list is empty

I think it needs a bit of polishing but it feels very powerful

@callebtc callebtc added question Further information is requested and removed needs review Someone should look at it labels Aug 5, 2025
@JaredBanyard JaredBanyard changed the title Migration of BluetoothMeshService and application state into foreground service with custom UI Migration of BluetoothMeshService and application state into foreground service Aug 6, 2025
@JaredBanyard
Copy link
Author

JaredBanyard commented Aug 6, 2025

  • you've added new hamburger buttons in the UI, I think those should be removed too
  • the buttons allow the user to kill the service, can we do that in the notification? I understand it's a bit tricky since we don't have any settings yet.

Agreed, the back button and notification take care of this.

  • when I put the app into the background and open it back up from the notification all works but that used to work before too. does the foreground service improve anything about that?

The BluetoothMeshService will be shutdown when the process/activity/viewmodel is paused and eventually garbage collected. With a service, it can stay alive in that background process and then the UI can live in an activity that binds to the service to retrieve application state. This is how media apps work, the player is in a service.

  • when I kill the app, I see the notification, but when I press it, it restarts the app again, the peer list is empty and doesn't fill up anymore

Yeah so thats a force close and an unknown app state, the notification is there but the process its connected to is dead. Reopening and re-initialization of the app is expected since it died. I'm not sure how to detect this state off the top of my head.

  • which features of the app keep working as a foreground service? all of them? only parts? does routing work for example?

We can put whatever we want in it. The bluetooth service and all its dependencies will live there. And then any state you need for the foreground notification (peer data, message data, etc0

  • the app seems to be able to receive DMs because I can see the read receipts but notifications aren't fired. when I reopen the app, I can't read the DMs because of the fact that the peer list is empty

Yeah I think we will need to integrate DMs into the foreground service UI. I have stubbed that out.
I also added new screenshots showing mock users. It updates every 5 seconds.

I've done a bunch of cleanup real quick. it looks like I need to rebase and check out DMs 👍

@JaredBanyard
Copy link
Author

Okay so I believe ive restored the DM notifications,, we arent tied to the primary foreground service for notifications. We can also easily make a custom layout for the foreground service that can handle routing directly to DMs. There will be more work needed on audio/vibration notifications around the foreground service and DMs.

@JaredBanyard
Copy link
Author

Sorry I have been busy! I will get back to rebasing this at some point!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants