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

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

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@cdjackson
Contributor

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

Initial commit of BLE bundles (BLE, BlueGiga, YeeLightBlue)
Signed-off-by: Chris Jackson <chris@cd-jackson.com>
Update BlueGiga bridge properties
Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@svilenvul

This comment has been minimized.

Show comment
Hide comment
@svilenvul

svilenvul Jun 26, 2017

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.

Contributor

svilenvul commented Jun 26, 2017

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

This comment has been minimized.

Show comment
Hide comment
@cdjackson

cdjackson Jun 26, 2017

Contributor
Contributor

cdjackson commented Jun 26, 2017

@svilenvul

This comment has been minimized.

Show comment
Hide comment
@svilenvul

svilenvul Jun 26, 2017

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 ?

Contributor

svilenvul commented Jun 26, 2017

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

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Jun 26, 2017

Member

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.

Member

kaikreuzer commented Jun 26, 2017

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

This comment has been minimized.

Show comment
Hide comment
@cdjackson

cdjackson Jun 26, 2017

Contributor
Contributor

cdjackson commented Jun 26, 2017

@svilenvul

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.

Show outdated Hide outdated ...ng/org.eclipse.smarthome.binding.ble.bluegiga/ESH-INF/thing/bluegiga.xml Outdated
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",

This comment has been minimized.

@svilenvul

svilenvul Jun 26, 2017

Contributor

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

@svilenvul

svilenvul Jun 26, 2017

Contributor

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

This comment has been minimized.

@kaikreuzer

kaikreuzer Jun 26, 2017

Member

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.

@kaikreuzer

kaikreuzer Jun 26, 2017

Member

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.

This comment has been minimized.

@cdjackson

cdjackson Jun 26, 2017

Contributor

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

@cdjackson

cdjackson Jun 26, 2017

Contributor

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

This comment has been minimized.

@maggu2810

maggu2810 Jun 26, 2017

Contributor

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.

@maggu2810

maggu2810 Jun 26, 2017

Contributor

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.

This comment has been minimized.

@kaikreuzer

kaikreuzer Jun 30, 2017

Member
  • 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 :-))

@kaikreuzer

kaikreuzer Jun 30, 2017

Member
  • 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 :-))

This comment has been minimized.

@maggu2810

maggu2810 Aug 22, 2017

Contributor

Any news about using Bluetooth over DBus?

@maggu2810

maggu2810 Aug 22, 2017

Contributor

Any news about using Bluetooth over DBus?

This comment has been minimized.

@kaikreuzer

kaikreuzer Aug 22, 2017

Member

The latest news is over here: openhab/openhab2-addons#2489 - expecting a new PR with the merged result soon!

@kaikreuzer

kaikreuzer Aug 22, 2017

Member

The latest news is over here: openhab/openhab2-addons#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>

This comment has been minimized.

@svilenvul

svilenvul Jun 26, 2017

Contributor

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>

@svilenvul

svilenvul Jun 26, 2017

Contributor

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>

This comment has been minimized.

@cdjackson

cdjackson Jun 26, 2017

Contributor

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.

@cdjackson

cdjackson Jun 26, 2017

Contributor

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();

This comment has been minimized.

@svilenvul

svilenvul Jun 26, 2017

Contributor

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 ?

@svilenvul

svilenvul Jun 26, 2017

Contributor

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 ?

This comment has been minimized.

@cdjackson

cdjackson Jun 26, 2017

Contributor

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

cdjackson Jun 26, 2017

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@cdjackson

cdjackson Jun 26, 2017

Contributor
Contributor

cdjackson commented Jun 26, 2017

Fix bridge-type definition for bluegiga
Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@kaikreuzer

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Jun 30, 2017

Member

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!

Member

kaikreuzer commented Jun 30, 2017

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";

This comment has been minimized.

@kaikreuzer

kaikreuzer Jun 30, 2017

Member

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

@kaikreuzer

kaikreuzer Jun 30, 2017

Member

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";

This comment has been minimized.

@kaikreuzer

kaikreuzer Jun 30, 2017

Member

This also already exists as BLE_CHANNEL_RSSI.

@kaikreuzer

kaikreuzer Jun 30, 2017

Member

This also already exists as BLE_CHANNEL_RSSI.

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

This comment has been minimized.

@kaikreuzer

kaikreuzer Jun 30, 2017

Member

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

@kaikreuzer

kaikreuzer Jun 30, 2017

Member

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

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

This comment has been minimized.

@kaikreuzer

kaikreuzer Jun 30, 2017

Member

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.

@kaikreuzer

kaikreuzer Jun 30, 2017

Member

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

This comment has been minimized.

Show comment
Hide comment
@cdjackson

cdjackson Jun 30, 2017

Contributor
Contributor

cdjackson commented Jun 30, 2017

@kaikreuzer

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Jul 24, 2017

Member

@cdjackson Any updates on this?

Member

kaikreuzer commented Jul 24, 2017

@cdjackson Any updates on this?

@kaikreuzer

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Aug 25, 2017

Member

This is now superseded by #4112.

Member

kaikreuzer commented Aug 25, 2017

This is now superseded by #4112.

@kaikreuzer kaikreuzer closed this Aug 25, 2017

@kaikreuzer

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Oct 7, 2017

Member

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

Member

kaikreuzer commented Oct 7, 2017

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

@kaikreuzer kaikreuzer reopened this Oct 7, 2017

@openhab-bot

This comment has been minimized.

Show comment
Hide comment
@openhab-bot

openhab-bot Oct 8, 2017

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

Contributor

openhab-bot commented Oct 8, 2017

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

This comment has been minimized.

Show comment
Hide comment
@martinvw

martinvw Nov 4, 2017

Contributor

@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.

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

This comment has been minimized.

Show comment
Hide comment
@martinvw

martinvw Nov 4, 2017

Contributor

@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?

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

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Nov 5, 2017

Member

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").

Member

kaikreuzer commented Nov 5, 2017

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

This comment has been minimized.

Show comment
Hide comment
@cdjackson

cdjackson Nov 6, 2017

Contributor

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).

Contributor

cdjackson commented Nov 6, 2017

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

This comment has been minimized.

Show comment
Hide comment
@martinvw

martinvw Nov 7, 2017

Contributor

@cdjackson were you able to check it?

Contributor

martinvw commented Nov 7, 2017

@cdjackson were you able to check it?

@cdjackson

This comment has been minimized.

Show comment
Hide comment
@cdjackson

cdjackson Nov 7, 2017

Contributor
Contributor

cdjackson commented Nov 7, 2017

@kaikreuzer

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Nov 7, 2017

Member

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.

Member

kaikreuzer commented Nov 7, 2017

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

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Nov 10, 2017

Member

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!

Member

kaikreuzer commented Nov 10, 2017

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

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Nov 12, 2017

Member

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.

Member

kaikreuzer commented Nov 12, 2017

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

This comment has been minimized.

Show comment
Hide comment
@maggu2810

maggu2810 Nov 13, 2017

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

Contributor

maggu2810 commented Nov 13, 2017

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 join this conversation on GitHub. Already have an account? Sign in to comment