search

oyvindwe / connectlife-ha

ConnectLife integration for Home Assistant
GNU General Public License v3.0
52 stars 22 forks source link

What is the reason for all properties to be enabled, but hidden by default, rather than disabled by default? #112

Closed psycho202 closed 1 month ago

psycho202 commented 1 month ago

For an unknown device I can understand that these should be enabled to facilitate the implementation of a new device, but for a "known" device, the values that don't return any data or that aren't transformed into readable data should be disabled by default.

On other HA integrations, the default behavior is that these are set as Disabled, so I'm wondering why it's different for this integration?

oyvindwe commented 1 month ago

My idea of setting to hidden instead of not showing them at all was that if a new property is added in the API for an already mapped device, this property will become available in Home Assistant.

But it might be a good idea to support disabling known properties if they are redundant or useless in HA, and maybe also support disabling all unmapped properties for a device.

Something like this:

- property: my_useless_property
  disable: true

and/or

disable_unknown: true
psycho202 commented 1 month ago

I can see your reasoning for why to set it as hidden. As the API can change this indeed seems like a good way.

Having the ability via device mapping to set a redundant or unused property as disabled would be welcomed though!

Monacoslo commented 1 month ago

There are really a lot of properties that for smart appliane monitoring do not have much sense (for example for washing machine you need max 5 of more than 50. Ok, maybe additional 3 alarms could be used to send notifications, but this is all. So having the rest of them disabled would really make things cleaner

oyvindwe commented 1 month ago

Please review PR #119 - is this an OK solution?

psycho202 commented 1 month ago

Please review PR #119 - is this an OK solution?

Looks like it would indeed be a good solution. Tested on my end with the desired end results.

I created a branch on my own repo for the adjustment of 009-128 for inclusion into this PR, which filters out all the properties with unchanging values. What would be the correct way to push this. Create a PR after you merge your PR into main? Or is there a way for me to add on to your PR with this file?

https://github.com/psycho202/connectlife-ha/tree/feature/disable-property

oyvindwe commented 1 month ago

I have merged PR #119 now, so you can create a PR from your branch. (In my experience, this is usually the easiest way to handle a PR that depends on another.)