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.

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

Filtering on companyIdentifier does not work #854

Closed
tsuijten opened this issue Feb 5, 2025 · 13 comments · Fixed by #855
Closed

Filtering on companyIdentifier does not work #854

tsuijten opened this issue Feb 5, 2025 · 13 comments · Fixed by #855
Labels
bug Something isn't working javascript
Milestone

Comments

@tsuijten
Copy link

tsuijten commented Feb 5, 2025

I am trying to filter devices by companyIdentifier this work for Android, but in JS I get an error.

requestPeripheral options:

private val options = Options {
    filters {
        match {
            manufacturerData = listOf(Filter.ManufacturerData(0x0A61, byteArrayOf()))
        }
    }
}

Error:

SystemLogEngine.kt:35 [Kable/requestDevice] TypeError: Failed to execute 'requestDevice' on 'Bluetooth': 'dataPrefix', if present, must be non-empty.
  options: Options(filters=[FilterPredicate(filters=[ManufacturerData(id=2657, data=, dataMask=null)])], optionalServices=[])
  processed: {"filters":[{"manufacturerData":[{"companyIdentifier":2657,"dataPrefix":{}}]}]}

This code in "normal" WebBLE api works:

      device = await navigator.bluetooth.requestDevice({
         filters: [{ manufacturerData: [{ companyIdentifier: 0x0A61 }] }],
       });
@twyatt twyatt added bug Something isn't working javascript labels Feb 5, 2025
@twyatt twyatt added this to the 0.36.0 milestone Feb 6, 2025
@twyatt twyatt marked this as a duplicate of #856 Feb 6, 2025
@twyatt twyatt changed the title Filtering on companyIdentifier does not work in JavaScript Filtering on companyIdentifier does not work Feb 6, 2025
@tsuijten
Copy link
Author

tsuijten commented Feb 6, 2025

I now get this error:

throwableExtensions.kt:25 IllegalArgumentException: If data is present (non-null), it must be non-empty
    at new ManufacturerData (webpack-internal:///./kotlin/kable-kable-core.js:692:13)
    at options$lambda$lambda$lambda (webpack-internal:///./kotlin/sample.js:10515:55)
    at protoOf.build_wq9w6y_k$ (webpack-internal:///./kotlin/kable-kable-core.js:906:7)
    at protoOf.filters_l35td1_k$ (webpack-internal:///./kotlin/kable-kable-core.js:7644:28)
    at options$lambda (webpack-internal:///./kotlin/sample.js:10503:19)
    at Options_0 (webpack-internal:///./kotlin/kable-kable-core.js:7605:5)
    at _init_properties_RequestDeviceLocator_kt__4wd3jl (webpack-internal:///./kotlin/sample.js:10522:17)
    at get_options (webpack-internal:///./kotlin/sample.js:10340:5)
    at protoOf.doResume_5yljmg_k$ (webpack-internal:///./kotlin/sample.js:10393:25)
    at protoOf.invoke_d9fzmj_k$ (webpack-internal:///./kotlin/sample.js:10370:16)


@tsuijten
Copy link
Author

tsuijten commented Feb 6, 2025

Ah, I see, this was with the byteArrayOf() still in place. I removed that and now it works!

@tsuijten
Copy link
Author

tsuijten commented Feb 6, 2025

@twyatt Stop the presses :)

Seems like this change now breaks companyIdentifier filtering on Android. I now see a list of all devices

@twyatt

This comment has been minimized.

@twyatt
Copy link
Member

twyatt commented Feb 6, 2025

Looks like a bug in Android.

Fixed in https://android-review.googlesource.com/c/platform/packages/modules/Bluetooth/+/3188842.

I don't have access to the associated bug, so I'll see what I can do to track down what version of Android this was fixed in so that I can try to figure out a workaround in Kable for it.

@twyatt
Copy link
Member

twyatt commented Feb 6, 2025

Looks like bug still existed in Android 15.

It is fixed in the main branch, so presumably will ship in Android 16. 🤞

@twyatt
Copy link
Member

twyatt commented Feb 6, 2025

Can you try the following (which is based on 0.35.0 with #855 cherry-picked):

repositories {
    maven("https://oss.sonatype.org/content/repositories/snapshots")
}

dependencies {
    implementation("com.juul.kable:kable-core:0.35.0+issue854-2-SNAPSHOT")
}

It falls back to flow based (non-native) filtering when only company ID is specified on pre Android 16.

@tsuijten
Copy link
Author

tsuijten commented Feb 7, 2025

I'll try, but when I tested companyId filtering prior to the changes made in #855 it worked on Android, I just tested with com.juul.kable:kable-core:0.35.0 on my device (Pixel 7, Android 15) and that works. When testing com.juul.kable:kable-core:0.35.0+issue854-SNAPSHOT the filter stopped working on Android while fixing it on iOS and JS.

@twyatt
Copy link
Member

twyatt commented Feb 7, 2025

Does your device's advertisement have manufacturer data in addition to the company ID?

If it doesn't (i.e. if the advertisement manufacturer data is an empty ByteArray) then it would explain why it worked with com.juul.kable:kable-core:0.35.0 — as specifying a ManufacturerData(0x0A61, byteArrayOf()) would filter for advertisements with a company ID of 0x0A61 AND empty data, which would align with your device. Unfortunately, that filter would require that the data be an empty ByteArray.

The hope is, that 0.35.0+issue854-2-SNAPSHOT allows consumers to make the data optional (by specifying it as null), and when non-null: it will appropriately filter against it.

In other words, w/ 0.35.0+issue854-2-SNAPSHOT:

Filter Advertisement Matches
id=0x0A61, data=[] id=0x0A61, data=[0x1, 0x2]
id=0x0A61, data=null id=0x0A61, data=[0x1, 0x2]
id=0x0A61, data=[] id=0x0A61, data=[]

..at least, that is the hope. 🤞

@tsuijten
Copy link
Author

tsuijten commented Feb 7, 2025

I actually do have manufacturing data in the advertisements 🤷 So no idea why it worked then previously. Anyway, I'll test the filter with 0.35.0+issue854-2-SNAPSHOT and report back to you

@twyatt
Copy link
Member

twyatt commented Feb 7, 2025

Hmm, since your advertisement did have data in the manufacturing data, I'll dig deeper to try and figure out what is going on.
Thanks for all the help debugging this, btw!

@twyatt
Copy link
Member

twyatt commented Feb 7, 2025

Ok, I've made the necessary changes to keep the previous behavior (on Android) whilst applying the fix to the iOS and JS sides.

You can give 0.35.0+issue854-3-SNAPSHOT a try, if you want.

It essentially provides Android (pre 16) an empty ByteArray for data even when null is specified on the Kable side.

This means that native scan filtering can still occur on Android when null is specified for data.

@tsuijten
Copy link
Author

tsuijten commented Feb 7, 2025

0.35.0+issue854-3-SNAPSHOT is giving me compile errors in the project, not sure why, but I was not able to test that version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working javascript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants