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

Add duration flag (in seconds) to scan with options #967

Merged
merged 5 commits into from
May 20, 2023

Conversation

peitschie
Copy link
Collaborator

This allows different scan modes to be easily selected while still allowing the scan to be time-limited

peitschie added 2 commits May 6, 2023 13:53
This allows different scan modes to be easily selected while
still allowing the scan to be time-limited
@peitschie peitschie force-pushed the scan-with-timeout branch from 002301f to 6d507fd Compare May 6, 2023 03:53
@mikerusso-tsi
Copy link

mikerusso-tsi commented May 8, 2023

Looks like you're cutting off the 'scan' function, which will force all consumers of that to switch. Do you normally want a deprecation period to ease people over? I.e., change 'scan' to internally call 'startScanWithOptions'.

Also, will this be available in the 'slim' variant?

BTW, here is my code to replace the 'scan' call. I believe I am seeing the same scan performance with it as before the API change you indicated!
let options = { scanMode: 'lowLatency', duration: timeout }; ble.startScanWithOptions(services, options, success, failure);
where timeout is a number in seconds.

@peitschie
Copy link
Collaborator Author

Bothscan and startScan remain fully supported. If you pay close attention to ble.js, you'll see that I've routed these to the equivalent scanWithOptions call instead, as all the native paths trace through the same code.

@mikerusso-tsi
Copy link

Bothscan and startScan remain fully supported. If you pay close attention to ble.js, you'll see that I've routed these to the equivalent scanWithOptions call instead, as all the native paths trace through the same code.

Ah, that's what I was expecting! Sorry.
But is the @slim variant included in this PR? I'm hoping so!

@peitschie
Copy link
Collaborator Author

Slim runs on a different branch. I'll publish both at the same time once I merge this 🙂

…started

The this::stopScan syntax seems to lose the exact reference for the runnable,
preventing the remove callback from properly clearing it.
@peitschie peitschie force-pushed the scan-with-timeout branch from df6929e to 7a3db00 Compare May 19, 2023 14:16
@peitschie peitschie force-pushed the scan-with-timeout branch from 7a3db00 to 4601ef1 Compare May 20, 2023 02:02
@peitschie peitschie force-pushed the scan-with-timeout branch from 4601ef1 to 3946f97 Compare May 20, 2023 04:20
@peitschie peitschie merged commit b4704db into master May 20, 2023
@peitschie peitschie deleted the scan-with-timeout branch May 20, 2023 04:25
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