"Color" feature depends on "Brightness" feature

[fwaggle]

The problem

My wife bought these "RGB" christmas lights, they're Mirabella Genio 200 LED "Colour Wheel Fairly Lights". They mostly work, the configuration modes for the various scenes are a pain and they play a bit fast+loose with the definition of "RGB" (despite the fact that they're specified in HTML style hex-triplets, you're limited to about 7 specific colors in the scenes), but my main concern is that the "set any single color" mode doesn't work, and I believe this is because of an incorrect assumption in localtuya.

Namely, that SUPPORT_COLOR being enabled implies SUPPORT_BRIGHTNESS, but these lights do not follow that - they don't have a brightness datapoint, but they do have a color one. See attached later.

Environment

• Localtuya version: Updated to git master, commit hash is 64dcb15
• Last working localtuya version (if known and relevant): None.
• Home Assistant Core version: 2022.9.7
• Are you using the Home Assistant Tuya Cloud component ?
• Are you using the Tuya App in parallel ?

Steps to reproduce

1. Add the device to HA using the cloud import, set only the DP 20 (on/off), and it works fine. I can poke values at the other DPs using the set_dp service without issue.
2. If you add the device and specify the color DP (24) as well, it adds, but immediately goes unavailable with two error messages in the logs (see attached).
3. There's no DP on the device for brightness, I've enumerated the rest of the DPs, they're all in use for other things (programming scenes, etc). I tried forcing the typical one (22) too, no luck.
4. If I edit the code in light.py to force brightness to some level if it's not present, the color picker works correctly.

Configuration configuration.yaml or config_flow

Not sure what config to put in here, I didn't configure anything manually it's all done via the HA front-end.

DP dump

      "model": "Smart RGB II",
      "dps_strings": [
        "20 (value: True)",
        "21 (value: colour)",
        "24 (value: 001e023b0164)",
        "102 (value: ffff3207ff000000ff00ffff000000ffff00ff00ffffffffff)",
        "103 (value: ffff3207ff000000ff00ffff000000ffff00ff00ffffffffff)",
        "104 (value: ffff0107ff000000ff00ffff000000ffff00ff00ffffffffff)",
        "105 (value: ffff3207ff000000ff00ffff000000ffff00ff00ffffffffff)",
        "106 (value: 00000100000000000000000000000000000000000000)",
        "107 (value: 00003200000000000000000000000000000000000000)",
        "108 (value: 00320000)",
        "109 (value: 00640400ff00ff00ffffffffff000000)",
        "110 (value: 00010601ff0000ffff0000ff0000ffff0000ffff00ffffffff)",
        "111 (value: 00320700ff0000ffff0000ff0000ffff0000ffff00ffffffff)",
        "112 (value: 0200020000ffff00ff00)"
      ],
      "product_key": "keyfkrfu7f9k55j7"

Provide Home Assistant taceback/logs

2022-10-01 14:56:41.254 ERROR (MainThread) [homeassistant] Error doing job: Exception in callback _SelectorDatagramTransport._read_ready()
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/local/lib/python3.10/asyncio/selector_events.py", line 1027, in _read_ready
    self._protocol.datagram_received(data, addr)
  File "/config/custom_components/localtuya/discovery.py", line 70, in datagram_received
    self.device_found(decoded)
  File "/config/custom_components/localtuya/discovery.py", line 79, in device_found
    self._callback(device)
  File "/config/custom_components/localtuya/__init__.py", line 142, in _device_discovered
    device = hass.data[DOMAIN][TUYA_DEVICES][device_id]
KeyError: 'DEVICE_ID_OMITTED'
2022-10-01 14:56:42.029 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 520, in async_update_ha_state
    self._async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 573, in _async_write_ha_state
    attr.update(self.state_attributes or {})
  File "/usr/src/homeassistant/homeassistant/components/light/__init__.py", line 926, in state_attributes
    data[ATTR_BRIGHTNESS] = self.brightness
  File "/config/custom_components/localtuya/light.py", line 189, in brightness
    return map_range(
  File "/config/custom_components/localtuya/light.py", line 97, in map_range
    mapped = (value - from_lower) * (to_upper - to_lower) / (
TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'

Additional information

Here's a diff of the changes I made that make the color wheel mode work, but are obviously not the correct way to handle it. I originally wrapped it in a try/except because of the TypeError, but doing so throws an exception elsewhere because self._brightness is packed into the values.

So this patch isn't applicable, it's just showing my thought process for below.

--- localtuya/custom_components/localtuya/light.py        2022-10-01 04:36:19.484557030 +0000
+++ localtuya/custom_components/localtuya/light.py      2022-10-01 05:20:50.463398553 +0000
@@ -186,9 +186,15 @@
     def brightness(self):
         """Return the brightness of the light."""
         if self.is_color_mode or self.is_white_mode:
-            return map_range(
-                self._brightness, self._lower_brightness, self._upper_brightness, 0, 255
-            )
+            if not self._brightness:
+                self._brightness = 1000
+            try:
+                return map_range(
+                    self._brightness, self._lower_brightness, self._upper_brightness, 0, 255
+                )
+            except TypeError:
+                return None
         return None
 
     @property

So I'm not sure how to fix this, it seems like SUPPORT_COLOR needs decoupling from SUPPORT_BRIGHTNESS, and some arbitrary value picked for brightness if polling it is unavailable or if the DP isn't configured. At the very least not having it throw an exception so the device still works and doesn't go unavailable if the color DP is set and the brightness one isn't seems appropriate. Unfortunately either of these solutions is probably beyond my abilities at the moment, but I'll take a look at it if no one smarter has a better idea.

I'm thinking that maybe in map_range it should check if self._brightness is None (and not just zero), and if so it should maybe assume a value (maybe upper_brightness?), that'd probably get the device working without breaking a bunch of stuff, but I'm not sure of that.

Any suggestions are welcome.

[fwaggle]

Good news for anyone else who buys these - frankly disappointing - RGB christmas lights. I can get it to work without any code changes on localtuya 64dcb15.

You must set the following DPs for RGB mode to work correctly:

• ID (on/off) is 20.
• Color mode is 21
• Color is 24

This clears all the hurdles needed to get self._brightness derived from the HSV color data point, and everything "just works". If you set color without the color mode data point, the device will immediately go unavailable, which I think is pretty user-hostile behaviour so I'll leave the issue open, but I'm almost certain I'm not smart enough to fix this as it'll require some rethinking of at least one of the functions in light.py.

In case anyone (including future me) wants to take a crack at it, I'll leave my notes here:

• self._brightness is initialized as None. If you're not using the color datapoint, this is fine as it's never referenced.
• If you enable the color DP, self._brightness is referenced, causing map_range to raise an exception because you can't do math between an int and None.
• In status_updated(), none of the things that are supposed to set self._brightness execute, because it depends on either having a brightness DP, or if color DP is set, is_white_mode needs to be False (is_white_mode returns True (because __get_color_mode defaults to MODE_WHITE).

So I think this is pretty user-hostile because I'm fairly sure you could accidentally get into this situation with a supported bulb just by plugging in some DPs but not all when configuring the device? But anyway what I think the code should do in order to do it's best:

• Try to derive the brightness from HSV/RGB even if white mode's on? I'm guessing this check's there so it doesn't accidentally clobber the brightness pulled from the brightness DP, but I think that's trivially fixed by moving lines 406 and 407 after line 409, or just checking if the brightness DP is available.
• If self._brightness is referenced in map_range and it's None, try to pick a default value that's sensible to avoid an exception... not sure if picking the max or something else would be appropriate though.

[dhutchison]

I've managed to hit this same case with a Galaxy Projector lamp. It's weird as nothing in the Smart Life app seems to allow changing back to that "white" colour mode (and it doesn't really make sense with what the device exposes). Changing the brightness in Smart Life changes the colour mode to "color" then the entity becomes available again.

Going to take a look at your suggestion as there is definitely something weird in that bit of logic.

[CloCkWeRX]

#337