Je te laisse me redire pour finaliser les essais sur la 3ème PR.
EDIT :@pierre-gilles, je ne sais pas comment cela se passe, mais pourrais-tu ajouter @S-Axel au projet sur Github, il n’est pas sélectionnable en reviewer !
J’ai répondu sur ta réflexion sur le nouveau composant (je suis d’accord avec toi) et fait les corrections.
J’ai ajouté une réflexion sur la modification faite dans le composant Gauge ? Ainsi que celui dans le composant Chart dont je suis en cours d’ajout. Que penses-tu de la modification réalisée dans Gauge, et de la poursuite dans Chart ?
D’autant plus que dans le composant Chart, cela signifie qu’il faut convertir toutes les valeurs …
Dans mon optique elle va gérer tous les format impactant les distances comme volumes, vitesses, distances, niveau, voir même les pressions non ? Où il faudrait ajouter une option pour cette dernière dans les settings du user ?
Comme vu ensemble, l’ensemble des correctifs proposés est corrigé.
Je pense en effet que c’est beaucoup plus clean.
Pour info, j’ai tenté le nom de variable ‹ _DISTANCE_UNIT_CONVERSIONS › pour la variable interne au fichier unit-conversions.js mais eslint ne l’accepte pas. Je l’ai donc laissé en ‹ DISTANCE_UNIT_CONVERSIONS ›.
Tu peux me pointer vers le bon message où m’en reparler ici ? Il y a eu beaucoup d’activités depuis mon dernier message, je ne sais plus de quoi tu parles.
Yes, on va devoir le faire Surtout avec l’arrivée de Matter qui est un standard fixe en terme d’unité, donc la conversion devient nécessaire pour tous les utilisateurs qui ne sont pas d’accord avec l’unité choisit par Matter. C’était moins le cas avec Zigbee2mqtt, mais ça devient nécessaire avec Matter.
Tout ce que tu cites me semble effectivement lié aux unités de distance, non ?
Pour te donner un peu de contexte sur pourquoi j’ai séparé temperature_unit_preference et distance_unit_preference dans Gladys :
L’idée, c’est de se placer du point de vue de l’utilisateur. Certains pays utilisent le système impérial pour les distances, mais affichent les températures en Celsius. Même aux États-Unis, on trouve des utilisateurs qui préfèrent les miles pour les trajets, mais conservent les températures en Celsius, d’où la séparation actuelle dans Gladys.
Concernant les unités que tu mentionnes, j’ai un peu de mal à imaginer un cas d’usage où un utilisateur voudrait un paramètre différent pour la distance et la vitesse, ou encore pour la pression.
S’il y a de la demande pour aller plus loin dans le détail des unités, on pourra bien sûr l’envisager, mais en l’état, un seul paramètre me semble suffisant
Effectivement, les underscore en début de nom de constante, pas très fan
Je suis tout à fait d’accord avec cette réflexion. Après comme le mentionne @S-Axel dans sa review, il est vrai que le terme « distance » dérive un peu pour le volume ou la pression. Le terme ‹ system_unit › semble plus adapté dans ce cas de figure sauf si
On sépare une préférence pour le volume et la pression => comme la température, on pourrait très bien supposer que quelqu’un veuille du système IMPERIAL US pour la distance et des BAR pour la pression ou des LITRE (Gallon sinon) pour le volume.
Entièrement d’accord ^^ Le fait de devoir faire les conversions ici amène juste la réflexion, je ne dis pas que c’est à faire maintenant. Mais dans une prochaine PR qui viendra vite, la conversion PSI => BAR devra être ajoutée. C’est juste pour cela. ^^
@S-Axel a l’habitude de travailler avec les underscore pour les variable interne à un fixhier non exportées. On a tenté ^^
Les pressions, contrairement aux températures, ça reste intimement lié aux unités de distances ( inches of mercury pour la météo, pounds per square inch pour la pression des pneus ), donc ça me choque pas de garder l’unité de distance pour l’instant.
Si on tombe à l’avenir sur une horde d’utilisateur US qui ont des besoins très spécifique, ça n’empêche pas de rajouter un paramètre plus tard
@Terdious J’aimerais bien faire une release de Gladys aujourd’hui pour publier les différents bugfixs, on est d’accord que tout ce qui est actuellement sur master peut partir en l’état ?
On est d’accord, tout est bon, sauf, pour moi, la petite correction pour les portes et fenêtre ouverte qui n’est pas dans le bon sens (comme je te disais en dev, je ne les vois pas et ça ne m’avait pas choqué) mais, du coup je te fais une réponse rapide ici :
Nom de la feature spécifique : « Porte ouverte » :
Non => 0 => Lock, je ne comprend pas le raisonnement inverse, c’est une icône censée être représentative. Cela signifie que les portes sont bien verrouillées
Oui => 1 => Unlock.
Après si toi ça te convient ainsi alors rien à changer.
Pour le coup dans la PR4 j’ai fais une proposition pour le changer en badge couleur vert/rouge.
Pour les portes, pour moi il faut juste que le comportement lié aux voitures soit le même que pour les portes de maison actuellement en production dans Gladys