Add support for scanning for mock devices using ScanSettings and Scan… #703

nrbrook · 2020-07-21T16:35:19Z

…Filter(s)

Added implementation and unit tests

…Filter(s)

CLAassistant · 2020-07-21T16:35:23Z

All committers have signed the CLA.

Also fixed bug when filtering

dariuszseweryn · 2020-07-23T16:33:12Z

Hello,
This PR looks awesome. I will look into the code in a moment.
There are some checkstyle rule violations could you address them?

dariuszseweryn

Some comments for discussion

mockrxandroidble/src/main/java/com/polidea/rxandroidble2/mockrxandroidble/RxBleClientMock.java

dariuszseweryn · 2020-07-23T17:00:53Z

mockrxandroidble/src/main/java/com/polidea/rxandroidble2/mockrxandroidble/RxBleClientMock.java

+        return bb.array();
+    }
+
+    private static boolean filterDevice(RxBleDevice rxBleDevice, ScanFilter... scanFilters) {


I am not sure why it is RxBleDevice here where it is clearly meant for RxBleDeviceMock?

Me neither...copied from the original method private static boolean filterDevice(RxBleDevice rxBleDevice, @Nullable UUID[] filterServiceUUIDs) {

Do I need to change this? I don't think it matters really.

mockrxandroidble/src/main/java/com/polidea/rxandroidble2/mockrxandroidble/RxBleClientMock.java

...androidble/src/main/java/com/polidea/rxandroidble2/mockrxandroidble/RxBleScanRecordMock.java

…xandroidble/RxBleClientMock.java Link in comment block Co-authored-by: Dariusz Seweryn <[email protected]>

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanFilter.java

nrbrook · 2020-07-24T10:58:39Z

PR #706 is based on this PR and would be preferred instead of this PR to avoid creating new APIs that are later deprecated, although if both are merged between releases it wouldn't really matter.

dariuszseweryn

I thought I have posted this review before the weekend but it turned out I didn't :/

To not be overwhelmed by stacked changes I would like to merge your PRs sequentially starting with this one and proceeding with next.

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanFilter.java

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanResultInterface.java

nrbrook · 2020-07-27T10:17:17Z

To not be overwhelmed by stacked changes I would like to merge your PRs sequentially starting with this one and proceeding with next.

This PR is merged into #706, and that is merged into #707 already

dariuszseweryn · 2020-07-27T10:22:31Z

To not be overwhelmed by stacked changes I would like to merge your PRs sequentially starting with this one and proceeding with next.

This PR is merged into #706, and that is merged into #707 already

I know, unfortunately partial code reviews are not currying and I would like to first merge this one before starting review of the next — this way #706 will show only differences with master (now it is showing both this and next changes)

nrbrook · 2020-07-27T10:48:23Z

Oh sorry yes that's fine – I misread your comment and thought you wanted me to do some merging. I was only concerned about having to maintain deprecated constructors if releases were made between PRs.

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanResult.java

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanFilter.java

rxandroidble/src/test/groovy/com/polidea/rxandroidble2/scan/ScanFilterTest.groovy

…lder

nrbrook

Like this? eb7d009

dariuszseweryn · 2020-07-27T15:07:35Z

Like this? eb7d009

Also move the implements ScanResultInterface from ScanResult onto RxBleScanResultMock.java

dariuszseweryn · 2020-07-27T15:20:28Z

ScanFilter.matches uses the interface to accept ScanResult and RxBleInternalScanResult objects

Correct. If you would move the interface from ScanResult to RxBleScanResultMock then ScanFilter.matches would accept RxBleScanResultMock and RxBleInternalScanResult which would be convenient and would not change ScanResult API at all :) Or am I wrong?

…vert ScanResult changes

nrbrook · 2020-07-27T15:24:08Z

ScanFilter.matches uses the interface to accept ScanResult and RxBleInternalScanResult objects

Correct. If you would move the interface from ScanResult to RxBleScanResultMock then ScanFilter.matches would accept RxBleScanResultMock and RxBleInternalScanResult which would be convenient and would not change ScanResult API at all :) Or am I wrong?

Got it, see latest commit. Got confused for a minute there!

dariuszseweryn

Almost there :)

mockrxandroidble/src/main/java/com/polidea/rxandroidble2/mockrxandroidble/RxBleClientMock.java

…xandroidble/RxBleClientMock.java Co-authored-by: Dariusz Seweryn <[email protected]>

dariuszseweryn

