elupus / hass_nibe

Home Assistant Nibe Uplink Integration
184 stars 54 forks source link

Set state class to total increasing for Wh, kWh or MWh #141

Closed wilfredktlr closed 2 years ago

wilfredktlr commented 2 years ago

I am currently testing this change and hope it fixes #103. When it is ready I will change the status so it is ready for review.

elupus commented 2 years ago

Please add the parameters in the list of sensors instead.

wilfredktlr commented 2 years ago

Please add the parameters in the list of sensors instead.

Thank you for your reply. Do you mean to add new NibeSensorEntityDescription items to PARAMETER_SENSORS? I thought about doing that but then I'd have to add a large number of parameters (see https://www.openhab.org/addons/bindings/nibeheatpump/ and search for "energy") which can also differ from system to system. My system (a Nibe F1255-6 PC) has 7 parameters which are energy sensors (see list below). Other systems might have different parameters so the list is never complete. My solution seems more generic and does the same kind of checks as in def device_class(self). Can you tell me why adding NibeSensorEntityDescription to PARAMETER_SENSORS is a better solution?

elupus commented 2 years ago

Because that is how it is generally done in HA. Try to be explicit rather than generic. Its fine if its incomplete, we can add them as we go along.

wilfredktlr commented 2 years ago

Thank you for the explanation. I'm new at developing for Home Assistant and Python is also not my native programming language. I'm a Sitecore developer so C# and .NET is my daily poison. 😉

I've removed my previous commit and did a force commit on my own repository (because it accidentally contained my work email address). Then I added the new entities to PARAMETER_SENSORS and created a new commit (https://github.com/wilfredktlr/hass_nibe/commit/87a087e911da44746cdb24cdfa35818e543f7275).

PS. I see the pull request has been automaticly closed. Can you reopen it or do I have to create a new pull request?

elupus commented 2 years ago

I dont think i can since you managed to first push an empty branch. So it ended up marked as merged. Just open a new pull.