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

bluetooth: Add acceptAllDevices #458

Merged

Conversation

beaufortfrancois
Copy link
Member

You can give a try to this updated samples at https://beaufortfrancois.github.io/samples/web-bluetooth.

https://beaufortfrancois.github.io/samples/web-bluetooth/device-info.html and any sample previously using anyNamedDevice have been updated to use acceptAllDevices.

R: @g-ortuno @jeffposnick

log('Requesting Bluetooth Device...');
navigator.bluetooth.requestDevice({filters: filters})
log('with ' + JSON.stringify(options));
navigator.bluetooth.requestDevice(options)
Copy link

Choose a reason for hiding this comment

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

Note that you are not allowed to use acceptAllDevices with filters so you might need some logic for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I've fixed that in 4ebbcd1

return navigator.bluetooth.requestDevice(
{filters: anyNamedDevice(), optionalServices: ['battery_service']})
return navigator.bluetooth.requestDevice({
acceptAllDevices: true, optionalServices: ['battery_service']})
Copy link

Choose a reason for hiding this comment

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

It makes me sad that we are using acceptAllDevices so liberally. I recognize the desire to make our samples super easy to use but every developer that copies this code will end up using it which will result in a bunch of unrelated devices being shown in the chooser and energy being wasted because we have no filters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would 9533907 be enough?

@@ -3,9 +3,9 @@ var bluetoothDevice;
async function onButtonClick() {
bluetoothDevice = null;
try {
log('Requesting any Bluetooth Device...');
log('Requesting any Bluetooth Device... (NOT RECOMMENDED - USE FILTERS)');
Copy link

@g-ortuno g-ortuno Dec 7, 2016

Choose a reason for hiding this comment

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

I don't think it's that clear what exactly is not recommended?

navigator.bluetooth.requestDevice({
  acceptAllDevices: true /* NOT RECOMMENDED - USE FILTERS */});

Also fwiw: 10% of the devices requests have been using anyNamedDevice().

Copy link

Choose a reason for hiding this comment

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

Or maybe:

navigator.bluetooth.requestDevice({
  // filters [{...}] <- Prefer to use filters to save energy and show only relevant devices.
  acceptAllDevices: true
});

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @g-ortuno! How about 5f0a821?

@g-ortuno
Copy link

lgtm

@beaufortfrancois
Copy link
Member Author

I'll wait for M56 to land in Stable channel before merging this CL then.

@beaufortfrancois beaufortfrancois merged commit dbed7f9 into GoogleChrome:gh-pages Feb 8, 2017
@beaufortfrancois beaufortfrancois deleted the acceptAllDevices branch February 8, 2017 20:13
markafoltz pushed a commit that referenced this pull request Oct 14, 2024
* bluetooth: Add acceptAllDevices

* Don't use both acceptAllDevices and filters

* Add warning about acceptAllDevices

* Converted missing link-loss sample

* Remove autofocus for prefilled fields
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