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

Optimize scan filtering #890

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Optimize scan filtering #890

wants to merge 4 commits into from

Conversation

twyatt
Copy link
Member

@twyatt twyatt commented Apr 6, 2025

Closes #889

Optimizes scan filtering when non-native filter(s) are combined with native filter(s), by favoring native scan filtering and moving non-native supported filters to flow filtering.

Take the following example:

Code Graphical representation
filter {
    match {
        service = listOf(Filter.Service(B))
        // AND
        name = listOf(Filter.Name.Prefix("A_"))
        // AND
        serviceData = listOf(Filter.ServiceData(..))
    }
}

Issue 890

This PR will optimize the scan by establishing a native filter for the Filter.Service and Filter.ServiceData and having the Filter.Name.Prefix filtering occur via flow operator.

Before the PR After the PR
Issue 890 before Issue 890 after

This optimization is only possible when a single match is specified (i.e. filters will not be OR'd). This is because when there are multiple filter match declarations, then it would be difficult to reliably determine which native filter was applied to know what "overlayed" filters should occur at the flow operator level. To illustrate this limitation, take the following example:

Code Graphical representation
filter {
    match {
        service = listOf(Filter.Service(A))
        // AND
        name = listOf(Filter.Name.Prefix("A_"))
    }
    // OR
    match {
        service = listOf(Filter.Service(B))
        // AND
        name = listOf(Filter.Name.Prefix("B_"))
    }
}

Issue 890 limitation

Then, during a scan there isn't a clear way to filter at the flow operator:

Issue 890 limitation2

As such, for these situations (more than one match with non-native filters): all filters will occur at the flow operator level.

@twyatt twyatt added android patch Changes that should bump the PATCH version number labels Apr 6, 2025
@twyatt twyatt changed the title Optimize scan filtering on Android Optimize scan filtering Apr 6, 2025
@twyatt twyatt requested a review from Copilot April 6, 2025 05:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

kable-core/src/androidMain/kotlin/BluetoothLeScannerAndroidScanner.kt:164

  • Combining multiple native filters into a single ScanFilter may inadvertently merge distinct filtering criteria. Please review if this merging accurately reflects the intended native filtering behavior.
native = listOf(nativeFilters.values.toList().toNativeScanFilter()),

@twyatt twyatt marked this pull request as ready for review April 7, 2025 05:41
@twyatt twyatt requested a review from a team as a code owner April 7, 2025 05:41
@twyatt twyatt added this to the 0.37.1 milestone Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android patch Changes that should bump the PATCH version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android Scanner should filter by service first, then name when both are provided
2 participants