Improves light support #155

danielcaceresm · 2024-11-19T18:46:05Z

I added support to 2 lights I owns:

The first one doesn't have colder/warmer commands, it has commands per specific color values. Also, it doesn't have an on command, you just send the brightness and color values and it turns on.
The second one, as the first, it doesn't have colder/warmer commands, it has commands per specific color values. Also, it only has a toggle command for on/off.

Example:

{
    "manufacturer": "Sulion",
    "supportedModels": ["Marina S"],
    "supportedController": "ESPHome",
    "commandsEncoding": "Raw",
    "brightness": [26, 51, 77, 102, 128, 153, 179, 204, 230, 255],
    "colorTemperature": [
      2700, 4600, 6500
    ],
    "commands": {
      "off": "",
      "brighten": "",
      "dim": "",
      "colorTemperature": {
        "2700": "",
        "4600": "",
        "6500": ""
      }
    }
  }

Enhancements:

Added support for CMD_COLORTEMPERATURE and CMD_BRIGHTNESS commands to control color temperature and brightness, allowing specific commands per brightness or color values instead of up/down steps.
Updated async_turn_on method to handle cases where CMD_POWER_ON is not available by setting the last known color temperature and brightness.
Updated the async_turn_off method to check the current state before sending the power-off command.

Bug Fixes:

Use the delay config value when sending the command.
Fix the brightness step calculation to use the correct value

…teps

make-all

Looks good to me

litinoveweedle · 2024-12-05T03:49:12Z

Hello @danielcaceresm and @make-all, as I do not have any IR controlled light I completely depend on you and your testing. ;-) So If you say it is OK, I will merge it. ;-)

danielcaceresm · 2024-12-05T15:10:13Z

Hello @litinoveweedle! I have these changes working with my lights from I made the PR without any issue. I don't have lights using the colder/warmer commands and I can't test the old behavior but I tried to not modify it.
The only change that can 'affect', but I think it is a bug fixing, is that this PR adds the handling of the delay from config (I need it to get my lights working correctly), but the old behavior can be achieved setting this value to 0.

make-all · 2024-12-05T23:59:16Z

I just did some testing, and it appears the color temperature adjustment is acting strangely for small adjustments. It seems to yoyo between colder and warmer, and reaching the end (where multiple commands are sent to resync) when the HA control is half way.

make-all · 2024-12-06T00:28:30Z

I think I found the issue:

2024-12-06 09:19:12.631 DEBUG (MainThread) [custom_components.smartir.light] Changing color temp from 4811K step 5 to 4670K step 5
2024-12-06 09:19:12.631 DEBUG (MainThread) [custom_components.smartir.light] Sending on remote command 1 times.

The code is sending an "on" command when the color temperature lookup results in no change needed. This has the side effect of "memory recall" on my light. It is also happening with brightness.

litinoveweedle · 2024-12-06T08:20:21Z

By coincidence I have recently related discussion. Overall the SmartIR is assumed state device (as we do not know state of the device). We can take two ways - be optimistic (trust that real device is in the state as reported by HA) or pessimistic (send always command to converge device to the required state). I am probably going to implement config switch to be able to implement both option so user can decide per device which approach to use. This feature is originally aked for media_player class, but to keep SmartIR consistent I will implement it for every class - lights as well.

This would then solve the mentioned issue - if you can set mode to optimistic. Will this work fork you?

litinoveweedle · 2024-12-06T08:41:31Z

@danielcaceresm Thank you once more for the contribution. As the light device file becomes more versatile ( and complex ;-) ), could you please also try to document format (based on you example) in the CODES_SYNTAX.md. I am also open to splitting this file into fourth - one for each class, as it rather seems growing fast. ;-)

make-all · 2024-12-06T11:24:19Z

My lights do get out of sync, so "optimistic" is not exactly what I need. I added code to give extra commands to make sure it is at the end of the range for brightness and color temperature when reaching what HA thinks is the end of the range to deal with the sync problem for example. I guess if someone has a light that wraps instead of no-op when a remote up/down button tries to go beyond the range, they would want to turn that off, so that toggle would still be useful.

