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

initial websocket bindings #124

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

Conversation

jacobrosenthal
Copy link
Member

First pass at websocket bindings. Sorta works, but seems to spawn a new device every time.

Feedback obviously welcome.

I dont see any unit tests for bindings?

var child_process = require('child_process');

var BlenoBindings = function() {
var port = 0xB1f;
Copy link
Member Author

Choose a reason for hiding this comment

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

Arbitrarily chose 0xB1f, one up from nobles websocket.

@jacobrosenthal
Copy link
Member Author

I also missed this os check, https://github.com/sandeepmistry/bleno/blob/master/lib/bleno.js#L132

It seems innapropriate to os check a second time, and its (currently) impossible to know what bindings are under the WS hood...

Perhaps the bleno api should be identical no matter the platform, but rather send a null value for the unimplemented variables?

@jacobrosenthal
Copy link
Member Author

Its not exactly unit tests, but I found the debug=bindings helpful in trying to determine if ws is properly passing through and executing the same bindings calls. All examples appear identical, heres the battery characteristic logs https://gist.github.com/jacobrosenthal/3aa67244397d2ffdb01a

@jacobrosenthal
Copy link
Member Author

Interesting.. stop isnt working from the websocket bindings at the moment even thought the bindings logging look perfect. @sandeepmistry notice the super fast return time on the not working copy (15ms vs 155) Thoughts on that?
https://gist.github.com/jacobrosenthal/9728ad4ddb1d3c0bdc04

@sandeepmistry
Copy link
Collaborator

@jacobrosenthal looks ok on my Mac:

var eddystoneBeacon = require('./../../index');

eddystoneBeacon.advertiseUid('00010203040506070809', 'aabbccddeeff');

setTimeout(function() {
  console.log('stop');
  eddystoneBeacon.stop();
}, 5000);

Using LightBlue on iOS, could not discover my Mac. What are you using for the central?

@jacobrosenthal
Copy link
Member Author

I havent been able to reproduce it anymore yet. Maybe just a weird blued state that day..

@jacobrosenthal
Copy link
Member Author

Eddystone does a platform check as well which doesnt square with websocket:
don/node-eddystone-beacon#23

I have a platform check branch bringing os.platform call into bleno api so it can be used behind websocket that after some more testing and discussion Im going to merge here:
https://github.com/jacobrosenthal/bleno/tree/websocket-platformcheck

@sandeepmistry
Copy link
Collaborator

I have a platform check branch bringing os.platform call into bleno api so it can be used behind websocket that after some more testing and discussion Im going to merge here:
https://github.com/jacobrosenthal/bleno/tree/websocket-platformcheck

Like we've been discussing in don/node-eddystone-beacon#25 (comment) - I think we both agree the platform check can be a property instead of a async function

@jacobrosenthal
Copy link
Member Author

Added a platform check

@rzr
Copy link

rzr commented Mar 5, 2020

May I suggest to forward your changes to:

https://github.com/abandonware/bleno/

Meanwhile, feedback welcome on latest release:

https://www.npmjs.com/package/@abandonware/bleno/v/0.5.1-2

@rzr
Copy link

rzr commented Feb 20, 2021

Please confirm this issue is resolved with @abandonware version

Relate-to: abandonware#17

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.

3 participants