search

watou / vera-ecobee

Vera Plugin for ecobee Thermostat
7 stars 5 forks source link

Exposing fanMinOnTime setting #27

Closed edutibau closed 6 years ago

edutibau commented 8 years ago

Hello,

Thanks for all the work you've put into this plugin. I've been using it for a while and always wished it supported the fanMinOnTime setting. Today I finally looked at ecobee's API and saw that it was exposed and decided to get it done :)

I added the action to I_Ecobee1.xml and the ability to use it in a scene in S_Ecobee1.xml.

Usages:

luup.call_action("urn:ecobee-com:serviceId:Ecobee1", "SetFanMinOnTime", {["MinOnTime"] = "{number between 0 and 60}"}, {deviceNumber})

http://{veraip}:3480/data_request?id=action&output_format=xml&DeviceNum={deviceNumber}&serviceId=urn:ecobee-com:serviceId:Ecobee1&action=SetFanMinOnTime&MinOnTime={number between 0 and 60}

This is my first time modifying a vera plugin... Let me know if there are any changes I should make for you to consider adding it to the main repo.

watou commented 8 years ago

Thank you for this pull request! Please see the 2 comments I've added regarding state variable. I would be happy to queue up your change for a v1.7 release.

edutibau commented 8 years ago

See my comment above.

I could try to get SendMessage/SetClimateHold working tonight, and update this pull request. Would you be able to test this in UI7?

Also, including it in the next release would be great!

watou commented 8 years ago

I have a Vera Edge here running UI7, so if you update this PR and ping me to test it on UI7, I will!

edutibau commented 8 years ago

Great. I'll update it tonight.

edutibau commented 8 years ago

Pushed the code to fix the other two options under Ecobee (SetClimateHold and SendMessage).

Tested using UI5: screen shot 2016-04-15 at 6 37 39 pm

watou commented 8 years ago

I can confirm the change looks good on UI5 (1.5.622 on a Vera 3), but on UI7 (1.7.1707 on a Vera Edge), none of the actions appear for the thermostat device. But I don't think this is new with this PR -- giving action arguments most likely never worked before.

Is there anything you can do to check what changes would be required to make this work on UI7 as well? This PR is still an overall improvement for UI5, but it would be nice to solve this for both UI5 and UI7. I don't have the time to track it down myself...

edutibau commented 8 years ago

Unfortunately documentation about plugin development and differences between UI5 and UI7 is nowhere to find. I went through all the plugins that I currently have installed that support UI7, and made a small change to the stateVariables. If that doesn't work, then I really have no idea :(

watou commented 8 years ago

Unfortunately documentation about plugin development and differences between UI5 and UI7 is nowhere to find.

Ain't that the truth. 😞

I will try your latest change in the next day or two, and if the plugin at least works as well as it did before this PR, and continues to solve the problem on UI5, I will merge it for v1.7. Thanks again.

watou commented 8 years ago

I'm not sure that sendEvents="yes" is compatible with <scpd xmlns="urn:schemas-upnp-org:service-1-0">. I've seen service XML files that use sendEvents="yes" but don't specify the xmlns attribute. It's possible that Vera doesn't mind, but I want to be conservative about things we can't verify (which, when it comes to writing Vera plugins, is a lot of things).