briancmpbll / home_assistant_custom_envoy

173 stars 76 forks source link

Add net counters #147

Closed testuser7 closed 10 months ago

testuser7 commented 10 months ago

Add counters:

Consumption should be metered only when consumption CTs are installed.

catsmanac commented 10 months ago

Thanks @testuser7, I'll merge with v0.0.17 into dev-test as v0.0.18-dev-01.

First inspection looks good, only 1 minor issue I'll update in the merge process.

lifetime_production_net should be set to some value containing 'not available' if it's not available on some Envoy types, same as you did for consumption_net. Currently it's set to null causing the entity to be created. Minor issue as said.

      "lifetime_production_net": null,
      "consumption_net": "Consumption data not available for your Envoy device.",

I'll let you know when done so you can take it for a test ride, I only have a plain vanilla Envoy so I can't really test it, just confirm it doesn't crash or create unneeded entities.

catsmanac commented 10 months ago

Hi @testuser7, when merging I noticed below code added. Since consumptionnet starts with consumption_ as well I'd expect the consumption net phase data would be fed from envoy_reader.consumption_phase rather then from envoy_reader.consumption_net_phase. If I'm not mistaking we need to swap the order so consumptionnet test is first and same for the other alphabetical tests?

afbeelding

testuser7 commented 10 months ago

Thank you for the review. All these things need to be changed in the code.

But after adding the import/export indexes, this PR makes the same changes in a different way.