Add callback support to advertise* and stop API's

[sandeepmistry]

Alternative to #22, need to update read me later.

Example:

eddystoneBeacon.advertiseUid('00010203040506070809', 'aabbccddeeff', {}, function() {
  console.log('advertiseUid started');

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

@jacobrosenthal thoughts?

[jacobrosenthal]

Looks like it solves async problems, and gives the bleno start and stop error in the process. It still seems wierd though that if ble is off, you just wouldnt get anything back from the start callback right? Because eddystone will keep spinning waiting for statechange to be powered on? Also theres event to either get a global error (not sure why you would) or more likely to be notified that state change poweredoff and advertising has presumably stopped

[sandeepmistry]

It still seems wierd though that if ble is off, you just wouldnt get anything back from the start callback right? Because eddystone will keep spinning waiting for statechange to be powered on?

Correct. What are you thinking, should we call the callback with an error in this case (similar for other states: unauthorized, unsupported)

[jacobrosenthal]

Sure, but when? State isnt exposed to user currently so they have no way to wait until state before calling. So if you just return the error theyd have to just keep calling until success which isnt ideal.

Or if you keep it as it currently is, youd use a timeout? but that feels awful.

Think youd have to expose state. so you can do it the way bleno does it, wait for state then go

[sandeepmistry]

Sure, but when? State isnt exposed to user currently so they have no way to wait until state before calling. So if you just return the error theyd have to just keep calling until success which isnt ideal.

Good point.

Think youd have to expose state. so you can do it the way bleno does it, wait for state then go

We can leave the callbacks in, but need to do something like:

var eddystoneBeacon = require('eddystone-beacon');

eddystoneBeacon.on('adapterStateChange', function(adapterState) {
  if (state === 'poweredOn') {
    eddystoneBeacon.advertiseUrl('http://www.google.com');
  } else {
    console.log('can\'t start advertising, adapter state = %s', adapterState);
  }
});

So changes I'm thinking about are:

1. Re-emitting the bleno state change event as adapterStateChange
2. Making the advertise API's throw an error or call the supplied callback with an error, when the adapter state is non powered on.

This is a breaking change, but I think it's for the best. @jacobrosenthal is this what you were thinking?

cc/ @don

[don]

I like the idea of calling the callback with an error if the adapter state is not poweredOn. But, re-emitting the bleno state changes makes the code work like bleno, which is a good thing.

Is it worth duplicating the events or just using bleno? (Probably worth duplicating as long as we don't have to duplicate a bunch of other stuff too.)

var bleno = require('bleno');
var eddystoneBeacon = require('eddystone-beacon');

bleno.on('stateChange', function(state) {
  if (state === 'poweredOn') {
    eddystoneBeacon.advertiseUrl('http://www.google.com');
  } else {
    console.log('can\'t start advertising, adapter state = %s', state);
  }
});

[jacobrosenthal]

Turns out I need the state change in order to do my platform check. Currently we platform check every time we advertise, which can be 10+ times a second. I want to cache that on startup, but theres currently no way to know when bleno is ready (in my case, that the websocket is open)

[sandeepmistry]

Hmm, I'm wondering if it would be better to have a static property, like bleno.platform instead of using the OS check. In web socket mode this could be sent just before the current state is synced on socket connect.

[jacobrosenthal]

I think you're saying what I thought I was saying.

On Mon, Sep 28, 2015 at 2:15 AM, Sandeep Mistry notifications@github.com
wrote:

Hmm, I'm wondering if it would be better to have a static property, like
bleno.platform instead of using the OS check. In web socket mode this
could be sent just before the current state is synced on socket connect.

[sandeepmistry]

@jacobrosenthal right, let's bring the discussion over to bleno then?

We need to split this discussion up:

1. Adding callback param support
2. Add state change event, and not allow users to start advertising without waiting for a powered on state
3. Platform check, already tracked in Platformcheck #24, but need something in bleno (I think)

[jacobrosenthal]

So back on callback support here, I actually get an error on linux with uid/with-tlm/js every time startadvertising is called, Command Disallowed, do you know what that one might refer to @sandeepmistry ?

[sandeepmistry]

@jacobrosenthal that's the response to the HCI command bleno is issuing, you can run an hcidump capture to take a closer look at what state of the adapter is. I'll have to take a closer look later on.