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

Fixing issues 47, 49, 53 and 55 #56

Merged
merged 5 commits into from
Mar 8, 2024
Merged

Fixing issues 47, 49, 53 and 55 #56

merged 5 commits into from
Mar 8, 2024

Conversation

luismalddonado
Copy link
Contributor

@luismalddonado luismalddonado commented Mar 1, 2024

Fixing issues 47, 49, 53 and 55

@CV8R
Copy link

CV8R commented Mar 1, 2024

Hi @luismalddonado I have been working on this too and looks like we need some additional changes for the actions. I am learning how to use GitHub and not show how to to a Pull Request on the v2.0.0 release. I will share what I have done to assist if you can import the changes.

I also made some other small mods that we can work in.
Is there a way I can share climate.py and intesisbox.py with you.

@CV8R
Copy link

CV8R commented Mar 1, 2024

I pushed a commit to my fork, its a bit of a mess the way I have done it. I am not a dev and learning how to use Github. The changes I made are commented with # CHANGED.

@CV8R
Copy link

CV8R commented Mar 1, 2024

Should add a breaking changes note to the readme since these changes are likely to mean changes to references in Automations.

@luismalddonado luismalddonado changed the title Fixing issues 49 and 55 Fixing issues 47, 49, 53 and 55 Mar 2, 2024
@jnimmo
Copy link
Owner

jnimmo commented Mar 4, 2024

Thanks both, just checking this includes what you were working on too @CV8R ?
@luismalddonado Could you please change into the component directory and run '''black .''' to tidy up the formatting then I can merge it.

@luismalddonado
Copy link
Contributor Author

luismalddonado commented Mar 4, 2024

Thanks both, just checking this includes what you were working on too @CV8R ? @luismalddonado Could you please change into the component directory and run '''black .''' to tidy up the formatting then I can merge it.

Done! Sorry. I have 0 experience with python.

@CV8R
Copy link

CV8R commented Mar 4, 2024

Will review and add if needed later today.

@luismalddonado
Copy link
Contributor Author

Will review and add if needed later today.

Anyway when I have time and get used to homeassitant and pyton I will review all the code and try to make things in a better way. Now it is just working! At least for me.

@CV8R
Copy link

CV8R commented Mar 5, 2024

With my lack of GiHub experience I am struggling to follow the changes and the files I pull down with GiHub desktop appear earlier than the v2.0.0 release. I have been editing them on my system direct...

Note, without adding the following to climate.py the new climate feature lags are not being used and its defaulting to the legacy off mode. Once added, it is likely to break Automations (it did on mine) but easy to fix - we should add a breaking change warning once set to false.

_enable_turn_on_off_backwards_compatibility = False

As per this blog post

Otherwise. Best go ahead and then I will try again later once the changes are merged.

I found some other things like CHN MODE and ONOFF being sent out of order, but will try add those later after these changes.

@luismalddonado
Copy link
Contributor Author

luismalddonado commented Mar 8, 2024

What is the status @jnimmo ??? I have this code working since 7 days is far more stable than "main" branch. Current "main" branch is not working :-(

@jnimmo jnimmo merged commit 06e6c83 into jnimmo:main Mar 8, 2024
1 of 2 checks passed
@jnimmo
Copy link
Owner

jnimmo commented Mar 8, 2024

Please ensure to install the pre-commit hooks in future to ensure it passes linting as I can't modify your pull request to fix it -

pip3 install pre-commit
pre-commit install
pre-commit run --all-files

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.

3 participants