Add DP suggestions to switch platform

[postlund]

Basic framework for suggesting datapoints for options in config flow. Currently very basic as it only supports a list of probable datapoints, but we can extend that with for instance regexp later.

Already used datapoints are currently not respected (I.e.ignored in any way). I'm not sure what would be the best way to do that, as you might want to use the same datapoint in several entities (e.g. current consumption as both a switch attribute and a sensor). One idea is to apply this specifically to the ID, so we at least don't suggest an ID that is already used by another entity. One case for this is a double gang socket, where we should suggest DP 2 for the second switch we add. But that's minor future improvement as well.

This feature is opt-in by platforms and only switch implement it so far. Basically define DP_SUGGESTIONS and that is all. See switch.py for an example.

Relates #93

[ultratoto14]

Hi @postlund i adapted light.py (because i just have lights) with the following code:

from homeassistant.const import CONF_ID, CONF_BRIGHTNESS, CONF_COLOR_TEMP
....
DP_SUGGESTIONS = {
    CONF_ID: [1, 20],
    CONF_COLOR_MODE: [2, 21],
    CONF_BRIGHTNESS: [3, 22],
    CONF_COLOR_TEMP: [4, 23],
    CONF_COLOR: [5, 24],
}

The good point:
The config flow is perfectly filled with the desired DPs, well done 👍
The bad point:
having some of my bulbs defined in the yaml, applying this patch raised an config check error:

2020-10-31 18:26:47 ERROR (MainThread) [homeassistant.config] Invalid config for [localtuya]: extra keys not allowed @ data['localtuya'][0]['entities'][0]['brightness']. Got 22
extra keys not allowed @ data['localtuya'][0]['entities'][0]['brightness_upper']. Got 1000
extra keys not allowed @ data['localtuya'][0]['entities'][0]['color']. Got 24
extra keys not allowed @ data['localtuya'][0]['entities'][0]['color_mode']. Got 21
extra keys not allowed @ data['localtuya'][0]['entities'][0]['color_temp']. Got 23
extra keys not allowed @ data['localtuya'][0]['entities'][0]['color_temp_max_kelvin']. Got 6500
extra keys not allowed @ data['localtuya'][0]['entities'][0]['color_temp_min_kelvin']. Got 2700
value is not allowed for dictionary value @ data['localtuya'][0]['entities'][0]['id']. Got 20
value is not allowed for dictionary value @ data['localtuya'][0]['entities'][0]['platform']. Got 'light'. (See ?, line ?)

[postlund]

Ah, yes, right... there's some code sharing going on here and suggestions should not be done for YAML validation. Will fix that.

[postlund]

@ultratoto14 Pushed an update now which should fix the problem, please give it a spin!

[ultratoto14]

@postlund unfortunately, still the same error.

[postlund]

@ultratoto14 What about now?

[ultratoto14]

@postlund yes !! Everything is working fine now 👍 config_flow + yaml

If you want you can add the ones for the lights there

[postlund]

@ultratoto14 Added now, thanks for testing and providing values 👍

[ultratoto14]

@postlund tested your last one and everything is Ok

[postlund]

@rospogrigio I think this is ready now. Additional suggestions can be added in other PRs (feel free to do so for other platforms if you have something useful to add).

[postlund]

I added support for not suggesting IDs already in use by other entities as well.

[ultratoto14]

@postlund just tested your last commit a switch then a light, the ID dp used in switch is not selected by default in the light 👍

A remark: as the value is selected by default, we do not have the ability to remove it, i mean, i added a RGBW light, the dps are well detected, i wanted to remove the color configuration and make my light white only, but i cannot remove the selected one as the field is already filled and the combo does not contain a "None" entry. Not sure i'm clear, tell me if it's not the case.

[postlund]

@rospogrigio Right, that's a particular use case I didn't think of. We could add a check box in the dialog where entity type is selected to allow opt-out of automatic suggestions as a work-around?

Ultimately we should support unsetting values, but it's a bit tricky so I have not prioritized that right now. One problem is that in a lot of places (which I believe I have mentioned before) we don't treat optional settings as optional, e.g. when a default value is specified we assume the value to always be set so no need to check if it's not. If we allow unsettling values, there's potentially a lot of issues that might happen. We should fix that though.

[rospogrigio]

