ady624 / webCoRE

webCoRE is a web version of CoRE
GNU General Public License v3.0
251 stars 984 forks source link

Hubitat smarthings merge #70

Open jp0550 opened 6 years ago

jp0550 commented 6 years ago

Works between Hubitat and Smartthings with just an import change needed at the top of the piston source file.

If this is merged the fuel stream PR can be ignored as this supports both local and external fuel streams.

idpaterson commented 6 years ago

Amazing work, thank you for moving forward with the merge. I will set this up for minions to test both sides of the integration (a few of us have Hubitat) and hopefully we'll be able to push it through in the next release within a few weeks.

idpaterson commented 6 years ago

It looks like there will be a small bug fix release before this goes into testing. You're on deck for testing once that small release is deployed.

jp0550 commented 6 years ago

Sounds good! There probably be another commit to this to iron out some issues I found, and to handle a fw change from Hubitat.

idpaterson commented 6 years ago

I've had good results with this pattern for the HubAction and Protocol classes. May not be the most idiomatic way to accomplish it but is this something you can run with?

private static Class HubActionClass() {
    try {
        return 'physicalgraph.device.HubAction' as Class
    } catch(all) {
        return 'hubitat.device.HubAction' as Class
    }
}
sendHubCommand(HubActionClass().newInstance(requestParams, null, [callback: localHttpRequestHandler]))

Since it is used everywhere to distinguish between ST and HE I would also love to see an abstraction of the hubUID test. Is there a good way to use an alternate self-documenting name like isHubitat?

jp0550 commented 6 years ago

Sure thing, I'll pull it in and give it a try next chance I get, along with changing hubUID references.

jp0550 commented 6 years ago

This one should be ready to go. There's a lot of performance improvements here for Hubitat and for the first time I was seeing timings lower than SmartThings. Looks like the biggest slowdown in that system was the state size so I've reduced it wherever possible mostly with the stats and logs limits which are now limited to 10 and 50 with a setting that can be changed per piston if the user wants more. Also I removed atomic state loading for both systems after getRuntimeData if the piston didn't have to wait at a semaphore which helped as well.

If you get a chance, let me know what you think and if you see any changes that should be made.

idpaterson commented 6 years ago

Excellent! Should I include a warning when importing pistons to a Hubitat instance, something along the lines of "Unlike the computing cloud that runs pistons on SmartThings, your Hubitat Elevation hub runs all of your pistons. Please import and test in small batches to avoid overloading your hub." Perhaps if they are importing more than say 20 pistons? Not the best wording since the running pistons are what will overload the hub rather than the import, but if you have any other performance guidelines I think the import feature will be an important place for this reminder.

jp0550 commented 6 years ago

I think that's a great idea as I can see users coming to Hubitat using this tool to pull in their ST pistons. I'll get together a list of performance guidelines and send them to you.

jp0550 commented 6 years ago

Biggest things in Hubitat as far as performance are below:

  1. After creating a piston go into the Hubitat apps section, click the newly created piston, and hit done. Child apps in Hubitat can't (yet) create the settings by themselves so it can cache devices. You'll get a warning in the logs until you do so, and the piston will run around 150ms slower as it grabs devices from the parent on each execution.

  2. Having multiple pistons reference the same event creates slowdowns as two pistons have to spin up to handle that event. It's better to put multiple statements referencing the event into one piston for situations where you have an event like motion triggering multiple actions.

  3. Write pistons keeping performance in mind following some of the guidelines from this thread: https://community.webcore.co/t/conditions-and-triggers-the-difference/164

I have some changes coming soon to fix timeToday in Hubitat. You can't create a time variable right now with something like '04:00PM'. It will just return the current time.