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

[bluetooth] Add support for service data #10278

Merged
merged 5 commits into from
Aug 25, 2022

Conversation

PRosenb
Copy link
Contributor

@PRosenb PRosenb commented Mar 4, 2021

Bluetooth low energy allows broadcasters to advertise not only their presence but also Service Data. In Service Data, data of a Bluetooth Service's Characteristic can be advertised. Doing so allows a central like openHAB to receive data from a Bluetooth device without the need to connect to it. This helps to reduce power consumption on the peripheral.

Supplement to the Bluetooth Core Specification, 1.11 SERVICE DATA, page 19

The aim of this pull request is to add basic support of Service Data to openHAB so that a temperature/humidity sensor broadcasting its value as Service Data is supported.

I'm curious what you think about this idea in general @cpmeister and also on how I suggest to implement it. Input and feedback is very welcome.

@PRosenb
Copy link
Contributor Author

PRosenb commented Mar 24, 2021

What do you think @cpmeister? Will you review my suggestion?

@PRosenb
Copy link
Contributor Author

PRosenb commented Apr 5, 2021

@Hilbrand, @kaikreuzer, @weymann it seams that @cpmeister has no time to do a review. Is there someone else who could do that?

@wborn wborn added the enhancement An enhancement or new feature for an existing add-on label Apr 24, 2021
@kaikreuzer
Copy link
Member

Hi @PRosenb! Yes, it is a pity that @cpmeister does not find the time as he is our BLE expert...
But I am also very interested in getting support for service/characteristic data for beacon devices as this is a very common use case for BLE.
I had a look at your code and while it looks good in general, my concern is that it won't scale - you have implemented support for a single characteristic, but there exist more than 200 of them. I guess we'd hence have to rather use some map/library and do dynamic lookups.
Also, we will likely require support for BLE units, so that the values can always be correctly parsed and converted to openHAB channel types.
Did you spend any thoughts on this already?

@PRosenb
Copy link
Contributor Author

PRosenb commented Jul 10, 2021

Hi @kaikreuzer, thanks for having a look at my implementation. I'm currently moving overseas and have no access to my equipment at the moment.
You are certainly right that there are many characteristics we could support. I thought to start with one I specifically need and then extend it later to support more.
Did you think on using a map in BeaconBluetoothHandler to map service UUIDs to channel types? I thought on that too but as it maps to specific channel types defined in channels.xml, I didn't how we can make it dynamic.
What's your idea there?

What do you mean with BLE units?

@kaikreuzer
Copy link
Member

