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

Linux close BlueZClient #99

Open
pimlie opened this issue Sep 12, 2024 · 8 comments
Open

Linux close BlueZClient #99

pimlie opened this issue Sep 12, 2024 · 8 comments
Labels

Comments

@pimlie
Copy link

pimlie commented Sep 12, 2024

Is it on purpose that this package doesn't expose BlueZClient.close()?

Atm it seems that devs will be unable to put their app into a fully clean state on Linux, cause if you ever created a CentralManager then the BlueZClient will always remain active / connected to dbus. Not sure I fully understand how everything works on every client, but at least on Linux it looks like we could benefit from being able to connect then close (ie by disposing CentralManager) the BlueZAgent repeatedly?

@yanshouwang
Copy link
Owner

yanshouwang commented Sep 12, 2024

The CentralManager() is a singleton factory constructor, every time you call this constructor it will return the same instance of the CentralManger.

I think it's OK to keep the connection in the singleton?

@pimlie
Copy link
Author

pimlie commented Sep 12, 2024

Thanks! I noticed this issue because I was listening for stateChanged, but if you call CentralManager() twice (ie in my case open a bluetooth input form twice) then only the first time it will trigger a stateChanged event. I guess that is because the CentralManager singleton still existed and in the context of the singleton the state did not change. Only later did I realize that you should check CentralManager().state at init, then only add the listener for actual changes.. :)

But that behaviour felt a bit counter-intuitive, at least in my case it would be nice if I could fully dispose the singleton and all it's resources

@yanshouwang
Copy link
Owner

This behavior is the same as other platforms, You need to call getState before listen the stateChanged stream on other platforms either.

it's possible to use multi instances instead of singleton, but this will make the plugin more complicated. I will do this when it's necessary. For now it's better to keep this logic, and you don't need to dispose anything with this plugin.

Note: when the CentralManager do not use the Singleton pattern, it's should be a breaking change as all the instances of CentralManager must be disposed after that.

@pimlie
Copy link
Author

pimlie commented Sep 12, 2024

it's possible to use multi instances instead of singleton

I don't think I am asking for that. I.e. isn't the singleton mainly used to ensure there is no concurrent discovery etc which is not supported by any platform? That makes sense, but I'm talking about the case where the singleton is not needed anymore at all.

Given an app flow like this:

  • Start app
  • View data list screen
  • Add new data from bluetooth screen -> this inits the singleton
  • Back to view data list screen -> singleton is not needed anymore, but on linux BluezClient is still active

Maybe I'm overlooking something in the API but at the moment when you go back to the view data screen, the singleton is kept alive basically forever right? So on linux the Bluez dbus connection etc is also still available? If so, then I'm only asking to be able to just dispose of any resources the singleton still possesses (f.e. on linux to close the BluezClient connection). Hope I explained it clearly :)

Let me rephrase it otherwise, how do I ensure on Linux that BlueZClient.close is called when I don't need any Bluetooth interaction anymore in my app?

@yanshouwang
Copy link
Owner

The singleton is a lazy instance and once it's created the connection will keep alive until the app exit. If you want to close the connection and still want the CentralManager to be a singleton, how do you reopen the connection after you close it? Then we need to add openDbusConnection and closeDbusConnection methods to the CentralManager API just for Linux, it's obviously not a good way to solve this.

If there is a must to close the connection, It's a necessory to change the singleton to instances which can be disposed at any time. But I think it's OK to keep the connection in the singleon. You can treat it as a socket connection. It's normal to keep such a connection alive during the whole app lifecycle.

@yanshouwang
Copy link
Owner

Or think about the method channel. We create the method channel when app started, but we never dispose the method channel instances. I think the debus connection plays the same role as the method channel, right?

@pimlie
Copy link
Author

pimlie commented Sep 19, 2024

It's a bit confusing as bluez.dart uses connect/close but you would expect either connect/disconnect or open/close and not a combination. Looking at what the code does it seems that connect/close are each others opposites tho.

In any case, I would like to be able to be in control whether that dbus connection is open or not. Cause if we know the user doesn't need it anymore and we know we can re-open the connection if needed, why would we keep it open all the time? So moving dbus to a Channel and never dispose of the channel wouldn't solve my request, as I'd like that dbus connection to close ;)

Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants