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.

Sign up for GitHub

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

Jump to bottom

fixes bluez 5.55 service registration allowing muka to be updated. #74

Merged
merged 1 commit into from
Sep 3, 2021
Merged

fixes bluez 5.55 service registration allowing muka to be updated. #74

merged 1 commit into from
Sep 3, 2021

Conversation

james-lawrence
Copy link
Contributor

@james-lawrence james-lawrence commented Aug 10, 2021 •
edited
Loading

muka/go-bluetooth does some magic so you can use short UUIDs and it'll auto
expand them to the full 128 bit uuid. setting these flags disables that behavior.

related issues:

I am unsure how safe this change is for previous versions as I do not have any to test prior to bluez 5.60.

	svc := &bluetooth.Service{
		UUID: bluetooth.ServiceUUIDNordicUART,
	}

	if err = adapter.AddService(svc); err != nil {
		return errors.Wrap(err, "failed to add bluetooth service")
	}

before

INFO[0000] muka/go-bluetooth generated uuid 6e400001-b5a3-f393-e0a9-e50e24dcca9e -> 12346e400001-b5a3-f393-e0a9-e50e24dcca9e-0000-1000-8000-00805F9B34FB 
2021/08/10 16:24:42 failed to add bluetooth service: Failed to create entry in database

after:

INFO[0000] muka/go-bluetooth generated uuid 6e400001-b5a3-f393-e0a9-e50e24dcca9e -> 6e400001-b5a3-f393-e0a9-e50e24dcca9e

bluetooth logs describing failure:

Aug 10 16:24:42 dambli bluetoothd[538246]: src/gatt-database.c:database_add_service() Failed to read "UUID" property of service
Aug 10 16:24:42 dambli bluetoothd[538246]: src/gatt-database.c:database_add_app() Failed to add service
Aug 10 16:24:42 dambli bluetoothd[538246]: src/gatt-database.c:client_ready_cb() Failed to create GATT service entry in local database

@james-lawrence
fixes bluez 0.55 service registration.
cacc520 
muka/go-bluetooth does some magic so you can use short UUIDs and it'll auto
expand them to the full 128 bit uuid. setting these flags disables that behavior.

related issues:
- allows updating muka library to resolve tinygo-org#35
- fixes the regression the upgrade caused tinygo-org#46
- commit causing the regression in upstream muka/go-bluetooth@1c4c1c8
@james-lawrence james-lawrence changed the title fixes bluez 0.55 service registration allowing muka to be updated. fixes bluez 5.55 service registration allowing muka to be updated. Aug 10, 2021
@deadprogram deadprogram changed the base branch from release to dev August 11, 2021 06:31
@deadprogram
Copy link
Member

@aykevl I see that the CI failed for this PR but I do not think it is directly related: https://app.circleci.com/pipelines/github/tinygo-org/bluetooth/204/workflows/840c1286-b4e4-4b3b-b9f7-98e9c5f8adb2/jobs/334?invite=true#step-103-33

Could you please look? Thanks!

@deadprogram
Copy link
Member

Working on BlueZ 5.53 at least. And it does correct the issue at least after updating muka/go-bluetooth to the latest version.

@deadprogram
Copy link
Member

Confirmed, so merging, and will subsequently commit update to modules to fix build.

@deadprogram deadprogram merged commit d985dcb into tinygo-org:dev Sep 3, 2021
@james-lawrence james-lawrence deleted the bugfix/service-uuid branch September 5, 2021 01:07
aykevl added a commit that referenced this pull request Jan 3, 2024
@aykevl
linux: rewrite everything to use DBus directly
d231e0c 
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.
aykevl added a commit that referenced this pull request Jan 3, 2024
@aykevl
linux: rewrite everything to use DBus directly
cf80768 
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.
@aykevl aykevl mentioned this pull request Jan 3, 2024
Merged
deadprogram pushed a commit that referenced this pull request Jan 3, 2024
@aykevl @deadprogram
linux: rewrite everything to use DBus directly
d775214 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Field not found: Roles
2 participants
@james-lawrence @deadprogram