search

999LV / SmartVirtualThermostat

Smart Virtual Thermostat python plugin for Domoticz home automation system
MIT License
41 stars 31 forks source link

Fix the 2 opened issues #41

Closed ultrasuperpingu closed 3 years ago

ultrasuperpingu commented 3 years ago

Fix default economy temp value Add delta max temperature as a parameter

Accidentally (it was supposed to be in another branch) added custom pages to visualize thermostats data (to copy in www/template)

ultrasuperpingu commented 3 years ago

Those parameters was supposed to be edited before copying the files (Smart Thermostats.html and svt_viewer.js) in the www/templates directory. The domoticz example of custom pages (custom.example) is using a hard coded url too... I changed and just use the same address/port than the current page.

999LV commented 3 years ago

@ultrasuperpingu , many thanks for the change. May I make one more request to automate the installation ? possibly move the html and js files to a ./templates/ subfolder of the plugin folder and copy/remove these files to the ./www/templates/ folder in the onStart and onStop methods as per suggestion in https://www.domoticz.com/wiki/Developing_a_Python_plugin#Setup_custom_page_within_plugin Not that I want to be a pain (your work is a great addition), but this makes the whole thing more transparent to the user. Cheers.

ultrasuperpingu commented 3 years ago

Ok, I'll look at that this evening.

ultrasuperpingu commented 3 years ago

Ok, I added file copying on plugin startup. I don't delete files on plugin stop since you can have (and usually do) multiple instances on the plugin and you don't want the viewer to be deleted when you stop an instance... I'm really not sure this feature is a good idea though. If one do not want to have this custom page, it can not be suppressed as the custom page id copied at plugin startup. Manual install is to me a better option.

999LV commented 3 years ago

@ultrasuperpingu , thanks for this and your comments. I’ll test and think through it, hopefully over the weekend (I am super busy these days with my day job). The plugin is very popular and we do not want to introduce breaking changes but your work also makes me thinking about more user friendly set up options (I am not familiar with js so never focused on it).

ultrasuperpingu commented 3 years ago

The viewer is independant from the plugin code. There is no breaking changes either if you use the startup install or choose to let the users to manually install the custom page (maybe just write a simple bash script to copy the files is enough?).

That being said, I'm using the plugin for 2 weeks now and I'd like to propose some propositions for improvement and some will probably be breaking changes. But I think it's not a huge problem as long as there is a documentation to tell users how to migrate to a new version. They'd have the choice to do the migration or to stick with the old version...

And all that being said, thanks again for the plugin. I really think it should be integrated to domoticz as core :)

Edit: The viewer code is pretty basic old fashion js code. The "proper" way to do it would be to make an angular component I guess but I've done it in first way for me, and it does the job. I don't have enough time to do it with angular. The idea was just to share what I've done for me because I think some people may have use of it but I know it's not the best way to do it.

999LV commented 3 years ago

@ultrasuperpingu , I agree with you about no automatic install (my bad for causing extra work). I adjusted your PR accordingly and made a few cosmetic updates. Accepting you pull request with many thanks. Ideally the plugin page in the domoticz wiki needs to be updated as well. This gave me some ideas about a "setup" custom page to replace the cluttered plugin parameters page and all its CSV data entry, but this is a lot of work (and I need to catch up with js and angularjs ...). May be for the next lock-down.

ultrasuperpingu commented 3 years ago

@999LV No problem for extra work, I can handle this little work :). For plugin configuration, I opened an issue with some proposition including an other way to to it...

jakenl commented 3 years ago

My first response to the GUI for SVT was 'Wow'. Many times, when I try to understand how SVT managed to do a good job, or that it needs a little tweaking, I am switching back and forth between the switch page, temperature page and setpoint page. This puts it all nicely together!

A big point of improvement can be made in the scaling. With current low temperatures outside, the setpoint and inside temperatures are more or less a flat line on the top and the outside temperature on the bottom of the graph. I can't learn anything from that. Both inside, outside and setpoint should have their own vertical scale. While multiply y-axes can be difficicult to interpret in normal life, it is no problem for understanding and checking the behaviour of SVT

ultrasuperpingu commented 3 years ago

I agree that when all curves are displayed, then graph can be hard to interpret, but outside, inside and setpoint using the same scaling does provide an information. I'm not sure it is a good idea to remove it. You can click on the curve legend to show/hide it (and then, the scaling change to fit the displayed curves) so there is no need IMHO to display outside temperature with a different scale.