With BLE units, I referred to "BLE GATT Units" that also have UUIDs assigned in the specification (see e.g. here.
We would need to know the unit of a value to correctly map it to an openHAB item type (preferably a Number with dimension).

I thought to start with one I specifically need and then extend it later to support more.

As mentioned above, by concern is that this won't scale at all. The hope expressed in this post was that a lot of work/features that were done in the previous 3rd-party bluetooth binding can be ported over to this official one here.

This binding used a dedicated GATT parser library and says in its README:

If a bluetooth device supports the Bluetooth specification 4.0, then the standard GATT services and characteristics can be automatically recognized and corresponding thing channels be created for each GATT characteristic and its GATT fields.

This was done in a very generic way, which would imho also be the best way to go here.

@PRosenb
Copy link
Contributor Author

PRosenb commented Jan 18, 2022

I implemented a generic solution but cannot test it without a debugger due to ForbiddenClassException. @kaikreuzer have you seen that issue as well or know how to solve it?

@jlaur
Copy link
Contributor

jlaur commented Jun 30, 2022

@kaikreuzer - are you interested in looking into this generic solution implemented by @PRosenb? Also there is a request for help, I don't know if you or anyone else would have the knowledge to be able to help.

@PRosenb
Copy link
Contributor Author

PRosenb commented Jun 30, 2022

Thanks @jlaur for your comment.

The ForbiddenClassException happens because the library bluetooth-gatt-parser does not support newer versions of xstream. openHAB uses newer versions of xstream and the newer version is on the classpath.

The existing Generic Bluetooth Device Binding of openHAB is currently broken because it depends on bluetooth-gatt-parser.
I created a pull request to fix bluetooth-gatt-parser but unfortunately does it seam abandoned.

@kaikreuzer
Copy link
Member

@kaikreuzer - are you interested in looking into this generic solution implemented by @PRosenb?

Yes, I definitely am, sorry for not commenting any earlier!

I created a sputnikdev/bluetooth-gatt-parser#15 to fix bluetooth-gatt-parser but unfortunately does it seam abandoned.

How about creating a fork of it in the openHAB organisation? Then we can fix it ourselves and release our own versions of it.

@kaikreuzer
Copy link
Member

How about creating a fork of it in the openHAB organisation?

I just did so. I included your pull request and released a version 2.0.0 under the new org.openhab namespace: https://github.com/openhab/bluetooth-gatt-parser/releases/tag/bluetooth-gatt-parser-2.0.0

I will prepare a PR to update the Generic Bluetooth Device Binding to make use of this version.

@kaikreuzer
Copy link
Member

Here we go: #13128

@PRosenb
Copy link
Contributor Author

PRosenb commented Jul 15, 2022

Great, thank you @kaikreuzer

@jlaur
Copy link
Contributor

jlaur commented Jul 19, 2022

@PRosenb - #13128 is merged now, so you should be able to resolve conflict/rebase.

@PRosenb
Copy link
Contributor Author

PRosenb commented Jul 21, 2022

Thanks for letting me know @jlaur, I rebased it but cannot test it at the moment because I'm overseas. I think review can still go ahead.

@jlaur
Copy link
Contributor

jlaur commented Aug 1, 2022

@kaikreuzer - gentle reminder that this PR is ready for review.

@jlaur jlaur requested a review from kaikreuzer August 11, 2022 21:21
@jlaur
Copy link
Contributor

jlaur commented Aug 11, 2022

@PRosenb - should anything be mentioned in the README?

@kaikreuzer
Copy link
Member

Thanks @PRosenb, The code looks good to me in general. I currently cannot test it as I only have a BlueGiga stick at hand, but not a Linux/BlueZ setup. That also brings me to my only concern: The PR only adds support for BlueZ, but not for BlueGiga. I assume you do not have such a stick, so it will be too much to ask you for adding support.

@cdjackson As you are the original author of the BlueGiga bundle: Do you happen to know if it is also possible to extract service data from scan notifications on it? From the code itself, I unfortunately do not see any clear path to do so. 😞

Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
…ngStatus.ONLINE

Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
@cdjackson
Copy link

Sorry @kaikreuzer it's been a few years since I've looked at the BlueGiga stuff so I'm not sure to be honest.

@PRosenb
Copy link
Contributor Author

PRosenb commented Aug 22, 2022

Thank you @kaikreuzer to review the code. BlueZ uses the built in Bluetooth chip of the Raspberry Pi. If you have such a device I guess it should work with openHABian.
I'm back home by the way and tested it with the new changes from main branch.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

If you have such a device I guess it should work with openHABian.

I certainly have, but I didn't take it with me on vacation. 😉

Ok, since you have tested it and the code is fine, we can merge - many thanks for your work!

I'll see if I can find any time to do some debugging with a BlueGiga stick - it would be really nice if this feature could likewise be supported there.

@kaikreuzer kaikreuzer merged commit 34bdc21 into openhab:main Aug 25, 2022
@kaikreuzer kaikreuzer added this to the 3.4 milestone Aug 25, 2022
@PRosenb
Copy link
Contributor Author

PRosenb commented Aug 25, 2022

Great, thank you @kaikreuzer

leifbladt pushed a commit to leifbladt/openhab-addons that referenced this pull request Oct 15, 2022
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
marcelGoerentz pushed a commit to marcelGoerentz/openhab-addons that referenced this pull request Nov 14, 2022
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants