Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial support for Tornado 16X SQ air conditioner (0x4e2a) #430

Closed
wants to merge 28 commits into from

Conversation

enosh
Copy link

@enosh enosh commented Sep 22, 2020

Proposed changes

Add support for Tornado 16X SQ air conditioner (sold in Israel link). I think it'll also work for related models. Their app is based on AC Freedom so it might work with some models from that brand also. There are features like sleep curves, timers and ambient temperature I didn't implement.

@felipediel
Copy link
Collaborator

felipediel commented Sep 22, 2020

Thank you very much for the contribution! If you try to control it with the hysen class, does it work?

@enosh
Copy link
Author

enosh commented Sep 22, 2020

It's the first thing I tried, but it fails, the payloads are different in size, info and don't use a CRC for checksum. Figuring out the values in the payload took me the most time, your broadlink-hacktools was invaluable.

broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
@felipediel
Copy link
Collaborator

Sorry for asking all this. You had already done a great job of cracking the codes. But if I change the code after we merge this I won't be able to test the changes because I don't have the device.

data['display'] = (payload[0x16] & 0x10 == 0x10)
data['health'] = (payload[0x14] & 0b11 == 0b11)

checksum = self._calculate_checksum(payload[:0x19]) # checksum=(payload[0x1a] << 8) + payload[0x19]
Copy link
Collaborator

@felipediel felipediel Sep 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something like this?

checksum = payload[0x19] | (payload[0x1a] << 8)
if sum(payload[:0x19], 0x20017) - sum(payload[0x19:0x1b]) & 0xffff != checksum:
    raise exception(-4008)  # Checksum error.

Are you sure 0x20017 is not an int (20017)? Please double check, this number does not feel right.

If so, you can use this:

checksum = payload[0x19] | (payload[0x1a] << 8)
if sum(payload[:0x19], 0x4e31) - sum(payload[0x19:0x1b]) & 0xffff != checksum:
    raise exception(-4008)  # Checksum error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully understand how the checksum works, but I've done a lot of testing with this implementation without a problem showing up, will gladly send packet captures if you want to try. I'm pretty certain it's not a straight sum like your example.

Sorry for asking all this. You had already done a great job of cracking the codes. But if I change the code after we merge this I won't be able to test the changes because I don't have the device.

It's actually nice to have someone looking over my code as I don't have a lot of experience.

broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
@thewh1teagle
Copy link

Hi guys
I have Tornado 12X SQ air conditioner
This should work on it?
How can i test it?
Thank you so much!

@KTibow
Copy link
Contributor

KTibow commented Nov 6, 2020

@thewh1teagle download the repo, checkout the branch, build the wheel, and install with pip.

@thewh1teagle
Copy link

thewh1teagle commented Nov 6, 2020

@thewh1teagle download the repo, checkout the branch, build the wheel, and install with pip.

Thanks. I used @enosh Fork (Tornado branch), I installed it with
pip3 install .
Then i used his Tornado class. and it works!!.
I used his get_ac_info() method and i received {'state': True}
then i tried to use set_advanced() method like this
dev.set_advanced(state=False,mode="auto", speed="low", target_temp=16)
and It worked like a charm!

@KTibow
Copy link
Contributor

KTibow commented Nov 6, 2020

Mind fixing the conflicts @enosh?

@thewh1teagle
Copy link

thewh1teagle commented Nov 6, 2020

Hi
Sometimes it says checksum failed.
It works always after i use something from the app to control the ac, then it works once, and in the second try it says always checksum failed and it's not controlling the ac.

Edit

  1. sometimes it says checksum failed, but it actually control the ac!
  2. dev.set_name("random name") is always works.
  3. dev.set_lock(True/False) is always works.
  4. dev.check_power() is always working.
  5. dev.set_advanced(state=True/False) always working when the ac on cooling mode, However always not working when the ac on heating mode

on Tornado 12X SQ air conditioner.
Do you have hints where do i start to check why i get that?

broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
args = received_state
logging.debug("filled args %s", args)

PREFIX = [0xbb, 0x00, 0x06, 0x80, 0x00, 0x00, 0x0f, 0x00,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can assume that all requests start with bb 00 06 80 (and responses with bb 00 07 00). We can abstract this with self._encode() and self._decode(), respectively.

Comment on lines 349 to 355
data['mode'] = {
0x00: 'auto',
0x20: 'cooling',
0x40: 'drying',
0x80: 'heating',
0xc0: 'fan'
}.get(payload[0x11] &~ 0b111, 'unrecognized value') # noqa E225
Copy link
Collaborator

@felipediel felipediel Dec 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My opinion on how to handle this like a boss. Import the IntEnum class and create child classes inside the controller class. So we would end up with something like this:

from enum import IntEnum, unique

class sq1:

    @unique
    class Mode(IntEnum):
        AUTO = 0
        COOLING = 0x20
        DRYING = 0x40
        HEATING = 0x80
        FAN = 0xc0

We can generate the mode in many different ways...

mode = self.Mode.FAN
mode = self.Mode["FAN"]
mode = self.Mode(0xc0)

We can use:

# For reading
mode = self.Mode(payload[0x11] &~ 0b111)

# For writing
packet[0xf] = mode | sleep << 2

This is like creating an interface for a normal integer.

# If we want to see the name
>>> mode.name
'COOLING'

# If we want to see the value
>>> mode.value
32

A ValueError is generated for invalid input:

>>> d = blk.hello("192.168.0.16")  # Example HVAC
>>> d.Mode(1000)
Traceback (most recent call last):
  File "<pyshell#59>", line 1, in <module>
    d.Mode(1000)
  File "/usr/lib/python3.6/enum.py", line 293, in __call__
    return cls.__new__(cls, value)
  File "/usr/lib/python3.6/enum.py", line 535, in __new__
    return cls._missing_(value)
  File "/usr/lib/python3.6/enum.py", line 548, in _missing_
    raise ValueError("%r is not a valid %s" % (value, cls.__name__))
ValueError: 1000 is not a valid Mode

External applications have access to an interface. They can see what options they have.

>>> d = blk.hello("192.168.0.16")  # Example HVAC
>>> [m for m in d.Mode.__members__]
['AUTO', 'COOLING', 'DRYING', 'HEATING', 'FAN']

Best of all: this is a normal integer that can be appended to the packet and serialized normally. What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! Will implement.
Would you then pass a Mode object in the dictionary as input or the string representation?

broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
broadlink/climate.py Outdated Show resolved Hide resolved
@felipediel
Copy link
Collaborator

felipediel commented Jan 22, 2021

Hi @enosh. Thanks again for this. I will solve conflicts and create a PR to the dev branch, we will continue from there, ok? Do you want to create the PR?

@felipediel
Copy link
Collaborator

@enosh I couldn't leave all of our effort behind, so I rebased and created a PR on the dev branch: #520. After I make some improvements, could you please help me to test?

@thewh1teagle
Copy link

Any update on this? does the library supports tornado ACs?

@felipediel
Copy link
Collaborator

Closed with #520. Thanks a lot @enosh!

@felipediel felipediel closed this Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants