Mqtt light refactor

[emontnemery]

Description:

Refactor of MQTT light as discussed in #18180 to add support for config entry to mqtt_json and mqtt_template

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7601

Example entry for configuration.yaml (if applicable):

Before change (mqtt_json):

light:
  - platform: mqtt_json
    command_topic: "home/rgb1/set"

After change (mqtt_json):

light:
  - platform: mqtt
    schema: json
    command_topic: "home/rgb1/set"

Before change (mqtt_template):

light:
  - platform: mqtt_template
    command_topic: "home/rgb1/set"
    command_on_template: "on"
    command_off_template: "off"

After change (mqtt_template):

light:
  - platform: mqtt
    schema: template
    command_topic: "home/rgb1/set"
    command_on_template: "on"
    command_off_template: "off"

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in home-assistant.io

[OttoWinter]

This would be a breaking change. And especially for the MQTT discovered platforms I really do not like breaking changes as everyone would need to update their firmwares/wait for them to update too. It's not the end of the world, but maybe we could be a bit smarter with working around this?

I'm thinking: now that the platform option in discovery payloads has no use anymore, maybe we could use it as an alias for schema? This way we could have a gradual deprecation/removal cycle.

[OttoWinter]

This is not the way this should be done. If we already have a schema option, why not use it?

def validate_mqtt_light(value):
    SCHEMAS = {
        'basic': schema_basic.PLATFORM_SCHEMA_BASIC,
        'json': schema_json.PLATFORM_SCHEMA_JSON,
        ...
    }
    return SCHEMAS[value[CONF_SCHEMA]](value)

PLATFORM_SCHEMA = vol.All(vol.Schema({
    vol.Optional(CONF_SCHEMA, default='basic'): vol.All(vol.Lower, vol.Any('basic', 'json', 'template'))
}, allow_extra=True), validate_mqtt_light)

[emontnemery]

Thanks!
However, what should the custom validator do if it finds a violation?
Print error messsage? Throw?

[OttoWinter]

Voluptuous already takes care of that using the code above. (Internally, the schema will throw an vol.Invalid or vol.MultipleInvalid which will propagate up through the custom validator into a validation error)

[OttoWinter]

Alternatively, we could do

PLATFORM_SCHEMA = vol.Any(schema_basic.PLATFORM_SCHEMA_BASIC, schema_json. ...)

# in basic schema
PLATFORM_SCHEMA = mqtt.MQTT_RW_PLATFORM_SCHEMA.extend({
  vol.Optional(CONF_SCHEMA, default='basic'): 'basic',
  # ...

# in other schemas (for example json)
PLATFORM_SCHEMA = mqtt.MQTT_RW_PLATFORM_SCHEMA.extend({
  vol.Required(CONF_SCHEMA): 'json',

but the validation error messages this alternative approach would produce are quite bad.

[emontnemery]

Thanks a lot! Rewritten as you suggest.

[emontnemery]

@OttoWinter Thanks a lot for reviewing! I added backwards compatibility by replacing platform in the discovery message with schema as you suggest.

[emontnemery]

How should the documentation be changed to reflect these changes?

Obviously, the configuration.yaml samples need to be updated and some wording rephrased (it's now one platform, supporting multiple schemas).
Should the documentation source files also be renamed in some way, or it does not matter?

[balloob]

Docs should be renamed to just be light.mqtt. You can add redirect_from to make the old urls redirect to the new page.