Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

WIP: Initial commit of BLE bundles (BLE, BlueGiga, YeeLightBlue) #3633

Closed
wants to merge 3 commits into from

Conversation

cdjackson
Copy link
Contributor

@cdjackson cdjackson commented Jun 10, 2017

Overview

This provides an initial PR for the BLE binding, with a BlueGiga bridge, and a YeeLightBlue thing handler implemented. It is still WIP but working. It's based on the discussions elsewhere.

Note that I've not currently implemented a discovery participant. Since BLE discovery in other APIs is handled through discovery filters, I've currently implemented this. So all a handler needs to do is add a property to the thing XML and it will be auto discovered. Maybe the discovery participant is also required for more complex implementations, but this can be added.

I also have a handler for the Blukii beacons coded, but I don't want to add everything here and make the PR more difficult to review.

Comments appreciated ;)

Chris
Signed-off-by: Chris Jackson chris@cd-jackson.com

Work to be completed

Currently this is working, but there is further testing, tidying and improving required.

Better connection management is needed to cope with disconnects. There may also be some arbitration required in the BlueGiga handler to handle clashes between different thing handlers.

BLE Binding overview

The BLE binding is implemented to allow bundle fragments to extend the main BLE bundle to add new thing handlers, and new bridges. This architecture means that fragment bundles must follow specific naming to avoid conflicts when the fragments are merged, and must always utilise the binging name ble.

A base class structure is defined in the org.eclipse.smarthome.binding.ble bundle. This includes the main classes required to implement BLE -:

  • BleBaseBridgeHandler. This implements the main functionality required by a BLE bridge including management of devices that a bridge discovers.
  • BleDevice. This implements a BLE device. It manages the notifications of device notifications, BLE service and characteristic management, and provides the main interface to communicate to a BLE device.
  • BleService. Implements the BLE service. A service holds a number of characteristics.
  • BleCharacteristic. Implements the BLE characteristic. This is the basic component for communicating data to and from a BLE device
  • BleDescriptor. Implements the BLE descriptors for each characteristic.

Implementing a new BleBridge bundle

A BLE bridge handler provides the link with a BLE master hardware (eg a dongle, or system Bluetooth API). The new bridge bundle needs to implement two main classes – the BridgeHandler which should implement BleBridgeApi, and a ThingFactory required to instantiate the handler.

The BleBridgeHandler must implement any functionality required to interface to the Bluetooth layer. It is responsible for managing the Bluetooth scanning, device discovery (ie the device interrogation to get the list of services and characteristics) and reading and writing of characteristics. The bridge needs to manage any interaction between the interface with any things it provides – this needs to account for any constraints that a interface may impose such that Things do not need to worry about any peculiarities imposed by a specific interface.

Classes such as BleCharacteristic or BleService may be extended to provide additional functionality to interface to a specific library if needed.

The BleBridgeHandler must register the BleDiscoveryService to allow BLE thing discovery.

Implementing a BleThing bundle

A BLE thing handler provides the functionality required to interact with a specific BLE device. The new thing bundle needs to implement two main classes – the ThingHandler, and a ThingFactory required to instantiate the handler.

Two fundamental communications methods can be employed in BLE – beacons, and connected mode. A BLE thing handler can implement one or both of these communications. In practice, a connected mode Thing implementation would normally handle the beacons in order to provide as a minimum the RSSI data.

Thing Filter

Things are defined in the XML files as per the ESH architecture. A filter has been defined in the Thing properties in the XML that is used to select the thing type. The filter allows selection of the thing type from data available in the beacon - the property name for this filter is bleFilter as per the example below.

        <properties>
            <property name="bleFilter">NAME=Yeelight Blue II</property>
        </properties>

The available filters are as follows -:

  • MANUFACTURER: The manufacturer ID specified as a hexadecimal value
  • NAME: Provides selection on the long or short name in the device beacon
  • SVC:

Multiple filters can be separated with a comma. The system requires that all filters match before the thing type is selected.

Thing Naming

To avoid naming conflicts with different bundle fragments a strict naming policy for things and thing xml files is proposed. This should use the bundle fragment name and the thing name, separated with an underscore - eg for the YeeLight binding Blue2 thing, the thing type is yeelight_blue2.

Connected Mode Implementation

The connected mode BleThingHandler needs to handle the following functionality

  • Get the things bridge handler. This will implement the BleBridgeApi.
  • Call the BleBridgeApi.getDevice() method to get the BleDevice class for the requested device. The getDevice() method will return a BleDevice class even if the device is not currently known.
  • Implement the BleDeviceListener methods. These provide callbacks for various notifications regarding device updates – eg when the connection state of a device changes, when the device discovery is complete, when a read and write completes, and when beacon messages are received.
  • Call the BleBridgeApi.connect() method to connect to the device. Once the device is connected, the BleDeviceListener.onConnectionStateChange() callback will be called.
  • Call the BleBridgeApi.discoverServices() method to discover all the BleServices and BleCharacteristics implemented by the device. Once this is complete, the BleDeviceListener.onServicesDiscovered() callback will be called.
  • Call the readCharacteristic or writeCharacteristic() methods to interact with the device. The BleDeviceListener.onCharacteristicReadComplete() and BleDeviceListener.onCharacteristicWriteComplete() methods will be called on completion.
  • Implement the BleDeviceListener.onCharacteristicUpdate() method to process any read responses or unsolicited updates of a characteristic value.

Beacon Mode Implementation

The beacon mode BleThingHandler needs to handle the following functionality

  • Get the things bridge handler. This will implement the BleBridgeApi.
  • Call the BleBridgeApi.getDevice() method to get the BleDevice class for the requested device. The getDevice() method will return a BleDevice class even if the device is not currently known.
  • Implement the BleDeviceListener.onScanRecordReceived() method to process the beacons. The notification will provide the current receive signal strength (RSSI), the raw beacon data, and various elements of generally useful beacon data is provided separately.

Signed-off-by: Chris Jackson chris@cd-jackson.com

Signed-off-by: Chris Jackson <chris@cd-jackson.com>
Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@svilenvul
Copy link
Contributor

Many thanks for this PR and the detailed instructions!

We have taken a BlueGiga BLED112-V1 USB stick and we will try to write a simple binding for a beacon device using this API.

From first glance I miss the BLEDiscoveryParticipant and I have a few small comments regarding the thing descriptions. I will try to suggest some changes in another branch in the next days.

@cdjackson
Copy link
Contributor Author

cdjackson commented Jun 26, 2017 via email

@svilenvul
Copy link
Contributor

As a matter of interest, why do you need the discovery participant?

Because as a binding developer I expect to extend the BaseThingHandler and to implement a Discovery participant as this is the case with UPnP and MDNS, this allows me to create custom discovery results for different devices at least.

@kaikreuzer, what do you think about this ?

@kaikreuzer
Copy link
Contributor

Yes, I would have hoped for an BLEDiscoveryParticipant as I had sketched out here:

The "base" BLE bundle [...] contains an implementation of a discovery service
Specific device support can [...] register a BLEDiscoveryParticipant and thus can provide DiscoveryResults for specific devices.

@cdjackson
Copy link
Contributor Author

cdjackson commented Jun 26, 2017 via email

Copy link
Contributor

@svilenvul svilenvul left a comment

Choose a reason for hiding this comment

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

You can do this now with the existing system using the filters. Does this not meet your needs?

It meets my needs, but the framework provides almost the same functionality, why not reusing it.

My initial impression is that the API is a lot more easy to use than the previous version, I just want to add few comments.

xsi:schemaLocation="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0 http://eclipse.org/smarthome/schemas/thing-description-1.0.0.xsd">

<!-- Sample Thing Type -->
<thing-type id="bluegiga">
Copy link
Contributor

Choose a reason for hiding this comment

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

<bridge-type id="bluegiga"> otherwise class cast exception is thrown, when the handler is initialized.

lib/com.zsmartsystems.bluetooth.bluegiga-1.0.0-SNAPSHOT.jar
Fragment-Host: org.eclipse.smarthome.binding.ble
Import-Package:
gnu.io;version="3.12.0.OH",
Copy link
Contributor

Choose a reason for hiding this comment

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

Strange, but I couldn't find it in the Eclipse SmartHome target platform. When I use the openHAB target platform, it's ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point. We indeed do not have gnu.io available in ESH for license (GPL) reasons. We therefore cannot add it to our 3rd party p2 site for the target platform to pick it up.
We can check if we can create a works-with CQ that would at least allow us to compile against it (while not distributing it). The other option would be to add the bluegiga fragment simply to openhab2-addons and stick with the DBUS solution in ESH.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry - I forgot to mention this issue. ESH needs a serial driver ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we try to work with the Kura team and use + improve that EPL licensed serial driver?
https://github.com/eclipse/kura/tree/develop/target-platform/org.eclipse.soda.dk.comm-parent/org.eclipse.soda.dk.comm/src/main/java
It does not use gnu.io but the javax.comm interface.
But if it is working it does not matter.
Perhaps we can create an abstraction layer, so the real "backend driver" does not care.
But IMHO it would be a big step if we could use an EPL licensed serial driver.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • improve that EPL licensed serial driver

In general, I would love to have something like that. But I am a bit frightened of the implied efforts in that. Afair, the lib had it's only problems and bugs and especially didn't support many architectures (only armv6hf and x86_64?). Building the native parts of such libs can be pretty cumbersome.

It does not use gnu.io but the javax.comm interface.

