Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Window ByPass #143

Merged
merged 11 commits into from
Oct 30, 2023
Merged

Window ByPass #143

merged 11 commits into from
Oct 30, 2023

Conversation

adi90x
Copy link
Contributor

@adi90x adi90x commented Oct 28, 2023

Hello,

Comme discuté, une PR pour ajouter une fonction de bypass de l'ouverture/fermeture de la fenetre.
Dans le cas ou le bypass est désactivé alors que le dernier état de la fenetre est ouvert je bascule en HVAC_OFF.
C'est la deuxieme PR car je n'avais pas pris les enormes modifs 👍 => cette PR est mergeable tel quel !

Je regarde l'autre point ( interdire le passage en HVAC_HEAT si la fenetre est ouverte demain )

Bonne soirée,
Adrien

@jmcollin78
Copy link
Owner

Déjà !? Wahou, tu m'impressionnes

Est-ce tu peux ajouter un test unitaire stp pour prouver que ca marche ?

Pour lancer les tests unitaires, tu vas dans VSCode, en mode container, tu cliques sur
Capture d’écran 2023-10-28 à 23 27 57
puis sur
Capture d’écran 2023-10-28 à 23 28 15

Tout doit être au vert.

Tu peux t'inspirer de ce qui est fait dans le test test/test_window.py pour ajouter au moins un test.

Je fais une relecture quand même en attendant.

Copy link
Owner

@jmcollin78 jmcollin78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super job !
J'ai pas grand chose à dire.

README-fr.md Outdated Show resolved Hide resolved
README-fr.md Outdated Show resolved Hide resolved
README-fr.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
custom_components/versatile_thermostat/binary_sensor.py Outdated Show resolved Hide resolved
custom_components/versatile_thermostat/binary_sensor.py Outdated Show resolved Hide resolved
@adi90x
Copy link
Contributor Author

adi90x commented Oct 29, 2023

Bonsoir,
J'ai ajouté un test de bon fonctionnement du bypass ( i.e : le thermostat reste en HEAT malgré que la fenetre soit ouverte )
Une premiere donc il n'est surement pas parfait :)
Bonne soirée !

tests/test_window.py Outdated Show resolved Hide resolved
@jmcollin78
Copy link
Owner

Plus que 3 petits commentaires et on est bon.

@adi90x
Copy link
Contributor Author

adi90x commented Oct 29, 2023

Plus que 3 petits commentaires et on est bon.

Il manque quoi ?

@jmcollin78
Copy link
Owner

Il manque quoi ?

Ben il reste 3 commentaires. Tu ne les vois pas ?

  1. "Normalement rien n'a changé (puisqu'on ignore l'évent), donc pas la peine d'appeler self.update_custom_attributes."
  2. "Ajouter un test pour vérifier qu'il y a un réellement un changement de valeur de window_bypass_state et ne rien faire dans ce cas."
  3. "Déjà appelé par async_set_hvac_mode et par restore_hvac_mode donc je pense que ca ne sert à rien."

@jmcollin78 jmcollin78 merged commit 2786a6e into jmcollin78:main Oct 30, 2023
3 of 4 checks passed
@jmcollin78
Copy link
Owner

Merged ! Thank you @adi90x !

@adi90x
Copy link
Contributor Author

adi90x commented Oct 30, 2023

Merged ! Thank you @adi90x !

Yes les commentaires était gérés ! Merci pour tes commentaires et ta réactivité ! Je regarde dans la semaine pour empêcher le switch vers HEAT si la fenêtre est ouverte !

Bonne journée et bonne semaine !

@jmcollin78
Copy link
Owner

While testing I see:
VersatileThermostat-Thermostat switch 1 - Window ByPass is activated. Ignore window event
and
Capture d’écran 2023-10-30 à 08 04 15

Quand j'enlève le bypass ca revient bien actif. Donc ça marche pour le cas nominal.


Y a un cas qui ne marche pas:

  1. y a un mode de détection des ouvertures qui est fait lorsque la température chute. C'est le mode window auto.
  2. dans ce mode, le bypass ne fonctionne pas.

Je vais regarder si c'est simple à corriger.

@jmcollin78
Copy link
Owner

C'est corrigé. J'en ai profité pour ajouter des tests et un thème pour changer la couleur du binary_sensor Window bypass.
Toutes les modifs sont là si tu veux jeter un oeil: dc89e01

Je release dans la foulée.

@adi90x
Copy link
Contributor Author

adi90x commented Oct 30, 2023

J'ai juste ajouté un petit commentaire, avec le dernier commit, tu supprime l'enregistrement de l'état de la fenêtre lorsque le bypass est activé, alors que l'on souhaite garder en mémoire le _window.state

EDIT : Même "problème" dans la partie Window auto que tu as modifié

@jmcollin78
Copy link
Owner

Vu !

923d374

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants