ZacheryThomas / homeassistant-smartrent

Home Assistant Custom Component for SmartRent Locks πŸ”, Thermostats 🌑, Sensors πŸ’§ and SwitchesπŸ’‘
MIT License
84 stars 3 forks source link

feature: dimmable light switches #9

Closed ZacheryThomas closed 2 years ago

ZacheryThomas commented 2 years ago

Requested as a part of the last comment on this issue: https://github.com/ZacheryThomas/homeassistant-smartrent/issues/4

AppleTechy commented 2 years ago

Do you want me to repost the information under this issue? Please let me know if you need anything else :)

ZacheryThomas commented 2 years ago

Ehh, should be fine tbh! I've got enough to go off of. Thanks for running those scripts & getting all that info for me btw!

ZacheryThomas commented 2 years ago

Alright @AppleTechy! Should be good to test out now. Just pushed a new release.

Let me know if it works out for you / if anything needs to be changed.

cskwrd commented 2 years ago

@ZacheryThomas if I understand this correctly my lights aren't showing up in home assistant because they are dimmable? I shouldn't open a new issue, correct?

AppleTechy commented 2 years ago

@ZacheryThomas Must have missed the email notification. I upgrade to release v0.3.0 but I am not seeing the lights show up in Home Assistant. While I had configured the integration to automatically add any new entities, I thought maybe that was failing. I went ahead and removed the integration and try reconfiguring but still am not seeing the lights. Went and looked at the logs, looks like your commit broke setting up light entities. πŸ™ƒ

SEE EDIT 2 for CHANGES NEEDED

2022-09-27 19:56:34.896 WARNING (SyncWorker_0) [homeassistant.loader] We found a custom integration hacs which has not been tested by Home Assistant. This component might cause stability problems, be sure to disable it if you experience issues with Home Assistant
2022-09-27 19:56:45.513 WARNING (MainThread) [smartrent.api] Function "get_switches" will be removed in a future update! Please use "get_binary_switches" or "get_multilevel_switches".
2022-09-27 19:56:45.542 ERROR (MainThread) [homeassistant.components.light] Error while setting up smartrent platform for light
Traceback (most recent call last):
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/helpers/entity_platform.py", line 281, in _async_setup_platform
    await asyncio.shield(task)
  File "/home/homeassistant/.homeassistant/custom_components/smartrent/light.py", line 24, in async_setup_entry
    async_add_entities([LightEntity(ml_switch)])
TypeError: LightEntity() takes no arguments

Edit: In lights.py line 24 you call async_add_entities([LightEntity(ml_switch)]) if I change that to async_add_entities([SmartrentLightEntity(ml_switch)], my lights now show up in Home Assistant and currently reflect the current status of the lights, including changes. This error is still present-> 2022-09-27 20:14:10.061 WARNING (MainThread) [smartrent.api] Function "get_switches" will be removed in a future update! Please use "get_binary_switches" or "get_multilevel_switches".

However, when I attempt to change the status of a light via HASS, I get an error:

2022-09-27 20:14:56.609 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [redacted] 'MultilevelSwitch' object has no attribute 'set_level'
Traceback (most recent call last):
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/components/websocket_api/commands.py", line 200, in handle_call_service
    await hass.services.async_call(
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/core.py", line 1738, in async_call
    task.result()
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/core.py", line 1775, in _execute_service
    await cast(Callable[[ServiceCall], Awaitable[None]], handler.job.target)(
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/helpers/entity_component.py", line 204, in handle_service
    await service.entity_service_call(
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/helpers/service.py", line 676, in entity_service_call
    future.result()  # pop exception if have
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/helpers/entity.py", line 931, in async_request_call
    await coro
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/helpers/service.py", line 713, in _handle_entity_call
    await result
  File "/srv/homeassistant/lib/python3.10/site-packages/homeassistant/components/light/__init__.py", line 544, in async_handle_light_off_service
    await light.async_turn_off(**filter_turn_off_params(light, params))
  File "/home/homeassistant/.homeassistant/custom_components/smartrent/light.py", line 90, in async_turn_off
    await self.device.set_level(0)
AttributeError: 'MultilevelSwitch' object has no attribute 'set_level'

EDIT 2: In lights.py the following modifications need to be made: Line 80: await self.device.set_level(brightness) -> await self.device.async_set_level(brightness) Line 91 await self.device.set_level(0) -> await self.device.async_set_level(0)

So all changes that need to be made to fixe issues from this release: 1) Restore functionality for all lights   * lights.py Line 24: async_add_entities([LightEntity(ml_switch)]) -> async_add_entities([SmartrentLightEntity(ml_switch)]

2) Fix settings status for multi-level lights.   Line 80: await self.device.set_level(brightness) -> await self.device.async_set_level(brightness) Line 91 await self.device.set_level(0) -> await self.device.async_set_level(0)

UNTESTED 3) Remaining item log error to fix 2022-09-27 20:22:29.708 WARNING (MainThread) [smartrent.api] Function "get_switches" will be removed in a future update! Please use "get_binary_switches" or "get_multilevel_switches".

  * In switch.py line 18 switches = client.get_switches() -> switches = client. get_binary_switches()

ZacheryThomas commented 2 years ago

@cskwrd Looks like I introduced a bug, but we can just use this current issue to track everything still. Should be able to get out a patch today though!

ZacheryThomas commented 2 years ago

@AppleTechy thanks for all the info! Makes it real easy. I'll try to get things sorted out

AppleTechy commented 2 years ago

@ZacheryThomas Submitted a PR with all the changes to make life easy. Sorry for the messy comment earlier, was updating it as I resolved issues that came up. πŸ™ƒ

ZacheryThomas commented 2 years ago

Thanks for all the work @AppleTechy! I merged ur PR (yay) and made a new release!

Funny enough I did the same work you did in your PR. I merged my code and then right after saw you had a PR up. 😩 But I still merged your stuff in so you get some credit for fixin stuff though πŸ˜‰

AppleTechy commented 2 years ago

@ZacheryThomas I'll switch to the new release when I'm back on my local network with the HASS instance. Hopefully no other issues arise. @cskwrd can you confirm after upgrading to the latest release that your binary lights are back working?

cskwrd commented 2 years ago

@AppleTechy I was to upgrade without problems. I now have lights on my dashboard. The dimming controls for the lights are a bit wonky, however. When I first open the settings for a light, the brightness reports NaN. I was not able to adjust the brightness of the light (using the slider) until I clicked somewhere on the bar. After clicking, I could then slide the slider back and forth as expected. Another thing I noticed, was that the brightness level is not correctly reported to Google Home. I can set the brightness of a light from the app, but it says 0% on the app's home screen.

I am not sure how much of that is the responsibility of this integration, but I thought I would mention it.

AppleTechy commented 2 years ago

@cskwrd Hmm, the custom integration in HASS is working perfectly for me. Let's start with the brightness reporting NaN. Can you send a screenshot of what you are seeing? We might need you to run a script linked in https://github.com/ZacheryThomas/homeassistant-smartrent/issues/4 for one of those dimmable lights that you are seeing the issue with. I am wondering if it is reporting a wonky 'level' last read state.

As far as the second issue, I don't use Google Home so maybe someone else can step in and field that question but my guess is something is up with how the entity is being reported to Google Home from HASS and it not correctly updating the state to Google Home not necessarily an issue directly with this custom integration.

cskwrd commented 2 years ago

WRT Google Home wonkiness, I am not super interested in that issue because you are probably right that it isn't related to this integration.

Switch Issue

image

As requested a screenshot of what I am seeing. If I click and try to drag the slider along the bar it doesn't change position or value.

I do want to clarify that this only happens until it is fixed. This issue doesn't reappear after I click anywhere on the slider. After fixing it, the slider behaves normally. As I was writing this reply things seemed to change, the switch above is now behaving normally, but a switch that was behaving normally is now doing the NaN thing again.

I ran Script 1 and the attribute named level had a state of 97 when this screenshot was taken.

image

The second screenshot above is an example of another switch that has been fixed. This switch had a level of 0 at the time of the screenshot.

The following is me thinking out loud more than anything

Could it be an issue of how either this integration or HA handles 0? I could see how it might be confusing for a less technical user if the switch were to be in the ON state but a brightness of 0. Maybe the integration reports a 0 but HA doesn't allow that value?

Now that it seems that the behavior I am seeing is less consistent, maybe it is still an issue with the way 0 is handled, but maybe the data from the SmartRent API is not exactly stable either?

ZacheryThomas commented 2 years ago

So far im thinkin that the data from SmartRents API might be wonky at times. For example I had issues in the past with the termostats returning temperature related items as strings which is already weird enough. But sometimes the strings would be styled like ints (ex "79"), and sometimes they would be floats (ex "79.0").

What we can do for now is enable more debugging in homeassistant to see what state everything is in when this behavior occurs.

If you set up your configuration.yaml to be somethin like this:

logger:
  default: warning
  logs:
    custom_components.smartrent: debug
    smartrent: debug

We should be able to get a bit more data 🀞

cskwrd commented 2 years ago

I enabled logging as described.

The log messages don't include information about which device goes to which level that is shown in the logs. However, the dashboard currently shows 3 lights as on. In reality 2 lights are on, the third light is on in the sense that the off indicator light is not illuminated, but the brightness appears to be zero. This same light reports a 50% brightness to the smart rent website, and I am guessing that this is the light that the HA integration is reporting has a level of 1 (in the logs). The dashboard gives me NaN on the slider for all the lights that HA indicates are illuminated.

ZacheryThomas commented 2 years ago

@cskwrd Ok! So I ended up pushing a new release. I think I figured out the majority of your issues. I made a bit of a mistake when setting the brightness values in Home Assistant.

Let me know if the latest release clears that up for you

cskwrd commented 2 years ago

@ZacheryThomas I manually upgraded my integration by removing the old install and replacing it with the contents of the new zip archive. I no longer see any NaN sliders! The controls on my dashboard seem to work as expected. Thank you (to both of you) for your help!

ZacheryThomas commented 2 years ago

Glad it's workin out! Gonna close out this issue then. Feel free to open another issue if more stuff comes up!