search

cyr-ius / hass-livebox-component

Livebox Component for Home assistant
MIT License
64 stars 21 forks source link

Add WAN access switch entity for devices #87

Closed brenard closed 11 months ago

brenard commented 11 months ago

I added possibility to control devices WAN access using via a switch entity. This feature only works with my fix on the related aiosysbus scheduler method.

brenard commented 11 months ago

About the missing turn_on / turn_off methods in switch entities: it's not really relative to this PR, because, I just used the same method as another switch entities proposed by this integration, but I prepared a quick fix for that if you want : https://github.com/cyr-ius/hass-livebox-component/commit/8bc5c767b245b6ddbc84299319a4d03add5cc2ce

cyr-ius commented 11 months ago

About the missing turn_on / turn_off methods in switch entities: it's not really relative to this PR, because, I just used the same method as another switch entities proposed by this integration, but I prepared a quick fix for that if you want : 8bc5c76

Can you check and tell me if it works because I no longer have a livebox to do tests on. If ok, I will publish the final version

HACS , Beta , select 1.8.9-beta1

cyr-ius commented 11 months ago

I'm going to improve your code a little because in the async_get_device_schedule function, you return False if it is not a dictionary. But in this case, all your functions will crash. It is always better to return an empty dictionary.

and I don't see the call to this function. Why did you put it?

cyr-ius commented 11 months ago

I would have a livebox, I would have rewritten the code because I quite agree, we could write this much better. Unfortunately for me, I did not keep a test file simulating the aiosysbus API responses. If you have time to help me we could try to rewrite the code in a cleaner way

brenard commented 11 months ago

I'm going to improve your code a little because in the async_get_device_schedule function, you return False if it is not a dictionary. But in this case, all your functions will crash. It is always better to return an empty dictionary.

and I don't see the call to this function. Why did you put it?

This method is call by the coordinator in _async_update_data. The devices' schedule are truelly optional and when it's used (in switch entity, retrieved by using _get_device_schedule), I always test its value before accessing keys. I think it's could not be a problem, but change it if you prefer.

cyr-ius commented 11 months ago

I'm going to improve your code a little because in the async_get_device_schedule function, you return False if it is not a dictionary. But in this case, all your functions will crash. It is always better to return an empty dictionary. and I don't see the call to this function. Why did you put it? This method is call by the coordinator in _async_update_data. The devices' schedule are truelly optional and when it's used (in switch entity, retrieved by using _get_device_schedule), I always test its value before accessing keys. I think it's could not be a problem, but change it if you prefer.

Try it , 1.8.9-beta1 , please feedback

brenard commented 11 months ago

Try it , 1.8.9-beta1 , please feedback

Done and it's works as expected :smile:

I would have a livebox, I would have rewritten the code because I quite agree, we could write this much better. Unfortunately for me, I did not keep a test file simulating the aiosysbus API responses. If you have time to help me we could try to rewrite the code in a cleaner way

No problem, say me what you want I test for you!