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

Create SP3S class and improve energy monitoring #483

Closed
wants to merge 2 commits into from

Conversation

felipediel
Copy link
Collaborator

@felipediel felipediel commented Nov 26, 2020

Breaking change

Create a new SP3S class for these devices. Applications need to use the correct class to support get_energy().

The problem

Some SP2 devices (PID starting with 0x27) support get_energy(), but the firmware is different from SP3S, so we need to create another method.

From:
#413 (comment)
#200 (comment)

Proposed changes

  • Create a new class for SP3S with the current get_energy() method (only works for SP3S).
  • Create a new get_energy() method for the SP2 class.

Need help with tests

These smart plugs don't support energy monitoring. They respond to sp3s.get_energy() with CommandNotSupportedError and StorageError. This is the behavior we had in the past, so we were catching the exceptions eg here.

It would be interesting to see how they respond to this new sp2.get_energy() method before the code is merged. So, if anyone has the smart plugs on the list, please help to test this method.

On your PC:

# Create a venv (not mandatory, but recommended)
python3 -m venv venv
source venv/bin/activate

# Install this branch
pip3 install git+https://github.com/felipediel/python-broadlink.git@sp3s --force-reinstall

## Run Python 3
python3

Now let's see how the device respond to get_energy():

>>> import broadlink as blk
>>> d = blk.hello("192.168.0.16")  # Example device
>>> d.auth()
>>> d.get_energy()

Thanks in advance!

@felipediel felipediel marked this pull request as draft November 26, 2020 05:27
@felipediel
Copy link
Collaborator Author

Hi @ecylcje and @tottka! Could you guys please confirm if get_energy() works good for your devices with this code?

@tottka
Copy link

tottka commented Nov 26, 2020

Hi Felipe, Thank you for continuing to test, unforunately this version loses the power attribute on the Neo Pro.

@felipediel
Copy link
Collaborator Author

get_energy() doesn't work? You said that this formula worked, so I added it here. Any idea of what's going on?

@tottka
Copy link

tottka commented Nov 26, 2020

Let me look into it a little more and I'll report back, my mind is on the turkey right now. Probably not until tonight/tomorrow.

@tottka
Copy link

tottka commented Nov 27, 2020

My mistake, I was looking at the prior version of the code. Just tested your changes and good to go, THANKS

@felipediel
Copy link
Collaborator Author

Thank you!

@felipediel felipediel marked this pull request as ready for review November 30, 2020 19:39
@felipediel felipediel marked this pull request as draft December 1, 2020 05:59
@Stormhand
Copy link

Hello, im not sure if its related to this issue but get_energy() on my SP3S always returns like 30 watts more than in the IHC.

@felipediel
Copy link
Collaborator Author

Hi @Stormhand! Not the same thing. You need to open an issue an provide some data so we can understand what is going on. I would risk saying that it happens because we round up incoming data.

@Stormhand
Copy link

Thank you, ill do it. BTW the change here is throwing:

StorageError: [Errno -5] The device storage is full

@felipediel
Copy link
Collaborator Author

@tottka Sorry to bother you again. Do the SP2 get_energy() values match the app exactly? If not, does this method work?

    def get_energy(self) -> float:
        """Return the energy state of the device."""
        packet = bytearray(16)
        packet[0] = 4
        response = self.send_packet(0x6A, packet)
        check_error(response[0x22:0x24])
        payload = self.decrypt(response[0x38:])
        return float("{:x}{:x}.{:x}".format(*payload[0x06:0x03:-1]))

@felipediel felipediel changed the title Create SP3S class and fix get_energy() of SP2 class Create SP3S class and fix get_energy() Dec 9, 2020
@tottka
Copy link

tottka commented Dec 10, 2020

1st, The new version disabled ALL my Ankuoo devices in HA, even the ones that don't use wattage. I can't understand that line of code to even begin to offer any help.

2nd, I tested on both the kettle and a 60 watt bulb and neither matched the app. I could have sworn we had this working?
on the 60 watt bulb, the app says it is using 56, our logic says 218.72 (remember the magic number of 3.9? 218.72/3.9 = 56)
on the the kettle, the app says 1280, our logic says 2039 (no magic number here)

If you make another change I'll keep testing, no problem

@felipediel
Copy link
Collaborator Author

felipediel commented Dec 11, 2020

Thank you! I have a few guesses on how this works. Could you help me with some captures so that I have more information to work with?

  1. Check the energy in the app.
  2. Run this script.
import broadlink as blk
d = blk.hello("192.168.0.16")  # Example device
d.auth()
print(d.decrypt(d.send_packet(0x6a, bytes([4]))[0x38:]))
  1. Give me the results in tuples e.g. (60w, b'\x04\x00\x00\x00...')

@felipediel
Copy link
Collaborator Author

felipediel commented Dec 11, 2020

Lucky guess no 1:

    def get_energy(self) -> float:
        """Return the energy state of the device."""
        packet = bytearray(16)
        packet[0] = 4
        response = self.send_packet(0x6A, packet)
        check_error(response[0x22:0x24])
        payload = self.decrypt(response[0x38:])
        return int.from_bytes(payload[0x4:0x7], "little") / 1000

It matches the data you gave me. b'\x04\x00\x00\x00\x48\xda\x00... returns 56w in the app. This method returns 55.88w. Pretty close, but we need more data to confirm. Could you plug something powerful this time?

@tottka
Copy link

tottka commented Dec 11, 2020

