Skip to content

gap: Introduce AdvertisementPayload.ServiceUUIDs() #364

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 2 commits into from
Aug 18, 2025

Conversation

mook
Copy link
Contributor

@mook mook commented May 22, 2025

This adds a new method that returns all of the Service Class UUIDs found in the advertisement payload. This may require allocating a new slice of UUIDs, hence using HasServiceUUID where possible is still preferred.

Fixes #363

I have no idea what I'm doing, so please review the rawAdvertisementPayload implementation harder; I'm just adapting the HasServiceUUID code. I assume since go.mod claims go1.20 using the conversion in NewUUID is okay.

Please let me know if you'd like me to make changes; if you feel like making the directly that's fine too.

@deadprogram
Copy link
Member

Hello @mook thank you for looking into this.

If you could please add some tests for the new functions into https://github.com/tinygo-org/bluetooth/blob/dev/gap_test.go that would be greatly appreciated!

@mook
Copy link
Contributor Author

mook commented Jun 15, 2025

Thanks! Tests added. That raises some questions, though:

  • NewUUID takes bytes in the reverse order of UnmarshalBinary (and Bytes); when should each be used?
  • When implementing rawAdvertisementPayload#ServiceUUIDs, should I actually combine 0x03 and 0x02 (complete and incomplete lists of 16-bit UUIDs), instead of preferring the first? Similarly for 0x07 vs 0x06 (complete and incomplete list of 128-bit UUIDs). How about 0x05 / 0x04 (32-bit UUIDs)? Currently rawAdvertisementPayload#HasServiceUUID just ignores it.
  • Can I assume that the advertisement payload will not have multiple of the same EIR data type? Or should I actually just enumerate all EIR data instead? (Apologies if I have the terminology incorrect, I have no idea what I'm doing with respect to Bluetooth.)

mook added 2 commits August 17, 2025 09:36
This adds a new method that returns all of the Service Class UUIDs found in
the advertisement payload.  This may require allocating a new slice of
UUIDs, hence using HasServiceUUID where possible is still preferred.

Fixes tinygo-org#363
This catches an issue in rawAdvertisementPayload where NewUUID creates a
UUID with the reversed byte order.

Fixes: 873d49b
@mook mook force-pushed the advertisement-service-uuids branch from a2c505a to 89a1728 Compare August 17, 2025 16:46
@mook
Copy link
Contributor Author

mook commented Aug 17, 2025

Rebased; the relevant files had no changes, so there were no merge conflicts.

While the tests pass, it's probably still best if #364 (comment) gets resolved before merging.

@deadprogram
Copy link
Member

Thanks! Tests added. That raises some questions, though:

  • NewUUID takes bytes in the reverse order of UnmarshalBinary (and Bytes); when should each be used?

The Bluetooth spec tell us that this array is always little-endian.

  • When implementing rawAdvertisementPayload#ServiceUUIDs, should I actually combine 0x03 and 0x02 (complete and incomplete lists of 16-bit UUIDs), instead of preferring the first? Similarly for 0x07 vs 0x06 (complete and incomplete list of 128-bit UUIDs). How about 0x05 / 0x04 (32-bit UUIDs)? Currently rawAdvertisementPayload#HasServiceUUID just ignores it.

That might be a kind of bug? Let us address that as a future PR.

  • Can I assume that the advertisement payload will not have multiple of the same EIR data type? Or should I actually just enumerate all EIR data instead? (Apologies if I have the terminology incorrect, I have no idea what I'm doing with respect to Bluetooth.)

Given how small the size of data is, we can probably assume no duplication in this packet.

@deadprogram
Copy link
Member

Now merging, thanks for working on this @mook

@deadprogram deadprogram merged commit 0613dfc into tinygo-org:dev Aug 18, 2025
4 checks passed
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