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

Re-connection to already known hubs #114

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pwimmer
Copy link

@pwimmer pwimmer commented Dec 2, 2020

This pull requests improves the Web Bluetooth support. The scan method scans for advertisements of hubs that have been connected before. The old scan method that caused a popup in the browser has been renamed to requestDevice.

I've tested with Chrome 87. The following flags are required:

  • chrome://flags/#enable-experimental-web-platform-features
  • chrome://flags/#enable-web-bluetooth-new-permissions-backend

There are still issues that the LED color does not change in the sample application, but I had the same issue with the original code.

@aileo
Copy link
Contributor

aileo commented Dec 4, 2020

That is a nice improvement.
Instead of changing the behavior of the scan method (which would break backward compatibility), do you think it could be a parameter of the scan method ?

@pwimmer
Copy link
Author

pwimmer commented Dec 5, 2020

Instead of changing the behavior of the scan method (which would break backward compatibility), do you think it could be a parameter of the scan method ?

The reason why I renamed the scan method is that the new scan/stop methods act very similar to the Node implementation so I thought they should use the same names. The requestDevice method has no equivalent in Node so I thought it should have a new name.

As scan and requestDevice do different things it is cleaner to have separate methods. But I'm fine with a parameter if backward compatibility must be preserved.

@aileo
Copy link
Contributor

aileo commented Dec 7, 2020

Instead of changing the behavior of the scan method (which would break backward compatibility), do you think it could be a parameter of the scan method ?

The reason why I renamed the scan method is that the new scan/stop methods act very similar to the Node implementation so I thought they should use the same names. The requestDevice method has no equivalent in Node so I thought it should have a new name.

Seems legit.

This lib uses the term "Device" to refer to items connected to hubs (motors, sensors...), I think requestDevice could be confusing, maybe requestHub could be more accurate.

@nathankellenicki
Copy link
Owner

nathankellenicki commented Dec 23, 2020

@pwimmer I'm fine with breaking changes when good rationale is presented, and this is good rationale. :) Synchronicity between the Node.js and Web Bluetooth API is certainly a plus to reduce the amount of mental hoops would need to jump over when working between the two.

@aileo 's suggestion for requestHub over requestDevice is a good one to maintain naming conventions with the rest of the lib.

This is a great additional piece of functionality, many thanks for the contribution! My main concern is the necessity for end users to enable experimental features in the browser - one of the tenets I tried to follow is to make it so that the lib could just be published to a website which end users could visit with no further barriers. Do you have additional information on when these features may land proper? A link to some docs or discussions perhaps?

Thanks, and happy holidays to both!

@nathankellenicki
Copy link
Owner

@pwimmer I got around to testing this PR properly tonight. Unfortunately it looks like these features still haven't landed as they need to be turned on in Chrome flags.

In addition, I wasn't able to get this reliably working. After pairing, it wouldn't automatically reconnect to any previously paired hubs until I opened the pairing dialog again, at which point it would automatically connect, even without selecting the device and pairing. It seems unfortunately there are still some bugs in the implementation.

@pwimmer
Copy link
Author

pwimmer commented Nov 17, 2021

In addition, I wasn't able to get this reliably working. After pairing, it wouldn't automatically reconnect to any previously paired hubs until I opened the pairing dialog again, at which point it would automatically connect, even without selecting the device and pairing. It seems unfortunately there are still some bugs in the implementation.

Automatic reconnection requires a https server. It doesn't work with http.

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.

None yet

3 participants