sputnikdev / eclipse-smarthome-bluetooth-binding

Eclipse SmartHome Bluetooth Binding
46 stars 10 forks source link

Not recognizing custom characteristic/service #67

Open r00li opened 5 years ago

r00li commented 5 years ago

Hi! So, I am very new to Openhab (just installed it yesterday on my RPI 0W) and I am having some issues getting this binding to work with my custom devices.

So I have a few custom BLE devices (built by me) - powered by either a Microchip BLE module or the nordic nrf51822. Temperature sensors, blind controllers, switches and dimmers basically. I can get my devices to connect just fine, but I can't seem to get my own characteristics to show up.

For instance this is my device when connected (I have enabled for unknown channels to display as binary): screenshot 2018-11-10 at 18 36 57

This is my switch module. It has a single characteristic - you write 255 to turn the light on and you write 0 to turn the light off. It has read/write/notify properties.

This is my service xml file that I added:

<Service xsi:noNamespaceSchemaLocation="http://schemas.bluetooth.org/Documents/service.xsd"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="RSwitch V1 service"
         type="com.itag.service.simple_button" uuid="6a741280-71d6-11e5-952a-0002a5d5c51b" last-modified="2018-02-22">
<InformativeText>
    <Abstract>
        RSwitch V1 bluetooth service
    </Abstract>
    <Summary>
        RSwitch V1 bluetooth service
    </Summary>
</InformativeText>
<Dependencies>
    <Dependency>This service has no dependencies on other GATT-based services.</Dependency>
</Dependencies>
<GATTRequirements>
    <Requirement subProcedure="Write Characteristic Value">Mandatory</Requirement>
    <Requirement subProcedure="Notification">Mandatory</Requirement>
    <Requirement subProcedure="Read Characteristic Descriptors">Mandatory</Requirement>
    <Requirement subProcedure="Write Characteristic Descriptors">Mandatory</Requirement>
</GATTRequirements>
<Transports>
    <Classic>false</Classic>
    <LowEnergy>true</LowEnergy>
</Transports>
<ErrorCodes>
</ErrorCodes>
<Characteristics>
    <Characteristic name="RSwitch_V1"
                    type="com.r00li.bluetooth.characteristic.rswitchv1">
        <InformativeText>Button
        </InformativeText>
        <Requirement>Mandatory</Requirement>
        <Properties>
            <Read>Mandatory</Read>
            <Write>Mandatory</Write>
            <WriteWithoutResponse>Excluded</WriteWithoutResponse>
            <SignedWrite>Excluded</SignedWrite>
            <ReliableWrite>Excluded</ReliableWrite>
            <Notify>Mandatory</Notify>
            <Indicate>Excluded</Indicate>
            <WritableAuxiliaries>Excluded</WritableAuxiliaries>
            <Broadcast>Excluded</Broadcast>
        </Properties>
    </Characteristic>
</Characteristics>
</Service>

And this is my characteristic:

<?xml version="1.0" encoding="UTF-8"?>
<Characteristic xsi:noNamespaceSchemaLocation="http://schemas.bluetooth.org/Documents/characteristic.xsd"
                xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="RSwitch_V1"
                type="com.r00li.bluetooth.characteristic.rswitchv1" uuid="9ed90c00-71d7-11e5-977a-0002a5d5c51b" last-modified="2018-11-10"
                approved="No">
    <Value>
        <Field name="Switch">
            <Format>uint8</Format>
            <InformativeText>Switch status</InformativeText>
            <Enumerations>
                <Enumeration key="0" value="Off"/>
                <Enumeration key="255" value="On"/>
            </Enumerations>
        </Field>
    </Value>
</Characteristic>

To be honest, I am not entirely sure if what I am doing is correct, but going from examples this is what I came up with. Now I think that the binding loads my xmls correctly - the logs show that it was complaining when I had an error in the XML, and it's not complaining now.

One other thing that I find weird is that changing the value for the binary characteristic from that UI does nothing. At least it doesn't look like it does anything - the light doesn't turn on/off.

vkolotov commented 5 years ago

Hi @r00li, it is great that you built your own devices, this is effectively what I'm trying to do as well. I'll be very keen on helping you with this.

