bluetooth
bluetooth copied to clipboard
tinygo-org
linux & macos: adapter power state handling
A simple PR to address some of the issues brought up in #184. In particular, this adds a state change handler to the adapter which calls a provided callback when the powered state changes.
On macos this uses the existing CentralManagerDidUpdateState process that is already used to know when the adapter has been correctly enabled. On linux this watches for the Powered adapter D-Bus property to change.
This has been tested on an M1 Macbook Pro and a Raspberry Pi 4.
This PR modifies the existing API by adding (a *Adapter) SetStateChangeHandler(c func(newState AdapterState)). I also add some const states to provide generality between platforms, although these are probably unnecessary.
Unknowns:
- Does bare-metal have any comparable functionality that could be linked in? Or would this be a no-op?
- I do not have a windows machine handy so can not develop an implementation for that.
@xeanhort One additional note about #184, while this PR does not directly provide a mechanism to confirm if the adapter is scanning correctly or not. There is an adapter property called Discovering that can be watched to hopefully provide this insight. This can be easily integrated into the watching procedure added to adapter_linux.go in this PR.
If we can find a similar signal on the other platforms perhaps we can add it to the state change callback in a future PR.
Yay 🎉 Thank you so much for this.
Hmm, I am thinking how do we determine the initial adapter state on all platforms? Probably the library should always trigger an initial statechange event (after an Enable) based on whatever information it can get out of the platform.
The behaviour will need to belong to the library, as we can't trust the underlying implementations to behave the same.
On each platform, the Enable function will return an error if the adapter can't be turned on correctly. On macos this occurs after a timeout if the "Powered" signal doesn't arrive. This is the same powered signal used in this PR for the state-change callback. So at least on macos you could set up the callback before calling enable and expect to get the callback firing.
On linux the Enable function also enables a watch for the adapter property changes so the timing probably won't work as enable doesn't seem to actually turn anything on, it just fetches a reference to the D-Bus adapter object iiuc. In this case, what is probably more desirable is confirmation that the adapter is scanning correctly which can be confirmed by watching for the Discovering property on the adapter. However, there is not really an analog I can find on the macos side except for the isScanning property which looks to only be pollable but I will do some more digging.
I have no insight into the windows side, so if you are able to get something similar working on that end that would be awesome.
Also, I am not sure why tests are failing for some of the bare-metal targets. I need to dig into that as well.
So far my experience is that adapter.Enable does not fail when the adapter is disabled both on macOS and Windows.
Are you getting different behaviour? Maybe it's a different discussion 🤔
I can't speak for Windows, but on my M1 Macbook, if I start my application with Bluetooth turned off, calling Enable will fail after the 10-second timeout with the error timeout enabling CentralManager.
I have just tested on linux and I do not get an error when calling Enable with Bluetooth turned off. However, the new adapter state callback does fire when I eventually turn it on and from that you can kick things off correctly. The problem with this is that because there is no error the app can just assume everything is okay and continue to start scanning. Fortunately, the adapter.Scan function does return an error when you attempt to start it without the adapter actually being turned on. So as long as you catch that (make sure you subsequently call adapter.StopScan() to close the chan's) you should have a pretty robust solution.
Does bare-metal have any comparable functionality that could be linked in? Or would this be a no-op?
On bare metal, the Enable call directly enables the whole stack. An event system that waits until the stack is enabled doesn't make much sense there.
What actual problem are you trying to solve with this PR? I can think of a few:
- Wait for Bluetooth to be enabled on startup.
- Bluetooth is disabled, wait for the user to enable it.
For the first, if we know that Bluetooth is starting but not yet finished, I'd argue the proper way to fix it is not by introducing a new API but instead to make sure Enable waits until the adapter is fully enabled.
For the second, an event based system does make sense but I'm not sure this is actually the problem that you're trying to solve? Also, I'm not sure when that would be useful in practice.
In our case, we are building persistent Bluetooth gateway software that runs on MacOS and Linux to interface with our devices remotely. Think Bluetooth shell, SMP configuration, and OTA.
This software needs to be robust and persistent. On MacOS, and MacBooks in particular, having our software maintain functionality after the laptop has been closed and woken up again is critical, without the changes in this PR I wasn't able to find a way to capture the state changes that MacOS puts the Bluetooth adapter through. Less of a typical experience but still important is having the ability to respond to manual enabling and disabling of the adapter and having our software persist through such events.
I feel like there should be something stronger in this library's interface which allows a user to handle this in a normalized cross-platform way.
This issue currently happens with the connect / disconnect handler. We need a few more simple and explicit checks we can make so that we can ignore the underlying platform quirks. Also you don’t always receive these events depending on what happened.
So I think we should either:
-
Provide explicit methods to check: - Is the adapter enabled or disabled (without having to call Enable as that is very messy as a probe) - Is this device connected or disconnected (without having to constantly probe by making useless calls again)
These methods should belong to the relevant objects themselves. Otherwise you end up doing a lot of work with a dead device object, or disabled adapter, and then end up having to unwind everything. Also you cannot always unwind an entire user journey without pissing them off.
Or:
- Fire these events ourselves so that the behaviour is well defined (define these as library events so we can commit to enforcing some well-defined reliable behaviour)
We basically need to some way to find out the state (pull), and some way to be told the state (push). One without the other doesn't quite cover the cases.
On MacOS, and MacBooks in particular, having our software maintain functionality after the laptop has been closed and woken up again is critical, without the changes in this PR I wasn't able to find a way to capture the state changes that MacOS puts the Bluetooth adapter through.
Right. I assume these are pre-M1 laptops that fully shut down Bluetooth when suspended? I'm not sure about the newer ones but I think they keep Bluetooth alive when the lid is closed (at least that's what I experienced).
This seems like a good argument for adding some way to listen for changes. I hadn't considered suspend before.
Also, adding methods to check the current state seems reasonable. Those will probably be very simple.
Right. I assume these are pre-M1 laptops that fully shut down Bluetooth when suspended? I'm not sure about the newer ones but I think they keep Bluetooth alive when the lid is closed (at least that's what I experienced).
Actually, I am experiencing this on an M1, 14-inch Macbook Pro. I typically have it on power saving mode so that could be the explanation.
I have added some simple queries to get the adapter state on macOS and Linux.
Regarding Windows, it appears we can use https://learn.microsoft.com/en-us/uwp/api/windows.devices.bluetooth.bluetoothadapter.isperipheralrolesupported?view=winrt-22621 to check if the adaptor is enabled.
See https://stackoverflow.com/a/71404948
Regarding nrf, we cannot default to the unknown state until Enable() is called and still use the same logic to determine if adaptor is powered on. So that is a little trickier to handle?
We could default to PoweredOn on nrf, but that seems inconsistent with the meaning on the other platforms.
Looking at this PR again, I think this would be a helpful feature. @TirelessDev can you rebase this on the dev branch? It will likely require some relatively big changes for Linux (because I rewrote most of that on #216).