Skip to content
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

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

Closed
onkelandy opened this issue Oct 7, 2018 · 20 comments
Closed

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

onkelandy opened this issue Oct 7, 2018 · 20 comments
Labels
plugins Issue relates to a plugin

Comments

@onkelandy
Copy link
Member

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
Copy link
Member

ohinckel commented Oct 7, 2018

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member

msinn commented Nov 15, 2018

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
Copy link
Member

msinn commented Nov 15, 2018

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
Copy link
Member Author

I would totally vote for the second approach.

@bmxp
Copy link
Member

bmxp commented Nov 16, 2018

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
Copy link
Contributor

smaiLee commented Nov 16, 2018

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
Copy link
Member

msinn commented Nov 16, 2018

@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
Copy link
Member

msinn commented Nov 16, 2018

@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
Copy link
Member

msinn commented Nov 16, 2018

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
Copy link
Contributor

smaiLee commented Nov 16, 2018

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 😄

EDIT: You were faster than me 😃

@msinn
Copy link
Member

msinn commented Nov 16, 2018

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
Copy link
Contributor

smaiLee commented Nov 16, 2018

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
Copy link
Member

bmxp commented Nov 17, 2018

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
Copy link
Contributor

smaiLee commented Nov 17, 2018

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
Copy link
Member

msinn commented Nov 17, 2018

@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 #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
Copy link
Member

msinn commented Nov 17, 2018

@smaiLee

BTW: Is the triggering working for lists?

Since beginning of September (lists are complex datatypes too)

@msinn msinn added the plugins Issue relates to a plugin label Mar 26, 2019
@bmxp
Copy link
Member

bmxp commented Apr 19, 2019

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

@msinn
Copy link
Member

msinn commented Jan 30, 2022

This issue has been moved to the plugin repo: smarthomeNG/plugins#565

@msinn msinn closed this as completed Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins Issue relates to a plugin
Projects
None yet
Development

No branches or pull requests

5 participants