Constat :
Prenons l’exemple d’une commande binaire qui a été à 1 une fois durant 10 minutes pendant les 24 dernières heures et était préalablement à 0 les 6 jours précédents.
Si je demande duration(#maCommande#, 1, 24 hour) le résultat va être complètement faux car le code prend comme valeur de départ de la période la moyenne arithmétique de la valeur de cette periode (1 seul changement, 0.5).
Par exemple, on va obtenir 1010 minutes au lieu de 10 minutes…
Solution :
A la ligne 656 du fichier ScenarioExpresssion.class.php au lieu de faire une moyenne arithmétique $lastValue = round($histories[0]->getValue(), $_decimal);
il faudrait faire une moyenne temporelle $lastValue = round($cmd->getTemporalAvg($_startTime, $_endTime), 1);
Commentaire :
Cette correction donne toujours une approximation, mais une approximation très proche de la réalité contrairement à l’implémentation actuelle
Je sais que je devrais faire un pull request, mais j’ai encore besoin de regarder comment ça marche
Je me permet de te tagger @Loic , car je ne sais pas si tu regarde ce genre de post sans pull request…
J’essai de comprendre le raisonnement, mais je ne saisi pas, pourquoi parler de moyenne ? dans le cas de la fonction duration(), ce n’est simplement qu’un arrondissement de la valeur extraite de la BDD qui est effectué !
En fait si tu regarde le code, tu verra que pour obtenir la valeur de départ de la période, si jamais cette valeur n’a pas été changée dans les deux heures précédant la période, c’est la moyenne sur la période qui est prise pour essayer d’avoir une valeur approximative. Mais dans beaucoup de cas (pour moi, une commande booléenne qui n’est pas souvent modifiée), la moyenne arithmétique peut être complètement fausse alors que la moyenne temporelle sera plus approchante dans tous les cas.
Désolé mais je ne voit toujours pas ou il y a question de moyenne.
a ce que je comprend dans le code :
$histories = $cmd->getHistory(date('Y-m-d H:i:s', strtotime($_startTime . ' - 2 hours')), $_endTime); → extraction des données présente en BDD (dans un array) entre $_startTime et $_endTime, le -2h sert juste a pouvoir capté la valeur cherchée au cas ou, celle-ci soit en bdd avant $_startTime, mais pour une recherche sur 24h celle-ci est, je le conçois, insuffisante.
Exemple : si je cherche la valeur « 1 » sur une période de 24h et qu’il est 23h00
$_startTime vaut 2022-04-17 23:00:00
$_endTime vaut 2022-04-18 23:00:00
Si le 17/04 mon info est passé a « 1 » a 20h00 et remise a « 0 » le 18/04 a 2h00, elle ne sera pas comptabilisée car
2022-04-17 20:00:00 < $_startTime - 2h00 il manquera donc la duréé entre le $_startTime et le 18/04 a 2h00 soit 3h (180 min).
Pour ne perdre aucune valeur il faudrait plutôt que la fonction charge la totalité de l’historique : $histories = $cmd->getHistory(); (L650) et ainsi laissé le foreach faire le travail sur les données.
Ensuite le $lastValue = round($histories[0]->getValue(), $_decimal); sert juste a avoir le point de départ du 1er historique présent entre les bornes demandées pour pouvoir l’utiliser dans le foreach qui suit dans le code, celui-ci effectuera les étape de contrôles, tout en vérifiant la valeur recherchée.
Désolé, c’est peu être pas très claire, c’est pas facile a expliquer le code
Bonjour,
Pouvez vous faire un PR pour ce changement ? J’accepte tous les PR par défaut maintenant donc il n’y aura pas de soucis pour moi quelque soit le changement.
Je ne comprends pas comment ça peux répondre ça dans ton cas.
Le $lastValue de la ligne 656 va être évalué à 1 (première valeur de la liste de valeurs requêtées)
Du coup, dans la boucle qui suis on va arriver à la ligne 665 pour ton passage à 1 de 21h19 le 17
La durée va donc être incrémentée alors qu’en réalité, la valeur aurait du être à 0 et la durée non incrémentée.
à la boucle suivante, le $lastValue sera à 1 et il va encore y avoir incrémentation
En fait, tout ton début de période (qui dépend de l’heure à laquelle tu as lancé l’évaluation) va être compté comme du temps à 1 alors que ça ne devrait pas.
Et pour ce qui est de quotes, chez moi c’est l’inverse : les valeurs sont délirantes si je quote
EDIT :
J’ai compris : si tu quotes, jeedom ne comprends pas l’expression et renvoie la totalité des valeurs de l’historique, d’où ton résultat : il prends en compte le 0 de départ !
Ton 1422 corresponds à 23 hures et 42 minutes, ce qui correspond à un lancement de l’évaluation à 21h36 le 27/04 → jeedom a considéré la valeur de départ à 1
EDIT2 :
Bon j’avoue que ma solution n’est pas satisfaisante. Je réfléchie à une solution propre et je ferai un nouveau PR.
J’ai effectivement prit un peu plus de temps pour comprendre, et je suis bien tomber sur la même analyse que toi .
Par rapport au PR,
pour byCmdIdAtDatetime de la class history, pas de soucis. J’avoue que je n’y avait pas pensé, et sa fait une fonction utile de plus dans le Core .
par contre, il y a toujours des problèmes avec le duration:
il ne prend pas en compte une valeur qui aurait commencé avant $_startTime.
il renvoie « rien » si il n’existe pas encore d’historiques, alors qu’il doit quand même renvoyer « 0 ».
la methode pour récuperer : $lastValue = round(history::byCmdIdAtDatetime($_cmd_id, $_startTime), $_decimal); est mal formaté.
Il faut avant initier la class : $historyAtDateTime = history::byCmdIdAtDatetime($cmd_id, $_startTime);
Puis après récuperer le $historyAtDateTime->getValue().
Je fait un PR en alpha, qui reprend toute cette liste, si tu as le temps de faire un test, n’hésite pas a faire un retour.
.