-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Smart Radiator Thermostat - comet-blue-eurotronic #14
Comments
Hi @matteopedani , it looks similar to eQ-3 radiator thermostat, is it the same device? |
I don't think so. May be. I don't have eQ-3 |
Looks like it is not the same device. Here is a python project with some good details about its characteristics: What you will need to do is:
It will give you a new experimental feature that can discover all characteristics (including "unknown" characteristics). To enable this feature:
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. 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. |
@bademux @vkolotov |
To find the right ID ############## [termosifone cucina piano terra]# devices |
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 |
@matteopedani, looks like these ones are the most interesting:
|
Thank's I will work on these.
Matteo
2018-01-16 11:42 GMT+01:00 Vlad Kolotov <[email protected]>:
… @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
<#14 (comment)>,
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
|
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? |
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: 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: #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. |
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. |
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:
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:
and
Notice C1/C2 tag. |
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. |
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... |
Thanks for your update @xrucka.
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? |
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. |
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. |
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. |
Just to sum up options we have in my mind to resolve your issue with the "magic number":
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. |
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. |
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. |
Hi @xrucka,
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.
Can you please elaborate this more? Are you saying that a single characteristics is used to represent several data points? |
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. |
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:
@vkolotov what would you suggest? |
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. 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. |
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:
This, however, means there would have to be some kind of semantics parsed out from the characteristic specification, perhaps based on characteristic name? |
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] 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? |
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 |
@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. 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 |
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. |
Here we go, I still have a spreadsheet with some analysis for that thermostat "clone": as you may see, read and write structures (fields) are different which makes it very hard to come up with a gatt spec. |
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? |
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: 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) |
agreed;
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.
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:
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. |
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
The idea was to create a new object in gatt parser that could reflect 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 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? |
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:
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. |
Hi @xrucka, thank you four your thoughtful response. I really appreciate this.
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.
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 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 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()); |
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).
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.
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.
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. |
vkolotov, xrucka, |
@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 ? |
Would it be okay to setup bountysource for this? |
Unfortunately there is a very little interest in this project (or maybe in the entire OpenHab?) from public. Not sure if this makes sense. |
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)
The text was updated successfully, but these errors were encountered: