[WIP]Add Xiaomi&Aqara air conditioner partner #10761

cxlwill · 2017-11-23T07:57:36Z

Description:

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

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

climate:
  - platform: xiaomi_miio
    name: Xiaomi AC Partner
    host: YOUR_DEVICE_IP
    token: YOUR_TOKEN
    target_sensor: 
    customize:

Checklist:

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:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
New dependencies are only imported inside functions that use them ([example][ex-import]).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

houndci-bot · 2017-11-23T07:57:45Z

homeassistant/components/climate/xiaomi_miio.py

+                self.climate.send('send_cmd', [mainCode])
+            else:
+                self.climate.send('send_ir_code', [mainCode])
+            _LOGGER.info('Send Customize Code: acmodel: %s, operation: %s , temperature: %s, fan: %s, swing: %s, sendCode: %s', model, self._current_operation, self._target_temperature, self._current_fan_mode, self._current_swing_mode, mainCode )


line too long (246 > 79 characters)
whitespace before ')'

houndci-bot · 2017-11-23T07:57:45Z

homeassistant/components/climate/xiaomi_miio.py

+
+        try:
+            self.climate.send('send_cmd', [mainCode])
+            _LOGGER.info('Change Climate Successful: acmodel: %s, operation: %s , temperature: %s, fan: %s, swing: %s, sendCode: %s', model, self._current_operation, self._target_temperature, self._current_fan_mode, self._current_swing_mode, mainCode )


line too long (252 > 79 characters)
whitespace before ')'

houndci-bot · 2017-11-23T07:57:45Z

homeassistant/components/climate/xiaomi_miio.py

+                    index += 1
+
+
+        try:


too many blank lines (2)

houndci-bot · 2017-11-23T07:57:45Z

homeassistant/components/climate/xiaomi_miio.py

+                        temp = hex(temp)[2:].upper()
+                        mainCode = mainCode.replace('t4wt', temp)
+                    index += 1
+


blank line contains whitespace

houndci-bot · 2017-11-23T07:57:45Z

homeassistant/components/climate/xiaomi_miio.py

+                        temp = hex(temp)[2:].upper()
+                        mainCode = mainCode.replace('t6t', temp)
+                    if tep ==  "t4wt":
+                        temp = (int(codeConfig['t4wt']) + int(self._target_temperature) - 17) % 16


line too long (98 > 79 characters)

houndci-bot · 2017-11-23T07:57:45Z

homeassistant/components/climate/xiaomi_miio.py

+                    mainCode = mainCode
+                    # mainCode = mainCode.replace('li', codeConfig['li']['on'])
+                index += 1
+


blank line contains whitespace

houndci-bot · 2017-11-23T07:57:46Z

homeassistant/components/climate/xiaomi_miio.py

+                    if self._current_swing_mode == 'on':
+                        mainCode = mainCode.replace('sw', codeConfig['sw']['on'])
+                    else: 
+                        mainCode = mainCode.replace('sw', codeConfig['sw']['off'])


line too long (82 > 79 characters)

houndci-bot · 2017-11-23T07:57:46Z

homeassistant/components/climate/xiaomi_miio.py

+                if tep == "sw":
+                    if self._current_swing_mode == 'on':
+                        mainCode = mainCode.replace('sw', codeConfig['sw']['on'])
+                    else: 


trailing whitespace

houndci-bot · 2017-11-23T07:57:46Z

homeassistant/components/climate/xiaomi_miio.py

+                    mainCode = mainCode.replace('wi', wiCode);
+                if tep == "sw":
+                    if self._current_swing_mode == 'on':
+                        mainCode = mainCode.replace('sw', codeConfig['sw']['on'])


line too long (81 > 79 characters)

houndci-bot · 2017-11-23T07:57:46Z

homeassistant/components/climate/xiaomi_miio.py

+                        wiCode = '2'
+                    else:
+                        wiCode = '3'
+                    mainCode = mainCode.replace('wi', wiCode);


statement ends with a semicolon

cxlwill · 2017-11-23T08:05:20Z

The main contributor is Mac_zhou. We will work together to make it to HA~

Mylife4air · 2017-11-23T09:13:38Z

用 pycharm 的话只要把画灰色线的解决了，格式检查就没问题

homeassistant · 2017-11-24T03:14:40Z

Hi @mac-zhou,

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 · 2017-11-24T03:14:44Z

homeassistant/components/climate/xiaomi_miio.py

+            self._customize_swing_list = list(self._customize['swing'])
+            self._swing_list = self._customize_swing_list
+            self._current_swing_mode = 'idle'
+        else:  


trailing whitespace

houndci-bot · 2017-11-24T03:14:44Z

homeassistant/components/climate/xiaomi_miio.py

+from homeassistant.const import (
+    TEMP_CELSIUS, TEMP_FAHRENHEIT, ATTR_TEMPERATURE, ATTR_UNIT_OF_MEASUREMENT,
+    CONF_NAME, CONF_HOST, CONF_TOKEN, CONF_TIMEOUT)
+from homeassistant.helpers import condition


'homeassistant.helpers.condition' imported but unused

houndci-bot · 2017-11-24T03:14:44Z

homeassistant/components/climate/xiaomi_miio.py

+from homeassistant.components.climate import (
+    PLATFORM_SCHEMA,
+    ClimateDevice, ATTR_TARGET_TEMP_HIGH, ATTR_TARGET_TEMP_LOW)
+from homeassistant.const import (


'homeassistant.const.TEMP_FAHRENHEIT' imported but unused

houndci-bot · 2017-11-24T05:43:52Z

homeassistant/components/climate/xiaomi_miio.py

+                    maincode = maincode
+                index += 1
+
+            if (model in __Presets__) and ('EXTRA_VALUE' in __Presets__[model]):


line too long (80 > 79 characters)

tinloaf · 2017-11-24T12:26:47Z

homeassistant/components/climate/xiaomi_miio.py

+CONF_SYNC = 'sync'
+CONF_CUSTOMIZE = 'customize'
+
+__Presets__ = {


This looks a lot like protocol-level magic strings. We try to keep protocol implementations out of home assistant as far as possible. Perhaps it would be better to include this low-level protocol stuff in the python-miio package?

tinloaf · 2017-11-24T12:28:15Z

homeassistant/components/climate/xiaomi_miio.py

+    vol.Required(CONF_HOST): cv.string,
+    vol.Optional(CONF_TIMEOUT, default=DEFAULT_TIMEOUT): cv.positive_int,
+    vol.Required(CONF_SENSOR, default=None): cv.entity_id,
+    vol.Optional(CONF_CUSTOMIZE, default=None): dict,


You seem to use the _customize member to set fan and swing modes. Why don't you just let the user configure a list of possible fan and swing modes, as for example the mqtt climate platform does?

tinloaf · 2017-11-24T12:29:50Z

homeassistant/components/climate/xiaomi_miio.py

+    customize = config.get(CONF_CUSTOMIZE)
+
+    add_devices_callback([MiAcPartner(
+        hass, name, None, None, None, 'auto', None,


Why does the class constructor have all these arguments if you just statically pass None for them?

tinloaf · 2017-11-24T12:31:19Z

homeassistant/components/climate/xiaomi_miio.py

+        token, sensor_entity_id, sync, customize)])
+
+
+class ClimateStatus(ClimateDevice):


What is the idea behind this? Is that just for decoding the response from the device? If so, (a) it might also belong into python-miio, and (b) it should not derive from ClimateDevice. Otherwise: What kind of entity is this?

This class should belong to the upstream lib.

From the looks of it this is similar to https://github.com/rytilahti/python-miio/pull/129/files#diff-f3dc2005ebf7fddbea7c921f3bcce1f5R18 .

tinloaf · 2017-11-24T12:32:56Z

homeassistant/components/climate/xiaomi_miio.py

+        """get states from climate"""
+        getstate = self.climate.send("get_model_and_state", [])
+        _LOGGER.info(getstate)
+        self._state = ClimateStatus(getstate)


Here you set the state of an entity do another entity (remember that ClimateStatus derives from ClimateDevice). That is… weird. self._state should be a simple (string) representation of the device's current state.

tinloaf · 2017-11-24T12:46:45Z

homeassistant/components/climate/xiaomi_miio.py

+        self.schedule_update_ha_state()
+
+    def set_swing_mode(self, swing_mode):
+        """Set new target temperature."""


Comment is also wrong.

tinloaf · 2017-11-24T12:47:02Z

homeassistant/components/climate/xiaomi_miio.py

+        self.schedule_update_ha_state()
+
+    def set_fan_mode(self, fan):
+        """Set new target temperature."""


Comment wrong

tinloaf · 2017-11-24T12:47:22Z

homeassistant/components/climate/xiaomi_miio.py

+        self.schedule_update_ha_state()
+
+    def set_operation_mode(self, operation_mode):
+        """Set new target temperature."""


Comment wrong.

tinloaf · 2017-11-24T12:47:52Z

homeassistant/components/climate/xiaomi_miio.py

+        """Return the list of available fan modes."""
+        return self._fan_list
+
+    def set_temperature(self, **kwargs):


set_temperature can also be called with a new operation mode, to set both in one go. That must be handled here.

tinloaf · 2017-11-24T12:48:36Z

homeassistant/components/climate/xiaomi_miio.py

+        self._state = ClimateStatus(getstate)
+        return self._state
+
+    def sendcmd(self):


This is also very much protocol-level and should be factored out into a library.

rytilahti · 2017-11-26T18:11:32Z

Just FYI, some of the upstream parts of this are being worked in rytilahti/python-miio#129 . It probably makes sense to combine the effort, right?

rytilahti · 2017-11-26T18:12:27Z

homeassistant/components/climate/xiaomi_miio.py

+from homeassistant.const import (
+    TEMP_CELSIUS, ATTR_TEMPERATURE, ATTR_UNIT_OF_MEASUREMENT,
+    CONF_NAME, CONF_HOST, CONF_TOKEN, CONF_TIMEOUT)
+# from homeassistant.helpers import condition


Commented out code lines should not be added.

rytilahti · 2017-11-26T18:15:12Z

homeassistant/components/climate/xiaomi_miio.py

+        token, sensor_entity_id, sync, customize)])
+
+
+class ClimateStatus(ClimateDevice):


This class should belong to the upstream lib.

rytilahti · 2017-11-26T18:30:15Z

homeassistant/components/climate/xiaomi_miio.py

+        token, sensor_entity_id, sync, customize)])
+
+
+class ClimateStatus(ClimateDevice):


From the looks of it this is similar to https://github.com/rytilahti/python-miio/pull/129/files#diff-f3dc2005ebf7fddbea7c921f3bcce1f5R18 .

thundergreen · 2017-12-01T21:11:32Z

Hi.was just wondering if this could also include the ir WiFi transmitter ... For people using the it transmitter from xiaomi

Danielhiversen · 2018-01-19T07:48:41Z

@cxlwill do you plan to finish this?

cxlwill · 2018-01-19T07:53:05Z

@Danielhiversen Yeah, but I want to wait for Aqara to update the protocol in order to use Gateway function as well. If you want to close it for now or continue the work, very welcome.

Comments updated.

Comment removed.

syssi · 2018-01-28T14:33:01Z

@cxlwill @mac-zhou I've made a lot of code changes. The update interval (called "sync" in the past) can be controlled by the configuration key "scan_interval" (cp. https://home-assistant.io/docs/configuration/platform_options/).

I didn't change the customization handling yet. Could you test the updated codebase extensive? You will need the current master of python-miio.

balloob · 2018-02-18T05:18:41Z

What's the status of this PR? This PR has been WIP since last year.

syssi · 2018-02-18T07:03:33Z

I've refactored the code and looking for somebody willing to test. If nobody is able to test the component the PR can be close d.

balloob · 2018-02-18T08:18:24Z

I suggest we close it. It has been 3 weeks since your changes and asking to test it. Nothing happened. We can reconsider when testing happens

syssi · 2018-02-18T09:13:13Z

The component is available as custom_component now: https://github.com/syssi/xiaomi_airconditioningcompanion

cxlwill · 2018-02-18T09:20:06Z

Sorry for late response. It’s Chinese New Year now so I bet everyone is on vacation and leave HA stuffs behind. Thanks to Syssi for all your efforts. I didn’t notice about the PR Of custom_component before. And original author is quite busy since last year so he paused the update of this component. I will test it this evening. If everything works well, I will reopen this.

balloob · 2018-02-18T16:14:26Z

@cxlwill happy new year! 🎉🐕

I don't think you can reopen this PR, but just post here and we'll reopen.

cxlwill · 2018-02-20T08:56:35Z

@balloob Cheers~ @syssi No luck here. I test the custom component and issued an error:

xiaomi_miio: Error on device update!
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/homeassistant/helpers/entity_platform.py", line 197, in _async_add_entity
    yield from entity.async_device_update(warning=False)
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/homeassistant/helpers/entity.py", line 325, in async_device_update
    yield from self.async_update()
  File "/Users/cxlwill/.homeassistant/custom_components/climate/xiaomi_miio.py", line 208, in async_update
    ATTR_SWING_MODE: state.swing_mode.name,
