Skip to content

linux: rewrite everything to use DBus directly #216

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

Merged
merged 1 commit into from
Jan 3, 2024

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants