dckiller51 / bodymiscale

Custom_components Body Metrics for Xiaomi Miscale 1 and 2 (esphome or BLE monitor for Homeassistant)
Apache License 2.0
209 stars 33 forks source link

Dev questions #61

Closed edenhaus closed 2 years ago

edenhaus commented 2 years ago

Hi @dckiller51,

first thank for this component. Currently I prepare the PR to add config flow for this component and I got some questions:

After I have added the config flow support, it easy to add a sensor for each attribute (#40) and also fix #39, #37, #25. What do you think?

dckiller51 commented 2 years ago

Hello , I don't have a problem with the changes but it must be compatible with the card that goes with it https://github.com/dckiller51/lovelace-body-miscale-card.

For the configuration options min_weight, max_weight, min_impedance, max_impedance it allows to have a quick state. https://community-assets.home-assistant.io/original/3X/a/c/ac505f58d1c54e97f02f633ae2b379ff9c7d66ef.png

edenhaus commented 2 years ago

I don't have a problem with the changes but it must be compatible with the card that goes with it https://github.com/dckiller51/lovelace-body-miscale-card.

That's of course, I use also this card :) The entity stays at it is only new will be added.

For the configuration options min_weight, max_weight, min_impedance, max_impedance it allows to have a quick state. https://community-assets.home-assistant.io/original/3X/a/c/ac505f58d1c54e97f02f633ae2b379ff9c7d66ef.png

Sorry I didn't understand it. I understand, that when they impedance is negative it is not a valid value and therefore still a "impedance low" error state will be created. I would only delete the ability for the user to change this values and internal use the default values as specified in const.py https://github.com/dckiller51/bodymiscale/blob/d5c03bffad048fb431af9485fd58173e7f385082/custom_components/bodymiscale/const.py#L53-L56

dckiller51 commented 2 years ago

Ok I thought you wanted to delete it. we can actually put it in hard.

edenhaus commented 2 years ago

I don't have a problem with the changes but it must be compatible with the card that goes with it https://github.com/dckiller51/lovelace-body-miscale-card.

We probably need also adjust the card in the future as HA development guidelines are saying the following: grafik https://developers.home-assistant.io/docs/core/entity#generic-properties

So probably after I have created a sensor for each attribute, we should adapt the card to use the new sensors and afterwards remove the old bodymiscale entity completely.

We don't have to hurry, but we should keep it on the radar.