Regression with minimum temperature in v0.6.6

[BenJewell]

This is in regard to #396. Unfortunately I did not see it before it got merged, and I don't have the ability to reopen that issue, so I'm opening this one to provide further insight.
I did a ton of testing with this about a year ago, but I never submitted PR for my code changes. The information that I did submit, I apparently submitted as an issue on the wrong repo, so it was never seen.

It's much more complicated than it looks at first glance and there are several factors that we need to consider here:

1. Firstly, the change made caused issues when using Fahrenheit instead of Celsius. There is a weird rounding issue with every 9th degree between Fahrenheit and Celsius. For example, the previous value for this constant, 17, translates to 62.6 F, but this gets correctly (or incorrectly, depending on how you look at it) rounded to 62 F, which is the minimum temperature many units support in Fahrenheit mode. The Midea units which support lower temperatures can support down to 60 F or 16 C… BUT, we should set the min temperature constant to 15.5 because this provides Proper compatibility with both Fahrenheit and Celsius (not thoroughly tested in Celsius). This is because 15.5 rounds to 16 C, and in Fahrenheit, 15.5 C converts to 59.9 F, which correctly rounds to 60 F.

Another nuance to this is that with certain hardware versions, the Midea units can be set in both Celsius and Fahrenheit modes separately than the setting used on the physical Midea IR remotes with screens built in, if using such models. For some reason, if you never use the official cloud app, the units will default back to Celsius mode on their own sometimes, so the communications from this integration don't get rounded correctly and everything ends up like a degree off. I analyzed the bits in the packets for multiple temperatures in both modes and was not able to find a way to identify from the unit’s message whether it's expecting the temperature values using the Fahrenheit or Celsius mapping, at any given time. I resolved the issue by simply using my Firewall to block the Midea units from reaching the cloud servers, causing them to lose their defaults. This is outside the scope of this issue, but just something to be aware of. Whatever issue was actually causing it may have been fixed by now.

2. The other thing to keep in mind is that different Midea units have a different minimum temperature. I have three different styles and versions of units and found that Midea decreased the minimum temperature from 62 F to 60 F on many of the newer models. It's also possible with mini-splits to have 2 indoor units on the same system with different possible minimum temperatures. The significance of this is that we can no longer simply declare a constant with the minimum temperature. Some ways we can resolve this, listed in order from most to least ideal in my opinion:

a. Determine a way to use the capabilities response data from the unit to determine the correct minimum temperature value, the same way the cloud servers would.
b. “Test” whether a unit can handle low temperatures by attempting to set the temperature to 60F (16 C) and seeing if it holds, or if the unit forces it to another supported value. Usually 62F (or 86F when a bit rollover in that segment occurs). Then we know whether or not the unit supports low set temps.
c. We could simply provide a parameter for the min (and I guess max while we’re at it) temperature for each unit using a parameter, just like we do with the “Temp Step” parameter already. This would be the easiest fix.

3. The third issue is that there is something that causes the odd/even bit in the set temperature byte to get flipped with values below around 63 F. It's been a while but I remember encountering a specific combination (all zeros I think) that was reserved and skipped before things wrap around. When setting a temperature of 63F (17 C) or lower, we need to apply an offset. I use an if statement to catch this condition:

I am attaching a full copy of the AC device class message.py file that I use with the issue fixed. But it needs some more cleanup of comments and stuff. It's mainly just that one change in the above screenshot. message - Backup.py.txt

Also for reference, here is my example test data, showing where the issue begins:

Thank you for looking into this more and let me know what questions you have.

[wuwentao]

@BenJewell thank you very much for your detail test and issue.

I'm not test these minimum temperature value with my AC device, as it has been power off in my loaction.

if you set it with a NOT supported value, what's the error result and device status or HA error ?

from your description and find a way to identify from the unit’s message whether it's expecting the temperature values using the Fahrenheit or Celsius mapping

Solution 1: for 0xAC device, it's easy to do, if you using Celsius， the value range maybe 16-30, if the value in this range, it MUST be Celsius, so we can convert it to Fahrenheit in range 60-86, so we don't need any flag to check it, just the input value is enough. in fact, midea also using this solution， below is the origin lua script code

local function convert_to_F(temp)
	local tempTable = {}
	tempTable[10] = 50
	tempTable[10.5] = 51
	tempTable[11] = 52
	tempTable[11.5] = 53
	tempTable[12] = 54
	tempTable[12.5] = 54
	tempTable[13] = 55
	tempTable[13.5] = 56
	tempTable[14] = 57
	tempTable[14.5] = 58
	tempTable[15] = 59
	tempTable[15.5] = 59
	tempTable[16] = 60
	tempTable[16.5] = 61
	tempTable[17] = 62
	tempTable[17.5] = 63
	tempTable[18] = 64
	tempTable[18.5] = 65
	tempTable[19] = 66
	tempTable[19.5] = 67
	tempTable[20] = 68
	tempTable[20.5] = 69
	tempTable[21] = 70
	tempTable[21.5] = 71
	tempTable[22] = 72
	tempTable[22.5] = 73
	tempTable[23] = 73
	tempTable[23.5] = 74
	tempTable[24] = 75
	tempTable[24.5] = 76
	tempTable[25] = 77
	tempTable[25.5] = 78
	tempTable[26] = 79
	tempTable[26.5] = 80
	tempTable[27] = 81
	tempTable[27.5] = 82
	tempTable[28] = 82
	tempTable[28.5] = 83
	tempTable[29] = 84
	tempTable[29.5] = 85
	tempTable[30] = 86
	tempTable[30.5] = 87
	tempTable[31] = 88
	tempTable[31.5] = 89

	local temperature = temp
	if(type(temp) == "string")then
		temperature = tonumber(temp)
	end
	return tempTable[temperature]