Awesome 👍 🚀

dariuszseweryn · 2020-07-28T16:17:07Z

rxandroidble/src/test/groovy/com/polidea/rxandroidble2/scan/ScanFilterTest.groovy


-class ScanFilterTest extends Specification {
+@Config(manifest = Config.NONE, constants = BuildConfig, sdk = Build.VERSION_CODES.LOLLIPOP)
+public class ScanFilterTest extends ElectricSpecification {


Yeah. I totally forgot about it. 👍

rfrowe · 2021-03-03T00:43:03Z

I'd love to have this for my unit tests. However, there hasn't been a release in nearly a year. Is another planned soon?

nrbrook · 2021-03-03T08:59:25Z

@rfrowe It seems Polidea was bought out - I’m trying to reach out to Dariusz to see what’s happening and if he’ll continue maintaining it. If not I’ll look at maintaining and releasing a fork as we need this to work for commercial projects.

rfrowe · 2021-03-03T18:50:56Z

@nrbrook oof did this happen quite recently? I figured this project was still active when I chose to use it, since the last commit was in January. For now, I set Gradle to install this library from git via JitPack so I can use this change, but if you do end up forking and releasing to Maven, let me know :)

nrbrook · 2021-03-03T19:14:13Z

Nice I didn’t know you could do that via jitpack. Acquired February 8th according to linked in.

rfrowe · 2021-03-03T21:35:56Z

@nrbrook if you're interested:

testImplementation "com.github.Polidea.RxAndroidBle:rxandroidble:$rx_android_ble_version"
testImplementation "com.github.Polidea.RxAndroidBle:mockclient:$rx_android_ble_version"

where rx_android_ble_version is master-SNAPSHOT and you must have:

repositories {
    maven { url 'https://jitpack.io' }
}

dariuszseweryn · 2021-03-31T21:35:57Z

There is now a 1.12.0-SNAPSHOT release available (see readme for snapshots)

Add support for scanning for mock devices using ScanSettings and Scan…

b10a24c

…Filter(s)

nrbrook added 4 commits July 21, 2020 21:06

Make RxBleScanRecordMock.Builder static

377ab2e

Removed duplicate tests

14668a1

Include scan record service UUIDs when filtering mocks

9b55902

Also fixed bug when filtering

Filter mocks on scan record name, not device name

1b76a1e

Fix style violations

78514e2

dariuszseweryn reviewed Jul 23, 2020

View reviewed changes

nrbrook and others added 4 commits July 23, 2020 18:13

Update mockrxandroidble/src/main/java/com/polidea/rxandroidble2/mockr…

d0b9820

…xandroidble/RxBleClientMock.java Link in comment block Co-authored-by: Dariusz Seweryn <[email protected]>

Fix ScanFilter match bug

7b89793

Use ScanFilter.matches for mock scan result filtering

2a21a89

Throw exception on getBytes

0086b51

nrbrook commented Jul 23, 2020

View reviewed changes

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanFilter.java Outdated Show resolved Hide resolved

nrbrook added 2 commits July 23, 2020 21:16

Style fixes

0ddbc19

Readme update

b958a4f

nrbrook mentioned this pull request Jul 24, 2020

Mock structure changes #706

Merged

nrbrook added 2 commits July 24, 2020 17:28

Merge branch 'master' into mock-scan-record

9d7729b

Removed test – no longer required

6a19cdd

dariuszseweryn reviewed Jul 27, 2020

View reviewed changes

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanFilter.java Outdated Show resolved Hide resolved

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanResultInterface.java Outdated Show resolved Hide resolved

nrbrook added 2 commits July 27, 2020 11:43

Add documentation to ScanResultInterface

1552f84

Revert removal of ScanFilter match on cached device name

0414620

Move ScanFilterInterface to internal

9ab2a05

dariuszseweryn reviewed Jul 27, 2020

View reviewed changes

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanResult.java Outdated Show resolved Hide resolved

rxandroidble/src/main/java/com/polidea/rxandroidble2/scan/ScanFilter.java Outdated Show resolved Hide resolved

nrbrook added 2 commits July 27, 2020 15:41

Fix device name filtering and add unit test

cdefd74

Added failing UUID test

a5b547b

nrbrook commented Jul 27, 2020

View reviewed changes

rxandroidble/src/test/groovy/com/polidea/rxandroidble2/scan/ScanFilterTest.groovy Outdated Show resolved Hide resolved

nrbrook added 2 commits July 27, 2020 15:56

Create and use mock scan result class

eb7d009

Mock scan record byte array should be compiled in constructor not bui…

35804c7

…lder

nrbrook commented Jul 27, 2020

View reviewed changes

Move ScanResultInterface implementation to RxBleScanResultMock and re…

1e21387

…vert ScanResult changes

Fixed RxBleClientMock

1bd5d7b

dariuszseweryn reviewed Jul 27, 2020

View reviewed changes

mockrxandroidble/src/main/java/com/polidea/rxandroidble2/mockrxandroidble/RxBleClientMock.java Outdated Show resolved Hide resolved

nrbrook and others added 3 commits July 27, 2020 16:44

Update mockrxandroidble/src/main/java/com/polidea/rxandroidble2/mockr…

b1cb0ca

…xandroidble/RxBleClientMock.java Co-authored-by: Dariusz Seweryn <[email protected]>

Added full set of tests for ScanFilter.matches, but failing

67ff87b

Fixed ScanFilter unit tests

d997b9f

dariuszseweryn approved these changes Jul 28, 2020

View reviewed changes

dariuszseweryn merged commit da9692e into dariuszseweryn:master Jul 28, 2020

rfrowe mentioned this pull request Mar 4, 2021

RxBleScanRecordMock missing in maven artifact #736

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for scanning for mock devices using ScanSettings and Scan… #703

Add support for scanning for mock devices using ScanSettings and Scan… #703

nrbrook commented Jul 21, 2020

CLAassistant commented Jul 21, 2020 •

edited

Loading

dariuszseweryn commented Jul 23, 2020

dariuszseweryn left a comment

dariuszseweryn Jul 23, 2020

nrbrook Jul 23, 2020

nrbrook Jul 27, 2020

nrbrook commented Jul 24, 2020 •

edited

Loading

dariuszseweryn left a comment •

edited

Loading

nrbrook commented Jul 27, 2020

dariuszseweryn commented Jul 27, 2020

nrbrook commented Jul 27, 2020 •

edited

Loading

nrbrook left a comment •

edited

Loading

dariuszseweryn commented Jul 27, 2020

dariuszseweryn commented Jul 27, 2020

nrbrook commented Jul 27, 2020

dariuszseweryn left a comment

dariuszseweryn left a comment

dariuszseweryn Jul 28, 2020

rfrowe commented Mar 3, 2021

nrbrook commented Mar 3, 2021

rfrowe commented Mar 3, 2021

nrbrook commented Mar 3, 2021

rfrowe commented Mar 3, 2021

dariuszseweryn commented Mar 31, 2021

Add support for scanning for mock devices using ScanSettings and Scan… #703

Add support for scanning for mock devices using ScanSettings and Scan… #703

Conversation

nrbrook commented Jul 21, 2020

CLAassistant commented Jul 21, 2020 • edited Loading

dariuszseweryn commented Jul 23, 2020

dariuszseweryn left a comment

Choose a reason for hiding this comment

dariuszseweryn Jul 23, 2020

Choose a reason for hiding this comment

nrbrook Jul 23, 2020

Choose a reason for hiding this comment

nrbrook Jul 27, 2020

Choose a reason for hiding this comment

nrbrook commented Jul 24, 2020 • edited Loading

dariuszseweryn left a comment • edited Loading

Choose a reason for hiding this comment

nrbrook commented Jul 27, 2020

dariuszseweryn commented Jul 27, 2020

nrbrook commented Jul 27, 2020 • edited Loading

nrbrook left a comment • edited Loading

Choose a reason for hiding this comment

dariuszseweryn commented Jul 27, 2020

dariuszseweryn commented Jul 27, 2020

nrbrook commented Jul 27, 2020

dariuszseweryn left a comment

Choose a reason for hiding this comment

dariuszseweryn left a comment

Choose a reason for hiding this comment

dariuszseweryn Jul 28, 2020

Choose a reason for hiding this comment

rfrowe commented Mar 3, 2021

nrbrook commented Mar 3, 2021

rfrowe commented Mar 3, 2021

nrbrook commented Mar 3, 2021

rfrowe commented Mar 3, 2021

dariuszseweryn commented Mar 31, 2021

CLAassistant commented Jul 21, 2020 •

edited

Loading

nrbrook commented Jul 24, 2020 •

edited

Loading

dariuszseweryn left a comment •

edited

Loading

nrbrook commented Jul 27, 2020 •

edited

Loading

nrbrook left a comment •

edited

Loading