search

genielabs / HomeGenie

HomeGenie, the programmable automation intelligence
https://homegenie.it
GNU General Public License v3.0
401 stars 157 forks source link

Added new device handler that allows one to assign multiple device handl... #125

Closed hybridview closed 9 years ago

hybridview commented 9 years ago

...ers to a single device. Added AEON HomeEnergyMonitor device that demonstrates this new class. Added correct message parsing to Meter generic based on ZWAVE specifications. Changed ZWaveNode to handle Meter command class as a basic report, since it is not multi-instance. Added multiinstance param support to ZWave class (Meter only for now).

hybridview commented 9 years ago

Just adding a note on testing results: Performance has not been degraded with this change. My tests showed no consistent increase in response time. Also, I tested this using both POLLING (via script) and automated reporting.

genemars commented 9 years ago

@hybridview I created a new branch zwave-rework and will post there a proposal for zwavelib rework. So we can discuss and improve it.

genemars commented 9 years ago

@hybridview see https://github.com/genielabs/HomeGenie/commit/9ac3334ea0c31b6014af6274877488e270b72c53 feel free to suggest any other solution. This is just a very starting draft of how it could be.

hybridview commented 9 years ago

Sounds like a great idea! I can't wait to check it out.

What should I do with the changes in this pull request? They don't really belong in zwave rework, but I currently need these commits to properly support my HEM reports (until zwave rework is stable enough). I can keep a separate branch, but it will end up being a lot of work to keep it up to date with your main branch.

genemars commented 9 years ago

I will continue on this zwave-rework branch and rewrite all other command classes. after that I will need help testing that everything is still working as before. by then probably your HEM devices will work without any further modification to the library.

hybridview commented 9 years ago

Ok. That makes sense. It will force me to learn GIT a little better, which I should be doing anyways. :)

genemars commented 9 years ago

Hey @hybridview , if you want to help testing, the main zwavelib rework is completed. You can try to see if it work with your zwave devices. I think it's better not to merge anything in the current master, but rather work on the zwave-rework branch.

genemars commented 9 years ago

well... gotta fix something first =) wait for my next push

genemars commented 9 years ago

ok now it's ready for testing

hybridview commented 9 years ago

The zwave rework, after a small fix for sending instance# for encapped meter params, is working well in my local build. Thanks for all of the work!

Regarding the meter handler, at some point, it might be a good idea to create a MeterValue class instead of using the EnergyValue class, since meter supports more than just energy (gas, etc). It also supports a "Previous Value", which might be useful later in the project. The commit in this pull request includes a MeterValue class that could be useful as a reference.

hybridview commented 9 years ago

I updated my previous comment to reflect the successful test. This PULL request is not applicable anymore, so I will close it to clean things up.

genemars commented 9 years ago

Yes a MeterValue class is a good idea, just need to earn more knowledge on the topic.