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

Enhanced Scan Filtering #695

Merged
merged 15 commits into from
Jul 10, 2024
Merged

Enhanced Scan Filtering #695

merged 15 commits into from
Jul 10, 2024

Conversation

davidtaylor-juul
Copy link
Contributor

@davidtaylor-juul davidtaylor-juul commented Jun 18, 2024

Closes #586

Possibly relates to #627?

After noodling around on the and/or thing, it was found a mini-DSL/builder style to be simpler to implement, and also prevents the ability to specify non-sensical or conflicting filters.

Description

Everything in a match block is a single "predicate" which must all match to satisfy the condition (aka ANDed together). And any of the match predicate blocks can match to satisfy the overall set of filters (aka ORed together). The model of the composed logic is like this:

filters {
    match {
        filter1 AND filter2 ... AND filterN
    }
    OR
    match {
        filter1 AND filter2 ... AND filterN
    }
}

Example, to match on all devices with a certain name:

filters {
    match {
        name = Exact("bob")
    }
}

To match on a name AND a service uuid:

filters {
    match {
        name = Exact("bob")
        services = listOf(<uuid>)
    }
}

To match on a name OR a service uuid:

filters {
    match {
        name = Exact("bob")
    }
    match {
        services = listOf(<uuid>)
    }
}

System Filters

The native platforms provide an option to inject filters into the system scanner so that BLE radio usage can be optimized. This new filter model can be transcribed to match the system filter shape in many cases, so the preference is to convert it and inject it when possible, and fallback to a broader scan with filtering applied on the consumer side if unable to convert.

Web

Injection of the filter into the system "scanner" (aka picker) is the only option. For Web, we are always able to convert as it uses the same general shape and the exact same predicate logic.

Apple

For Apple the system only wants service uuids and the predicate logic takes the form of "uuid-1 OR uuid-2 ... OR uuid-n". So the approach here is to inject all uuid filters, but only if every clause has at least one uuid. This narrows the incoming advertisements to only those of interest, but we still must apply the provided filters to execute the full logic of the requested scan against each received advertisement.

Android

The Android system scanner supports matching on name, address, uuid and manufacturer data (and a whole lot more that we do not yet support). But the limitations are that each filter accepts only a single uuid or manufacturer data, and it does not support name prefix matching. We could get fancy and combinatorially unroll any compound uuid or manufacturer data clauses to maximize ability to enable the system filter, leaving only name-prefix as the exception. But, for now at least, decided on the simpler approach of detecting a direct translation fit, and falling back to a "scan everything and post-filter" if there are any name-prefix or compound clauses that do not directly convert.

Breaking Changes

As a breaking change it was decided to replace Filter.Name and Filter.NamePrefix with the sealed class Name { Exact; Prefix }. Having these separated breaks the logical operations because Name and NamePrefix are mutually exclusive, so are best expressed as a union/sum type. Currently users may specify both a Name and a NamePrefix which makes no real sense. (it is ok to OR them but not to AND them)

Also removed all filter related API that was marked as deprecated with level ERROR.

@davidtaylor-juul davidtaylor-juul added the minor Changes that should bump the MINOR version number label Jun 18, 2024
@davidtaylor-juul davidtaylor-juul changed the title Filter predicates Enhanced Scan Filtering Jun 19, 2024
@davidtaylor-juul davidtaylor-juul marked this pull request as ready for review June 19, 2024 18:03
@davidtaylor-juul davidtaylor-juul requested review from twyatt and a team as code owners June 19, 2024 18:03
Copy link
Member

@twyatt twyatt left a comment

Choose a reason for hiding this comment

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

Pass 1/x

Overall, I love this PR! Amazing work!

I created a branch with some small changes (twyatt/FilterPredicateSet), let me know what you think.

I'm using a typealias but I think just using the collection type directly would work.

If you wanted type safety not offered by the collection type, then maybe a value class would be a better choice than data class?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
core/src/commonMain/kotlin/FilterPredicate.kt Outdated Show resolved Hide resolved
core/src/commonMain/kotlin/FilterPredicate.kt Outdated Show resolved Hide resolved
@twyatt twyatt self-requested a review June 21, 2024 07:06
Copy link
Member

@twyatt twyatt left a comment

Choose a reason for hiding this comment

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

Did some basic testing using SensorTag sample app (Android and Apple) and everything worked great!

Love all the unit tests! Awesome work!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
core/src/jsMain/kotlin/ScannerBuilder.kt Show resolved Hide resolved
Copy link
Contributor

@cedrickcooke cedrickcooke left a comment

Choose a reason for hiding this comment

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

Sorry I got to this late, this is awesome.

@davidtaylor-juul davidtaylor-juul merged commit 4b0842a into main Jul 10, 2024
2 checks passed
@davidtaylor-juul davidtaylor-juul deleted the dtaylor/filter_predicates branch July 10, 2024 17:14
@twyatt twyatt mentioned this pull request Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Changes that should bump the MINOR version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support AND based scan filtering
3 participants