dvd-dev / hilo

Home Assistant Hilo Integration via HACS
MIT License
122 stars 26 forks source link

CONF_TARIFFS not in the config object when setuping the utility meter #365

Closed elafontaine closed 3 months ago

elafontaine commented 5 months ago

Version of the custom_component

Configuration

No yaml used, HACS; image image

Describe the bug

I initially opened it on the home-assistant project thinking it was something that they didn't mean to do ; https://github.com/home-assistant/core/issues/108930

Turns out that they expect the field "tariffs" to be present in the configuration being passed down on an "update", but the Hilo integration is making a basic configuration object without that field ; https://github.com/dvd-dev/hilo/blob/main/custom_components/hilo/managers.py#L67

Weirdly enough, I see the field is present though in the other function "create" that create the entity ; https://github.com/dvd-dev/hilo/blob/main/custom_components/hilo/managers.py#L53

My guess is that the update is being called in HA for some reason every time when I reboot, and hence why I have this log being generated. I believe it may also be why the cost tracking doesn't work for me. I get the constant tariffs, but the dynamic one doesn't seem to work (always 0).

Debug log

Logger: homeassistant.components.sensor
Source: helpers/entity_platform.py:361
Integration: Sensor ([documentation](https://www.home-assistant.io/integrations/sensor), [issues](https://github.com/home-assistant/core/issues?q=is%3Aissue+is%3Aopen+label%3A%22integration%3A+sensor%22))
First occurred: January 26, 2024 at 8:33:02 PM (1 occurrences)
Last logged: January 26, 2024 at 8:33:02 PM

Error while setting up hilo platform for sensor
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 361, in _async_setup_platform
    await asyncio.shield(task)
  File "/config/custom_components/hilo/sensor.py", line 182, in async_setup_entry
    await utility_manager.update(async_add_entities)
  File "/config/custom_components/hilo/managers.py", line 71, in update
    await utility_setup(self.hass, config)
  File "/usr/src/homeassistant/homeassistant/components/utility_meter/__init__.py", line 141, in async_setup
    if not conf[CONF_TARIFFS]:
           ~~~~^^^^^^^^^^^^^^
KeyError: 'tariffs'

This is getting an other entry in my logs that may be unrelated ;

Logger: custom_components.hilo
Source: custom_components/hilo/__init__.py:530
Integration: Hilo ([documentation](https://github.com/dvd-dev/hilo), [issues](https://github.com/dvd-dev/hilo/issues))
First occurred: 9:46:45 PM (22 occurrences)
Last logged: 9:52:01 PM

check_tarif: Unable to find state for sensor.hilo_energy_total_low

By the way, thank you for the great work on this integration :)

ic-dev21 commented 5 months ago

French is fine here as well if you prefer.

Thanks for the logs. I know @valleedelisle tried working on this a few weeks back in #324 but so far it’s not quite ready.

Sit tight, we’ll try and fix it.

ic-dev21 commented 5 months ago

@elafontaine vois-tu toujours l'erreur de ton côté avec la dernière release?

elafontaine commented 4 months ago

yep, still have it , version is 2024.2.2;

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 361, in _async_setup_platform
    await asyncio.shield(task)
  File "/config/custom_components/hilo/sensor.py", line 184, in async_setup_entry
    await utility_manager.update(async_add_entities)
  File "/config/custom_components/hilo/managers.py", line 72, in update
    await utility_setup(self.hass, config)
  File "/usr/src/homeassistant/homeassistant/components/utility_meter/__init__.py", line 141, in async_setup
    if not conf[CONF_TARIFFS]:
           ~~~~^^^^^^^^^^^^^^
KeyError: 'tariffs'
ic-dev21 commented 4 months ago

Thanks

If you have any experience in python you’re more than welcome to give it a try. Mine and David’s time will be limited in the coming weeks. I’ll gladly test and merge any PR though.

gravelfreeman commented 4 months ago

Juste pour dire que j'ai corrigé mon installation et maj à la dernière version. J'ai corrigé toutes les erreurs de mes templates également. J'ai la même erreur que @elafontaine dès que je coche Generate energy meters. L'erreur ne s'affiche qu'une seule fois et revient seulement au reboot.

2024-03-04 19:54:37.369 ERROR (MainThread) [homeassistant.components.sensor] Error while setting up hilo platform for sensor
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 344, in _async_setup_platform
    await asyncio.shield(task)
  File "/config/custom_components/hilo/sensor.py", line 186, in async_setup_entry
    await utility_manager.update(async_add_entities)
  File "/config/custom_components/hilo/managers.py", line 72, in update
    await utility_setup(self.hass, config)
  File "/usr/src/homeassistant/homeassistant/components/utility_meter/__init__.py", line 143, in async_setup
    if not conf[CONF_TARIFFS]:
           ~~~~^^^^^^^^^^^^^^
KeyError: 'tariffs'
ic-dev21 commented 4 months ago

J'arrive pas à reproduire ici malheureusement... Quelle version de HA?

elafontaine commented 4 months ago

Salut, derniere version, mais je m'apprete a enabler les debugs logs. mais c'est clairement l'object de config passe qui ne contient pas le conf_tariffs puisqu'il est recreer a la ligne 68 de manager.py (https://github.com/dvd-dev/hilo/blob/main/custom_components/hilo/managers.py#L68)

Maintenant, c'est juste que je dois comprendre pourquoi c'etait comme ca avant et pourquoi ca fail seulement pour nous... setup de container ? J'essaie de trouver de la documentation HA pour le utility meter en terme de recommandation de code pour des integration, mais ca ne semble pas documenter... Any help finding it would be appreciated... I'm currently reading their source code...

ic-dev21 commented 4 months ago

Salut, derniere version, mais je m'apprete a enabler les debugs logs. mais c'est clairement l'object de config passe qui ne contient pas le conf_tariffs puisqu'il est recreer a la ligne 69 de manager.py

Maintenant, c'est juste que je dois comprendre pourquoi c'etait comme ca avant et pourquoi ca fail seulement pour nous... setup de container ? J'essaie de trouver de la documentation HA pour le utility meter en terme de recommandation de code pour des integration, mais ca ne semble pas documenter... Any help finding it would be appreciated... I'm currently reading their source code...

La documentation de développement HA est merdique pour vrai. Je pense qu'on aurait probablement plus de succès à checker d'autres intégrations qui génèrent des energysensors voir comment ça se produit exactement.

gravelfreeman commented 4 months ago

J'arrive pas à reproduire ici malheureusement... Quelle version de HA?

Dernière version. Est-ce que tu roules le PR #324? Parce que ce PR règle cette erreur. Ça expliquerait pourquoi tu n'as pas l'erreur.

elafontaine commented 4 months ago

De ce que je vois, cette PR ne fixera pas le probleme ; https://github.com/dvd-dev/hilo/pull/324/files#diff-4585c8c9cf896839d09af344586f0cc792a689217dd2ee79b424796964ccfc47R87 et tu etais le dernier a commenter comme quoi tu avais l'erreur dans cette PR... la PR n'était pas merger, je n'ai donc pas acces au code.

Par contre si vous regarder le lien, le developer "dvd" se demandait s'il devrait repousser la config ... la reponse est oui puisque c'est config[CONF_TARIFFS] qui doit exister (meme si c'est un array vide). Je crois qu'il serait mieux de prendre l'objet existant, mais encore une fois, je parle a travers mon chapeau puisque je ne connais pas l'api exposer pour le utility meter par HA..

ic-dev21 commented 4 months ago

@elafontaine dvd est l'auteur du code de cette intégration si ça peut te placer un peu. C'est du reverse engineer accoté via un swagger qui était public par inadvertance qui a mené à ça. Ajouter à cela que il y a pas si longtemps c'était le free-for-all côté codebase energy sur HA et tu as ce que tu vois devant toi.

Pour ma part il m'a embarqué comme write access car il n'a peu/pas de temps à dédier pour maintenir l'intégration. Je ne suis pas programmeur de formation, j'apprends sur le tas. Il est possible de télécharger le code de la PR#324 sauf que comme la branch n'est pas à jour j'pas certain que ça va marcher.

J'ai tout ce qui faut ici pour tester des trucs, 2 env de dev, donc y'a moyen de tester ça à la gang, mais je dirai jamais non à un coup de main!

elafontaine commented 4 months ago

Je dois m'assoir un bon 4-8heure pour comprendre le code que nous avons. Je ne veux pas faire de quoi que je vais peut-etre briser apres car je ne comprend pas quelque chose d'obscure ;). J'ai pas mal d'experience avec Python, juste pas publique (mon travail ne me permet pas de publier ce que je faisais).

Je veux aider, surtout que je peux :D. Par contre, si tu as encore la definition swagger a quelque part de sauvegarder, tu pourrais me l'envoyer par email ? (ou autre). J'aimerais simplifier aussi le code avec votre authorisation. Ca ne sera pas cette semaine malheureusement pour moi. Travail + vie personnelle prennent la priorite ;).

ic-dev21 commented 4 months ago

Je dois m'assoir un bon 4-8heure pour comprendre le code que nous avons. Je ne veux pas faire de quoi que je vais peut-etre briser apres car je ne comprend pas quelque chose d'obscure ;). J'ai pas mal d'experience avec Python, juste pas publique (mon travail ne me permet pas de publier ce que je faisais).

Je veux aider, surtout que je peux :D. Par contre, si tu as encore la definition swagger a quelque part de sauvegarder, tu pourrais me l'envoyer par email ? (ou autre). J'aimerais simplifier aussi le code avec votre authorisation. Ca ne sera pas cette semaine malheureusement pour moi. Travail + vie personnelle prennent la priorite ;).

Pas de problème, c'est bénévole après tout. Je n'ai pas le swagger par contre.

Si tu as des suggestions, critiques ou autres sur les bouts que j'ai fait, te gènes pas, je suis en apprentissage constant ;)

Tu peux passer nous dire bonjour sur discord, on est pas sorteux!

gravelfreeman commented 4 months ago

Je dois m'assoir un bon 4-8heure pour comprendre le code que nous avons. Je ne veux pas faire de quoi que je vais peut-etre briser apres car je ne comprend pas quelque chose d'obscure ;). J'ai pas mal d'experience avec Python, juste pas publique (mon travail ne me permet pas de publier ce que je faisais).

Je veux aider, surtout que je peux :D. Par contre, si tu as encore la definition swagger a quelque part de sauvegarder, tu pourrais me l'envoyer par email ? (ou autre). J'aimerais simplifier aussi le code avec votre authorisation. Ca ne sera pas cette semaine malheureusement pour moi. Travail + vie personnelle prennent la priorite ;).

@ic-dev21 Je suis certain que si on pouvait avoir un dev qui s'y connait et conserve le Github à jour, certains seraient prêt de faire des donations. Je sais que le temps de tous et chacun est précieux alors un petit incentive pourrait pas faire de mal. J'imagine qu'on pourrait mettre des bounties. Genre un bounty pour réparer les énergy meters. Un autre pour cleaner le code et le documenter, etc.

elafontaine commented 3 months ago

Oh interesting that you can see what I did from my side. I created a PR with what I believe should work. I'll try to figure out how I can test it later.

I would also like to propose that I refactor the whole "sensor" part as it was made harder to figure out with all the async that happens in there... I believe it could be greatly simplified.

ic-dev21 commented 3 months ago

Oh interesting that you can see what I did from my side. I created a PR with what I believe should work. I'll try to figure out how I can test it later.

I would also like to propose that I refactor the whole "sensor" part as it was made harder to figure out with all the async that happens in there... I believe it could be greatly simplified.

Pas de problème je suis setupé pour tester.

ic-dev21 commented 3 months ago

Oh interesting that you can see what I did from my side. I created a PR with what I believe should work. I'll try to figure out how I can test it later.

I would also like to propose that I refactor the whole "sensor" part as it was made harder to figure out with all the async that happens in there... I believe it could be greatly simplified.

Un docker container ça se monte super vite et marche bien pour le dev ;)