nielstron / ha_bayernluefter

Custom component for the Bayernluefter
Apache License 2.0
8 stars 4 forks source link

Added "Humidity_Transport" in sensor.py #13

Closed MaxiKingXXL closed 1 year ago

MaxiKingXXL commented 1 year ago

Added "Humidity_Transport" to show "Feuchtetransport" in HA

ABS_HUM_MEASURES = [ "abs_Humidity_In", "abs_Humidity_Out", "Humidity_Transport", ]

nielstron commented 1 year ago

Hi @MaxiKingXXL I appreciate the request to improve this package! It looks like you created this change through the WebUI of Github - which makes me believe you did not test this change locally to ensure that it will work properly.

Please try this change out in your local home-assistant installation first and report back with the results in your UI.

For creating this patch you have already created a fork of the project, which you should be able to add in HACS as custom repository and load/try the changes in your own setup. Note it might be you need to merge your patch branch (patch-1) into master in your own fork before the changes take effect.

MaxiKingXXL commented 1 year ago

Hi @nielstron I have tested this change on my local HA environment already and the correct "Feuchtetransport" value show up properly. But you are right that the unit is not matching which I overlooked. :) The Humidity_Transport is in g/24h. Maybe just GRAMS = "g" as unit would fit better here.

The is my first change request in github ever and I'm also new in Python. I would be grateful is you could help to with this change.

image image

nielstron commented 1 year ago

That's good to know! To change the unit type, it would be easiest to introduce a new sensor list/map for this class - you see the two lists for abs and rel humidity sensors are grouped together since they share the unit type.

Grams seems unlikely to be the right unit - is it grams/s or grams/24h or something like that? grams would imply it is the accumulated total

nielstron commented 1 year ago

Hi @MaxiKingXXL I adjusted your PR so that humidity transport should now have the correct unit. Can you please check if it behaves as expected?

MaxiKingXXL commented 1 year ago

@nielstron Thanks, for making this change. But for it doesn't show up. Might it be possible that you have nit check in the latest change in sensor.py?

nielstron commented 1 year ago

I did not test it yet. I'll try to setup homeassistant with a mock server, then merge into master