Mqtt cover modifications by cribbstechnologies · Pull Request #7841 · home-assistant/core

cribbstechnologies · 2017-05-30T22:17:03Z

Description:

Making command topic optional so a tilt-only cover doesn't show up/down controls
Adding ability to set up/down position including ability to template the value

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

**also has modifications for polymer project - PR not ready yet

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

cover:
  - platform: mqtt,
    name: test,
    state_topic: 'state-topic',
    command_topic: 'command-topic',
    set_position_topic: 'position-topic',
    set_position_template: '{{100-62}}',
    payload_open: 'OPEN',
    payload_close: 'CLOSE',
    payload_stop: 'STOP'

Checklist:

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

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

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.

removing command_topic being required

mention-bot · 2017-05-30T22:17:05Z

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

houndci-bot · 2017-05-30T22:17:12Z

+            }
+        }))
+
+        state_attributes_dict = self.hass.states.get(


local variable 'state_attributes_dict' is assigned to but never used

houndci-bot · 2017-05-30T22:17:12Z

+            }
+        }))
+
+        state_attributes_dict = self.hass.states.get(


local variable 'state_attributes_dict' is assigned to but never used

houndci-bot · 2017-05-30T22:17:12Z

+                         self.mock_publish.mock_calls[-2][1])
+
+
+    def test_no_command_topic(self):


too many blank lines (2)

houndci-bot · 2017-05-30T22:17:12Z

+        current_cover_position = self.hass.states.get(
+            'cover.test').attributes['current_position']
+        self.assertEqual(22, current_cover_position)
+


blank line contains whitespace

bruhautomation · 2017-05-31T04:45:42Z

Hey @balloob. Sorry for the direct tag. Anyway this can make it into 0.46? Would be clutch to have it out in the next release cycle. Me and @cribbstechnologies worked through my typos and I'm good to share my project soon.

WoodsterDK · 2017-05-31T05:13:00Z

Will it be possible to set the position with a customized json payload, e.g. like:

set_position_template: '{"covers": [1,2],"position" : value.position}'
where "value.position" gets substituted with the target position and then the complete json object is sent by mqtt?

emlove

I really like the method of passing the service call through a template. It gives great flexibility to handle scaling or whatever else the endpoint might need.

emlove · 2017-05-31T14:37:32Z

+        self._position_topic = position_topic
+        self._set_position_template = set_position_template
+        if set_position_template is not None:
+            self._set_position_template.hass = hass


Can you put this logic in async_setup_platform so we don't need to pass hass in? Let's make it look the same as value_template.

emlove · 2017-05-31T14:38:43Z

+        if self._position_topic is not None:
+            supported_features |= SUPPORT_SET_POSITION

        if self.current_cover_position is not None:


I think this should now be removed since we're using self._position_topic to define SUPPORT_SET_POSITION. It probably shouldn't have been set before.

Did you mean remove the block starting at line 262?

Yes, that's right.

emlove · 2017-05-31T14:54:27Z

+            position = kwargs[ATTR_POSITION]
+            if self._set_position_template is not None:
+                try:
+                    position = self._set_position_template.async_render()


Change this to async_render(**kwargs). This will make the requested position from the service available to the template. Should also be noted in the docs.

emlove

LGTM, except the polymer submodule update needs to be removed.

This reverts commit 9cfc5ed.

cribbstechnologies · 2017-05-31T15:29:46Z

I think I reverted it. Let me know if I didn't do it right.

…

On Wed, May 31, 2017 at 11:19 AM, Adam Mills ***@***.***> wrote: ***@***.**** approved this pull request. LGTM, except the polymer submodule update needs to be removed. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7841 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABjyfUX-fCr8LZxR7wiY6ie6aMdLI1m0ks5r_YUXgaJpZM4Nq626> .

balloob · 2017-06-05T05:55:01Z

@armills if you approve, you can merge it too in the future 👍

Sorry, didn't make 0.46 as I am not really paying attention to notifications anymore (too many).

bruhautomation · 2017-06-07T20:05:19Z

No worries! I'm trying out a new 12v stepper right now since I have the extra time. Should be a good video once I have everything ready. Thanks much for everyone's work getting the MQTT tilt component integrated!

WoodsterDK · 2017-06-07T20:41:59Z

Untill it gets into the released version, will it then be enough to copy the modified cover/mqtt.py file into the custom component library?

* adding set position ability removing command_topic being required * flaking * flaking test * updating docs * requested updates * Revert "updating docs" This reverts commit 9cfc5ed. * forgot to update constructor calls in tests

balloob · 2017-06-09T04:36:24Z

Will cherry-pick it for 0.46.1

WoodsterDK · 2017-06-19T10:55:50Z

Can anyone guide me in the right direction setting this mqtt cover up.
My cover supports different positions between 0-100 percent - the topic is given, but the payload is a json object, where the parameter is part of, e.g.:

{"covers": [1,2],"position" : 50}

How is this done, using the latest template feature?

cribbstechnologies added 2 commits May 30, 2017 17:49

adding set position ability

42f26bb

removing command_topic being required

flaking

0d93c3c

homeassistant added platform: cover.mqtt cla-signed labels May 30, 2017

houndci-bot reviewed May 30, 2017

View reviewed changes

flaking test

e36e0b3

homeassistant added the cla-signed label May 30, 2017

This was referenced May 30, 2017

MQTT Cover Tilt State Not Reflecting Correctly #7764

Closed

updating docs for MQTT Cover changes home-assistant/home-assistant.io#2729

Closed

Control modification for cover home-assistant/frontend#298

Merged

emlove reviewed May 31, 2017

View reviewed changes

updating docs

9cfc5ed

homeassistant added the cla-signed label May 31, 2017

requested updates

43747e5

balloobbot added the platform label May 31, 2017

emlove approved these changes May 31, 2017

View reviewed changes

Revert "updating docs"

9931659

This reverts commit 9cfc5ed.

homeassistant added the cla-signed label May 31, 2017

forgot to update constructor calls in tests

5c71458

balloob merged commit 774f584 into home-assistant:dev Jun 5, 2017

balloob added this to the 0.46.1 milestone Jun 9, 2017

balloob mentioned this pull request Jun 9, 2017

0.46.1 #7968

Merged

balloob mentioned this pull request Jun 16, 2017

0.47 #8055

Merged

balloob mentioned this pull request Jun 25, 2017

Remove unnecessary thread_ident assignment #8194

Merged

home-assistant locked and limited conversation to collaborators Oct 20, 2017

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

		self.mock_publish.mock_calls[-2][1])


		def test_no_command_topic(self):

Uh oh!

Conversation

cribbstechnologies commented May 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

mention-bot commented May 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bruhautomation commented May 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WoodsterDK commented May 31, 2017

Uh oh!

emlove left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emlove May 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emlove left a comment

Choose a reason for hiding this comment

Uh oh!

cribbstechnologies commented May 31, 2017 via email

Uh oh!

balloob commented Jun 5, 2017

Uh oh!

bruhautomation commented Jun 7, 2017

Uh oh!

WoodsterDK commented Jun 7, 2017

Uh oh!

balloob commented Jun 9, 2017

Uh oh!

WoodsterDK commented Jun 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

cribbstechnologies commented May 30, 2017 •

edited

Loading

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

bruhautomation commented May 31, 2017 •

edited

Loading

emlove May 31, 2017 •

edited

Loading