Skip to content

Conversation

@aykevl
Copy link
Member

@aykevl aykevl commented Jan 3, 2024

This is a big rewrite to use DBus calls directly instead of going through go-bluetooth first.

This is a big change, but I believe it is an improvement. While the go-bluetooth works for many cases, it's a layer in between that I believe hurts more than it helps. Without it, we can just program directly against the BlueZ D-Bus API. The end result is about 20% more code, so not even that much more verbose (makes me think why we needed the go-bluetooth package in the first place).

With this rewrite, I fixed the following issues:

  • Fix a bug in Adapter.AddService where it would only allow adding a single service. Multiple services can now be added. This was actually the motivating bug that led me down to rewrite the whole thing because I couldn't figure out where the bug was in go-bluetooth (it's many layers deep).
  • All MapToStruct warnings are gone, like in invalid field detected *device.Device1Properties.Bonded #193. These were rather distracting.
  • Advertisements can be restarted after they were stopped. Previously this resulted in a panic.
  • The WriteEvent callback in a characteristic now also gets the 'offset' parameter which wasn't provided by go-bluetooth.
  • Looking at the source code of go-bluetooth, it appears that it includes devices from a different Bluetooth adapter than the one that's currently scanning. This is fixed with the rewrite.

This rewrite also avoids go-bluetooth specific workarounds like #74 and #121.

I have tested all six examples in the smoketest-linux Makefile target. They all still work with this rewrite on BlueZ 5.71 (Fedora 39) and BlueZ 5.66 (Raspberry Pi OS 64-bit running on a Raspberry Pi 3).

This is a big rewrite to use DBus calls directly instead of going through go-bluetooth first. This is a big change, but I believe it is an improvement. While the go-bluetooth works for many cases, it's a layer in between that I believe hurts more than it helps. Without it, we can just program directly against the BlueZ D-Bus API. The end result is about 10% more code. With this rewrite, I fixed the following issues: * All MapToStruct warnings are gone, like in #193. * Advertisements can be restarted after they were stopped. Previously this resulted in a panic. * Looking at the source code of go-bluetooth, it appears that it includes devices from a different Bluetooth adapter than the one that's currently scanning. This is fixed with the rewrite. * Fix a bug in Adapter.AddService where it would only allow adding a single service. Multiple services can now be added. This was actually the motivating bug that led me down to rewrite the whole thing because I couldn't figure out where the bug was in go-bluetooth (it's many layers deep). * The `WriteEvent` callback in a characteristic now also gets the 'offset' parameter which wasn't provided by go-bluetooth. This rewrite also avoids go-bluetooth specific workarounds like #74 and #121. I have tested all examples in the smoketest-linux Makefile target. They all still work with this rewrite.
@deadprogram
Copy link
Member

Tested on Ubuntu 20.04 with kernel 5.4.0-73 and worked perfectly! Thank you so very much @aykevl for doing this!!!

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

Labels

None yet

3 participants