First thing to check is file names. How do you name them? Should be the same as you put in the "type" tag, e.g.:

com.itag.service.simple_button.xml 
com.r00li.bluetooth.characteristic.rswitchv1.xml

I also suggest to change service name to something like that:

com.r00li.bluetooth.service.rswitch.xml

Also, you must remove and add your device in OH when you put your custom gatt specs. Basically, if a binary channel is already added, then it will not replaced with "user" channels from gatt files. So you have to remove your "thing", change "GATT services and characteristics parsing strategy" to "Recognised only" or "Recognised + system ..." and then add your thing again.

In theory this should work. However, there is something you could improve in the design of your GATT structure. There is a fundamental design consideration that you might like to take into account. The thing is that, even if you enable a flag (send 255) this would not 100% mean that your device switches its state, there are lots of things can happen preventing it to switch its state. Therefore, from my point of view there should be at least two characteristics:

The one that reports its state should support BLE "notifications", e.g. it should notify clients when its state changes. The one that changes state, effectively it is a "command" characteristic.

If you look at the way how standard characteristics are designed, you may notice that if a BLE device supports changing its state, it has a special characteristic called "control point", for example: https://github.com/sputnikdev/bluetooth-gatt-parser/blob/master/src/main/resources/gatt/characteristic/org.bluetooth.characteristic.heart_rate_control_point.xml

There are lots of similar examples in the standard characteristics like that. Effectively they encourage people to use "control point" as a single entry for changing device state and multiple characteristics for reading device states. It really makes sense if you think about it.

I've got some good examples that I use to control some switches at my home, unfortunately I don't have them handy now, but I'll attach it here once I'm at home.

r00li commented 5 years ago

Hi... Excellent. I do plan on open sourcing these as soon as I have a bit of time to document everything properly. And maybe replace a bunch of capacitors and inductors with a single chip balun on the NRF51 - soldering 0402 components by hand is a pain... Anyways... to the issue.

Yeah apparently I did make a mistake while adding a type. But that is just because I copied and pasted the example files so many times that I apparently at one point forgot to change the type. I tried it again with the correct type - sadly it didn't make a difference. The service and characteristic are named v1 because there is also a v2 which supports dimming.

And yeah, I know that I need to remove and add a thing when I change the specs (I also change the path to the specs files to something random and then back to correct path again when changing those files - just to force it to reload them). My parsing strategy is set to "Recognised + system..." .

As for the GATT style... I do have separate channels for control and status on one of my other devices. These were the first devices that I made and they have a really simple characteristic set. Besides - it was never an issue (and I've been using these switches for two years now). Currently if you set the value to anything other than something valid the V2 switch (based on NRF51, V1 is based on the Microchip BLE module) will just discard the value and set the correct one again. If I write [ff] to the binary characteristic via openhab (which should turn the light on), the switch just resets it back to [00]. The Microchip module is slightly more weird though. At least for these modules this will stay as is - mainly because they are wired into my electrical panel and I don't really want to fish them out. But I will take this into consideration for the future.

vkolotov commented 5 years ago

Hi @r00li, please find an example here: https://www.dropbox.com/s/383i56s5ilbx3yx/bluetooth_smart.zip?dl=0

This is an example of gatt files for a smart lock. I believe it is a self explanatory. Please try to adapt it for your switch.

r00li commented 5 years ago

Tried it again... still no luck...

com.r00li.bluetooth.service.rswitchv1.xml:

<?xml version="1.0" encoding="utf-8"?><!-- Copyright 2011 Bluetooth SIG, Inc. All rights reserved. -->
<Service xsi:noNamespaceSchemaLocation="http://schemas.bluetooth.org/Documents/service.xsd"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" type="com.r00li.bluetooth.service.rswitchv1" uuid="6a741280-71d6-11e5-952a-0002a5d5c51b"
         name="RSwitch V1">
    <InformativeText>
        <Abstract>
            Switch service
        </Abstract>
        <Summary>
            Switch service
        </Summary>
    </InformativeText>
    <Dependencies>
        <Dependency>This service is not dependent upon any other services.</Dependency>
    </Dependencies>
    <GATTRequirements>
        <Requirement subProcedure="Write Characteristic Value">Mandatory</Requirement>
        <Requirement subProcedure="Notifications">Mandatory</Requirement>
        <Requirement subProcedure="Read Characteristic Descriptors">Mandatory</Requirement>
    </GATTRequirements>
    <Transports>
        <Classic>false</Classic>
        <LowEnergy>true</LowEnergy>
    </Transports>
    <ErrorCodes>
    </ErrorCodes>
    <Characteristics>
        <Characteristic name="RSwitch_V1" type="com.r00li.bluetooth.characteristic.rswitchv1">
            <InformativeText>
                Switch state
            </InformativeText>
            <Requirement>Mandatory</Requirement>
            <Properties>
                <Read>Mandatory</Read>
                <Write>Mandatory</Write>
                <WriteWithoutResponse>Excluded</WriteWithoutResponse>
                <SignedWrite>Excluded</SignedWrite>
                <ReliableWrite>Excluded</ReliableWrite>
                <Notify>Mandatory</Notify>
                <Indicate>Excluded</Indicate>
                <WritableAuxiliaries>Excluded</WritableAuxiliaries>
                <Broadcast>Excluded</Broadcast>
            </Properties>
            <Descriptors>
                <Descriptor name="Client Characteristic Configuration"
                            type="org.bluetooth.descriptor.gatt.client_characteristic_configuration">
                    <Requirement>Mandatory</Requirement>
                    <Properties>
                        <Read>Mandatory</Read>
                    </Properties>
                </Descriptor>
            </Descriptors>
        </Characteristic>
    </Characteristics>
</Service>

com.r00li.bluetooth.characteristic.rswitchv1.xml:

<?xml version="1.0" encoding="UTF-8"?><!-- Copyright 2011 Bluetooth SIG, Inc. All rights reserved. -->
<Characteristic xsi:noNamespaceSchemaLocation="http://schemas.bluetooth.org/Documents/characteristic.xsd"
                xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                type="com.r00li.bluetooth.characteristic.rswitchv1" name="RSwitch_V1"
                uuid="9ED90C00-71D7-11E5-977A-0002A5D5C51B">
    <InformativeText></InformativeText>
    <Value>
        <Field name="Switch state">
            <Requirement>Mandatory</Requirement>
            <Format>8bit</Format>
            <Enumerations>
                <Enumeration key="0" value="Off"/>
                <Enumeration key="255" value="On"/>
                <ReservedForFutureUse start="1" end="254"/>
            </Enumerations>
        </Field>
    </Value>
</Characteristic>

One thing that I've noticed is that none of these examples that I've seen use full 128 bit UUID for services/characteristics. Could be anything to do with that? Another thing that I've noticed is that this service also includes the Client Characteristic Configuration descriptor. Should this be included? Also... what does Excluded mean when configuring service - that it must not have this option or that it doesn't matter if it has this option or not?

Here is what nRF connect says about my device: screenshot_20181112-202847

vkolotov commented 5 years ago

Right.

One thing that I've noticed is that none of these examples that I've seen use full 128 bit UUID for services/characteristics.

Could you please try to use the very first bit of your UUIDs? E.g.:

<Characteristic xsi:noNamespaceSchemaLocation="http://schemas.bluetooth.org/Documents/characteristic.xsd"
                xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                type="com.r00li.bluetooth.characteristic.rswitchv1" name="RSwitch_V1"
                uuid="9ED90C00">
<Service xsi:noNamespaceSchemaLocation="http://schemas.bluetooth.org/Documents/service.xsd"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" type="com.r00li.bluetooth.service.rswitchv1" uuid="6a741280"
         name="RSwitch V1">

This just reminds me something... There is probably a bug matching UUIDs of real characteristics with GATT definitions. Worthwhile to try before we dig further.

r00li commented 5 years ago

Yes! This worked. My characteristic showed up correctly now.

And I could also turn the light on by choosing On from the drop down. Strangely enough I can't seem to turn it off by selecting Off. I don't see any errors in the log (at least not one that would be obvious), but the light just won't turn off. What is weird here is that if I connect to it with my phone I now have to write 16 bit 0 for it to turn off. But that only happens when I modify the value using this binding. If I change it with any of my phones or my Mac or a python BlueZ library on the Pi I just write 8 bits. So something is still a bit iffy here. I have to test my V2 module with the nrf chip.

vkolotov commented 5 years ago

Good, what is the type of the characteristic? Make sure you specify a correct on, quite important. Is it a signed/unsigned number?

vkolotov commented 5 years ago

Here is a list of supported types: https://www.bluetooth.com/specifications/assigned-numbers/format-types

play with uint8 and sint8

vkolotov commented 5 years ago

I assume that it is unsigned 8 bit byte, if you write 255 value to turn it on (all bits are 1), so try uint8.

r00li commented 5 years ago

It is uint8. I've tried uint8, 8bit and I've also tried uint16 just to see what happens. With uint16 I get errors from the binding that there was an issue while reading.

vkolotov commented 5 years ago

Are you saying that uint8 does not work?

r00li commented 5 years ago

Yes. That is exactly what I am saying.

vkolotov commented 5 years ago

Ok, give me some time to get a new unittest for this.

r00li commented 5 years ago

Ok. I will try it with my V2 switch as well when I come home to see if it behaves any differently. And I'll try connecting a debugger to it if I have some time.

vkolotov commented 5 years ago

Hey @r00li, is this what you would expect? https://github.com/sputnikdev/bluetooth-gatt-parser/blob/31506f47638fb3203ed68cffdd0c1f063a2a37ab/src/test/java/org/sputnikdev/bluetooth/gattparser/GenericCharacteristicParserIntegrationTest.java#L438

assertEquals("11111111", Integer.toBinaryString(Byte.toUnsignedInt(raw[0])));

In other words your 8bit characteristic gets written with all 1.

vkolotov commented 5 years ago

btw, that branch fixes the issue with 128 bit UUIDs

r00li commented 5 years ago

Yeah that is what I would expect to happen when writing 255. Though I would also check if 0 gets written correctly. So writing 0 to an 8bit field should get you 0b00000000.

vkolotov commented 5 years ago

Ah right, the problem not in "on" but with "off". Hold on...

vkolotov commented 5 years ago

Here we go: https://github.com/sputnikdev/bluetooth-gatt-parser/blob/f15e40a00c28151a103cefba23ccabeb1acccdd8/src/test/java/org/sputnikdev/bluetooth/gattparser/GenericCharacteristicParserIntegrationTest.java#L443

It seems to me working, this means that the problem somewhere else. I'll double check if this is what exactly happens in the binding.

What transport do you use? TinyB or BlueGiga?

r00li commented 5 years ago

Hmmm shouldn't you be checking the binary string for: assertEquals("00000000", Integer.toBinaryString(Byte.toUnsignedInt(raw[0]))); Not: assertEquals("0", Integer.toBinaryString(Byte.toUnsignedInt(raw[0]))); Not entirely 100% on that. I am not that familiar with Java. But it seems that you are only checking the first bit, but you should check all 8.

I use TinyB with BlueZ 5.47 on Raspberry Pi Zero W.

vkolotov commented 5 years ago

The minimum what gets send to device (according to BT spec, afaik) is 1 byte. This means that and empty byte (all zeroes) your device should get.

r00li commented 5 years ago

You are right. And you are checking for 0 anyways. Weird.

vkolotov commented 5 years ago

Do you get any log entries like that when you select on/off in PaperUI?

Could not not convert state to enumeration: blah blah : blah
vkolotov commented 5 years ago

Damn, that log entry gets printed if debug level is enabled, can you pls enable it in the OH console and try to select on/off values?

log:set DEBUG org.sputnikdev.esh.binding.bluetooth.handler
r00li commented 5 years ago

I will check when I get home (in about 8 hours). Don't have access to the device right now.

But I did enable debug logging already. And I don't remember anything like that from yesterday when I was checking.

vkolotov commented 5 years ago

Cool. Make sure you enable it for this package: org.sputnikdev.esh.binding.bluetooth.handler

vkolotov commented 5 years ago

Also I'd recommend to try to write on/off values by using bluetoothctl utility. Should be quite easy to do. Just in general it is a good way to debug things as it is very close to what the binding does.

r00li commented 5 years ago

Hmmm. Ok. So no log entries that would look similar. Actually I see this in the logs: 2018-11-13 17:25:11.330 [vent.ItemStateChangedEvent] - bluetooth_ble_001EC02132BA_6A741280_9ED90C00_switchstate changed from 255 to 0

And I've tried it with bluetoothctl, this is the result:

[RSwitch]# connect 00:1E:C0:21:32:BA
Attempting to connect to 00:1E:C0:21:32:BA
Connection successful
[RSwitch]# select-attribute 9ED90C00-71D7-11E5-977A-0002A5D5C51B
[RSwitch:/service0016/char0017]# write 0xff
Attempting to write /org/bluez/hci0/dev_00_1E_C0_21_32_BA/service0016/char0017
[RSwitch:/service0016/char0017]# write 0x00
Attempting to write /org/bluez/hci0/dev_00_1E_C0_21_32_BA/service0016/char0017

The light turned on/off just as expected.

I've also tried it with my V2 device. It is working just fine with that one. And I've even connected a debugger to my V2 device - the data received was 0x00 with a length of 1. So it must be something with the V1 device. Sadly the stupid Microchip module in my V1 device doesn't have a debugger to see what actually happens. And I also don't have a spare one to see what is going on.

vkolotov commented 5 years ago

Right, very interesting :)

the data received was 0x00 with a length of 1

do you mean the device receives an array of length 1 and the first element in the array is 0x0?

r00li commented 5 years ago

Yeah. That is exactly what I mean. Basically it's what one would expect.

On the other hand there obviously is something wrong since the V1 is working just fine with bluetoothctl and not with this binding.

r00li commented 5 years ago

I just noticed something. Not sure if it is important or not... I was playing around with nrf connect on my phone and noticed that the V2 switch (the one that works fine) works with either "wite request" and "write command", but the V1 switch (the one with the issue) only works with "write request". Could it possibly be something to do with this?

Though this doesn't really explain why writing 255 would work but writing 0 wouldn't.

vkolotov commented 5 years ago

Possibly yes, not sure what it does though. I assume these are two different methods to write characteristics, e.g. write with response and write without response. The binding uses both of these methods depending on what the device reports (e.g. what method is supported).Strange thing is that it works to turn your switch "on", but not for "off", I' expect it either to work on not to work :)

vkolotov commented 5 years ago

Anyway, I'll try to emulate this in my OH (add your xmls) and see what exactly gets written into a characteristic. I won't be able to do this today, Friday is more likely.

r00li commented 5 years ago

Yeah. It most certainly is weird.

Awesome. I am looking forward to the results. And I'll see if I can figure something else out.

