Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Light fixes and BLE bulbs support #425

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

Lurker00
Copy link

55482aa: Any bulb added with the current LocalTuya version, has empty CONF_SCENE_VALUES list.

ca3af1c: There were no custom scenes, actually, when I introduced general effect controls (Default, Color, Scene, Music). Now you've implemented it, and these controls should be added to the top of the list as well.

Now to the main feature: BLE bulbs support.

According to Tuya documents, they are always write-only. So, I've added "Write-only DPs (usually BLE bulbs)" control to the second page of the ligth configuration. Before (#309) I used DP value 0 for this purpose. The rest is the same.

I've also implemented scene selector for these bulbs, and it works! It would be even better if DP12 ("scene_data_raw") be selected automatically, but I don't know how to implement it in LocalTuya. I've selected it manually.

Here is diagnostics of one of such bulbs.

localtuya-944c1616039a1d1ec6b9745179bff74e-КЛ1_ LED BULB B5K-1ae4e56ffcf36ae73a5bd7ddf02dd49e.json

@Lurker00
Copy link
Author

With these changes, it would be good to auto-select DP 11 (color) and DP 12 (scene) for such bulbs in the configuration, but I don't know how to do it!

@xZetsubou
Copy link
Owner

xZetsubou commented Dec 17, 2024

I have actually tried to implement devices write-only detection before before but left it halfway, bear with me a little bit 😓

If the devices such as "BLE bulbs" doesn't update at all why do we need this condition for? status_updated shall not be called.

if self._write_only: return

Because by default dp_value will return none if dp is empty.

if so isn't the logic will detect if device is write only?

"0" in self._device_config.manual_dps.split(",") and self._device._status != RESTORE_STATES

according to the old PR

the BLE gateway which controls the group, notifies about status changes for the first bulb in the group

Does this affect BLE bulbs as well? and this what caused the issue.

The current Light implementation in LocalTuya relies on the bulb state at least here

this condition:
if not self.is_on: false can be something like None if self._state is None else self._state

edit:

Also now that brightness/colour values are always left None why not use home-assistant to store there states? if they changed via another instance or app it will stay on the latest remembered state. leaving them empty is more accurate at the point?

@Lurker00
Copy link
Author

Lurker00 commented Dec 17, 2024

if so isn't the logic will detect if device is write only?

I'm not sure this logic will always work. But I remember you were against using DP 0 for any other purpose. That's why I've implemented the switch and built the logic around it.

Does this affect BLE bulbs as well? and this what caused the issue.

Yes: this is exactly the case with BLE bulbs causing problems due to inconsistency of its DP states.

this condition:
if not self.is_on: false can be something like None if self._state is None else self._state

Sorry, I don't understand why it would be any better.

Also now that brightness/colour values are always left None why not use home-assistant to store there states?

Keeping the brightness is already implemented in the MR, self._brightness is set:
https://github.com/Lurker00/hass-localtuya/blob/d2de2e6d3f7331c8c5605988f0f52daac474463e/custom_components/localtuya/light.py#L473-L474

I've also implemented keeping the color mode, to make brightness control work for a color, but didn't test it yet. Will test and commit in a day.

Actually, keeping these values is required only to make HA Light control to work smooth and consistent way while a user uses it, and only for the color mode.

@xZetsubou
Copy link
Owner

I remember you were against using DP 0 for any other purpose

Yes and not, because I did mention the "0" DP was added to "restore state" not "write_only devices" ,

old PR:
the "0" will affect all devices originally and probably will be back that "0" should be used to restore old status

This is why I added self._device._status != RESTORE_STATES.

so if it works for both cases I wouldn't mind, however the old PR changed the purpose of 0 from restoring entities to BLE "lights".

What about other BLE devices are all of them behave the same? are all of them write_only? We can't be sure this is why I tried to implement a logic to works for both and try to detect automatically but as said I stopped halfway.

Sorry, I don't understand why it would be any better.

because you mentioned this in the other PR, if not then it will always consider is "OFF" so I suggested this.

Yes: this is exactly the case with BLE bulbs causing problems due to inconsistency of its DP states.

So to clarify the BLE lights can be updated If it's the first device on the group of smart life?

Keeping the brightness is already implemented in the MR, self._brightness is set:

I meant even if the integration reloaded brightness will return None, but we can restore old state.

@Lurker00
Copy link
Author

OK, here it is 😄 I had to implement full control for BLE bulbs this way, because the HA Light UI control does not reflect the state of a bulb that is not turned on. Also, the bulb state shall go through the whole data flow.

By the way, async_turn_on() and async_turn_off are called from a HA thread, meaning, it is not good to change any data that can be accessible from LocalTuya MainThread. But the current implementation works no different from any other LocalTuya device.

But I failed to implement auto-configuration of DP 11 and DP 12! I tried this:

    # Light
    # https://developer.tuya.com/en/docs/iot/categorydj?id=Kaiuyzy3eheyy
    "dj": (
        LocalTuyaEntity(
            id=DPCode.SWITCH_LED,
            name=None,
            color_mode=DPCode.WORK_MODE,
            brightness=(DPCode.BRIGHT_VALUE_V2, DPCode.BRIGHT_VALUE),
            color_temp=(DPCode.TEMP_VALUE_V2, DPCode.TEMP_VALUE),
            color=(DPCode.COLOUR_DATA_V2, DPCode.COLOUR_DATA_RAW, DPCode.COLOUR_DATA),
            scene=(DPCode.SCENE_DATA_V2, DPCode.SCENE_DATA_RAW, DPCode.SCENE_DATA),
            custom_configs=localtuya_light(29, 1000, 2700, 6500, False, True),
        ),

in the core/ha_entities/lights.py to no avail. Do you know how to do it?

@xZetsubou
Copy link
Owner

xZetsubou commented Dec 19, 2024

Add property or method into coordinator.py class

    def is_write_only(self) -> bool:
        """Return true if the device is connected and has no status."""
        return self.connected and self._status == RESTORE_STATES

In entity.py property

@property
def is_write_only(self) -> bool:
    """Return true if the device is connected and has no status."""
    return self._device.is_write_only()

In lights module we can do the check.

if self.is_write_only:
    # yes

This should works without needing for CONF_WRITE_ONLY selector. And this will handle the the BLE devices if auto-setup-entities. otherwise manual config will be necessary.

But I failed to implement auto-configuration of DP 11 and DP 12! I tried this:

It should works fine unless the DP is missing in dps strings list. and probably that dp 12 and 11 is missing because they don't have any values in cloud data, try adding 11, 12 in manual dps it probably will works.


edit: also custom scenes shall return to same as before and user can add the modes manually instead.

@Lurker00
Copy link
Author

@xZetsubou Thank you! Could you please suggest how to find out which DP name was used for a particular device? E.g. we have

            scene=(DPCode.SCENE_DATA_V2, DPCode.SCENE_DATA_RAW, DPCode.SCENE_DATA),

and the data format for a scene depends on whether it is SCENE_DATA_V2, or SCENE_DATA_RAW or SCENE_DATA.

@@ -44,6 +44,7 @@
from .coordinator import pytuya, TuyaCloudApi
from .core.cloud_api import TUYA_ENDPOINTS
from .core.helpers import templates, get_gateway_by_deviceid, gen_localtuya_entities
from .core.ha_entities.base import DPCode
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to make exceptions for DPS instead remove the current condition for now. If there are DPS without value and can be used then the condition is not always accurate.

Comment on lines 1116 to 1120
if add_dp and (
(value := func.get("value"))
or value is not None
or __is_special_dp(dp, cloud_dp_codes)
):
Copy link
Owner

@xZetsubou xZetsubou Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if add_dp and (
(value := func.get("value"))
or value is not None
or __is_special_dp(dp, cloud_dp_codes)
):
if (value := func.get("value", "null")) or add_dp:

@xZetsubou
Copy link
Owner

Could you please suggest how to find out which DP name was used for a particular device?

I wouldn't say it's impossible but the method depends on what are the use cases?

@Lurker00
Copy link
Author

Could you please suggest how to find out which DP name was used for a particular device?

I wouldn't say it's impossible but the method depends on what are the use cases?

Color and scene data format depends on that. They must be chosen by DPCode value for a DP, not by a heuristics.

Remember my Zigbee bulbs have DP 6 for the scene data!

@xZetsubou
Copy link
Owner

xZetsubou commented Dec 20, 2024

Without cloud won't be possible to rely on DPCode unless the tweak made in auto-configuration which isn't planned.

I bought BLE bulb however buy GW doesn't seems to connect to it I'll look into it later.


After more thoughts write_only should be implemented if is_ble instead and 0 check can works.

in coordinator.py we can add this property only

    @property
    def is_ble(self):
        """Return if this sub-device is BLE."""
        # BLE bulbs doesn't have status, we can rely on status to detect that but this workaround works fine.
        # we can also add status check this way even if somehow 0 was added by mistake it still works.
        return self.is_subdevice and "0" in self._device_config.manual_dps.split(",")

in lights.py

self._is_bluetooth = getattr(self._device, "is_ble", False)

@Lurker00
Copy link
Author

Lurker00 commented Dec 22, 2024

First of all, sorry for the massive number of commits, partially reverted or rewritten!

Implemented:

  1. Adding all the DPs provided by the cloud, regardless of their values.
  2. Detecting color and scene data formats from DP codes, if cloud information is available. If it is not, the old (partially broken) logic is still used.
  3. Detecting of write-only DPs from the cloud attribute. It may be useful in the future, not only for BLE bulbs.
  4. Newly added write-only bulbs are detected by write-only state DP. For existing bulbs, not to force users to re-add their bulbs, cloud source of this DP is used, instead of check for DP 0.

Also added more scenes for modern bulbs (scene_data_v2), including from your post. This should close the #392.

edit: also custom scenes shall return to same as before and user can add the modes manually instead.

Actually, SCENE_CUSTOM is redundant and misleading:

  • It can't be re-selected: once another Effect is selected, the SCENE_CUSTOM line is removed from the list.
  • Selecting "Mode Scene" Effect exactly means "the effect that was loaded into the bulb last time", i.e. the same as SCENE_CUSTOM, but it is better, because it can be re-selected.

I bought BLE bulb however buy GW doesn't seems to connect to it I'll look into it later.

Yes, I had a BLE-capable gateway (very expensive!) that didn't not allow to attach bulbs to it. That's why I purchased another BLE-only dirty cheap gateway as a reserve.

@xZetsubou
Copy link
Owner

Thanks for reply.

I can't merge new changes with the way it made, This is only working for Cloud users yes this will works without cloud later however If I extract a template of the device and move it into non-cloud hub this would broke even if it's the same device.

DPCode/ha_entites module shouldn't be used either in coordinator.py or entity.py I made these for auto-setup entities after that e.g. ha-platforms initialization cannot.

It's true that detecting the code would help us in the future as well but this would makes us implement more features for cloud users, However these should only helps users the entity setup config flow.

Now this can still be made without relying on cloud at all, I have been kidna busy with RF remote support

Actually, SCENE_CUSTOM is redundant and misleading:

I meant this line would always force modes even if I choose custom_scenes.

self._scenes = {**self._modes.as_dict(), **self._scenes}

My BLE bulb does reports the state when it goes off/on this line broke it by false updates since it assumes that BLE did received the message.

                if self.write_only:
                    # The device never replies, process its status change now
                    self.status_updated(payload)

If you don't mind I'll try to refactor this to makes it works for both cloud/non-cloud then you can review.

@Lurker00
Copy link
Author

This is only working for Cloud users

I thought the checks:

                scene_code = self.dp_code(CONF_SCENE)
                if scene_code is None:

and

            color_code = self.dp_code(CONF_COLOR)
            if color_code is None:

exactly replicate the old behaviour. Yes, a bulb added via template will not work exactly as with information from the cloud (my Zigbee bulb has incorrect effects list), but it is expected. What are other problems do you see here?

My BLE bulb does reports the state when it goes off/on

Is its DP reported as write-only by the cloud?

this line broke it by false updates since it assumes that BLE did received the message.

How does it breaks? If the hub reports state change, then, with those lines, then its state are updated twice, which should not be a problem.

I meant this line would always force modes

It only adds mode switches, e.g. to pre-loaded color or scene, or to white. How does it hurt?

If you don't mind I'll try to refactor this to makes it works for both cloud/non-cloud then you can review.

OK, let's go this way! I'm really curious what did I miss.

@Lurker00
Copy link
Author

@xZetsubou I think I realized what did you mean: I should keep that "Write-only" checkbox for LocalTuyaLight, and in flow_schema(dps) of light.py take the default value for the checkbox from the dps parameter, right? That would work for any case, but I don't see a similar behavior for CONF_MUSIC_MODE checkbox... If this is what you want to implement and you know how to do it, that would be great!

@Lurker00
Copy link
Author

Lurker00 commented Dec 24, 2024

Why not to put additional information into templates: DP codes, write-only?

@xZetsubou
Copy link
Owner

Why not to put additional information into templates: DP codes, write-only?

Devices may share the DP / dp type, but doesn't mean they also share access-mode, And technically I added that feature for non-cloud purpose.

I thought the checks:

I don't prefer to rely on at all as I mentioned I don't want to keep rely on it, Add conditions in entity modules that would works for cloud wouldn't makes sense for this localtuya, I would prefer to make user insert config manually then do that 😅

Additionally: The cloud API doesn't means that it will always works for all users or all the time, most of DP names and access-mode data require API subscription which is sometimes user will have to contact Tuya support so they enable it for them.

Now there is something I wanted to do before which is configure entity based on it returned status a function that would be called once on connection made, I haven't had device to test this on it, but it's now BLE would works.

We have three cases.

  • Sub-device and "0" in manual DPS We aren't sure yet that all BLE bulbs reqiure 0 to be added but for now all of them does.
  • Scenes can be detect by it's present and it's length.
  • Color as well can be detect by it's present and format.

To keep with updates so far everything works fine with the past few days of testing, my bulb for some reason doesn't switch to while mode.

Brief:

  • Added BLE scenes.
  • BLE will store values and restore it, brightness wasn't working unless color value present.
  • important status will be cached.
  • Added connection made function to configure entity based on returned status.

I have some concerns for restore states, but this did works for me the better than None values,

I may do more changes, But because I want to keep with updates and I may also missed something you can check it if you had time.

https://github.com/xZetsubou/hass-localtuya/tree/add_ble_support

Note: The goal here is to avoid rely on cloud by any means, and kinda avoid user inputs.

@Lurker00
Copy link
Author

I'll read and test it, but I already see that

The goal here is to avoid rely on cloud by any means

this will not work: see my comment about my Zigbee bulbs! Check for DP codes is the only way I've found to make them work correctly.

@Lurker00
Copy link
Author

Here is my Zigbee bulb diagnostics. I have 5 of them, from different vendors, internally all the same.

I've also pushed a fix for BLE bulb cloudless setup. I think, my solution is perfect for setup from the cloud data, and not worse than the current release for manual setup.

localtuya-944c1616039a1d1ec6b9745179bff74e-ВС_ Zigbee Bulb-6e477bc6604f0282c806c9399a0b3c50.json

@xZetsubou
Copy link
Owner

xZetsubou commented Dec 28, 2024

This bulb should works fine with add_ble_support as I mentioned! It should map scenes to SCENE_LIST_RGBW_1000

@Lurker00
Copy link
Author

It should map scenes to SCENE_LIST_RGBW_1000

OK, my bad: "20" was DP number, now it is scene data length!

@Lurker00
Copy link
Author

Sorry, it seems I can return to this only after January 10. Happy New Year!

@xZetsubou
Copy link
Owner

No worries have fun and happy new year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants