search

sputnikdev / eclipse-smarthome-bluetooth-binding

Eclipse SmartHome Bluetooth Binding
46 stars 10 forks source link

Add support for Smart Radiator Thermostat - comet-blue-eurotronic #14

Open matteopedani opened 6 years ago

matteopedani commented 6 years ago

I'm testing the binding of https://community.openhab.org/t/comet-blue-eurotronic/36590 Now I can bind.

How can I create the XML files? Where to put? How to add an extra coefficent to each radiator (to calculate the calories) How to elaborate data. (to calculate calories and for billing purpose)

vkolotov commented 6 years ago

Hi @matteopedani , it looks similar to eQ-3 radiator thermostat, is it the same device?

matteopedani commented 6 years ago

I don't think so. May be. I don't have eQ-3 They use the similar hardware but they program in a different way. They have different ID manufactor I can expose my raspberry and the 7 thermostat to internet if you like to play send me your ssh pubblic id

bademux commented 6 years ago

@matteopedani It can be same device https://translate.google.com/translate?sl=auto&tl=en&js=y&prev=_t&hl=en&ie=UTF-8&u=http%3A%2F%2Ftorsten-traenkner.de%2Fwissen%2Fsmarthome%2Fheizung.php&edit-text=

vkolotov commented 6 years ago

Looks like it is not the same device. Here is a python project with some good details about its characteristics: https://github.com/im-0/cometblue/blob/master/cometblue/device.py

What you will need to do is:

  1. Create GATT xml files similar to this (please do not think it is easy to accomplish):

  2. Install a new build of the Bluetooth binding from here (you will need to uninstall the binding from MarketPlace first): https://oss.sonatype.org/content/repositories/snapshots/org/sputnikdev/org.eclipse.smarthome.binding.bluetooth/1.1-SNAPSHOT/org.eclipse.smarthome.binding.bluetooth-1.1-20180114.100602-6.jar

It will give you a new experimental feature that can discover all characteristics (including "unknown" characteristics). To enable this feature: image

  1. Create a folder on your file system that is accessible by "openhab" user, something like that: /home/pi/.bluetooth_smart That folder should contain two subfolders:
    • characteristic
    • service

These two folders should contain your xml files from the point n1, e.g. "characteristic" folder contains XML file for characteristics and "service" contains XML files for your services. Make sure that file permissions are set correctly so that "openhab" user can read them.

Specify that folder here: image

I just want to warn you that this process is not easy, you should have some background in bluetooth technology (GATT) and in software engineering. So please do not expect any magic.

matteopedani commented 6 years ago

@bademux
Yes, Is this device In this page I linked for reference https://community.openhab.org/t/comet-blue-eurotronic/36590

@vkolotov I will try to follow your instructions to create GATT xml files.

matteopedani commented 6 years ago

To find the right ID Can I use ? sudo bluetoothctl

############## follow the full output of bluetoothctl

[termosifone cucina piano terra]# devices Device D0:B5:C2:FB:BE:B3 termosifone bagno blu primo piano Device D0:B5:C2:FB:BE:C3 termosifone 5 Device D0:B5:C2:FB:B9:40 termosifone 1 Device D0:B5:C2:FB:AF:64 termosifone 2 Device D0:B5:C2:F1:73:2E termosifone 3 primo piano Device D0:B5:C2:FB:B9:3A termosifone 4 Device D0:B5:C2:F1:72:4D termosifone cucina piano terra [termosifone cucina piano terra]# connect D0:B5:C2:F1:72:4D Attempting to connect to D0:B5:C2:F1:72:4D Connection successful [termosifone cucina piano terra]# info Device D0:B5:C2:F1:72:4D Name: Comet Blue Alias: termosifone cucina piano terra Paired: yes Trusted: no Blocked: no Connected: yes LegacyPairing: no UUID: Generic Access Profile (00001800-0000-1000-8000-00805f9b34fb) UUID: Generic Attribute Profile (00001801-0000-1000-8000-00805f9b34fb) UUID: Device Information (0000180a-0000-1000-8000-00805f9b34fb) UUID: Vendor specific (47e9ee00-47e9-11e4-8939-164230d1df67) ManufacturerData Key: 0xffff ManufacturerData Value: ff 00 01 ff ff ff ......
ManufacturerData Key: 0x2229 ManufacturerData Value: 20 00 00 ff 69 88 ...i.
ManufacturerData Key: 0x2f26 ManufacturerData Value: 23 01 00 ff af 69 #....i
RSSI: -82 TxPower: 0 [termosifone cucina piano terra]# list-attributes Primary Service /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b 47e9ee00-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0046 47e9ee30-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0044 47e9ee2e-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0042 47e9ee2d-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0040 47e9ee2c-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char003e 47e9ee2b-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char003c 47e9ee2a-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char003a 47e9ee27-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0038 47e9ee26-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0036 47e9ee25-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0034 47e9ee24-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0032 47e9ee23-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0030 47e9ee22-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char002e 47e9ee21-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char002c 47e9ee20-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char002a 47e9ee16-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0028 47e9ee15-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0026 47e9ee14-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0024 47e9ee13-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0022 47e9ee12-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char0020 47e9ee11-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char001e 47e9ee10-47e9-11e4-8939-164230d1df67 Vendor specific Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service001b/char001c 47e9ee01-47e9-11e4-8939-164230d1df67 Vendor specific Primary Service /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service0010 0000180a-0000-1000-8000-00805f9b34fb Device Information Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service0010/char0019 00002a29-0000-1000-8000-00805f9b34fb Manufacturer Name String Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service0010/char0017 00002a28-0000-1000-8000-00805f9b34fb Software Revision String Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service0010/char0015 00002a26-0000-1000-8000-00805f9b34fb Firmware Revision String Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service0010/char0013 00002a24-0000-1000-8000-00805f9b34fb Model Number String Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service0010/char0011 00002a23-0000-1000-8000-00805f9b34fb System ID Primary Service /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service000c 00001801-0000-1000-8000-00805f9b34fb Generic Attribute Profile Characteristic /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service000c/char000d 00002a05-0000-1000-8000-00805f9b34fb Service Changed Descriptor /org/bluez/hci0/dev_D0_B5_C2_F1_72_4D/service000c/char000d/desc000f 00002902-0000-1000-8000-00805f9b34fb Client Characteristic Configuration

vkolotov commented 6 years ago

hi @matteopedani, yes those "manufacturer specific" services and characteristics are those that you will need to create mapping for. Look at this project, if you can read python code, you will find what each characteristic can do: https://github.com/im-0/cometblue/blob/master/cometblue/device.py

vkolotov commented 6 years ago

@matteopedani, looks like these ones are the most interesting:

'datetime': {
            'description': 'time and date',
            'uuid': '47e9ee01-47e9-11e4-8939-164230d1df67',
            'read_requires_pin': True,
            'decode': _decode_datetime,
            'encode': _encode_datetime,
        },

        'flags': {
            'description': 'flags',
            'uuid': '47e9ee2a-47e9-11e4-8939-164230d1df67',
            'read_requires_pin': True,
            'decode': _decode_flags,
        },

        'temperatures': {
            'description': 'temperatures',
            'uuid': '47e9ee2b-47e9-11e4-8939-164230d1df67',
            'read_requires_pin': True,
            'decode': _decode_temperatures,
            'encode': _encode_temperatures,
        },

        'battery': {
            'description': 'battery charge',
            'uuid': '47e9ee2c-47e9-11e4-8939-164230d1df67',
            'read_requires_pin': True,
            'decode': _decode_battery,
        },

        'firmware_revision2': {
            'description': 'firmware revision #2',
            'uuid': '47e9ee2d-47e9-11e4-8939-164230d1df67',
            'read_requires_pin': True,
            'decode': str,
        },

        'lcd_timer': {
            'description': 'LCD timer',
            'uuid': '47e9ee2e-47e9-11e4-8939-164230d1df67',
            'read_requires_pin': True,
            'decode': _decode_lcd_timer,
            'encode': _encode_lcd_timer,
        },

        'pin': {
            'description': 'PIN',
            'uuid': '47e9ee30-47e9-11e4-8939-164230d1df67',
            'encode': _encode_pin,
        },
matteopedani commented 6 years ago

Thank's I will work on these. Matteo

2018-01-16 11:42 GMT+01:00 Vlad Kolotov notifications@github.com:

@matteopedani https://github.com/matteopedani, looks like these ones are the most interesting:

'datetime': { 'description': 'time and date', 'uuid': '47e9ee01-47e9-11e4-8939-164230d1df67', 'read_requires_pin': True, 'decode': _decode_datetime, 'encode': _encode_datetime, },

    'flags': {
        'description': 'flags',
        'uuid': '47e9ee2a-47e9-11e4-8939-164230d1df67',
        'read_requires_pin': True,
        'decode': _decode_flags,
    },

    'temperatures': {
        'description': 'temperatures',
        'uuid': '47e9ee2b-47e9-11e4-8939-164230d1df67',
        'read_requires_pin': True,
        'decode': _decode_temperatures,
        'encode': _encode_temperatures,
    },

    'battery': {
        'description': 'battery charge',
        'uuid': '47e9ee2c-47e9-11e4-8939-164230d1df67',
        'read_requires_pin': True,
        'decode': _decode_battery,
    },

    'firmware_revision2': {
        'description': 'firmware revision #2',
        'uuid': '47e9ee2d-47e9-11e4-8939-164230d1df67',
        'read_requires_pin': True,
        'decode': str,
    },

    'lcd_timer': {
        'description': 'LCD timer',
        'uuid': '47e9ee2e-47e9-11e4-8939-164230d1df67',
        'read_requires_pin': True,
        'decode': _decode_lcd_timer,
        'encode': _encode_lcd_timer,
    },

    'pin': {
        'description': 'PIN',
        'uuid': '47e9ee30-47e9-11e4-8939-164230d1df67',
        'encode': _encode_pin,
    },

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sputnikdev/eclipse-smarthome-bluetooth-binding/issues/14#issuecomment-357921554, or mute the thread https://github.com/notifications/unsubscribe-auth/ASiPBT7Z1oeB9jsVIDCt2L-Es3-O9Aztks5tLH0WgaJpZM4RfePM .

-- Matteo Pedani

www.pedani.it mobile +39 3343637690 phone +39 0699341466 phone +39 069415152

xrucka commented 6 years ago

Hi, thanks @vkolotov for pointing me in this dirrection, I have only skimmed through the original discussion and completely missed the roadmap. I suggest using my fork of cometblue control utility, as I had further developed the encoding of status triplet. You can find my fork at https://github.com/xrucka/cometblue . @matteopedani - is there any way I can contribute on developing the specs?

vkolotov commented 6 years ago

Hi @xrucka, any contribution is greatly appreciated. Looks like you have some knowledge of GATT stuff, If so you could try to come up with GATT files as I mentioned above: https://github.com/sputnikdev/eclipse-smarthome-bluetooth-binding/issues/14#issuecomment-357909251

I'll be supporting you in this answering any questions you have.

Please note that this might not be possible to add support for this device through GATT XML files. Please see here: https://github.com/sputnikdev/eclipse-smarthome-bluetooth-binding/issues/13

In short, the eQ-3 device has API that does not follow GATT specification, to be more precise the data structures for its main characteristic are different for read, write and notify operations. That's why for the eQ-3 device we will have to develop a custom bluetooth binding.

xrucka commented 6 years ago

Ok, thanks for the headsup; I should be able to look into it either on tomorrow or thursday evening. I'll get back to you guys once I manage to do something about it. As for odd data structures, I've not noticed anything special, except perhaps the status triplet, but I believe that it should be manageable with correct read/write order.

vkolotov commented 6 years ago

Hi @xrucka, thank you very much for your contribution for TinyB transport. Looking forward to merge it to the upstream.

In regards your thermostat, there is a good example of how to implement GATT spec files. Recently we have added support for a device that kind of follows GATT spec (however we are still having some issues with notifications, looks like Bluez is of old version or something). Here are those files and a unittest:

It is not still merged to the upstream as I mentioned above we have some issues with notifications, but unittest shows that data structure can be easily parsed for the given characteristic.

I'm not sure If I have some time this weekend (it is a long weekend), but I'll try to help you to create specs for your device.

The most interesting part is here:

<Field name="Flags">
            <Requirement>Mandatory</Requirement>
            <Format>8bit</Format>
            <BitField>
                <Bit index="0" size="8" name="DataTypeFlag">
                    <Enumerations>
                        <Enumeration key="1" value="False" requires="C1"/> <!-- DATA_GENERAL_4_CH_TH_TYPE_0 -->
                        <Enumeration key="-126" value="True" requires="C2"/>  <!-- DATA_GENERAL_4_CH_TH_TYPE_1 -->
                    </Enumerations>
                </Bit>
            </BitField>
        </Field>

This field in the characteristic defines overall structure of the characteristic. Basically it says if the very first byte is equal to 1 or -126, then C1 and C2 flags set respectively. This controls inclusion/exclusion of the rest of the fields, for example:

<!-- DATA_GENERAL_4_CH_TH_TYPE_0 -->
        <Field name="Base temp">
            <Requirement>C1</Requirement>
            <Format>uint16</Format>
            <DecimalExponent>-1</DecimalExponent>
        </Field>

and

<!-- DATA_GENERAL_4_CH_TH_TYPE_1 -->
        <Field name="Sensor 2 humidity min">
            <Requirement>C2</Requirement>
            <Format>uint8</Format>
        </Field>

Notice C1/C2 tag.

vkolotov commented 6 years ago

BTW, it does not mean that we have to do this all the time. This is required only for some characteristics that have "dynamic" data structure. If it is flat/fixed, then just a list of fields solves the problem.

xrucka commented 6 years ago

Huray, I've finally managed to figure what was preventing my specs from working. If anyone reads this - so far, the short form of service and characteristics uuid has to be in upper case, as the libraries use internally string matching, instead of comparing numeric value.

Anyway, the device - at least at some points - uses offseted values. For example, year is represented with uint8 value, offseted by 1999. I've been following the field descriptions (http://schemas.bluetooth.org/Documents/formats.xsd), yet they do not explicitly provide a way to offset the value. Can you please confirm (and perhaps suggest an alternative), @vkolotov ? The only idea I got so far is to enumerate all 256 years, yet this sounds too hacked, even for me...

vkolotov commented 6 years ago

Thanks for your update @xrucka.

so far, the short form of service and characteristics uuid has to be in upper case, as the libraries use internally string matching, instead of comparing numeric value.

Yeah, I think it can be improved in the gatt parser by comparing "ignore case".

I think I remember seeing this in the eq-3 thermostat device as well. As you have said, there is not any easy way to fix that by using standard GATT spec. But you could extend the gatt parser with a new tag, e.g. like "DecimalExponent" https://github.com/sputnikdev/bluetooth-gatt-parser/blob/d488bc07404cd5ddc61b1c84087138cc9435f432/src/main/resources/gatt/characteristic/com.xiaomi.bluetooth.characteristic.advertised_data.xml#L31 and add some basic logic like here: https://github.com/sputnikdev/bluetooth-gatt-parser/blob/d488bc07404cd5ddc61b1c84087138cc9435f432/src/main/java/org/sputnikdev/bluetooth/gattparser/FieldHolder.java#L373

It must be quite easy to implement, let me know what you think?

xrucka commented 6 years ago

I guess that extending the specs might be the easiest way to do, especially if there are multiple devices holding the same issue. I'll do the patches and see what happens (I need to recall extending xsd's though).

Furthermore, is there a way to specify order of read/write operations in the bluetooth binding? The issue is, that before reading any other attribute, a "pin" has to be written to the device, otherwise all reads fail.

vkolotov commented 6 years ago

Furthermore, is there a way to specify order of read/write operations in the bluetooth binding? The issue is, that before reading any other attribute, a "pin" has to be written to the device, otherwise all reads fail.

Damn... too bad. There is not any authentication mechanism supported by binding yet...

Is it a just "magic" number that must be written to a specific characteristic?

I was thinking to tweak the binding to allow specifying a magic number in the device settings... Xiaomi devices have the same issue, however they advertise all necessary readings, e.g. temperature, humidity etc so no need to connect to them actually.

vkolotov commented 6 years ago

Another way is to create a new custom bluetooth binding (by using public bluetooth API from the core binding) and publish it in the Eclipse Marketplace.

vkolotov commented 6 years ago

Just to sum up options we have in my mind to resolve your issue with the "magic number":

  1. Implement a new bluetooth binding which uses BluetoothManager API. I've been working on making the BluetoothManager API to be usable by multiple clients, I believe it is more or less ready. The new binding can be published in the Eclipse Marketplace.
  2. Change the existing bluetooth binding to add support for setting "magic number" (device thing parameters) before making any communications with target device. This might not be a bad option, the new "thing" parameter may look like:
  3. Extend the existing binding with a new thing-type "bluetooth-cometblue..." and a corresponding ThingHandler and ThingHandlerFactory. Implement authentication logic in the new ThingHandler.

I guess, this problem is a generic one - authentication. Ideally the binding should provide some mechanisms (settings or something...) to allow users to perform authentication.

Let me know what you think @xrucka.

xrucka commented 6 years ago

Hi, I'd rather go with # 2. I believe that both # 1 and # 3 would (eventualy) lead to fatware explosion or lack of support. The automated dropdown magick suggestion can parse characteristics names for magic words (i.e. PIN). I'm not familiar with the Xiaomi device you've mentioned, but cometblue uses the same characteristic for both unlocking & setting the pin (1st, resp. 2nd write). I'll try to come up with some code for it during the next week and get back to you.

xrucka commented 6 years ago

Hi, I have finally managed to get some time into this again. I've tested the 1.3-SNAPSHOT of bluetooth-manager with authentication and explicit UUID setting and hex byte array - the device now connects and authenticates successfuly. I'll get back once I manage to complete the gatt files.

In the mean time, is there a way to create a "combined channel"? Usecase: the device uses custom characteristic format to provide time setting (year, month, day, hour, minute). Similar to this, there are timeslots for heating provided in other characteristics.

vkolotov commented 6 years ago

Hi @xrucka,

Hi, I have finally managed to get some time into this again. I've tested the 1.3-SNAPSHOT of bluetooth-manager with authentication and explicit UUID setting and hex byte array - the device now connects and authenticates successfuly.

Good to know that this works for you. I've spent some significant effort to make it working, it turned out not very easy to do, lots of nuances, especially multithreading etc.

In the mean time, is there a way to create a "combined channel"? Usecase: the device uses custom characteristic format to provide time setting (year, month, day, hour, minute). Similar to this, there are timeslots for heating provided in other characteristics.

Can you please elaborate this more? Are you saying that a single characteristics is used to represent several data points?

xrucka commented 6 years ago

Hi, ok, I'll elaborate a bit more on this. The device is programmable radiator valve - you can configure up to 4 presets for each day of the week - either as primary heading phase (i.e. day temperature) or secondary heating phase (i.e. night temperature). This, as a result means, that the device needs to keep track on current time - and for this purpose, it uses internal clock. This internal clock is accessible through characteristic 47e9ee01, which provides 5 bytes of data, representing minute, hour, day, month, year (1st to 5th byte respectively )

The current characteristic description I wrote mirrors the structure of gatt date_time characteristic

And as a result, it appears as 5 distinct channels - Year, Month, Day, Hour, Minute.

join_fields

xrucka commented 6 years ago

Hmm... got yet another issue implementing the program settings. The program is given as 8 8-bit fields, described by:

. Where time is represented as 8-bit value in minutes, with 10 minute step. However, as a result, the heating program from from 7:00 .. 22:00 would be than given as minutes 420 .. 1320. This is rather ugly. As I see it, there are 3 possible ways to go: 1) Further extend the GATT XML specs with capability for printf/strftime-like (bidirectional) formating of the value given. This might be somewhat ugly to implement, however can pay off in future, as it might allow for support for devices that do not exactly fit GATT standard. 2) Enumerate all 144 values with time they represent. 3) Find a way to represent this as hours, using (integer) multiplier and binary/decimal exponent. @vkolotov what would you suggest?
vkolotov commented 6 years ago

Hi @xrucka,

Re the program issue: I believe this issue consists of two:

Re the "combined" fields: I'm actually working on it at the moment, the idea is to come up with "high level" data types composite field holders that consist of smaller types. In the GATT specification those fields are defined as references, e.g. <Reference>org.bluetooth.characteristic.exact_time_256</Reference>: here

Your example of the date is perfect. There is also a good example - colour picker.

Here is a test that demonstrates composite fields: https://github.com/sputnikdev/bluetooth-gatt-parser/blob/c58d3524170a5695771873877cc6008275406aa2/src/test/java/org/sputnikdev/bluetooth/gattparser/GenericCharacteristicParserIntegrationTest.java#L126

"Exact Time 256" is the composite field that is defined in GATT spec.

Effectively users will be able to set data by using both ways: low level fields (primitive) and high level field (composite field). I think it perfectly matches the standard GATT spec approach.

Let me know what you think.

xrucka commented 6 years ago

We could possibly use existing GATT mechanisms to construct Duration from 8bit number, e.g. DecimalExponent (-1).

Not sure how do you intend to accomplish this. Do you mean using the start time as the integer part and end time as the decimal one?

I believe that Reference mechanism fits better - as long as the References are nested. I can imagine something like: `

org.eurotronic.cometblue.program_duration org.eurotronic.cometblue.program_duration org.eurotronic.cometblue.program_duration org.eurotronic.cometblue.program_duration

`

` with org.eurotronic.cometblue.program_duration.xml as:

uint8 org.bluetooth.unit.time.minute uint8 org.bluetooth.unit.time.minute

` I'm not sure though that duration alone is enough - I was not able to find out whether it holds start time as well as the duration - I'm afraid that both the start time + duration would have to be maintained.

This, however, means there would have to be some kind of semantics parsed out from the characteristic specification, perhaps based on characteristic name?

vkolotov commented 6 years ago

Hi @xrucka,

I've been playing with it a bit. Here is what I think:

Composite fields are definitely nice to have to implement "program"-like fields (scheduling), e.g. like you've mentioned "program_duration". Otherwise we can end up with a loooong list of primitive fields, e.g. "Monday Primary Heating Program 1 start", Monday Primary Heating Program 1 end, Tuesday Primary Heating Program 2 start etc... which would not be very usable.

However... It might not be critical/required just for now. If you think about it in general, OpenHab is a platform that provides all sorts of scheduling, so we probably do not actually need to provide configurable "programs" at all... If we have a mechanism to star/stop heating and set temperature, then this should be sufficient. I would be focusing on providing basic controls for now rather than implementing something complex that potentially won't be usable/in use.

Furthermore, even of we had a mechanism to define "programs", how would we implement this on OpenHab UI? AFAIK OH does not provide this sort of UI controls.

Another question/issue is "transactioning". Does your thermostat provide a separate characteristic per program/ per day? If it is per day, then there is another layer of complexity. If user wants to edit only "Monday program 1 start" field, then we have to gather all other program values, combine them together and send it to the device. This raises more questions like "what if we don't have other values"? Likewise, if we had "characteristic per program" data structure, then it should not be too difficult.

If we wanted to implement this on OH UI, we could potentially just use a textual channel. We could come up with a format for this, e.g.

Monday: [8:10-12:40; 18:00-21.30; 23:00-23:30] Tuesday: [8.10-12.40; 18:00-21.30]

This could potentially resolve the "transactioning" issue if each of the channel (Monday, Tuesday etc) belongs to a specific characteristic.

Re java "Duration": as far as know, this class represents quantity or amount of time in terms of seconds and nanoseconds. We don't have to use it though, I thought it might be a bit more convenient for users to define the "start" and "stop" fields by using Duration rather than minutes (Integer).

What do you think?

xrucka commented 6 years ago

Ok, I was able to get some time to work on this. It's still work in progress, however can already do some good. Comments wellcomed.

Requires pullrequests #54 and bluetooth-gatt-parser#7

eclipse-smarthome-bluetooth-binding-device-cometblue

xrucka commented 6 years ago

@vkolotov - Following the discussion on #67 -- If I understand that correctly, do you expect to use control points instead of arbitrary characteristic writes? Or was it more of "best practices" idea?

I'm currently struggling with the need to update field in characteristic (sending the whole characteristic to device is OK), but the 1.9.5 version of bluetooth-gatt-parser refuses to validate the write request. I didn't have time to look into it deeper yet, but if this is such a case, it would explain the issues.

Example of such a characteristic; havinig similar problem with another, yet unpublished, characteristic holds a set of boolean values, some of them are contacts, some are switches.

EDIT: 1.9.2->1.9.5

vkolotov commented 6 years ago

hi @xrucka, yes it is more best practice, e.g. BT spec does not say anything about that. However, if you try to find a standard spec that defines a characteristic which can be read and written in the same time, you won't find any (as far as I remember), e.g. normally characteristics with "write" access type cannot be read. If you think about it more, you do really need to separate state and command logically, because simply if you try to change a state, this does not mean that device is able to change it (or will change it) as BT communication is not transactional or in other words write operations are not atomic.

For example, you may set a state "a", lets say to "true", device may accept it but won't "actuate" your request immediately because it may need to do some physical actions (like turning servo or motor to a specific angle to close/open a valve). Furthermore, there might be some issues/errors in actuator so that it won't be able to complete the action (change state) at all. So you really need a characteristic that changes state and a characteristic that notifies when state gets changed (for any reason). If you mix these two characteristics, then you may get some inconsistency on client side, e.g. you write a state, but it does not get though to device but you (as a client) still think that the state has been changed.

I remember I was dealing with that thermostat device (a clone with slightly different gatt structure), it has a characteristic with read/write access type. That was the main issue in implementing this. Also there was a problem that there were multiple "write" fields that are all mandatory. Effectively when you need to change a field, the binding has to re-read its internal state (UI state) or re-read device state itself, then fill in gatt request with all mandatory fields and then send them all even if only 1 has changed. The binding used to support this until some time when I understood that this is actually a bad idea to re-read state because of a number of reasons, e.g. performance and consistency between the binding state and device state.

Having said that, we might get away with this without splitting your characteristic on separate fields, instead we might have a number of predefined values (enumerations) for that characteristic so that users can select one of them from dropdownbox. Not sure if this is feasible though for your particular case.

vkolotov commented 6 years ago

Here we go, I still have a spreadsheet with some analysis for that thermostat "clone": https://docs.google.com/spreadsheets/d/1Q1fid_KpZRYmpW3k8GZCLgegTwPuaqUiZCPx8ELTYkE/edit?usp=sharing

as you may see, read and write structures (fields) are different which makes it very hard to come up with a gatt spec. However, I still believe that we can implement something custom (extend gatt specs) so that it is feasible to do.

vkolotov commented 6 years ago

I can see that in your case, they have mixed current temperature state with some settings in the same characteristic. That would be problematic to support honestly speaking, we will need to implement something that would make the binding to re-read internal state (all these temp settings) before it changes any of other settings.

Another option would be to come up with a special/custom characteristic type that will be represented as a single channel in OH, for example as a text field with some defined format, e.g.

47E9EE2B: "[22, 18, 5, 5]"

where all these numbers are actually settings/fields that are sent to device once user pushed enter (or a rule changed its state)

BTW, is there any characteristic that can be used to fetch these temperature settings?

vkolotov commented 6 years ago

Right... that reminded me that I actually worked on it a while ago, the idea was to come up with a new "composite" field that may hold a set of other fields. So that this composite field can be associated with an OH channel directly, effectively and OH channel can change it and the logic of parsing it would be in that composite field itself:

https://github.com/sputnikdev/bluetooth-gatt-parser/blob/composite-fields/src/main/java/org/sputnikdev/bluetooth/gattparser/CompositeFieldHolder.java

A good example could be a colour picker, e.g. RGB numbers. Or a date-time composite field (https://github.com/sputnikdev/bluetooth-gatt-parser/blob/composite-fields/src/main/java/org/sputnikdev/bluetooth/gattparser/fields/LocalDateTimeFieldHolder.java)

xrucka commented 6 years ago

... if you try to change a state, this does not mean that device is able to change it (or will change it)

agreed;

BTW, is there any characteristic that can be used to fetch these temperature settings?

The characteristic used to read the temperature settings is exactly the same, device keeps overwriting the current temperature field, so it does not realy matter what's written in there. The open-window sensitivity is somewhat different, it seems that any value other than enumerated is ignored.

Right... that reminded me that I actually worked on it a while ago, the idea was to come up with a new "composite" field that may hold a set of other fields. So that this composite field can be associated with an OH channel directly, effectively and OH channel can change it and the logic of parsing it would be in that composite field itself

Ok, I'll try to fetch and port on the composite fields (did that back then when dealing with temperature automation programming). However, I'm not sure I follow the ideas behind. So to, to clarify: 1) "Primitive"FieldHolder holds single field, and it's value. CompositeFieldHolder holds multiple fields and their values. 2) CompositeFieldHolder is associated with (probably string) channel. It takes care for formatting and parsing the information within. This sounds similar to functionality currently provided by my program characteristic parser 3) Who provides the composite field? Characteristic parser, or spec reader? 4) Are there "primitive" channels as well? Such that their value would get propagated to the "composite" channel (i.e. manual channel sync) and backwards?

What about overriding the functionality of CharacteristicHandler? For what I can see, it has access to both the openhab channels, device governor and characteristic parsers. Implementing such a twisted functionality here seems to me a plausible option. Although, this would not be a generic solution, as each such device, broken by design, would have to provide it's handlers, overriding the default ones.

vkolotov commented 6 years ago

Hi @xrucka, let's consider more obvious example: [date time characteristic] (https://github.com/sputnikdev/bluetooth-gatt-parser/blob/composite-fields/src/main/resources/gatt/characteristic/org.bluetooth.characteristic.date_time.xml).

