cyberjunky / home-assistant-toon_climate

This component provides a climate device for rooted Toon thermostats.
MIT License
32 stars 8 forks source link

Revised source code, bug fixes and new functionality #21

Closed rbak69 closed 3 years ago

rbak69 commented 3 years ago

I love this custom climate component for the Toon and I decided to personally contribute to this project. Please see the overview of the changes I made to the code below:

Revised the code in “climate.py” to support “auto” (scheduled) and “heat” (manual) work mode.

Also added the support for the “eco” preset. Instead of a normal preset this “eco” preset will allow you to put the Toon in “vacation mode”. The latter means it will change the setpoint in the Toon to what has last been set in the Vacation mode the Toon offers. It will keep this setpoint until you change it to one of the other (standard) setpoints supported by the Toon or change the actual work mode.

Changed the code so it uses the actual config parameters for the min and max temp. The previous version of the code had the parameters but did not change them in the code.

Changed to code to correctly update the work mode status in Home Assistant when the work mode is changed manually on the Toon device. The previous version did not support this and the status of the climate object in Home Assistant got out of sync.

Cleaned up and re-ordered the code a bit and added additional comments in the code to explain a bit more what it does and what the meaning is of some of the settings.

As for some reason home assistant does not like the comment with the triple quotation marks when I added them in different areas of the code I replaced all the previous comments with teh triple quotation marks with inline comments using the pound sign instead. After that Home Assistant seemed to be very happy.

Updated the “README.mb” file the reflect the above.

I have tested the updated code as much as I can on my own "Toon 1" and it seems to works fine hence why I am happy to commit the changes. All changes either in home assistant or on the manually Toon are working well and in sync with the status on the Toon or in Home assistant.

cyberjunky commented 3 years ago

@rbak69 Hi René thanks alot for your contribution! I have one request though, can you change the comments to use triple quotes instead of the hashes? I know both are allowed, but hashes look so ugly ;) So like so:

"""
If I really hate pressing `enter` and
typing all those hash marks, I could
just do this instead
"""
rbak69 commented 3 years ago

Hi Ron happy to be able to contribute to this project!

I actually replaced the triple quotes on purpose as Home Assistant did not like the triple quotes when I added the comments with the triple quotes in specific locations in the code earlier.

I found out that using the hashes the problem was resolved. It took me some time to find out why Home Assistant did not load correctly after certain updates and found that just removing the comment or changing it from triple quotes to hashes resolved it. So I do not think it was a Python issue but as it seems it was the Home Assistant check that seems to be picky.

Personally I do not mind whether the comment is between triple quotes or using the hashes. As long as there are some decent comments in the code I am usually happy :-) Although I must admit when I printed the code earlier today with the hashes it looks a bit more readable to me that way. But I guess it is all personal preference as well.

Anyway I can see whether I can change this later and give it another try.

Apart from that is there anything else or are you happy with the changes and additional feature I added to the code and the cleanup and re-organizing I have done?

rbak69 commented 3 years ago

Hi Ron I used the same code and just replaced the hashes with the triple quotes. Unfortunately Home Assistant still does not like it. I have attched the file so you can test it and see whether there is something obvious causing Home Assistant not to load the code. Again using the same code with the comment with hashes it all loads without any issues

climate.py.txt

cyberjunky commented 3 years ago

@rbak69 There were some syntax errors, try this one, I did some code changes, and didn't test it yet, only validated syntax and code quality. I suggest to look at setting up development environment using vscode editor, it's nice, then it automatically check for issues while writing.

BTW, we need to add apply the changes to README.md also to info.md, which is the readme file used inside HACS.

climate.py.txt

rbak69 commented 3 years ago

Hi Ron that works fine. Realized I was a bit too quick with my find and replace :-(. As a result some comments were actually uncommented in the process causing the issues. That was not the case with the previous code but that might have had some other root cause. Nevertheless this works fine and I have made a few minor cosmetical changes like removing some extra lines. I also made a few minor changes for code consistancy. Happy with the result and some quick tests seem to indicate it is working fine and syncing correctly both ways.

I will need to look into the test environment you mentioned. For this I used Spyder 4.2 that came with Anaconda. I am a Python newbe so any advice is much apreciated.

I have replaced the content of the "info.md" with the content of the "README.md" so both are now identical and will commit the updates.