smarthomeNG / smarthome

Device integration platform for your smart home
https://www.smarthomeNG.de
GNU General Public License v3.0
123 stars 92 forks source link

KNX Plugin improvement for DPT 2 (num instead of list) #291

Closed onkelandy closed 2 years ago

onkelandy commented 6 years ago

To easily change DPT 2 knx entries via VISU it would be more comfortable to set priorities not only as lists but also as a number. The ETS converts simple numbers to lists as follows: 0 = [0,0] 1 = [0,1] 2 = [1,0] 3 = [1,1]

Such an internal conversion for DPT2 items would be very nice or are there any reasons this is a bad idea?

ohinckel commented 6 years ago

If we want to change this, we should also change this for DPT 3

[...]
|  2          |  2 bit        |  list    |  [0, 0] - [1, 1]
|  3          |  4 bit        |  list    |  [0, 0] - [1, 7]
[...]

(from https://www.smarthomeng.de/user/plugins/knx/README.html)

Should be simple to implement: I would just check the given value's type and in case it's not a list try to convert it to an int and use value directly.

onkelandy commented 6 years ago

Problem might occur when receiving the value from the knx device. Maybe there is a scenario where someone wants to have the response as a list? So I guess there needs be an option in the config?

onkelandy commented 6 years ago

I had a closer look at the dpt handling. This could somehow solve the problem for DPT2:

def en2(payload):
    # control, value
    if isinstance(payload, list):
        return [(payload[0] << 1) & 0x02 | payload[1] & 0x01]
    else:
        return [int(payload)]

Although the decoding is contra-productive as it converts an int to a list again on receiving a value:

def de2(payload):
    if len(payload) != 1:
        return None
    return [payload[0] >> 1 & 0x01, payload[0] & 0x01]

So I guess the better solution might be to check the type of the item in the parse_item function and maybe internally change the dpt to a "subtype" like 2001 with the corresponding functions in dpts.py

msinn commented 5 years ago

maybe internally change the dpt to a "subtype" like 2001 with the corresponding functions in dpts.py

There are defined subtypes for dpt 2 in the KNX specification (2.001 - 2.012), so we should take care not to create conflicts. I am going to look deeper into the KNX specification.

Same goes for dpt3. Defined subtypes are 3.007 and 3.008

capto_capture 2018-11-15_09-25-16_am

msinn commented 5 years ago

dpt 2.003 to 2.012 are only for KNX internal use (function blocks) only dpt 2.001 and 2.002 are for general use.

We could do something like this:

knx_dpt: 2       # return a list (backward compatibility)
knx_dpt: 2.001   # DPT_Switch_Control: return an integer
knx_dpt: 2.002   # DPT_Bool_Control: return a list (of bool)

Another approach could be to use the casting functionality of items:

item1:
    type: list
    knx_dpt: 2

item2:
    type: num
    knx_dpt: 2

item1 would receive a list and item2 would receive a number.

This casting functionality is already used in the mqtt plugin. Since the MQTT payload is an array of byte, the content gets interpreted using the type-defiinition of the item.

onkelandy commented 5 years ago

I would totally vote for the second approach.

bmxp commented 5 years ago

I would prefer the first approach. As long as the knx dpt are just a single int or so it's not a problem but what is with composed datatypes like 6.5.1 DPT_DALI_Control_Gear_Diagnostics?

None of above conversion method seems appropriate. One might take a dict here to store the components but then it's not a real item any more. Any ideas for those cases?

smaiLee commented 5 years ago

I think the first approach is simpler to manage and thus less fault-prone. Otherwise you have a redundant information as you need to change type as well as dpt to switch the behavior.

DPT 3.007 and 3.008 could be represented as negative and positive integers if item is of type num.

@bmxp I don't share your point for two reasons:

  1. Not every dpt has to be convertible to any item type. But if there is a reasonable conversion, why don't do it?
  2. This issue is about dpt 2 (and secondarily dpt 3). For other dpt it could still be needful to define subtypes (like 5.001 for percentages).

BTW: Why is an item of type dict not a real item? And to avoid misunderstandings: You mean dpt 237.600, 6.5.1 is the chapter number in the sepecification.

The mention of a dict leads to another option: If an item with dpt 2 has type dict, it could be filled by something like { "control": Bit1, "value" Bit2 } Or for dpt 3 { "direction": Bit1, "value" Bit2-4 }

msinn commented 5 years ago

@smaiLee

Otherwise you have a redundant information as you need to change type as well as dpt to switch the behavior.

That assumtion is wrong. In variant 1 you have to change the knx_dpt (from 2 to 2.001). In the second variant you have to change the type (form list to num).

Why is an item of type dict not a real item?

Why do you think so? It is!

msinn commented 5 years ago

@bmxp

None of above conversion method seems appropriate.

I don't understand what you mean.

One might take a dict here to store the components but then it's not a real item any more.

Why? If you define the Item to be of type dict it is still a real item.

msinn commented 5 years ago

I was wrong above. For the first approach you have to change type and knx_dpt!

Another problem with the first approach is, that when using e.g. 2.001 to convert to a num value, you use one definition of "two bits" to convert to a list of bits and another definition which from the knx perspective is "two bits" too, and convert it to a num.

For second approach it could be said like "I know that it is a "two bit" value, but I want to interpret it as number (or to interpret it as list). We could even think about a third conversion (to a dict) like this:

item1:
    type: list
    knx_dpt: 2
    # result e.g.: [1,1]

item2:
    type: num
    knx_dpt: 2
    # result e.g.: 3

item3:
    type: dict
    knx_dpt: 2
    # result e.g.: { "control": 1, "value": 1 }
    #   or: { "control": True, "value": True }
smaiLee commented 5 years ago

Otherwise you have a redundant information as you need to change type as well as dpt to switch the behavior.

That assumtion is wrong. In variant 1 you have to change the knx_dpt (from 2 to 2.001). In the second variant you have to change the type (form list to num).

I'm not sure if I misunderstand something, but in variant 1 you have to change the knx_dpt and the item type, don't you?

Why is an item of type dict not a real item?

Why do you think so? It is!

@bmxp wrote this :smile:

EDIT: You were faster than me :smiley:

msinn commented 5 years ago

I'm not sure if I misunderstand something, but in variant 1 you have to change the knx_dpt and the item type, don't you?

Yes, but you wrote that for the second variant you would have to change both.

smaiLee commented 5 years ago

Yes, but you wrote that for the second variant you would have to change both.

Sorry, that was a misunderstanding because of the ambiguity of the word "otherwise". I did not mean "on the other hand" but "in other ways" (means here: in approach 2).

bmxp commented 5 years ago

The point with item type dict is that changes within the dict won't be detected as such.

a = sh.myItem() # a will have the dict of MyItem
a['Bit5': 1]

currently this will not be detected by SHNG, thus the new value won't trigger anything, not cached or written somewhere. I think this might be a problem. Otherwise I am fine with dict and would favour that.

You can use sh.myItem(a) then of course to set. But it is hard to do this in eval syntax. And we should keep in mind that many people using SHNG don't have a programming background.

Beautiful is better than ugly. Explicit is better than implicit. Simple is better than complex. Complex is better than complicated. Flat is better than nested. Sparse is better than dense. Readability counts.

smaiLee commented 5 years ago

currently this will not be detected by SHNG, thus the new value won't trigger anything, not cached or written somewhere.

But that's a problem which has to be solved anyhow as the type dict is already implemented and documented.

BTW: Is the triggering working for lists?

msinn commented 5 years ago

@bmxp

The point with item type dict is that changes within the dict won't be detected as such.

This is true in Python for all complex datatypes. And for shNG it is not true any more. A couple of weeks ago this changed. Since beginning of September complex datatypes are handles via deepcopy see https://github.com/smarthomeNG/smarthome/issues/274.

keep in mind that many people using SHNG don't have a programming background.

Because of that, I think approach 2 is easier to understand.

msinn commented 5 years ago

@smaiLee

BTW: Is the triggering working for lists?

Since beginning of September (lists are complex datatypes too)

bmxp commented 5 years ago

IMHO we should have lists for complex datatypes. If we needed dicts, we always need to have a key as well.

msinn commented 2 years ago

This issue has been moved to the plugin repo: https://github.com/smarthomeNG/plugins/issues/565