The problem: come up with a solution that would allow OH binding to write date time characteristic in atomic manner (all fields at once).

In other words, we need to wire up OH DateTime channel (date-time picker on UI) with org.bluetooth.characteristic.date_time characteristic, e.g.:

DateTime channel -> Date object in java -> List of date-time parts (year, month, day, hours etc) -> GattRequest -> raw data

The idea was to create a new object in gatt parser that could reflect org.bluetooth.characteristic.date_time structure and could effectively convert Date java object into multiple Gatt fields (getValue and setValue (here)[https://github.com/sputnikdev/bluetooth-gatt-parser/blob/c58d3524170a5695771873877cc6008275406aa2/src/main/java/org/sputnikdev/bluetooth/gattparser/CompositeFieldHolder.java#L51]) and https://github.com/sputnikdev/bluetooth-gatt-parser/blob/composite-fields/src/main/java/org/sputnikdev/bluetooth/gattparser/fields/LocalDateTimeFieldHolder.java.

Usage of this new object would look like that: https://github.com/sputnikdev/bluetooth-gatt-parser/blob/c58d3524170a5695771873877cc6008275406aa2/src/test/java/org/sputnikdev/bluetooth/gattparser/GenericCharacteristicParserIntegrationTest.java#L126

Now, when we have a special object that represents a set of fields, we could wire it up in the binding. This would be two places to change:

Similarly in your case, we would have something like ThermostatSettingHolder composite field that would extend CompositeHolder like LocalDateTimeFieldHolder does. E.g. something like that:

public class ThermostatSettingHolder extends CompositeFieldHolder<String> {

 @Override
    public String getValue() {
        Map<String, PrimitiveFieldHolder> components = getPrimitives();
        // get temperature settings and format them into a string, e.g. "[22, 25, 6 ... ]"
        return result;
    }

    @Override
    public void setValue(String value) {
        // parse the argument ("[22, 25, 6 ... ]") and set primitive fields
    } 
}

The difference here is that OH does not support "temperature configs", so we could use just textual channel to represent a list of configs, e.g. "[22, 25, 6 ... ]"

I hope this answers some of your questions, let me know what you think?

xrucka commented 6 years ago

I hope this answers some of your questions, let me know what you think?

Yep, it does. And I believe that we're mixing two ideas/properties.

The first property is semantics. The idea of date_time characteristic is, that it represents a single logical entity - time (precise up to minutes/seconds/...). Although we are used to decompose date into individual components, it's essentialy a single value (just like unix timestamp). Further, if there are multiple such entries in the characteristic, e.g. log of last n times the device has rebooted. This functionality is what you would expect from (perhaps higher-level) characteristic parser, providing logical fields holding corresponding type instead of list of low-level fields.

The other property is atomicity (with respect to characteristic write), for when the characteristic contains multiple logical fields. My approach can, of course, be biased, but for me, bluetooth-manager provides a way to exchange data in similar matter TCP/IP network stack does. I have data I want to exchange with the device. The stack handles the connections, data interchange, etc. The application protocols built over the stack then deal with transactions, write atomicity, etc. This leads me to believe that parser should not be responsible for write coordination, nor contents written. Contents should be generated and locked ready in channel handler, resp. CharacteristicHandler.

Summarizing my approach: 1) Channels should be isolated from characteristics, although 1:1 (1 channel = 1 field = 1 characteristic) or k:1 (k channels, k fields, 1 characteristic) mappings are not forbidden in any way. 2) Channels interacting with the same characteristic should hold the same handler (as it is now with CharacteristicHandler). 3) If I need to update multiple channels and comance only single write to the characteristic, than this should be handled solely in CharacteristicHandler.

Given that, I can actually imagine, that CharacteristicHandler would provide k+1 channels (k logical fields provided by parser + 1 virtual channel); where only explicit updates of the virtual channel trigger the characteristic write. Of course, such virtual channel could be the list of values of individual logical fields, as well as "flush button". Furthermore, the "flush button" idea can be easily extended to multiple characteristics, sharing single flush button, hopefully dealing with such broken-by-design devices once and for all. And to mitigate the obstructions for not-as-broken devices, I can even imagine that the explicit flush channel functionality would have to be enabled in thing config, similarly to the choice of connection strategy or authentication model.

vkolotov commented 6 years ago

Hi @xrucka, thank you four your thoughtful response. I really appreciate this.

The idea of date_time characteristic is, that it represents a single logical entity - time (precise up to minutes/seconds/...).

I see you point and mainly agree (If I were a gatt spec designer, I'd have a single epoch timestamp probably). However, date_time gatt pecs split date on several parts, they could have said that date_time is an epoch timestamp, but they explicitly say that the characteristic is a set of fields that must be set/read separately, this is what we have to deal with.

The other property is atomicity This leads me to believe that parser should not be responsible for write coordination, nor contents written.

I see your point. However, gatt spec has some attributes that say some fields are mandatory, meaning that some fields must be written in the same time, otherwise the operation is not valid. So Gatt parser (not BT manager) has to provide some mechanisms that would allow user to validate gatt request / or reject invalid requests.

And you are correct in your analogy in TCP/IP network. This is what BluetoothManager and Transport modules do. They don't cate about what data is written, they just handle whatever gets passed. But they are different modules to Gatt parser, this is important to note.

Anyway, I do really see your point saying that the logic to work with multiple fields for write operation could be moved on a higher level, e.g CharacteristicHandler etc. This would be even easier to implement I believe. However, the idea of this project (BT manager + Gatt parser) is to provide a generic mechanism for other projects to work with all dodgy BT API :)

