[Pilight switch] Allow arbitrary command scheme

[DavidLP]

Description:
There was an attempt to do command scheme validation with voloptuous. But since the command
scheme is arbitrary and pilight does not provide an interface (yet) to get allowed commands anything should be allowed.

Related issue (if applicable): fixes #5119

Example entry for configuration.yaml (if applicable):

pilight:
  host: 127.0.0.1
  port: 5000
switch:
- platform: pilight
  switches:
    light:
      on_code:
        protocol: intertechno_old
        unit: 3
        id: 4
        'on': 1
      off_code:
        protocol: intertechno_old
        unit: 3
        id: 4
        'off': 1
      on_code_receive:
        protocol: daycom
        state: 'off'
      off_code_receive:
        protocol: daycom
        state: 'on'

Checklist:

If the code does not interact with devices:

• Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  Tests have been added to verify that the new code works More like: test have been done with 3 different installations + hardware to be able to claim that this code works better.

Remark:
Since this could break something I am not aware of (for me it works) I would delay this merge until I get feedback in the forum.

[mention-bot]

@DavidLP, thanks for your PR! By analyzing the history of the files in this pull request, we identified @janLo, @fabaff and @balloob to be potential reviewers.

[houndci-bot]

'CONF_PROTOCOL' imported but unused

[DavidLP]

This fix seems to work and seems not to break pilight switch configs. @janLo Maybe you have time to check with your more fancy config, since you use templates etc.?

[balloob]

Probably want to use vol.Coerce(str) for key, that way you convert it to a string if it is not.

[DavidLP]

That should be fine, because numbers as a protocol key do not exist. Think of it as a python dictionary with string keys and string/int/float values.

[janLo]

I'm not convinced to that. Instead of providing a proper implementation to the API we decide to revert it and give the responsibility to implementing the pilight API correctly to the user I their configurations. And all because we fear, that pilight might change its API somehow in the future.

That's if we gust let the user put its own HTTP requests into the configuration for weather components because we fear that the service might change its API in the future. Of course, it's more flexible but the we should not call it an API implementation.

[DavidLP]

Instead of providing a proper implementation to the API

A proper implementation to the API is impossible without reimplementing it (the protocol info is not available by the c-library of pilight). And that would be not proper in my terms.

responsibility to implementing the pilight API correctly to the user I their configurations

It is only about Is the value a string or number? Yes the user has to chose the correct type.

And all because we fear, that pilight might change its API somehow in the future

That is only one point I mentioned. Additionally it is error prone and self defined protocols do not work. And it requires maintenance neither me nor you are able to provide. Is it clear to you e.g. that to fix #5119 with your proposed feature Protocol validation at startup you have to make a voloptuous rule with ALL pilight protocolls? Why don't you get that this is insane to do?

it's more flexible but the we should not call it an API implementation

You gave it this name. For me it just a generic way to use all pilight supported 433 MHz devices.

[balloob]

@DavidLP although pilight supports arbitrary protocols, if we know that a unit, unit code or id are always positive ints, a schema could proof useful. The schema is defined with extra=vol.ALLOW_EXTRA so any other keys can still be passed through.

[balloob]

We started adding voluptuous schemas all over Home Assistant to help guide our user. Either by coercing their passed in data into the correct values or being able to give them sensible feedback. This has helped the user experience tremendously.

[DavidLP]

@balloob

if we know that a unit, unit code or id are always positive ints

We do not really know that. For example this was wrongly assumed for id and implemented bug #5119. We know that every protocol has an id but already for this essential key we cannot define a type. Sometimes it is a string sometimes a number. If that is impossible already, I prefer not to do any checks at all.

We started adding voluptuous schemas all over Home Assistant to help guide our user. Either by coercing their passed in data into the correct values or being able to give them sensible feedback. This has helped the user experience tremendously.

I agree. But this is only possible for backends that either expose their API (pilight does not yet) or that are so simple that you can define the type for all possible keys (more than 100 keys in more than 60 different combinations for pilight). What I feared (and happened) is that we assume to know something about a key, are proven wrong at some time, and have to do bug fix after bug fix. I cannot provide this and if I weigh less feedback against possible bugs I go for the first in terms of user experience. Also keep in mind that pilight users might implement their own protocol and that this might be rejected by home assistant.

Since you are the maintainer I leave you two choises :-D:

1. No protocol validation as proposed here
2. Workaround Pilight Switch rejects alphanumeric IDs #5119 in this PR by also allowing string in id. Maybe even merge other PRs that go down this road. See issues piling up ;-)

[balloob]

In the case of id, if we coerce it to a string, would protocols expecting an integer still work?

[DavidLP]

Good idea, but unfortunately it does not work. I checked.

[DavidLP]

The only solution right now for protocol validation at hass start-up is to reverse engineer all protocols c-code and create voloptuous schemes from this. I wrote a protocol parser that is capable of doing this and it seems to work fine. But implementation into hass will take me some time and needs proper testing, and options to deactivate protocol validation and to set the protocol level (debug, release x).

[janLo]

I do not recommend parsing the c code. This is where you really go to a maintainance hell. Have you had a look at this approach: DavidLP/pilight#3 ?

[DavidLP]

I just saw. Yes let's do it this way. I will review you PR.