If we do not require anything special from gnu.io and we manage to have a good wrapping API from javax.comm to gnu.io, I would be fine to say that ESH code should use javax.comm for serial communication (and thus could use soda.dk), but it should also leave the option open to use RXTX as an implementation (through the wrapping API) instead.

But imho this is out of scope of this PR. To make progress here, I would suggest to move the bluegiga bundle over to openhab2-addons as it is "just another" bridge, while the official BLE support in ESH uses DBUS (btw. this bridge bundle still seems to be missing here :-))

Copy link
Contributor

Choose a reason for hiding this comment

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

Any news about using Bluetooth over DBus?

Copy link
Contributor

Choose a reason for hiding this comment

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

The latest news is over here: https://github.com/openhab/openhab2-addons/pull/2489 - expecting a new PR with the merged result soon!


<!-- Sample Thing Type -->
<thing-type id="yeelight_blue2" listed="false">
<label>YeeLightBlue II Light Bulb</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

I expected a reference to the bridge (maybe not the bluegiga, but some generic ?, I am not sure ):

 <supported-bridge-type-refs>
      <bridge-type-ref id="...." />
 </supported-bridge-type-refs>

Copy link
Contributor Author

@cdjackson cdjackson Jun 26, 2017

Choose a reason for hiding this comment

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

This is the issue that @kaikreuzer eluded to in the other post regarding specifying bridges. Currently it's not possible I think to specify "ble.*" bridge type and this needs to be looked at but is a current limitation of the framework.

*
* @return true if the scan was started
*/
boolean scanStart();
Copy link
Contributor

@svilenvul svilenvul Jun 26, 2017

Choose a reason for hiding this comment

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

It is not clear for me, how I am suppose to take the scan results. I expected that I could register some callback(or listener) that will be called(notified) when a scan result is found.

At the moment the BridgeHandler calls a method in the DiscoveryService, but what should we do if we want to decouple them and use only the BleBridgeApi interface ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I expected that I could register some callback(or listener) that will be called(notified) when a scan result is found.

Why do you want this callback? This is really only for discovery. Scanning should be enabled at all times - I'm not sure if I say this above or in the code of the bridge, but this is generally needed (at least in BlueGiga) in order to get the beacons at all. The startScan method is used for discovery only, and enables active scanning.

Are you looking for the beacons? If so, then all you need to do is implement BleDeviceListener.onScanRecordReceived() callback to receive the results of any records received.

If not, can you describe what you want to do and I'll try and answer...

@cdjackson
Copy link
Contributor Author

cdjackson commented Jun 26, 2017 via email

Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@kaikreuzer
Copy link
Contributor

I’m happy to add both concepts, but my focus on getting this implemented quickly was to have some discussion

Well, one goal of such a sketch should be to lay out the general architecture (as this is what we noticed that it needs to be changed in comparison to our first approach).
And I fully agree with @svilenvul here: The BLEDiscoveryParticipant is the approach that is in line with all the rest of ESH. The current BleThingTypeFilter is in contrast a very nasty example of how to not do code in OSGi - it is all static, while using OSGi services internally. As recently discussed on Zigbee/Zwave, such code must be avoided under any circumstances. So please only keep the discovery participant concept, that should be just fine :-)

Some further feedback:

  • Good news is that I was able to get the BlueGiga dongle ONLINE on my Mac 👍
  • I would suggest to not use fragments (that can cause many issues and are an unloved child of OSGi), but rather use proper OSGi bundles for all parts - I locally refactored it for myself and there is no issue with that.
  • The API indeed looks pretty straight forward, so definitely a huge gain over the old version!
  • I couldn't do any device testing as I do not have a Yeelight blue. I'd be happy if all unknown devices could be added to the inbox as some generic device, which only provide its RSSI (i.e. "beacon mode") as a channel - this would already allow many nice use cases without requiring a specific bundle that supports it. If you are afraid of too much noise in the inbox, this might arguably an optional feature that can be configured on/off.

Summary: Great work for such a short time, definitely going the right way!

