Add unique_id to MQTT switch by thepotoo · Pull Request #13719 · home-assistant/core

thepotoo · 2018-04-06T14:44:41Z

Description:

Add unique_id to MQTT switch for use with discovery. This enables easier management of large numbers of devices using discovery.

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for `configuration.yaml` (if applicable):

This feature is helpful precisely because you don't need to edit configuration.yaml; you can use discovery for almost all devices instead.

Here is an example json discovery topic that works:
{"state_topic":"homeassistant/relay/fishroom/520E4E","unique_id":"relay_520E4E","command_topic":"homeassistant/fishroom/520E4E","availability_topic":"homeassistant/online/fishroom/520E4E","payload_available": "online","payload_not_available": "offline","payload_on":"on","payload_off":"off","optimistic":"false","assumed_state":"false","qos":"0","retain":"true"}

On the backend (in the Arduino IDE), the unique_id is generated using switch_type (in this case, 'relay') + ESP.getChipId();

As long as each ESP only controls one switch, the id will be truly unique.

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass
Is there a guideline for what to use for tests? I'm happy to put something together, but I'm not really sure where to start.

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

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

This should not be necessary in this case.

If the code does not interact with devices:

Tests have been added to verify that the new code works.

Is there a guideline for what to use for tests? I'm happy to put something together, but I'm not really sure where to start.

Added unique_id to MQTT Switch for easier discovery and firmware management

homeassistant · 2018-04-06T14:44:42Z

Hi @thepotoo,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

houndci-bot · 2018-04-06T14:44:50Z

                 state_topic, command_topic, availability_topic,
                 qos, retain, payload_on, payload_off, optimistic,
-                 payload_available, payload_not_available, value_template):
+                 payload_available, payload_not_available, unique_id: Optional[str], value_template):


line too long (101 > 79 characters)

syssi · 2018-04-06T20:44:50Z

Duplicate of #13318

thepotoo · 2018-04-06T22:22:44Z

@syssi This isn't a duplicate - #13318 is for the mqtt sensor, this one is for a mqtt switch.

syssi · 2018-04-07T04:55:42Z

You are totally right, sorry!

syssi · 2018-04-07T07:28:58Z

The PR looks good. Please care about the lint issue and update the docs (new config parameter).

fabaff · 2018-04-07T07:41:09Z

Will also require a test.

Pretty much a straight copy from tests/components/sensor/test_mqtt.py

houndci-bot · 2018-04-08T16:20:58Z


        state = self.hass.states.get('switch.test')
        self.assertEqual(STATE_ON, state.state)
+


blank line contains whitespace

syssi · 2018-04-08T19:14:12Z

Unfortunately your introduced test is failing.

Changed assert in test to use async_entity_ids instead of self.hass.states.all

thepotoo · 2018-04-08T21:48:07Z

@syssi Yeah, I see that - If I'm reading the log correctly, the test code I copied/modified from sensor assert len(self.hass.states.all()) == 1 is failing because the switches have groups and sensors don't (?)

If I change this assert to assert len(self.hass.states.all()) == 2, would that make this a valid test? I tried changing it to async_entity_ids instead of all, but that still picked up group.all_switches.

Sorry for the total noob questions.

houndci-bot · 2018-04-13T14:13:26Z

+        fire_mqtt_message(self.hass, 'test-topic', 'payload')
+        self.hass.block_till_done()
+        assert len(self.hass.states.async_entity_ids()) == 2
+        #all switches group is 1, unique id created is 1


block comment should start with '# '

dgomes

LGTM

OttoWinter · 2018-05-06T15:25:07Z

Needs docs

thepotoo added 2 commits April 6, 2018 10:24

Added unique_id to MQTT Switch

47afc32

Added unique_id to MQTT Switch for easier discovery and firmware management

fixed typo

7813262

homeassistant added cla-needed platform: switch.mqtt small-pr PRs with less than 30 lines. labels Apr 6, 2018

houndci-bot reviewed Apr 6, 2018

View reviewed changes

homeassistant added cla-signed and removed cla-needed labels Apr 6, 2018

thepotoo mentioned this pull request Apr 6, 2018

Add MQTT Sensor unique_id #13318

Merged

4 tasks

syssi marked this as a duplicate of #13318 Apr 6, 2018

fabaff added the docs-missing label Apr 6, 2018

Fixed line too long

60eba92

homeassistant added the cla-signed label Apr 8, 2018

Added test for unique ID being truely unique

5e3fe2f

Pretty much a straight copy from tests/components/sensor/test_mqtt.py

houndci-bot reviewed Apr 8, 2018

View reviewed changes

Removed whitespace

884c4fb

thepotoo added 2 commits April 8, 2018 16:02

added command topic to test

cf466e9

Revised test

351a499

Changed assert in test to use async_entity_ids instead of self.hass.states.all

Update test_mqtt.py

3b5b7ee

homeassistant added the cla-signed label Apr 13, 2018

houndci-bot reviewed Apr 13, 2018

View reviewed changes

Update test_mqtt.py

243a9b7

dgomes approved these changes May 5, 2018

View reviewed changes

syssi merged commit 2bb1a95 into home-assistant:dev May 6, 2018

balloob mentioned this pull request May 28, 2018

0.70.0 #14657

Merged

home-assistant locked and limited conversation to collaborators Sep 5, 2018

ghost added integration: mqtt and removed platform: switch.mqtt labels Mar 21, 2019


		state = self.hass.states.get('switch.test')
		self.assertEqual(STATE_ON, state.state)

Uh oh!

Conversation

thepotoo commented Apr 6, 2018

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

homeassistant commented Apr 6, 2018

Uh oh!

houndci-bot Apr 6, 2018

Choose a reason for hiding this comment

Uh oh!

syssi commented Apr 6, 2018

Uh oh!

thepotoo commented Apr 6, 2018

Uh oh!

syssi commented Apr 7, 2018

Uh oh!

syssi commented Apr 7, 2018

Uh oh!

fabaff commented Apr 7, 2018

Uh oh!

houndci-bot Apr 8, 2018

Choose a reason for hiding this comment

Uh oh!

syssi commented Apr 8, 2018

Uh oh!

thepotoo commented Apr 8, 2018

Uh oh!

houndci-bot Apr 13, 2018

Choose a reason for hiding this comment

Uh oh!

dgomes left a comment

Choose a reason for hiding this comment

Uh oh!

OttoWinter commented May 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Example entry for `configuration.yaml` (if applicable):