[mideaac] Initial contribution- second try

[apella12]

This is a second attempt. The first attempt is closed #17395. Essentially all of the previous comments have been addressed. The main new change is the separation of the connection manager and the MideaACHandler, (largely rewritten by @lsiepel) these two files are basically new. I have worked and tested with this version (made some modifications) for several weeks and it seems to work as well as the first attempt now. Also there were a number of new supporting files added by @lsiepel.

For reference the previous intro is here.
I’d like to put this out for new binding review.
This binding was started by the original author as a forum topic but has since had other contributors. I picked it up when I installed a mini-split AC that used the Midea protocol earlier this year, originally to allow for longer polling frequencies. Then as a retirement challenge (no formal java training), decided to attempt to clean it up for official status. I cleaned up the summary report from over 150 items and believe I conformed to current developer guidelines (Java docs, naming, tests, etc.). Along the way added some additional functionality and corrected some code issues.
The discovery and security classes remain largely unchanged from the original author (who I haven’t been able to contact), and work well. The other classes I understand pretty well at this point and hopefully can address any concerns.
As a note, to conform to the guidelines there are breaking changes in naming from the version I published on the forum thread.

[lsiepel]

This is not forgotten, the time I have to spare is consumed by the ZUI binding at the moment. So don’t wait for me.

[apella12]

@lsiepel Well I'm not really waiting. I'm working on other things too. There shouldn't be much to review after you rewrote the connection manager. I did a few things to make it more robust to run without the A/C connected to the cloud and fixed some things that you left hanging. I have had it polling for months without issues. It is not exactly A/C season, so no one is exactly clamoring for an update. Hopefully you or someone else can have a look in a couple of months as we get towards spring. EDIT: I did update the version to OH5 and change the date to 2025 in the headers

[lsiepel]

Forgot about this one, sorry. I might have some time to review in the next days.
It was tested and all works as expected?

[apella12]

Yes, it is working fine. I have been using it since the last commit. Decided to rebase today in case something in the OH5.0 causes a problem, but it passed checks

[lsiepel]

Thanks for not leaving this PR/binding. It has improved, i also see more room for improvement. Besides some new minor issues that i probably missed before, i think the DTO and connection structure can benefit from some more work and thinking.

[lsiepel]

this logging is not needed when the thing state is set.

Suggested change

logger.warn("Can't retrieve Token and Key from Cloud; email, password and/or cloud parameter error");

[apella12]

See question above

[lsiepel]

Suggested change

logger.warn("Can't retrieve Token and Key from Cloud; email, password and/or cloud parameter error");

[apella12]

Ok in next commit

[lsiepel]

Stop processing and exit immediate when an InterruptException occurs.

[apella12]

As I recall that won't work as I intended (with 3 tries). The exceptions aren't serious and are ephemeral in nature. I haven't had that INFO log message in months (after I went from 2 to three tries. That's why I forgot it was there.

[lsiepel]

An InterruptException usually signals an external call to stop the proces / exit the application /shutdown. And therefor the method should exit fast cleaning up any resources but surely not trying to perform another retry attempt.

[apella12]

I have separated the catches

[lsiepel]

Marked some previous comments as resolved. Found some new ones, but nothing big. Let's slowly continue and narrow the remaining issues down and fix them together.

[lsiepel]

Looks like double logging. I guess this would be better:

Suggested change

	if (!deviceIsConnected) {
	logger.info("Connected to IP {}", ipAddress);
	}
	logger.debug("Connected to IP {}", ipAddress);
	logger.debug("Connected to IP {}: {}", ipAddress, deviceIsConnected);

[apella12]

What the intention here was one INFO for the first time and debug if it was recovering from some problem, but not disconnected. Otherwise there is nothing in the log to show it is running. Note starting date.
2025-01-24 16:45:59.796 [INFO ] [nternal.connection.ConnectionManager] - Connected to IP 192.168.0.246
This is when I updated to the rebased binding
2025-02-19 10:58:07.986 [INFO ] [nternal.connection.ConnectionManager] - Connected to IP 192.168.0.246
This is when I deleted the email and password this morning to recheck that it could be done
2025-02-20 08:54:25.405 [INFO ] [nternal.connection.ConnectionManager] - Connected to IP 192.168.0.246

