noble / noble-device

A Node.js lib to abstract BLE (Bluetooth Low Energy) peripherals, uses noble
MIT License
97 stars 66 forks source link

Suggestion for additional peripheral state #6

Closed simsalapim closed 9 years ago

simsalapim commented 9 years ago

Hey @sandeepmistry, what are your thoughts on adding an additional Noble device state that is "connectedAndSetup”? When connectAndSetup() has completed, the device’s state could automatically switch to “connectedAndSetup”, indicating that its services and characteristics are ready to be interacted with.

Looping in @raykamp here too.

sandeepmistry commented 9 years ago

@simsalapim that makes sense.

What are your thoughts on possible states? So, far I can think of:

Are you thinking events for state change to just a regular property?

raykamp commented 9 years ago

@sandeepmistry would you be adding a new state property to the NobleDevice class, or adding this state to noble' peripheral?

Some more context: The problem that we've been encountering is when an action is performed on our device, we first check to see it it's ready. Currently, we are using if(device._peripheral.state === "connected") to (incorrectly) check that the device is ready. In our application, we could add a state that is set to true when connectAndSetup() is returned successfully, but this seems valuable to most users and a worthy inclusion in noble-device.

A more ideal scenario could look like this (a little redundant, but safe): if(device && device._peripheral.state === "connected" && device.isSetUp === true){ //Then perform some action }

sandeepmistry commented 9 years ago

@raykamp was thinking off adding a new state property to NobleDevice. Agreed, it would be useful to have in node-device.

Would this work for you?

if(device && device.state === "setup"){ //Then perform some action }

No need for the caller to know about the internal peripheral object.

raykamp commented 9 years ago

@sandeepmistry, yes that way of accessing device state would be very helpful.

My one concern would be in naming the states. "SetUp" or "set up" could indicated that the BLE device is configured, but if mistaken with the word "setup", the user could interpret this stage as the being in the active process of "setting up" the peripheral.

Perhaps having a more verbose state name could help with this. Perhaps the following:

In the case of a NobleDevice, the states for "connected" and "setting up" seem like they could be combined into one state, right? In the list above, I proposed this as "connectedAndSettingUp"

sandeepmistry commented 9 years ago

@raykamp good point. I agree with your list, a small change I would make is a slight case change for "connectedAndSetUp" to "connectedAndSetup".

sandeepmistry commented 9 years ago

@raykamp any progress on this?

raykamp commented 9 years ago

Hey @sandeep, I haven't started yet! I'm going to push something tomorrow afternoon for you to look it over. If the concept looks good, I'll wrap up the rest.

Cheers -Ray

On Monday, April 6, 2015, Sandeep Mistry notifications@github.com wrote:

@raykamp https://github.com/raykamp any progress on this?

— Reply to this email directly or view it on GitHub https://github.com/sandeepmistry/noble-device/issues/6#issuecomment-90295123 .

raykamp commented 9 years ago

@sandeepmistry, I've settled on a very simple solution for this. It doesn't feel over-engineered, and it keeps a very simple interface for the user in the same spirit as the entire noble-device project. It's a simple binary state that shows when the device is ready to be used, or not.

Here's the PR: https://github.com/sandeepmistry/noble-device/pull/7

sandeepmistry commented 9 years ago

@raykamp thanks for the contribution! I've publish v1.0.1 with change from #7.