litinoveweedle / SmartIR

⏻ Control Home Assistant climate, fan, media_player and light devices via IR/RF controllers (Broadlink, Xiaomi, MQTT, LOOKin, ESPHome, ZHA)
MIT License
86 stars 28 forks source link

Not sending "off" and "on" command if those are same and device is already in the required assumed state #70

Closed klimofey closed 2 months ago

klimofey commented 2 months ago

Problem In my AC commands for turning "on" and "off" are same, so when I change the state of AC (temperature for example) - "ON + HVAC + TEMPERATURE" turns my AC off, if AC is on now and turning on if AC is off now.

Solution If ON and OFF commands in json are same, don't send those command if power state is not changing. If need to turn on from off current state - send command for turning on and after 5 sec send command for changing HVAC + temperature If need to turn off - just turn off if need change state without changing power state - not send all commands

Describe alternatives you've considered Add possibility to use in temperature field values: string like common IR code and object at the same time { "on": "dkawmdlkwamdlkawmdlawdlmaw", "regular": "wadmpp9iunjonj" }

Additional context My AC - Electra RC3 IR remote

javierugarte commented 2 months ago

Hello! I have the same problem and I'm surprised that there aren't more people experiencing it. Usually, the on/off button is the same.

If you want, I can also support you with any necessary tests.

Best regards,

litinoveweedle commented 2 months ago

Well this is actually not very common for HVAC units.the physical IR controller have very often one button, but it actually cycles between two different commands, where the on one is actually sends comple state of the device. But anyway I am planning to add this into ongoing release...

litinoveweedle commented 2 months ago

Hello, initial implementation is in this branch. You can install latest beta using HACS and then replace only climate.py from the branch above.

Please test it and report back. If any issues are found please attach debug log.

javierugarte commented 2 months ago

It works perfectly! All the tests I have done work well.

The only thing is, when I applied the beta version, I got an error saying that the property "temperatureUnit" does not exist in my custom_codes file. Could a default value be set to prevent failures for people who already have custom_codes created? Although the error message is descriptive, I'm not sure if the average user will read it.

Best regards and thanks! I will be looking forward to the final version.

litinoveweedle commented 2 months ago

Hello, thank you for testing. I am glad it worked as expected.

Regarding the default value for the Temperature Unit, There are many syntax checks added, so I kind of expect a lot of problems from users not reading the docs. I moved all the syntax checks from the climate.py into dedicated shared class, so now it raise error even before it reaches climate and failover default could be set. I fixed all (well most) of the shipped codes. So if the user was bright enough to create own device files, hopefully he will be able to find the error. But I will probably do some "breaking changes' stuff on the pages...

Anyway branch was merged with the master and I will probably release one more beta before new version release. I would appreciate you to retest it and point to other potential breaking changes ;-)

litinoveweedle commented 2 months ago

The fix was release in the latest beta