r00li commented 5 years ago

Hey! Have you managed to test anything?

I was just playing around a bit more today and found a way to replicate this behavior with my phone. Basically writing 0xffff (instead of 0xff) to the characteristic will turn on the light, but writing 0x00 to it afterwards will not turn it off again. After I write 0xffff (or 0xffffff) to it I need to actually write 0x0000 to turn it off instead of 0x00 like normal. I am not sure if it is useful since from the tests with the other device it doesn't appear like the binding is sending too much data, but it is an idea.

vkolotov commented 5 years ago

Hi @r00li, sorry I was really busy irl.

Interesting findings. Effectively, the "on" operation breaks your switch. Would you be able to do another experiment? Try to turn on your device with your phone, then connect it to the binding and try to turn it off. This may work if your device won't turn off automatically when disconnects.

This would give us more info on what's not right.

r00li commented 5 years ago

No worries.

I tried it... no luck sadly. The thing still won't turn off. And if I want to turn it off again with my phone I need to write 0x0000 again. This is starting to get on my nerves. I have the desire to rip the damned switch out of my wall and attach a debugger to it just to see what happens.

vkolotov commented 5 years ago

Can you share that piece of your firmware code that check incoming value and then make a decision what to do (on/off)?

Update: just wondering what is in your if statement, e.g. what value you compare with. And also type of variable/characteristic you use.