end

local function convert_to_C(temp)
	local tempTable = {}
	tempTable[50] = 10
	tempTable[51] = 10.5
	tempTable[52] = 11
	tempTable[53] = 11.5
	tempTable[54] = 12
	tempTable[55] = 13
	tempTable[56] = 13.5
	tempTable[57] = 14
	tempTable[58] = 14.5
	tempTable[59] = 15
	tempTable[60] = 16
	tempTable[61] = 16.5
	tempTable[62] = 17
	tempTable[63] = 17.5
	tempTable[64] = 18
	tempTable[65] = 18.5
	tempTable[66] = 19
	tempTable[67] = 19.5
	tempTable[68] = 20
	tempTable[69] = 20.5
	tempTable[70] = 21
	tempTable[71] = 21.5
	tempTable[72] = 22
	tempTable[73] = 23
	tempTable[74] = 23.5
	tempTable[75] = 24
	tempTable[76] = 24.5
	tempTable[77] = 25
	tempTable[78] = 25.5
	tempTable[79] = 26
	tempTable[80] = 26.5
	tempTable[81] = 27
	tempTable[82] = 28
	tempTable[83] = 28.5
	tempTable[84] = 29
	tempTable[85] = 29.5
	tempTable[86] = 30
	tempTable[87] = 30.5
	tempTable[88] = 31
	tempTable[89] = 31.5
	local temperature = temp
	if(type(temp) == "string")then
		temperature = tonumber(temp)
	end
	return tempTable[temperature]
end

solution 2: if device is NOT 0xAC device and temperature range can't match with solution 1, device protocol may have a flag like 0xC3 device, I have add a PR and parse this flag from midea device lua protocol, the source code and flag can be found in midea-lan/midea-local#345 with source code: https://github.com/midea-lan/midea-local/blob/bac0fb1497b3e78cb24ca6e97f4677673c7cabd9/midealocal/devices/cd/message.py#L181 and https://github.com/midea-lan/midea-local/blob/bac0fb1497b3e78cb24ca6e97f4677673c7cabd9/midealocal/devices/cd/__init__.py#L105

in this source code, I have checked this flag and return only Celsius to HA reuslt, if user want to use Fahrenheit, HA will convert it to Fahrenheit.

so we should confirm current issue and confirm the solution for you.

[BenJewell]

Thank you for the follow up.

Regarding the Celsius and Fahrenheit issue, I must have been logging the wrong spot in the code, as I thought it was the Midea units doing the conversion. This info leaves me with more questions than answers I think. But it's not a big deal for me at the moment.

Regarding the issue I mentioned with the bit flip at temperatures at or below 63, I'm probably going to just leave it alone for now since it's working fine for me with the workaround I shared (the IF statement). In addition to explaining what I found, I was hoping to get that merged in so I don't have to revert my fix every time I do an update. Assuming it doesn't cause issues for other people on course.

Regarding the issue I mentioned with the min temperature being different for different types of units, if you make a code change for this, I can help test it with both styles on units I have to make sure it gets handled correctly as 60F or 62F.

If you send a NOT supported value, the units will either set their temperature to the wrong value, or simply do nothing and not change it. Depends on the circumstance.

[wuwentao]

@BenJewell please provide the error details with debug log file and your device SN
also add a bried description with the error info for this debug log file.
we can't edit it with what you did, as it will impact all the user devices, we should support all user devices.

if possible, you can also file a new github issue with all the error description, log file , and close current issue.

[BenJewell]

@BenJewell please provide the error details with debug log file and your device SN also add a bried description with the error info for this debug log file. we can't edit it with what you did, as it will impact all the user devices, we should support all user devices.

if possible, you can also file a new github issue with all the error description, log file , and close current issue.

Thanks. Since this issue is so detailed and complex, it would be a lot easier go over everything during a screen sharing session, as it's difficult to fully explain in text. Perhaps I am just not good at explaining it well.

But the bottom line is that this issue probably affects everyone and most people just haven't noticed it since they don't select such low temperatures. Especially the bit flip issue. And the change that you recently made to the integration made things a little better for some people and worse for others. The different changes I made tries to make it work correctly for everyone.

I have 8 units setup with this. 3 different hardware versions, as described in the first post. So it sounds like to want me to undo any changes I have made, then turn on debug logging for the integration, then try setting the temperatures to different values and letting it jump to the wrong ones (like how 60 F jumps to 64 F after you select it), and sending you that log?

I am not sure that you’ll be able to see the issue this way, since it’s a low level issue and me fixing it required analysis of the binary data, which I don’t think the integration had logging for, before I added it during my testing. But you may be able to see what I mean.

I also forked midea-local to midea-local-ben in Pypi, with the changes made.