[apella12]

Added a comment to explain

[lsiepel]

Suggested change

	logger.info("Problem with reading response, skipping {} skipped count since startup {}", command,
	logger.debug("Problem with reading response, skipping {} skipped count since startup {}", command,

[apella12]

ok. I used that to track health, but probably no value to user

[lsiepel]

When an authentication exception occurs, i guess it is permanenet. When a timeout occurs, it is transient and the binding should recover. You might want to handle both cases seperate.

Suggested change

	logger.warn("Unable to proces command: {}", e.getMessage());
	updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());

[apella12]

Ok This was part of the code you rewrote and have never seen an exception, so don't know what else to say.

[lsiepel]

Suggested change

logger.warn("Can't retrieve Token and Key from Cloud; email, password and/or cloud parameter error");

[lsiepel]

Suggested change

logger.warn("Discovery needed, discovering....{}", thing.getUID());

[lsiepel]

Here you provide a lot more information then in the readme. This is what chatgpt makes out of it. Please check and adapt the text so that it gives the user enough guidance to set it up correctly.l

The key parameters—like deviceId, token, key, and version—are automatically discovered once the air conditioner is connected to the WiFi and you’ve provided the necessary cloud and password (unless it's a version 2 device).

However, you’ll still need to manually enter a few parameters such as: Polling time, Socket timeout, Prompt tone.

[lsiepel]

Suggested change

	Frame label="AC Unit" {
	Text item=outdoor_temperature label="Outdoor Temperature [%.1f °F]"
	Text item=indoor_temperature label="Indoor Temperature [%.1f °F]"
	Setpoint item=target_temperature label="Target Temperature [%.1f °F]" minValue=63.0 maxValue=78 step=1.0
	Switch item=power label="Midea AC Power"
	Switch item=temperature_unit label= "Temp Unit" mappings=[ON="Fahrenheit", OFF="Celsius"]
	Selection item=fan_speed label="Midea AC Fan Speed"
	Selection item=operational_mode label="Midea AC Mode"
	Selection item=swing_mode label="Midea AC Louver Swing Mode"
	Frame label="AC Unit" {
	Text item=outdoor_temperature label="Outdoor Temperature [%.1f °F]"
	Text item=indoor_temperature label="Indoor Temperature [%.1f °F]"
	Setpoint item=target_temperature label="Target Temperature [%.1f °F]" minValue=63.0 maxValue=78 step=1.0
	Switch item=power label="Midea AC Power"
	Switch item=temperature_unit label= "Temp Unit" mappings=[ON="Fahrenheit", OFF="Celsius"]
	Selection item=fan_speed label="Midea AC Fan Speed"
	Selection item=operational_mode label="Midea AC Mode"
	Selection item=swing_mode label="Midea AC Louver Swing Mode"

[apella12]

ok

[lsiepel]

Created a new comment with a suggestion. I meant to add indentation not removing a curly brace.

[lsiepel]

An InterruptException usually signals an external call to stop the proces / exit the application /shutdown. And therefor the method should exit fast cleaning up any resources but surely not trying to perform another retry attempt.

[apella12]

In no particular order, 1) I'm not sure how to use the OH standard conversions and where to put the .toLowerCase since I do not see that in the documentation. 2) The exception in the retry I'm trying to address is simply a timeout. 3) Elsewhere, I was trying to avoid breaking something that I fixed earlier would cause me to go back and fix again. It has been months and I'm not recalling all the issues I had getting to the fix. 4) I also attached a log of the DTO actions, it appears to me everything is needed. The only logic is to pull from the cloud responses to compose the next message.

EDIT: Regaring other comments;
First about the blocking method in the initialize(). The issue is that V.3 discovery is two steps. First step is to get the IP, port, deviceId and set the defaults. The V.3 initialization starts but hits a wall until the cloud provider, email and password are entered on the UI page (V.2 OK). That springs the method to get the token and key. We could force textual configuration and the use of a third party platform to obtain the token and key, but that doesn’t seem to fit the OH model.

Second I retested a change to remind myself of the problem. I dropped the return after getTokenKeyCloud(cloudProvider); (line 218) and the initialize() after logger.debug("Token and Key obtained from cloud, saving, back to initialize"); (line 372). The token and key are filled in on the UI, the connection manager is started, but chokes on the authorization. Resaving the thing with a change corrects the problem (I just added a location), but that is messy. With the current code everything works seamlessly when deleting the token and key when the cloud provider, password and email are in place.

[apella12]

Figured out how to leverage the OH core util hex/byte and separated the socket timeout exception from anything else. Seems to still work, but with the magnitude of changes am testing overnight. Have tested scan, retrieve key/token, normal polling and temperature changes. Had one timeout exception that worked as expected. 9 seconds = 4 for timeout and 5 delay for next try.

2025-02-21 17:19:56.320 [DEBUG] [nternal.connection.ConnectionManager] - Disconnecting from device at 192.168.0.246
2025-02-21 17:20:05.326 [DEBUG] [nternal.connection.ConnectionManager] - Socket retry count 1, Socket timeout connecting to 192.168.0.246: Connect timed out
2025-02-21 17:20:05.329 [DEBUG] [nternal.connection.ConnectionManager] - Device at IP: 192.168.0.246 requires authentication, going to authenticate

Normal connection is less that 10ms.

2025-02-21 17:09:53.751 [DEBUG] [nternal.connection.ConnectionManager] - Disconnecting from device at 192.168.0.246
2025-02-21 17:09:53.757 [DEBUG] [nternal.connection.ConnectionManager] - Device at IP: 192.168.0.246 requires authentication, going to authenticate

[lsiepel]

Ok, so i'll try to reboot this review proces, where are we with this PR? Previously we tried to seperate the handler and the connection manager, how did that work out?

[Copilot]

Pull Request Overview

Initial contribution of the MideaAC binding, introducing the core implementation, tests, and OpenHAB metadata for discovery, configuration, and channels.

• Added new binding module and registered it in the parent POM.
• Implemented security, packet/response handling, capability parsing, connection manager, handler logic, and cloud integration.
• Provided comprehensive unit tests and updated thing-types, i18n, and addon metadata.

Reviewed Changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
bundles/pom.xml	Register org.openhab.binding.mideaac module
src/main/java/.../internal	Add core binding implementation (handler, security, parser, packet, response, energy)
src/test/java/.../internal	Add unit tests for security, parser, response, energy, discovery, cloud, configuration
src/main/resources/OH-INF/thing	Define thing-types (channels, configuration)
src/main/resources/OH-INF/i18n	Provide localization properties
src/main/resources/OH-INF/addon	Add addon metadata for discovery and binding

Comments suppressed due to low confidence (2)

bundles/org.openhab.binding.mideaac/src/main/resources/OH-INF/thing/thing-types.xml:275

• The pattern="%d%%" implies a percentage but this channel reports energy in kWh; update to a more appropriate numeric or unit-aware pattern (e.g., %.2f %unit%).

		<state readOnly="true" pattern="%d%%"/>

bundles/org.openhab.binding.mideaac/src/test/java/org/openhab/binding/mideaac/internal/handler/capabilities/CapabilityParserTest.java:152

• [nitpick] Method name testParseWithtemperature does not follow camelCase; rename to testParseWithTemperature for consistency.

    void testParseWithtemperature() {

[Copilot]

The check k > 256 can never be true because k is derived from a byte; consider normalizing with int k = (crcValue ^ m) & 0xFF; and remove unreachable branches.

Suggested change

	int k = (byte) (crcValue ^ m);
	if (k > 256) {
	k -= 256;
	}
	if (k < 0) {
	k += 256;
	}
	int k = (crcValue ^ m) & 0xFF;

[apella12]

Changed this in latest commit

[apella12]

@lsiepel I didn't see your reopening comment. Must not have been notified (or thought it was related to my ZUI nofications)...

To recap; you wrote code to separate the connection manager and the MideaAC handler and I got it working again and addressed all the comments from February 21 and before. Since then I extracted several features from the HA version (and with the help of Copilot in VSC converted them to java). Also there have been a couple of new users of the forum version that had new messages that I incorporated using some Lua code references. Also because of the new features, reworked some part of the code. All that is complete now, but the binding is 25% longer. I was just making a final pass to ensure the ReadMe matched with the additional capabilities.