I like your idea to create a "fake" channel that would span all other gatt fields. This should work fine, I agree. I have not thought about that. However as I mentioned earlier we need to make a decision on where this logic should go, e.g. into a higher level layer (binding) or into gat parser.

I'd have thought that you would like the example of the CompositeField usage, but obviously you have your vision of things. What disadvantages do you think that approach has?

        GattResponse response = parser.parse("2A2B",
                new byte[] {(byte) 2017, 2017 >> 8, 1, 4, 11, 38, 45, 3, 1, 2});
        assertEquals(9, response.getSize());
        assertEquals(2017, (int) response.get("Year").getInteger(null));
        assertEquals(1, (int) response.get("Month").getInteger(null));
        assertEquals(4, (int) response.get("Day").getInteger(null));
        assertEquals(11, (int) response.get("Hours").getInteger(null));
        assertEquals(38, (int) response.get("Minutes").getInteger(null));
        assertEquals(45, (int) response.get("Seconds").getInteger(null));
        assertEquals(3, (int) response.get("Day of Week").getInteger(null));
        assertEquals(1, (int) response.get("Fractions256").getInteger(null));
        assertEquals(2, (int) response.get("Adjust Reason").getInteger(null));

        assertEquals(LocalDateTime.of(2017, 1, 4, 11, 38, 45),
                response.getCompositeHolder("Exact Time 256").getValue());
xrucka commented 6 years ago

I see your point. However, gatt spec has some attributes that say some fields are mandatory, meaning that some fields must be written in the same time, otherwise the operation is not valid. So Gatt parser (not BT manager) has to provide some mechanisms that would allow user to validate gatt request / or reject invalid requests.

Agreed that having all fields optional here (temp setting characteristic) would be more straightforward, however - how would you format the characteristic then? All I can think of is to deal with this on parser level, that should serialize the gatt request fields with corresponding placeholder. However, I was not able to get this approach working, as optional fields were not recognized (don't have the log anymore).

Anyway, I do really see your point saying that the logic to work with multiple fields for write operation could be moved on a higher level, e.g CharacteristicHandler etc. This would be even easier to implement I believe. However, the idea of this project (BT manager + Gatt parser) is to provide a generic mechanism for other projects to work with all dodgy BT API :)

I've tried to modify the bluetooth binding to support channel handler overrides, it required few changes in api I do not really like (exposing some internals as public because of subclassing by my classes in different package). I believe that this can be broken down into two kinds of characteristic channel handlers - simple (current) and cumulative, that internally remembers the other fields and basically allows the mandatory fields. Thing config would then explicitly mark characteristics handled by the cumulative handler.

I like your idea to create a "fake" channel that would span all other gatt fields.

However, in order to pick such problematic characteristic, handled either with fake flush channel, or with cumulative handler, would still require explicit marking in config, if they were not to complicate simple usage.

What disadvantages do you think that approach has?

I wouldn't say disadvantages, in my mental model, it simply does something different from the usecase I need. Your approach covers the idea of single logical value, broken down (who-knows-why) into several fields. Thus, the update of the logical value updates the values covered within, thus ensures that all mandatory fields covered by this value are updated simultaneously. It's a good approach for situations like comet's automated mode (per-day temperature programs).

The situation above is different, as the characteristic contains several unrelated values (3x temperature setting, 1x current temperature, 2x open-window detection settings). Furthermore, using string representation of such characteristic would easily confuse user, as he/she would easily get lost in semantics of individual fields if represented as array, and dictionary-like representation would be too verbose. [ 8, 17, 13, 10, 10, 4] - i simply cannot read this clear. As user, I would like to see channel for reporting current temperature, another for setting temperatures, another for open-window timeout, etc.; each with clear semantics. And if there is extra "flush" operation, I don't mind that. And I don't really care whether there is flush for entire device, or just for characteristic. It simply fits the prepare + commit model. I don't see any other level to implement the prepare + flush other than handler level.

Benjalien commented 4 years ago

vkolotov, xrucka, sorry to wake up an old thread, but did you finally came to a "noob-proof" solution? BR Benjamin

OSevangelist commented 3 years ago

vkolotov, xrucka, sorry to wake up an old thread, but did you finally came to a "noob-proof" solution? BR Benjamin

@vkolotov @xrucka would be interested in that as well is there any solution to get these devices (i.e. cometblue) working with OpenHAB at least a bit ? From your communication i didn't understand any viable solution to make it work so far. Am i right ?

aut0 commented 3 years ago

Would it be okay to setup bountysource for this?

vkolotov commented 3 years ago

Unfortunately there is a very little interest in this project (or maybe in the entire OpenHab?) from public. Not sure if this makes sense.