But I think turning the light on when we think it is already on should only be done when turn_on is called without arguments. That is effectively what the old code did, as it recorded "done_something" whenever processing the brightness and color_temp arguments, regardless of whether they resulted in a command being sent. But in this change it was modified to only record "done_something" when a command was actually sent, so it is falling through to send the on command when the brightness/color_temp is moved by increments smaller than the light's supported step size. This triggers when using adaptive lighting for example.

make-all · 2024-12-08T11:24:58Z

custom_components/smartir/light.py

+                    # commands to go the full range.
+                    if new_color_temp == len(self._colortemps) - 1 or new_color_temp == 0:
+                        steps = len(self._colortemps)
+                    did_something = True


This line (279) needs to move back up to above line 267, to avoid sending "on" commands when a color temp is given but does not result in a change.

Moved the assignation to line 245, affecting the both branches of the if.

make-all · 2024-12-08T11:31:31Z

custom_components/smartir/light.py

+                            or new_brightness == 0
+                        ):
+                            steps = len(self._brightnesses)
+                        did_something = True


This line (331) needs to move back to above line 316, actually it could move back up to above line 298 where it was previously and the one on line 307 removed, since after the change both branches of the if statement are setting it unconditionally.

The one on line 290 in the original code (aligned with 307) was left over from an earlier version where I originally did exactly what you did here, and has probably misled you about the intention. It should have been removed when it was moved to line 270 in the original code.

Moved the assignation to line 291, affecting the both branches of the if. Thanks!

danielcaceresm · 2024-12-08T19:27:10Z

@danielcaceresm Thank you once more for the contribution. As the light device file becomes more versatile ( and complex ;-) ), could you please also try to document format (based on you example) in the CODES_SYNTAX.md. I am also open to splitting this file into fourth - one for each class, as it rather seems growing fast. ;-)

@litinoveweedle I modified the file adding the documentation for the light format. Please let me know if any adjustments are needed.

make-all

Behaves OK now for the up/down controlled lights

danielcaceresm added 5 commits November 19, 2024 19:01

light using delay when command is sent

760b2dc

fix brightness step

c70c60d

allowing commands per brightness or color values instead of up/down s…

9d6a303

…teps

turning on the device without on command setting the last values

814993a

avoiding to emit the off command when the state is off

4175073

make-all approved these changes Nov 29, 2024

View reviewed changes

make-all reviewed Dec 8, 2024

View reviewed changes

danielcaceresm added 2 commits December 8, 2024 19:29

fix did_something

16af690

light format documentation

25bb9a3

make-all approved these changes Dec 8, 2024

View reviewed changes

make-all approved these changes Dec 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improves light support #155

Improves light support #155

danielcaceresm commented Nov 19, 2024

make-all left a comment

litinoveweedle commented Dec 5, 2024

danielcaceresm commented Dec 5, 2024

make-all commented Dec 5, 2024

make-all commented Dec 6, 2024 •

edited

Loading

litinoveweedle commented Dec 6, 2024

litinoveweedle commented Dec 6, 2024

make-all commented Dec 6, 2024

make-all Dec 8, 2024

danielcaceresm Dec 8, 2024

make-all Dec 8, 2024

danielcaceresm Dec 8, 2024

danielcaceresm commented Dec 8, 2024 •

edited

Loading

make-all left a comment

Improves light support #155

Are you sure you want to change the base?

Improves light support #155

Conversation

danielcaceresm commented Nov 19, 2024

Enhancements:

Bug Fixes:

make-all left a comment

Choose a reason for hiding this comment

litinoveweedle commented Dec 5, 2024

danielcaceresm commented Dec 5, 2024

make-all commented Dec 5, 2024

make-all commented Dec 6, 2024 • edited Loading

litinoveweedle commented Dec 6, 2024

litinoveweedle commented Dec 6, 2024

make-all commented Dec 6, 2024

make-all Dec 8, 2024

Choose a reason for hiding this comment

danielcaceresm Dec 8, 2024

Choose a reason for hiding this comment

make-all Dec 8, 2024

Choose a reason for hiding this comment

danielcaceresm Dec 8, 2024

Choose a reason for hiding this comment

danielcaceresm commented Dec 8, 2024 • edited Loading

make-all left a comment

Choose a reason for hiding this comment

make-all commented Dec 6, 2024 •

edited

Loading

danielcaceresm commented Dec 8, 2024 •

edited

Loading