Sorry @postlund and others, I am quite in the middle of a shitstorm (my daughter's nursery is closed due to 2 Covid-positive teachers and we are currently quarantined) so I'm having difficulties in testing and reviewing... hopefully the next days will be better, I'll review as soon as I can. Also @postlund I'll need your help with #111 because I have something weird going on, maybe the trout can help too 😄 . By the way, any news about hacs/default#662 ? Will get back to you soon, bye!

[rospogrigio]

@postlund I get the same problem as with #124 (discovery doesn't work).
Also, I wonder whether this approach would conflict with the future idea of building a sort of preset database for the devices... you'll tell me because you have a clearer idea of the coding.

_Edit: sorry, my fault, my main HA instance was still running, even if I had stopped it Will test it again later. _

[postlund]

Shouldn't be any problem, I think we can fix that anyway in a good way.

[postlund]

And oh, no pressure @rospogrigio, family goes first. Take care!

(No news regarding HACS PR, but I saw someone asking for a review and the answer was in all-caps, so we shouldn't do that)

[postlund]

@rospogrigio I added a check box so you can opt-out from suggestions.

[ultratoto14]

Hi @postlund, i see more and more issues that may be fixed once this one is merged.
What do you need to go further on this one ?
I can do tests if needed.

I just bought some sockets with power consumption and I also have multiple kind of lights (as you already know 😄 )

[postlund]

@ultratoto14 Nothing more on my end, we just need to wait for the situation to ease up a bit so @rospogrigio can review 😊

[rospogrigio]

Hi @postlund , I'm back and I tried this PR with my 2-gang switch, however I have 2 issues:

1. there is no way to un-set the DPs for voltage/current as attributes, which would be needed if one wants to see them using sensors instead of attributes; also, in this way you get all the 3 attributes for each gang
2. I cannot create the integration: it says "success" but cannot associate it to a Room, and also it does not have devices or entities associated (see pic). And no errors in the logs...
   Let me know how I can help you debug, bye...

[postlund]

@rospogrigio Welcome back 🎩

1. I think we touched that before. You can either get all fields filled with suggestions or tick the box and get nothing. But as unsetting options is not supported, you can't kinda get half of it. In reality, it doesn't matter that you have those attributes filled in because you can still set up sensors. Unique IDs contains the platform name by default, so you can create a switch and binary_sensor using the same ID for instance.
2. Normally that is what happens when the connection fails after adding the device. You should enable debug logs and see if tries to connect at all.

[rospogrigio]

1. ok, got it
2. ok will retry tomorrow

PS, what about hacs/default#662 ? I really don't want to be pushy but it's nearly one month since I created the PR... is there any way we can ask for a review?

[postlund]

Yeah, the HACS issue is really taking a lot of time. There was a race a couple of days ago, lots of PRs were merged but not yours for some reason. Not sure what to do.

[rospogrigio]

I really don't know, I see that there are also other PRs slightly older than ours waiting for merge but would really know why this is taking so long... let's give them a couple of more days and then let's try to ask for a review and keep fingers crossed.

[ultratoto14]

In light DP_SUGGESTIONS, you can add
CONF_SCENE: [6, 25],

24 is usually used for color value and 25 for scene but one user reported that on his light, there's no scene support and color is on 25.

@postlund, @rospogrigio I initiated a wiki page for known working devices if you want to add yours Known Devices

[lishan89uc]

Hi guys I am new here but this addon has made my life SO MUCH BETTER! that I want to chip in. I am using:

Feit Dimmer (from Costco)
Product ID: tebc75erlgslgpn5
Main Module: V3.1.2
MCU: V1.0.8
ID: 1
Brightness: 2
Minimum Brightness: 3

Feit Electric Wi-Fi Smart Plug (from costco)
Product ID: ezrzkbpajujvaoa4Type
Main Module: V1.0.7
MCU: V1.0.7
ID: 1

Both work Super well also local tuya fixed issue with home assistant tuya integration where if I turn off the light with a classic 3 way switch the light would stay "On" on Home Assistant. That said the instructions for this integration needs some work. Maybe even including some pictures. I will be glad to help with refining the instructions. However I am new to this. So I am not sure how I would go about doing that.

[postlund]

I will continue working on this PR as my next task (or rather, I already have). To allow more flexibility, to support the rather complex light platform for instance, I intend to change DP_SUGGESTIONS into a method instead so each platform programmatically decide which datapoints to suggest. It will look something like this (current draft):

def suggest(dps, used_dps):
    ...

The dps argument contains a dict, mapping integers to objects. The used_dps contains a set with integers of all datapoints already used by other entities.

Expected return value is a dict with options, mapping to datapoints to use. Something like this:

def suggest(dps, used_dps):
    return {
      CONF_ID: 1,
      CONF_FOOBAR: 20,
    }

I'll see if this hold and hopefully have something ready by tonight or tomorrow. Will only focus on the switch platform now and take other platforms in separate PRs.

[ultratoto14]

@postlund, I like the id of already used dps, do you think we can also use the product key with that ? I mean another device of the same kind is already configured with the same product key, then suggest the same dps ?

[postlund]

@ultratoto14 I guess that it would be possible to some extent. It might be a future improvement, but I won't consider it now. Once we have the database, we can populate that locally with devices that have been manually set up and treat them as part of the database. That way we get a generic implementation.