Set temperature of 63F gets rounded wrong and sent as 62F instead (Fix Included)
Closed this issue · 7 comments
One issue I've always had using this integration in Fahrenheit is that the Midea AC units don’t take the temperature value of 63 F correctly. If you try to set to 62, 64, or any other option, it works fine, but if you select 63, the screen on the unit and the official app both get a value of 62 instead. There's no way to set 63 through Home Assistant, only the official app.
Another issue with this library is that it does not actually get the available temperature RANGE attributes from the unit. Some Midea units have a minimum set temperature of 60 F and some have a minimum of 62 F. This library has the minimum hard-coded as 62 F (17 C). In my instance, I modified it to allow use of the lower values that some of my units support. If you would like to try and reverse engineer this and add this feature, I can get some packet captures from both models of units for comparison and send them. If not, being able to manually set a custom minimum and maximum limit for each unit would be okay as well.
Anyway, back to the main issue of 63 F not working. I spent all day really digging into the code and trying to figure out how it worked, and I identified the issue. The last 5 bits of the 2nd byte of the body are for the set temperature in Celsius. For some strange reason, the Midea units expect 17 (Binary 10001) to get a set temp of 63 F. Following the Bitwise operator pattern, we would expect the Midea units to take 1 (Binary 00001) for a set temperature of 63 F.
So basically, the odd/even bit gets flipped. I was not sure of a way to add this exception to the bitwise expression, so I simply added an IF statement in the “message.py” file to check for 17.0 and change it to 17.5 before passing into the bitwise, in order to get the correct output:
As you can see in my little debug table, 63 F being converted directly to Celsius is 17.2222 C, which technically rounds down to 17, but we need it to round up to 17.5 in this case. My guess would be Midea reserved this number 1 for some other purpose and therefore could not use it for this value. This guess is based on the fact that this rounding error does NOT occur with OTHER Fahrenheit values whose rounding ends in .2222, such as 72 F (22.2222) and 81 F (27.2222).
I did NOT submit a pull request for this fix yet because:
- There may be a better way to implement it.
- It would possibly break stuff for users that are using Celsius instead of Fahrenheit, so may need to add a check for that.
Please let me know if you need any other info.
I found out that the modification I posted did not fully fix the issue. There were several other places in the code that needed to be modified as well (Such as the different types of messages that the HVAC units send out). Please let me know if you need me to share a copy of the fixed code I have. It's not well written, but it works.
17 C being the minimum is a problem for me, i tried to set the temperature_min to 16, yet it still won't propely accept it.
Could you share your code @BenJewell ?
Sure, I can this weekend. Are you using Home Assistant in Fahrenheit or celsius?
Oops, I opened this bug report on the wrong repo, lol. I use this library: https://github.com/georgezhao2010/midea_ac_lan
So not sure how much I can help unless you want to switch integrations, but here is my code for the bit exceptions in the 60-63F range, if it's any help.
I have noticed this issue with certain temps not being able to be sent correctly as well. The midea app does work, but something with this is not converting something correctly. I always thought because home assistant runs as "C" in the background and just does conversions to F for display and input, as I have also run into this with other integrations as well.
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.