r00li commented 5 years ago

Here is the whole thing: https://gist.github.com/r00li/e3965849ee128e95faec422a16ef6d6b processToken function is the one that actually does turning on/off. The whole thing is using a Microchip RN4020 BLE module paired with an STM32F0 micro controller and the code is pretty much as ugly as possible.

vkolotov commented 5 years ago

Nice one :) That would be difficult to debug that thingy, too many moving parts, e.g. bt module, uart and your controller... Have you already tried to specify 9bit type or 16bit type for your characteristic? Worth trying before ripping off from the wall :) If you set 9bit, you will need a proper key value as well, e.g. 511 in decimal (0b11111111).

vkolotov commented 5 years ago

Or you can even try to leave the key as 255.

vkolotov commented 5 years ago

Same thing for 16bit test case. The key value for key could be:

I'd try these ones. Who knows what your bt module would do/send to your mcu, hopefully one of these values will work ;)

r00li commented 5 years ago

That's why I switched to nrf51 and never looked back. The official nordic SDK has awesome documentation and their help team is very helpful if you have issues. The learning curve is a bit steep for a first time user maybe.

There is a 9 bit type? Interesting. I will try these tomorrow. I did try uint16 before without any luck though.

I don't know why I haven't seen this issue before now though. I've been using this switch from 2015 almost daily with various phones (iOS and android), linux distros with custom software written in python to BLE explorer on my Mac.

vkolotov commented 5 years ago

I know this is strange.

Yes, you can use an arbitrary number in Xbit notation. The binding will send only these X bits.

nrf51 chips are very good, I like to work with them. I'd also recommend to look at mbed framework, which is heaps easier to use comparing to nordic sdk and you also don't have to have any chaintool installed as this all gets built in cloud.

r00li commented 5 years ago

Well I tried these values today... 9bit doesn't work... Though with that (or 16bit) the log is full of: Not enough bits to parse field "Switch state". Data length: 1 bytes. Looks like your device does not conform SIG specification.

16bit doesn't work either. 65535 and 511 turn on the light, but 0 doesn't turn it off. 256 doesn't turn it on or off. Though I did notice something strange... when I turn off the light with my phone by writing 0x0000 to the device and then reconnect to it with the binding, the value shown in the UI is 256 instead of 0.

r00li commented 5 years ago

Well I went for the extreme measures... I ripped the switch out of the wall and connected it to the debugger.

The funny thing is - as soon as I connect to it with the binding no breakpoints gets triggered anymore. It just seems like the Microchip BLE module is flooding my micro controller with UART interrupts. And not even receive interrupts.

When I connect to it with my phone everything works normally. No excessive interrupts, and other breakpoints work just fine.

But for now I am done messing around with this. I'll just replace the V1 switch with my V2 module and be done with it.

I have one final question... when will the update with the support for 128 bit characteristics be available to download via the openhab UI?

vkolotov commented 5 years ago

Good to know.

I have one final question... when will the update with the support for 128 bit characteristics be available to download via the openhab UI?

I'm planning to cut a new release soon, potentiall next week. Sorry I'm a bit busy nowadays, can't do releases too often. Also I'm working on a feature that will help you to manage your devices, e.g. that feature will allow you to configure things so that they won't keep connection always open and will only establish connection when required (eg. a characteristic is changed).

vkolotov commented 5 years ago

BTW, since your v2 switches are working, is this something that you could share? I mean that would be good if you could create a PR with your gatt files for the gatt parser project. This would mean that you won't have to add any external configs for your OH and it also will provide some good examples for others.

r00li commented 5 years ago

Sure, no problem. I'll add the V2 switches and the other devices after after the 128 bit UUIDs are in so I can actually see if everything is working as it should.

vkolotov commented 5 years ago

Hey @r00li, could you please try that build (with 128 bit uuid fixes)? https://oss.sonatype.org/content/repositories/snapshots/org/sputnikdev/org.eclipse.smarthome.binding.bluetooth/1.1.7-SNAPSHOT/org.eclipse.smarthome.binding.bluetooth-1.1.7-20181130.002654-2.jar