-
Notifications
You must be signed in to change notification settings - Fork 786
[bluetooth] WIP: Merging BluetoothSmart and Ble Bindings #4112
Conversation
…bluetoothsmart binding into two bundles: bluetooth and bluetooth tinyb transport Signed-off-by: Vlad Kolotov <vkolotov@gmail.com>
Signed-off-by: Vlad Kolotov <vkolotov@gmail.com>
…in.includes property) Signed-off-by: Vlad Kolotov <vkolotov@gmail.com>
Many thanks for the efforts @vkolotov!
Just to proactively mention a few that matter to me:
@cdjackson Is this something you could start looking into? It might be a good opportunity to see how well the architecture fits to your expectations and to provide early feedback, if you think something is not ideal. |
@vkolotov Another question: Where did you get the binary of tinyb from? Is there a single binary that works for x86 and arm platforms? |
…w look like: bluetooth:ble:<adapter>:<device>, e.g.: bluetooth:ble:001A7DDA7104:CFFC9EB20E63 Signed-off-by: Vlad Kolotov <vkolotov@gmail.com>
Hi @kaikreuzer for your valuable feedback. Re thing UIDs:This is working exactly like you said. The binding does not know anything about transport type. I have managed to keep transport notion (protocol bit of URL) in the BluetoothManager so it does not leak from it to the binding. Thing UIDs looks look like that now: bluetooth:ble:'adapter':'device', e.g.: bluetooth:ble:001A7DDA7104:CFFC9EB20E63. Re BluetoothManager dependency:I totally understand your point of view as a product owner of ESH. You need to have control over the API. However, I'd like you to understand my point view as well so that we can find reasonable compromise. I personally happy to contribute to ESH, I like opensource. However, that would be a dead-end for the BluetoothManager if we moved it to ESH (because Eclipse foundation simply is not flexible enough). I believe it's got future and can be used by other projects, like kura etc. It is quite unique, I could say. This is what I can propose. iIf you believe that the BM is not stable enough and its future unpredictable (API), then we could come up with a facade and adapters in ESH for it. However, I personally think that there will be no risk in having it as a main API for Bluetooth binding. I will be looking after that project for quite awhile. I also believe that we can find more contributors for it if it was outside of the ESH as a separate library which can be used just by referencing it through maven (btw it is already in the central maven repo, all components including GATT parser). I'm happy to discuss this further. Re Gatt Parser:I agree, it is not a central part. Re BlueGiga:I had a delivery of a BlueGiga dongle. I could do the initial merge of @cdjackson transport bundle into this PR to validate our architectural decisions. I will need to have a look how it goes, I may expect to see some issues (some incompatibility of APIs between dbus and serial) that I will need to be addressed in the BM first. As soon as I'm sure that everything goes fine and there is not major issue, then I would like to have some help from @cdjackson. I believe that would be more efficient for now. Happy to discuss this approach as well. Re TinyB:I built that library locally with no modifications to it. This will probably require separate binaries for x86 and ARM platforms. I will need some help from you @kaikreuzer to figure out how to implement this in ESH (e.g. loading different native libs for different CPUs). I do monitor TinyB github project and I'm aware of the issues they have fixed recently. That would be great if we could use the same (kura) approved lib by Eclipse. Ultimately, we should create a transport bundle purely based on dbus for java implementation. I have this in my mind as I believe TinyB is just a temporary solution (which is actually not too bad). |
Signed-off-by: Vlad Kolotov <vkolotov@gmail.com>
This could be solved easily by OSGi itself -- so correct manifest headers...
Personally I don't like that "move libraries code base to ESH repo" approach. Shouldn't be a "stable" API the relevant requirement? Why do the API need to be under full control of the ESH project? |
If I checkout https://github.com/intel-iot-devkit/tinyb and build the sources, I get the shared library.
@vkolotov Can you point me to the jar? -- edit -- Okay, should read the README 😉
|
Hi @kaikreuzer @cdjackson, good news. I've made some good progress implementing/merging Bluegiga transport with just some minimal changes for BluetoothManager. @cdjackson looks like you've got some uncommitted changes in your project bluegiga. I'm trying to make an improvement for it and build a new jar. However, what's in git is not the same what you have in your ESH lib folder. For example, the source in ESH has NotificationService.java class, but there is not such in git. Would you be able to have a look? The bluegiga project obviously has a dependency on rxtxlib. How are we going to deal with it? As far as I can see there is not any bundle in ESH which supplies that lib. |
BTW, @kaikreuzer do you have any thoughts around the BM dependency issue discussed earlier? |
…cteristicAccessType) Signed-off-by: Vlad Kolotov <vkolotov@gmail.com>
Sounds wonderful!
@cdjackson Could you clarify this?
This is indeed a problem and due to license issues, we won't be able to get rxtx into ESH itself. We had discussed this here already and my suggestion was to put the BlueGiga-Bridge bundle into the openhab2-addons repo as a workaround.
Yes, an I also had quite some internal discussions meanwhile. I fully understand your argument that you do not want to tie the BM to ESH code as this would make it impossible to use it independently in other projects.
You should note that ESH is nothing else but a set of JARs. So it is on us to put the BM in a separate bundle with no dependencies on any other ESH stuff - as a result, this would be available as a jar in Maven, which can be used in any context, i.e. completely independently from ESH. |
On 11 Sep 2017, at 15:55, Kai Kreuzer ***@***.***> wrote:
@cdjackson <https://github.com/cdjackson> Could you clarify this?
It was resolved a week or so back - everything is committed.
|
Hi @kaikreuzer thanks for you feedback.
Putting it in a different project would be disappointing. Re your question what architectures rxtx lib supports: it support arm, x86. That said it is running on windows, linux and OSX. Which is great, I believe, because it spans a great number of OSs and would be a good addition to tinyb/dbus. In fact, tinyb works only in linux (arm and x86). I made a good progress porting/merging @cdjackson work with his awesome library for bluegiga adapter. Looks like everything is working, including bluetooth notifications (which is awesome!). BTW @cdjackson, it has not been updated for 3 months, so looks like you have not pushed your changes... How about tibyb? Is it going to be approved any time soon?
It is sad to hear that the Eclipse Foundation has the such problems with external references. The way how that "foundation" behaves lays much behind opesource side of things. I completely agree with @maggu2810 that it is not right to own the code/api just because they want it. This is not right, the freedom of opensource is vastly constrained in that way. If Eclipse foundation forces the good will of contributors to do the things like "they know better", I feel really sorry for the contributors. That's why probably during my career/work experience I used only a little of Eclipse products. Honestly speaking I do not see any future of BM within Eclipse Foundation. All these politics do not do any good job neither for OH nor for end-users. I would better keep working in my branch providing the bundle (and the libraries) to people as a 3rd party plugin for OH2, rather than letting the big hippopotamus to own the code and "control" it. What you propose is neither a trade-off nor a compromise. It is just an ultimatum. Definitely not an opensource approach. I don't think that the such governance plays a good role in the Eclipse products, for example, I have never seen Eclipse IDE working great. In my humble opinion, It feels sad that Eclipse actually was selected as a main platform for OH. Anyway, I will continue to work on the Bluetooth plugin, it is up to you to merge my stuff or not. |
I will double check, but I think everything is pushed and from a quick check of the latest source, it looks like the changes I made last are in the master. The last changes are indeed a few months ago, but that doesn't mean that everything is not pushed as the date in the repository is not the push date.... |
Please ignore that, it is there now. Thank you. |
What do you mean by missing? |
It was missing, but now it is there. Cheers. I'll create some PRs soon for some minor improvements. Would you be able to release the lib @cdjackson ? |
Ok - thanks. I've not pushed anything since last week or so, and I was a bit confused as your link above links directly to the file that is in my repo.... ;) |
I didn't ask that question, I am well aware as we are using it in openHAB through nrjavaserial, which nicely supports all the different platforms.
I am sorry if I didn't express myself properly. I really tried to explain the arguments in detail and they are not because anyone wants to put arbitrary constraints on anyone to make life more difficult. Please also note that I am not speaking on behalf of the Eclipse Foundation, so please do not generalise any complaints on them.
No, it is not. It is up to the committers to decide on the evolution of the project. |
Here I fully disagree. I can only look at the Gihub project, but from what I can see there are 0 contributors, 0 stars, 0 issues, 0 pull requests and 0 contributors. With this track record, I see rather a big risk in making this library an integral part of the Eclipse SmartHome APIs. I fully trust that you are planning to take care of further evolving and maintaining your library and I deeply respect the fact that you do not want to contribute it to Eclipse SmartHome. That indeed is up to you to decide. And I really hope that it's going to be successful! However, at the same time I kindly ask you to respect a project's decision whether or not to add a dependency to another project at this point in time.
You are mixing things up - there are two completely different aspects that you have to distinguish clearly: The Eclipse Foundation and the Eclipse SmartHome project. The major benefit of the Eclipse Foundation in our case is (in my personal, non-lawyer words) to make sure that this project adheres to the Open Source license (EPL in our case) and guarantee that the intellectual property within the project's deliverables is free of any legal risks to potential commercial adopters. This put some minor burden on projects like e.g. maintaining a clean track record of every contribution and also for bigger contributions and dependencies to undergo an in-depth review process. It's only goal is to check if the contributed code really is under the license it claims and the author has the permissions to contribute it under this license, i.e. make sure it is not copy'n'pasted from somewhere else. So in contrast to your belief, the Eclipse Foundation is not about owning any code, it is only about reducing the potential legal risk of using open source software which might not be what it claims to be. Unless you are working in an environment where you are commercially working with software, this might sound like a useless burden to you.
My vote goes to reviving #3633 then. |
Hi @SJKA thanks for your comments.
I must agree. Thank you for pointing this out. It looks indeed a bit dodgy. However, does it mean that it is in bad condition (or will it be in not decent condition)? The reason of this PR is to bring contributors attention and decide if it is ready to merge or not. Will they be looking at "stars" and the number of contributors? If so, that process will look not less dodgy comparing to the repo statistics.
I do respect this. Furthermore, I admire OH project, it is a good piece of software art. Well done guys. However, I do not like/follow the fact that contributors are forced to move their code to ESH codebase. This vastly limits soul and freedom of opensource. You may say that all ESH sources are open, but let's be honest, who would use some internal ESH pom references from their projects? Definitely - nobody. It is simple as this. I would define the problem we discuss here as the following: I personally understand the significance of being legally clean and protected. However, how that can be a problem of using external libraries if they are compatible with Eclipse Foundation license? As you said, one of the reason of PRs is to check if the code conforms with EPL. So what's wrong with the BluetoothManager code? Are there any copy-pastes? Or, does it not conform EPL? Anyway, if you think that the community should decide between the two PRs, then would you mind if I create a poll for this? I'm thinking to make two options like that:
Would this be ok for you? Just want to make this fair for both sides. |
It is not a problem at all. From a legal point of view your code might be very well suited for a dependency in ESH.
It is maintained only by you, no one else is using it, nobody except you is able to maintain it. And in case you are not willing to keep it online the ESH project would loose a core dependency for the foundation of all bluetooth bindings. So in case we dont agree in this case its a high risk for the ESH project to depend on your code. |
Hi @htreu thank you for your input.
Can't see any problem here. Let's imagine I have finished the bundle, the community approves it and merges to ESH. Everyone is happy and the bundle brings the whole world of bluetooth to the ESH. Users are happy. I continue to contribute and fix bugs. Then... from all of a sudden I disappear (which is nonsense)... What prevents you from forking the project and continue contribution (giving that prior the approval the code is fully documented)? It is opensource world. Update: you don't event need to create a fork. Just continue to work in the same repo. The release instructions (central maven) will be provided. |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/xiaomi-smart-scale-binding/34196/2 |
@kaikreuzer @vkolotov |
@kaikreuzer @bademux I can see here several options:
There are lots of options here. Anyway it will be available to people. PS. The development of this binding is still full speed ahead, e.g. it is not available to merge to the upstream at the moment, but will be available soon. |
…ies) Signed-off-by: Vlad Kolotov <vkolotov@gmail.com>
…ies) - adding some missing jars Signed-off-by: Vlad Kolotov <vkolotov@gmail.com>
From the discussion above, I think it became clear that the Eclipse Foundation is not the right place for continuing this as its professional open source project management frame is rather regarded as a hindrance and not as a plus. This means that the code is primarily meant for hobbyists, i.e. specifically for openHAB users. |
@vkolotov Please note that no transport bundles must be added to the marketplace as only binding entries are supported there for installation through the marketplace extension. |
Hi @kaikreuzer sorry for taking over your #4535. I just wanted to let you guys know that it is in progress and soonish will be released.
That's strange but I have tested this and the transport marketplace extensions work fine. Is this yet another regulatory rules? I thought this would be better to split transport into two "extensions" just because not all people are going to use them together. |
If I create a transport bundle "transport_a" that is used by a few of my bindings e.g. "binding_b", "binding_c" and "binding_d" it wouldn't make any sense to add the transport bundle content into every binding. Would it? If my reading has been correct, the bundle Shouldn't we enhance that bundle to support a transport type, too? |
There isn't such a thing as a "transport add-on/extension". Transport bundles are internal bundles that are technical dependencies, which get automatically installed (e.g. through Karaf if they are declared in a Karaf feature). For the marketplace, we do not have a dependency mechanism in place. What is prepared is to have other packaging formats than "bundle" in future, thus as "kar" - this would allow packaging multiple bundles that are required for a single add-on. But this is yet something to implement for the marketplace support. Until then, we only have "binding" and "rule templates" as valid categories. |
hi all, the binding is available for betta testing. Please see instructions here: https://github.com/sputnikdev/eclipse-smarthome-bluetooth-binding |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/bluetooth-binding-beta-testers-needed/38492/152 |
This PR is to merge vkolotov's OH BluetoothSmart binding and cdjackson's BLE binding.
Objectives and KPIs
Objectives
KPIs
Progress
Merging BluetoothSmart binding into ESH projectSplitting BluetoothSmart binding into two bundles: bluetooth and transport (tinyb) bundlesMaking adapter things to be bridgesImplementation details
1. Bluetooth URL
This is a useful component which allows us to identify any BT resource. It is similar to @cdjackson
BluetoothAddress, but it also supports locating BT adapters, services, characteristics and GATT fields. Even if multiple adapters are used, the URL can precisely identify resource of a BT device. That class is reasonably documented, so take a look at the comments in the file for more info.
The Bluetooth URL can help to address Flexibility and Extensibility KPIs.
2. Transport abstraction layer
This is an API which is supposed to provide an abstraction layer between hardware and OpenHab. The API provides common interfaces for BT adapter, device, GATT service, GATT characteristic. It also supports plugging in different implementations for different transport types, such us serial port transport or BDus transport. In order to do so, a new transport layer must implement a limited number (actually only 4 interfaces, an example here) of interfaces.
This abstraction layer can help to address Flexibility and partially Robustness.
As a first step, I would leave TinyB transport as a main transport for linux based set-ups, however I agree that this should be converted to be using native DBus adapter. I can even suggest a good candidate for this work - hypfvieh. I had a chat with David a couple of months ago, he might be interested in this.
3. Bluetooth Manager
It is a set of APIs and processes which are responsible for robustness of the system. The central part of it is "Governors" (examples and more info BluetoothGovernor, AdapterGovernor, DeviceGovernor and BluetoothManager). These are components which define lifecycle of BT objects and contain logic for error recovery. They are similar to the transport APIs, but yet different because they are active components, i.e. they implement some logic for each of BT objects (adapter, device, characteristic) that make the system more robust and enable the system to recover from unexpected situations such as disconnections and power outages.
Apart from making the system more stable, the Governors are designed in a such way that they can be used externally, in other words it is another abstraction layer which hides some specifics/difficulties of the BT protocol behind user-friendly APIs.
Obviously the Bluetooth Manager is supposed to help us with Robustness and Extensibility KPIs.
4. GATT Parser
It is a component which can translate BT raw data into user-friendly format. In short, it reads GATT specifications (xml files) and converts raw data into map of: Field name -> value. Works both ways, e.g. raw data -> fields (read operation) and fields -> raw data (write operation). All complex logic related to bit operations, type conversions, validations etc are hidden in this component. More info here.
This component will allow us to create "generic" OH handlers which can work with any devices that conform GATT specifications OR users can supply their own implementations for GATT specs in XML files to add their own custom made devices.
Architecture design
As you can see that diagram reflects the diagram you pointed out earlier. It is quite similar. There are bundles to add transport, bluetooth binding APIs and some specific binding for a particular device (YeeLightBlue).
The only significant difference is the way how transport is plugged in to OH. In contrast to @cdjackson implementation, the transport implementations are plugged in to Bluetooth Manager via well defined transport APIs.
Another difference is that all OH handlers are supposed to work with the "Governors API" which provides/separates handler specific logic from dealing with BT protocol nasties.
Please note that on the diagram green colour means that it is already implemented (well to certain extend that it can be thought that it is implemented), blue and red colours - something we need to develop/merge. In my mind, the blue components can be implemented/merged by @cdjackson and the red components by me. Of course we all need to put some effort in polishing the green components, however I promise to take the main responsibility in supporting the green stuff.
PS. I'm still transferring/merging useful information/comments from the two PRs. There will be added more info/details/design notes.
cc @kaikreuzer @cdjackson