AttributeError: 'bool' object has no attribute 'name'

syssi · 2018-02-20T09:14:54Z

@cxlwill Please report all issues here: https://github.com/syssi/xiaomi_airconditioningcompanion/issues

If the custom component is stable again I will move the code.

syssi · 2018-02-20T09:43:33Z

@cxlwill fixed.

cxlwill requested review from andrey-git and rytilahti as code owners November 23, 2017 07:57

homeassistant added cla-signed new-platform platform: climate.xiaomi_miio labels Nov 23, 2017

houndci-bot reviewed Nov 23, 2017

View reviewed changes

cxlwill changed the title ~~Add Xiaomi&Aqara air conditioner partner~~ [WIP]Add Xiaomi&Aqara air conditioner partner Nov 23, 2017

This was referenced Nov 23, 2017

Add Xiaomi&Aqara AC Partner home-assistant/home-assistant.io#4050

Closed

Xiaomi Mi Home Air Conditioner Companion support rytilahti/python-miio#76

Closed

tinloaf self-requested a review November 23, 2017 15:20

homeassistant added the cla-needed label Nov 24, 2017

houndci-bot reviewed Nov 24, 2017

View reviewed changes

homeassistant added cla-signed and removed cla-needed labels Nov 24, 2017

balloobbot added platform requirement labels Nov 24, 2017

homeassistant added the cla-signed label Nov 24, 2017

houndci-bot reviewed Nov 24, 2017

View reviewed changes

tinloaf suggested changes Nov 24, 2017

View reviewed changes

rytilahti mentioned this pull request Nov 26, 2017

Xiaomi Air Conditioner Companion support rytilahti/python-miio#129

Merged

rytilahti reviewed Nov 26, 2017

View reviewed changes

homeassistant added the cla-signed label Jan 21, 2018

syssi added 9 commits January 28, 2018 15:13

Method "climate" refactored.

9435bd6

Comments updated.

Constants introduced.

fbc1873

Operation mode parameter of set_temperature is processed properly now.

c1081c8

"Continuation line over-indented for visual indent" fixed.

d3ebf8f

Some names and comments updated.

d3aae82

Infrared command assembly refactored and moved to python-miio.

ab1d756

Refactoring.

623013f

Unused import removed.

c1512eb

Comment removed.

Test requirements removed.

6d8b36e

syssi force-pushed the miio-acpartner branch from 06ad2e6 to 6d8b36e Compare January 28, 2018 14:15

homeassistant added the cla-signed label Jan 28, 2018

syssi added 2 commits January 28, 2018 15:17

requirements_test.txt reverted.

b26c404

Async methods.

b609bdf

requirements_all order fixed.

b87ea00

homeassistant added the cla-signed label Jan 28, 2018

schedule_update_ha_state calls removed.

332803e

balloob closed this Feb 18, 2018

home-assistant locked and limited conversation to collaborators May 29, 2018

ghost added integration: xiaomi_miio and removed platform: climate.xiaomi_miio labels Mar 21, 2019

		token, sensor_entity_id, sync, customize)])


		class ClimateStatus(ClimateDevice):

[WIP]Add Xiaomi&Aqara air conditioner partner #10761

[WIP]Add Xiaomi&Aqara air conditioner partner #10761

Conversation

cxlwill commented Nov 23, 2017 • edited

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cxlwill commented Nov 23, 2017

Mylife4air commented Nov 23, 2017

homeassistant commented Nov 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rytilahti commented Nov 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thundergreen commented Dec 1, 2017

Danielhiversen commented Jan 19, 2018

cxlwill commented Jan 19, 2018

syssi commented Jan 28, 2018

balloob commented Feb 18, 2018

syssi commented Feb 18, 2018

balloob commented Feb 18, 2018

syssi commented Feb 18, 2018

cxlwill commented Feb 18, 2018 • edited

balloob commented Feb 18, 2018

cxlwill commented Feb 20, 2018 • edited

syssi commented Feb 20, 2018

syssi commented Feb 20, 2018

cxlwill commented Nov 23, 2017 •

edited

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

rytilahti commented Nov 26, 2017 •

edited

cxlwill commented Feb 18, 2018 •

edited

cxlwill commented Feb 20, 2018 •

edited