I am so over my head!

Here's how I've been testing, I go to /srv/homeassistant/lib/python3.8/site-packages/broadlink/switch.py
and replace the get_energy section as per your request. Then I restart HA & check the wattage in HA State against the app. With your latest change request, all my NEO's are disabled when HA is restarted.

@felipediel
Copy link
Collaborator Author

Sorry, there was a typo. Could you please try again?

@tottka
Copy link

tottka commented Dec 11, 2020

The APP & HA update on different cycles and electricity usage varies depending on the device.
The bulb holds very steady and the applicances have fluctuations.
I would call this perfect!
Thank you

Kettle
app: 1282.67 HA: 1282.299 & 1282.851

60 watt bulb
app: 57.47 HA: 57.471

Air Cleaner on Low
app: 122.76 HA: 122.758
app: 121.75 HA: 121.747

Air Cleaner on Med
app: 147.86 HA: 147.494
app: 147.13 HA: 147.126

Air Cleaner on High
app: 190.99 HA: 191.08
app: 190.62 HA: 190.62
app: 190.53 HA: 190.528

@felipediel
Copy link
Collaborator Author

Good! Thank you very much for the tests!

@tottka
Copy link

tottka commented Dec 12, 2020

Whatever you decide to do is fine by me. I only use it to tell if a device is currently running or not.

@felipediel felipediel marked this pull request as ready for review December 16, 2020 05:29
@felipediel felipediel changed the title Create SP3S class and fix get_energy() Create SP3S class and improve energy monitoring Dec 16, 2020
@felipediel
Copy link
Collaborator Author

Hi @Amir974. Sorry to bother you. I just remembered that you have a device that will be affected by this change. Do you mind helping me test how your SP2-IL respond to the new sp2.get_energy() method? I need this information to handle the exception correctly in Home Assistant.

@felipediel
Copy link
Collaborator Author

Hi @tottka. Yes, me again. Sorry. Mind checking how your Ankuoo NEO (0x2717) respond to sp2.get_energy()? I need this information to handle the exception correctly in Home Assistant.

@Amir974
Copy link

Amir974 commented Dec 23, 2020

@felipediel with pleasure
(My device does not support energy tracking),
I’ll look into the instructions and get back to you with results

@tottka
Copy link

tottka commented Dec 23, 2020 via email

@felipediel
Copy link
Collaborator Author

felipediel commented Dec 24, 2020

I made a debug script to make it easier. Download the file, open the folder on a terminal and type:

python3 sp2.py 192.168.0.16  # Example device

You can do it on your PC. I need the output.

@Amir974
Copy link

Amir974 commented Dec 26, 2020

@felipediel I might need some assistance to move forward

I am getting the following:

(venv) C:\project>pip3 install git+https://github.com/felipediel/python-broadlink.git@sp3s --force-reinstall
Collecting git+https://github.com/felipediel/python-broadlink.git@sp3s
  Cloning https://github.com/felipediel/python-broadlink.git (to revision sp3s) to c:\users\user\appdata\local\temp\pip-req-build-nkwecwh5
  Running command git clone -q https://github.com/felipediel/python-broadlink.git 'C:\Users\User\AppData\Local\Temp\pip-req-build-nkwecwh5'
  ERROR: Error [WinError 2] The system cannot find the file specified while executing command git clone -q https://github.com/felipediel/python-broadlink.git 'C:\Users\User\AppData\Local\Temp\pip-req-build-nkwecwh5'
ERROR: Cannot find command 'git' - do you have 'git' installed and in your PATH?

@felipediel
Copy link
Collaborator Author

It means you need to install git and add it to the PATH. You can avoid dependencies with that script above. It only needs Python 3 installed.

@Amir974
Copy link

Amir974 commented Dec 26, 2020

@felipediel I hope I got it,
does the following help?

Python 3.8.5 (tags/v3.8.5:580fbb0, Jul 20 2020, 15:43:08) [MSC v.1926 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import broadlink as blk
>>> d = blk.hello("192.168.1.59")
>>> d.auth()
True
>>> d.get_energy()
0.0
>>> 

@felipediel
Copy link
Collaborator Author

That's exactly what we were looking for! Thank you very much @Amir974!

@tottka
Copy link

tottka commented Dec 26, 2020

This is probably not what you are looking for?
Traceback (most recent call last):
File "/home/homeassistant/.homeassistant/xPrograms/sp2.py", line 540, in
main(sys.argv)
File "/home/homeassistant/.homeassistant/xPrograms/sp2.py", line 536, in main
print(d.get_energy())
File "/home/homeassistant/.homeassistant/xPrograms/sp2.py", line 431, in get_energy
check_error(response[0x22:0x24])
File "/home/homeassistant/.homeassistant/xPrograms/sp2.py", line 150, in check_error
raise exception(error_code)
main.CommandNotSupportedError: [Errno -4] Command not supported

@felipediel
Copy link
Collaborator Author

We were looking for the exception too. The devices are different, we need to handle the invalid request for both. Thank you very much @tottka!

jbsky and others added 2 commits January 23, 2021 00:35
* Refactor the LB1 class

* General improvements

* Enumerate bulb color modes

* Clean up encoder

Co-authored-by: Felipe Martins Diel <felipe-diel@hotmail.com>
@felipediel
Copy link
Collaborator Author

Closing to merge into the dev branch.

@felipediel felipediel closed this Jan 23, 2021
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.

None yet

5 participants