Service Netatmo

Documentation terminée !! Add Netatmo integration doc by Terdious · Pull Request #238 · GladysAssistant/v4-website · GitHub
Y aurait-il du monde pour me dire si tout est claire ?

Airtable ‹ en › et ‹ fr › mis à jour (Airtable - Netatmo NATherm1).
Toutefois je ne vois pas comment l’intégrer à la documentation, l’ID de la documentation ‹ netatmo › n’existant pas et ne voyant pas comment l’ajouter :

Pour info, j’ai ajouté aux liens Amazon &tag=gladproj-21. La redirection fonctionne mais je ne suis pas vraiment sûr de la prise en compte de l’affiliation (‹ 21 › = année 2021 ? Doit-on le changer chaque année ?)

Merci pour la documentation ! C’est très complet c’est propre.

Pour le titre, est-ce que tu peux mettre un titre plus long qui explique clairement ce que ce tutoriel permet de faire ?

Exemple: Connecter ses thermostats Netatmo à son installation domotique

ça permet de se placer sur Google sur ce genre de mot clé avec un titre accrocheur et pas lié à Gladys (ne pas mentionner Gladys dans le titre)

Pour la partie roadmap, je serais pour retirer toutes les dates, on ne s’engage jamais sur Gladys sinon ça créé juste de la déception :slight_smile: Les priorités changent, et c’est impossible de s’engager… Met juste une liste de choses que tu veux faire !

C’est gentil mais pas besoin d’ajouter les liens affilié, ils sont mis automatiquement! Et non 21 c’est juste un chiffre généré une fois par Amazon il y a longtemps :wink:

Au niveau de l’intégration, tu en es où et qui a pu tester au final ?

1 Like

Salut @pierre-gilles !!
Et bon retour, meilleurs voeux 2024 à toi.

Ok, je fais les changements pour la doc ^^

Pour le coup j’ai repris comme Sonos pour la fin : « Connecter ses thermostats Netatmo à sa maison connectée »

Désolé je comprend que le file est très long à reprendre ^^
Donc comme dis plus haut, la PR de l’intégration est prête,
je t’ai fait une demande de review.
Testée par @_Will_71 (test avec son thermostat) et par @Tlse-vins (test sans équipements compatibles + tests avec mon instance avec thermostat). A part le bug d’affichage de la consigne (non lié à cette PR) normalement tout est bon. Tourne chez moi depuis presque 3 semaines.

L’avantage étant quelle peut etre testée par n’importe qui, me suffit de partager mon instance. Si ca te tente… dis moi.

Redis moi.

C’est bon c’est fait pour la documentation et pour airtable.

Du coup pour cette partie ? Je laisse comme ça et tu t’en charges ?

parfait!

Ok je vais regarder semaine prochaine, j’ai pas mal de trucs à rattraper avant :slight_smile:

Tu ne peux pas créer une option sur Airtable? Si tu tapes « netatmo » ça te propose pas de le créer?

1 Like

Oki !

Non ^^ J’ai testé mais rien de possible, liste fixe (droit peut-être mais je ne pense pas…). J’ai farfouillé un peu partout mais rien trouvé :sweat_smile:

Je n’aibpas eu le temps de tout regarder, mais j’ai relevé ça :

  • Dans la doc sonos ont a mis le lien vers les applications Android et Apple, faire la même chose pour Netatmo.
  • J’ai mi tout les boutons à cliquer dans Gladys en preformaté ( Intégration -> Netatmo ) avec la petite flèche aussi.
  • Rajouter à la fin : Je vous invite à poster un message sur le forum, si jamais vous avez besoin d’aide.

Salut @Tlse-vins,

C’est déjà fait !

C’est corrigé :

  • Avant
    image


…etc…

  • Après
    image


…etc…

Fait, j’ai également rajouté un accès à la documentation « Tableau de bord » et « Scènes » :

Tout est live. Merci pour ton retour.

Edit :

Au temps pour moi … manque Apple ^^ => C’est fait !

@pierre-gilles please need help,

Petite question, que je t’ai posé en 1er lieu sur la PR de l’intégration Netatmo, mais je pense que tu n’y prête pas attention :
Suite à la sortie de la v4.34.0 j’ai mis à jour la branche, mais comme régulièrement il y a eu plantage sur le Codecov report. Dans ta vidéo live coding, tu avais répondu à ma question pour relancer les tests de la PR :


Or de mon côté, je n’ai pas du tout d’option « Re-run jobs » :sob: n’aurais-tu pas des droits à mettre pour pouvoir effectuer cette action ?

Merci par avance, je serais déçu si tu passais ton chemin sur la PR en considérant que les tests ne passent pas encore alors que ce n’est pas le cas …

Ah, ça arrive souvent que codecov pète, c’est hyper instable ^^

Tu as vérifié dans ton repo si tu as l’action ? ( dans ton fork, pas dans le repo Gladys source )

Yep j’ai regardé aussi …rien … peut-être une option chez moi du coup !!

Bizarre, bon j’ai relancé moi même mais c’est vraiment pas terrible si un développeur d’un fork ne peut pas relancer les tests de sa propre PR ! ^^

Je t’ai fais une première review de cette PR : https://github.com/GladysAssistant/Gladys/pull/1951#pullrequestreview-1814951634

Je suis un peu étonné, la PR ne ressemble plus du tout à ce que j’avais vu lorsque j’étais allé voir ton code suite au live coding… Le code est assez loin de ce que je montre en live coding je trouve !

Je t’ai indiqué pas mal de points à corriger. Passe du temps sur la PR et essaie de la relire en entier pour appliquer ces changements partout :slight_smile:

Au niveau de la quantité de code, on est de retour sur une PR assez fat donc je suis vraiment content qu’on se soit limité au thermostat, c’est déjà un gros bout à review

Merci encore pour le travail effectué sur cette PR :pray:

1 Like

Bonjour @pierre-gilles,

Merci beaucoup pour ta review. Je m’attele à cela aujourd’hui. Je vais y passer le temps qu’il faut.

Il est vrai, mais est-ce que ça te parait normal du fait de ne pas passer par un module externe comme pour sonos, ou est-ce que ce n’est vraiment pas normal à ton sens. Car sinon, ça ne vaut peut être pas le coup de corriger, mais plutôt de revoir entièrement le code …

Pour le « netatmoHandler » passé partout … Je suppose que c’est une mauvaise compréhension / configuration de mon vscode. Le « this. » est déclaré en erreur eslint partout dans mon interface vscode, du coup je passais le ‹ netatmoHandler › pour cette raison seulement.

On est d’accord que c’est le fait de contrôler le code au complet et de ne pas passer par un node_module « netatmo » ? Ou pour toi c’est lié à un mauvais code ?

Je pose ces questions, car je prend énormément de temps (avec plaisir hein) mais si c’est pour te faire perdre beaucoup de temps ainsi qu’à moi, il ne faut pas qu’on poursuive. Si par contre avec les correctifs que tu m’indiques et le temps que tu y passes on est « sûr » d’aboutir, alors je donnerais tout :sweat_smile:

Merci à toi pour cette relecture en tout cas !

Non non tkt l’esprit de la PR et la façon de faire globale est bonne :slight_smile:

1 Like

Salut @pierre-gilles,

J’ai terminé les modifications demandées (j’ai tout vérifié et relu tes commentaires 3 fois, j’espère ne rien avoir oublié, sinon je suis vraiment mauvais) et mis à jour les tests, normalement tout est corrigé.

Comme je te l’ai mentionné dans la PR par contre, je ne comprend pas le souci que tu as eu

Blockquote
On the functional side

When I first open the integration, here is what I see:

There is an error message immediately (I didn’t do anything), which is a bit weird for the user.

J’ai tenté de reproduire mais je n’arrive même pas à reproduire ta première erreur de connexion… Personne n’a visiblement eu le cas sur les 2 testeurs et je viens de recommencer avec une nouvelle installation sur wsl, je n’ai pas ce problème…, mon état première connexion :


J’ai également testé via l’image docker et mon état première connexion avec DB vide est également :

Pourrais-tu voir de ton côté pour me guider si tu rencontres à nouveau le souci ? Logs, inspecteur navigateur ?

Je précise que les visuels sont avant les correctifs.

Merci par avance pour la prochaine review.

Pour info, après quelques dernières modifications, voici ce que j’ai sur mes 3 pages du services pour les états :

5 Likes

Merci @Terdious pour les correctifs !! :slight_smile: C’est très chouette.

Je crois que c’était juste un souci de connexion au serveur local lors de mes tests, je pense pas que ce soit lié à la PR ! Je vais re-tester et je te tiens au courant :slight_smile:

1 Like

N’hésite pas si tu veux un accès, pour la visu des devices en réel et le testen réel, ça peut être mieux, et ce n’est normalement pas compliqué ! Je viens de faire le test avec une autre adresse mail, ça marche bien même sans être en possession d’équipements Netatmo.

1 Like

@pierre-gilles ,

Comme je suis en attente sur cette PR, puis-je commencer une PR en partant de cette première PR ? En sachant que ce ne sera pas du travail perdu, il sera facile de récupérer le travail même en cas de gros changement demandé lors de la nouvelle review. Mais ça me permet de rester dedans et de gagner du temps au final.

Ma question porte surtout sur, pour toi quel impact ? Est-ce contraignant ou problématique une fois la 1ère PR mergée ?

Non ce n’est pas un souci du moment que tu sépare bien les deux PRs :slight_smile:

Désolé en ce moment je suis très occupé par le chantier dont je parlais dans le live de rentrée, donc je n’ai pas encore eu le temps de faire de la review de PR !

1 Like