-
Notifications
You must be signed in to change notification settings - Fork 159
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
Inject GATT descriptors #29
Conversation
Thanks, can you try to split this change set into several pull requests on upstream too ? Reviewers welcome too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts:
- I don't care for the naming of the new events. It seems like the original events were just misspelled, so adding these new events with the names the originals should have had is confusing. Why not just add the service/characteristic objects as additional data to the existing events? That should retain backwards compatibility while being more clear.
- Can you update the README and maybe add an example in
examples/
? - It seems strange for a developer to have to reinitialize an object manually. Is there a way that noble could (optionally) cache these data structures internally, rather than forcing the developer to hold on to internal objects they can't do much with?
Thanks for your thoughts @mrstegeman, let me add my own:
|
examples/cache-gatt-reconnect.js
Outdated
} | ||
console.log() | ||
|
||
let temperatureChara = undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it well named ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be better to have all temperature related code into a separate file ?
Please also submit upstream 1st
I intended to write a small but functional example. I don't think the effort of extracting 7 lines or so is worth a separate file. The point here is the difference in timing of the same task under different conditions (with cache / without).
I'm commiting to my fork which you are seeing automatically. This is fine for me, esp. as this allows you and others to comment here. I'd like to push an agreed-upon version upstream and not double work on every commit. |
I suggested to isolate temperature in separate place to make it scalable for other types... |
Well then let me suggest to use an environment variable, that way you can change it on the fly, as in If you have something else in mind, suggestion welcome :) |
examples/cache-gatt-discovery.js
Outdated
@@ -88,7 +94,7 @@ let findServices = function (noble, peripheral) { | |||
|
|||
peripheral.discoverServices([], (error, services) => { | |||
|
|||
let temperatureCharacteristic = undefined | |||
let sensorCharacteristic = undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be set explicitly ?
@@ -8,7 +8,13 @@ | |||
var noble = require('../index'); | |||
const fs = require('fs'); | |||
|
|||
// the sensor value to scan for, number of bits and factor for displaying it | |||
const CHANNEL = process.env['CHANNEL'] ? process.env['CHANNEL'] : 'Temperature' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or just "Unknown"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer a working example... at least for parts of the universe ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this, what about other reviewers ?
Seems excellent - anything else holding this back? |
Unless there is any negative feedback I can merge this next month cc: @mrstegeman, @gfwilliams also @readeral have you considered to join the abandonware co-maintainers team see |
My primary request at this point is that the README is updated with the new events and APIs. |
…ered, characteristicsDiscovered) that enables caching of gatt information. The cached objects can be used to initialize a device via addServices() and addCharacteristics(), which return an array of Noble objects ready for connecting in the usual way. examples/cache-gatt-discovery.js provides an example for collecting GATT data and persisting the information, examples/cache-gatt-reconnect.js uses the data to connect to the device using the cached data.
f1cc0e4
to
ed5fc14
Compare
Please review other PR then I plan to release something in a couple of weeks |
(serviceDiscovered, characteristicsDiscovered) that enables caching of gatt information. The cached objects can be used to initialize a device via addServices() and addCharacteristics(), which return an array of Noble objects ready for connecting in the usual way. examples/cache-gatt-discovery.js provides an example for collecting GATT data and persisting the information, examples/cache-gatt-reconnect.js uses the data to connect to the device using the cached data. Author: Simon Vogl <simon@voxel.at> Relate-to: #29 Change-Id: I68bc5208529f872e5f60d7c0315a6346d24df2fb
(serviceDiscovered, characteristicsDiscovered) that enables caching of gatt information. The cached objects can be used to initialize a device via addServices() and addCharacteristics(), which return an array of Noble objects ready for connecting in the usual way. examples/cache-gatt-discovery.js provides an example for collecting GATT data and persisting the information, examples/cache-gatt-reconnect.js uses the data to connect to the device using the cached data. Author: Simon Vogl <simon@voxel.at> Relate-to: #29 Change-Id: I68bc5208529f872e5f60d7c0315a6346d24df2fb
(serviceDiscovered, characteristicsDiscovered) that enables caching of gatt information. The cached objects can be used to initialize a device via addServices() and addCharacteristics(), which return an array of Noble objects ready for connecting in the usual way. examples/cache-gatt-discovery.js provides an example for collecting GATT data and persisting the information, examples/cache-gatt-reconnect.js uses the data to connect to the device using the cached data. Author: Simon Vogl <simon@voxel.at> Relate-to: #29 Change-Id: I68bc5208529f872e5f60d7c0315a6346d24df2fb
The mac bindings do not call |
(serviceDiscovered, characteristicsDiscovered) that enables caching of gatt information. The cached objects can be used to initialize a device via addServices() and addCharacteristics(), which return an array of Noble objects ready for connecting in the usual way. examples/cache-gatt-discovery.js provides an example for collecting GATT data and persisting the information, examples/cache-gatt-reconnect.js uses the data to connect to the device using the cached data. Author: Simon Vogl <simon@voxel.at> Relate-to: abandonware#29 Change-Id: I68bc5208529f872e5f60d7c0315a6346d24df2fb
(serviceDiscovered, characteristicsDiscovered) that enables caching of gatt information. The cached objects can be used to initialize a device via addServices() and addCharacteristics(), which return an array of Noble objects ready for connecting in the usual way. examples/cache-gatt-discovery.js provides an example for collecting GATT data and persisting the information, examples/cache-gatt-reconnect.js uses the data to connect to the device using the cached data. Author: Simon Vogl <simon@voxel.at> Relate-to: abandonware#29 Change-Id: I68bc5208529f872e5f60d7c0315a6346d24df2fb
(serviceDiscovered, characteristicsDiscovered) that enables caching of gatt information. The cached objects can be used to initialize a device via addServices() and addCharacteristics(), which return an array of Noble objects ready for connecting in the usual way. examples/cache-gatt-discovery.js provides an example for collecting GATT data and persisting the information, examples/cache-gatt-reconnect.js uses the data to connect to the device using the cached data. Author: Simon Vogl <simon@voxel.at> Relate-to: #29 Change-Id: I68bc5208529f872e5f60d7c0315a6346d24df2fb
We have a number of known BLE devices the we (re-)connect to often. Speeding up the connection setup phase is good for the user experience (less delay, less potential for packet loss), so we have extended noble a little to report the GATT data structures for services and characteristics and to initialize its inner workings with this data.
In order to not change default behavour, there are two new events one can listen for when discovering GATT entries, simply register them before the actual discovery call:
returns an array of services resp. characteristics including low-level information that can be used to initialize noble:
addServices returns an array of service objects, addCharacteristics an array of Characteristic objects that can be used to discover, subscribe for data etc.