*/
public class YeeLightBlueBindingConstants {

private static final String BINDING_ID = "ble";
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not re-define it here, but simply use BleBindingConstants.BINDING_ID instead.

public final static String CHANNEL_SWITCH = "switch";
public final static String CHANNEL_BRIGHTNESS = "brightness";
public final static String CHANNEL_COLOR = "color";
public final static String CHANNEL_RSSI = "rssi";
Copy link
Contributor

Choose a reason for hiding this comment

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

This also already exists as BLE_CHANNEL_RSSI.



<!-- RSSI Channel -->
<channel-type id="rssi">
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be defined in the ble binding itself and just be used by Yeelight.

*
* @author Chris Jackson - Initial contribution
*/
public interface BleBridgeApi {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why this should not be called BleBridgeHandler (and possibly also extend BridgeHandler?
After all, all instances are BridgeHandlers and that's how you get hold of them. This way, it would also be clear that they respect the Thing lifecycle, so e.g. using the api after disposal of the handler is not a good idea.

@cdjackson
Copy link
Contributor Author

cdjackson commented Jun 30, 2017 via email

@kaikreuzer
Copy link
Contributor

@cdjackson Any updates on this?

@kaikreuzer
Copy link
Contributor

This is now superseded by #4112.

@kaikreuzer kaikreuzer closed this Aug 25, 2017
@kaikreuzer
Copy link
Contributor

Re-opening this PR as per #4112 (comment).

@kaikreuzer kaikreuzer reopened this Oct 7, 2017
@openhab-bot
Copy link
Contributor

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/oh-is-now-profeffionsl-like-eclipse-ide-framework/34914/1

@martinvw
Copy link
Contributor

martinvw commented Nov 4, 2017

@cdjackson is there anything I can do to help get this forward, I already purchased a BLED112

@svilenvul would I also need your code to get me kickstarted with this BLE stick.

@martinvw
Copy link
Contributor

martinvw commented Nov 4, 2017

@kaikreuzer what do you exactly mean with this and do you still have the code?

I would suggest to not use fragments (that can cause many issues and are an unloved child of OSGi), but rather use proper OSGi bundles for all parts - I locally refactored it for myself and there is no issue with that.

Would it mean exporting all classes from the BLE binding which should be available for the other bindings and using Import-Package in the bluegiga bundle etc

@cdjackson, you once mentioned the following:

I will take a look at refactoring over the next week or two. I think this will also cover your points about constants since this will all change.

A lot has happened since then (I read the sad story as well). Did you at that moment have a specific direction in mind or would it already help if I do a PR against your repo to work on the mentioned comments?

@kaikreuzer
Copy link
Contributor

Thanks @martinvw for picking this up! I definitely wanted to drive this forward again as well.

what do you exactly mean with this and do you still have the code?

Good question. I am not sure if I still have the code, but it wasn't much work anyhow.
If @cdjackson does not have any local code changes that he didn't push, I can offer to do some refactoring and create a new PR with that against which anyone (you, @svilenvul, @cdjackson,...) can create PRs for further collaboration on it.

Would it mean exporting all classes from the BLE binding which should be available for the other bindings and using Import-Package in the bluegiga bundle etc

Yes, it is about defining proper interfaces that are exported by this bundle and implemented by other bundles (both for the "BLE driver" as well as for the "BLE device application code").

@cdjackson
Copy link
Contributor Author

Hi @martinvw. Thanks for the offer. If you have some time to make the changes that @kaikreuzer suggested above to the structure it would be great. I'll double check that I don't have any outstanding commits that aren't pushed (I don't think so, but let me check tonight).

@martinvw
Copy link
Contributor

martinvw commented Nov 7, 2017

@cdjackson were you able to check it?

@cdjackson
Copy link
Contributor Author

cdjackson commented Nov 7, 2017 via email

@kaikreuzer
Copy link
Contributor

Thanks for checking, @cdjackson!
@martinvw I'd then suggest to continue as mentioned above: I'll try to refactor the code by the end of this week and create a new PR from it from which we can move things further then.

@kaikreuzer
Copy link
Contributor

I'll try to refactor the code by the end of this week and create a new PR from it

Just as a head-up: I have heavily worked on this during the past days already, but as it is with refactorings, one gets deeper into it than expected. 😲
Today I wasn't able to continue on it as I was too busy doing a new stable ESH release, but I hope to be able to push the code out on Sunday night - so stay tuned!

@kaikreuzer
Copy link
Contributor

Ok, as promised, I have just pushed #4535 as a follow-up.
A short summary of what major changes I have done (not mentioning smaller fixes and improvements):

  • Changed name from BLE to Bluetooth
  • Renamed BleBridgeApi to BluetoothAdapter
  • Replaced BleThingTypeFilter by a BluetoothDiscoveryParticipant
  • Introduced a generic thing type that supports every unknown Bluetooth device in beacon mode and offers its RSSI as a channel.
  • Found that we had an approved works-with CQ for nrjavaserial, so I was able to add this to the targetplatform, such that BlueGiga support works in ESH as well and we don't have to move the bundle anywhere else!

I cannot promise that Yeelight support still works as I do not own such a bulb and they are discontinued, so pretty hard to get. Instead, I have started on an AwoX implementation, but didn't finish this yet (therefore it is not yet included).

I'd suggest to do all further discussion on #4535.

@kaikreuzer kaikreuzer closed this Nov 12, 2017
@maggu2810
Copy link
Contributor

Found that we had an approved works-with CQ for nrjavaserial, so I was able to add this to the targetplatform, such that BlueGiga support works in ESH as well and we don't have to move the bundle anywhere else!

Nice

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants