KiwiHC16 / Abeille

Abeille pour Jeedom (Gateway ZiGate)
GNU Affero General Public License v3.0
60 stars 52 forks source link

String hexa, choix de la casse #1308

Closed tcharp38 closed 3 years ago

tcharp38 commented 3 years ago

@KiwiHC16 De nombreux soucis sont liés au fait que naturellement on ecrira l'hexa en minuscules dans le code. Manque de pot on compare des strings, donc la casse est critique pour la comparaison ex: if ($addr == "00AC") vs if ($addr == "00ac")

Bref,

Tu en penses quoi ? Dans tous les cas faut rendre les choses + robustes de ce cote la non ?

Ca veut dire par ex de modifier AbeilleSerialRead pour qu'il renvoi TOUJOURS la bonne casse choisie. Actuellement il fait rien donc ca reste ce qui est renvoyé de la zigate. Ma preference reste le "lower case" car ecriture naturelle mais je te laisse choisir. Il me semble que tu avais une preference plutot pour "upper case".

KiwiHC16 commented 3 years ago

Je ne comprends toujours pas d ou vient le soucis. Pendant des mois cela fonctionnait sans soucis et maintenant on se retouve avec ces soucis.

Donc comme tu dis il va falloir faire un changement radical même si on ne comprend pas la source du problemes.

L'Hexa dans la code a toujours été choisi pour être en ligne avec "le texte recu". Upper, Lower, Mix: je me souviens avoir vu des adresses IEEE en mixte.

Je n'ai pas de preference pour le upper ou lower, il faut juste qu on passe à l'un ou l autre mais qu'on ne melange pas les deux.

Je pense qu on n a pas le choix et qu'il faut le faire. Les endroits ou faire les modifications:

Lors des soucis avec IEEE, j'avais tout passé en Upper. Dans les templates j'ai probalement mis en Upper. Je viens d en regarder quelque uns, ils sont en Upper. Dans AbeilleCmd les Hex sont codés en Upper.

Il y aurait certainement moins de boulot à passer en Upper.

Le write vers la Zigate est encodé avec un pack() pour passer du Hex au binaire. Je ne sais pas s'il faudrait faire des motifs à ce niveau.

Le read depuis la Zigate est un $byte = fread($f, 01); $byte = bin2hex($byte);

J'ajoute un strtoupper

KiwiHC16 commented 3 years ago

Maintenant que le read est strtoupper on doit pouvoir enlever tous les strtoupper que j avais mis pour le soucis IEEE. Faire et defaire c'est toujours travailler !

KiwiHC16 commented 3 years ago

Je suis entrain de passer en revue AbeilleParser.

tcharp38 commented 3 years ago

Ok donc je te laisse pousser tes modifs et je referai une passe ce soir, si jamais tu livres aujourd hui. Donc choix interne abeille = UPPERCASE.

C'est plus joli pour l'affichage d'ailleurs mais moins naturel pour l'ecriture.

KiwiHC16 commented 3 years ago

Oui je suis aussi de cet avis. Il va falloir aussi aller chercher les valeurs en cache ou en DB !!!

KiwiHC16 commented 3 years ago

Quelle galère ....

tcharp38 commented 3 years ago

Oui mais une fois ce truc clarifié je pense que ca peut grandement ameliorer la robustesse globale. Pour la mise à jour de la DB j'ai une idée, basée sur sur la mise à jour du format au demarrage que j'avais implémenté. Je peux my coller si tu veux (ce soir :) je vais bricoler autre chose dans la journée. Faut aussi que les travaux de l appart avancent :))

KiwiHC16 commented 3 years ago

Moi je voulais faire des trucs dehors mais il pleut. Je viens de faire la premiere passe d'AbeilleParser. Je regarde Abeille.class

KiwiHC16 commented 3 years ago

Ok je te laisse la BD. Il y a l IEEE dans la config des equipements $eqLogic->getConfiguration('IEEE','none') Et la valeur des commandes (cache): $commandIEEE = $eqLogic->getCmd('info', 'IEEE-Addr');

Il y a aussi la version de la base pour ne pas faire la boulot a chaque fois: $dbVersion = config::byKey('DbVersion', 'Abeille', ''); Voir Abeille.class en ligne 551 dans fct deamon_start_cleanup()

KiwiHC16 commented 3 years ago

Premiere passe faite pour: AbeilleSerialRead AbeilleLog Abeille.class AbeilleCmd AbeilleInterrogate AbeilleMsg AbeilleParser AbeilleSocat AbeilleZigate

Attention j ai fait toutes les modifs sans verifier que le code tourne toujours.

KiwiHC16 commented 3 years ago

Je viens de corriger 2 petites erreurs et le master fonctionne de nouveau.

tcharp38 commented 3 years ago

tu me dira si tu penses en rester la histoire qu'on ne soit pas en conflit quand je m'y colle ce soir.

KiwiHC16 commented 3 years ago

J'ai fini pour aujourd'hui, je te laisse la main. (J'ai remis les modeles de cette nuit, je n'aurai jamais du y toucher).

Master semble fonctionner.

tcharp38 commented 3 years ago

Je viens de réaliser un truc que tu savais peut etre déja. PHP fait des comparaisons intelligentes. Par ex:

if ("5e46" == "5E46") echo "Identique";

Ca va afficher "Identique". C'est pourquoi un certain nombre de trucs devait fonctionner malgre la diff de casse.

Une comparaison reellement identique s'ecrit avec 3 '=' if ("5e46" === "5E46") echo "Identique"; else echo "Diff"; Cette fois c'est "Diff" qui serait écrit.

Voila. En ce qui me concerne je vais me coucher moins bête mais je vais pousser une update avant.

KiwiHC16 commented 3 years ago

C est la flexibilité et le soucis de php. J ai mis ton PR dans le master.

tcharp38 commented 3 years ago

Salut Je ne comprends pas ce que tu entends par la

Et la valeur des commandes (cache): $commandIEEE = $eqLogic->getCmd('info', 'IEEE-Addr');

Pour moi il n'y a rien à faire la. Ca ne fait que donner la commande à executer pour recuperer l'adresse IEEE. Elle sera executée via AbeilleCmd puis retour par AbeilleParser donc les pbs de casse sont deja couverts. De + rien n'est socké dans la DB Jeedom côté "commande". Je me trompe ?

KiwiHC16 commented 3 years ago

Historiquement les IEEE sont des commandes infos dans Abeille. La valeur des commandes info n est pas en base mais en cache. Mais si on pert le cache on perd l adresse IEEE. Donc il y a quelques semaines, j'ai mis l IEEE comme une constante dans la configuration de l equipement. Mais pour ne pas perdre la retro compatibilité j'ai gardé la commande info IEEE Donc si on la met a jour dans la constante de configuration, il faut aussi la mettre à jour dans la commande IEEE.

Ou alors on arrete le support de la retro compatibilité qui complexifie le truc et on eneleve toute les commandes IEEE. Mais lors du prochiane upgrade les utilisateur risque de perdre l info IEEE. Mais on leur dit de fair un getLQI pour recuperer les IEEE.

Un moindre mal pour un mieux a long therme.

tcharp38 commented 3 years ago

Ok mais qu'entends tu par "en cache". Désolé, je suis long à la detente. J'ai pas vu ce stockage. C'est un fichier peut etre ?

KiwiHC16 commented 3 years ago

En cache - je devrais certainement plutot dire en memoire. Ma comprehension du fonctionnement de Jeedom:

tcharp38 commented 3 years ago

Si c'est en RAM comme tu dis, je suppose que ces infos sont raffraichies au rédémarrage du plugin. Seules les données historisées sont en DB je pense et dans ce cas je ne vois pas quelle info soufrirait du pb de casse. Une idée ? On n'historise pas les adresses par ex.

Du coup vois tu encore un pb potentiel ? Pour moi on peut clore le sujet. Pas vu de soucis particuliers sur mes differentes manips.

KiwiHC16 commented 3 years ago

Ok on clos. Ca tourne sur ma version de test et je ne vois pas de soucis.

tcharp38 commented 3 years ago

Il me reste à livrer un petit bout.. celui qui enregistre reellement la nouvelle version de config et la suppression de qq messages de debug