Nebula83 / openhab2-addons

Add-ons for openHAB 2.x
Eclipse Public License 1.0
8 stars 2 forks source link

Updates for the evohome binding / PR #15

Closed magnayn closed 5 years ago

magnayn commented 6 years ago

Hi -

I have been using the evohome binding, and I've tried to fix some of the PR issues from your pull request into openhab2-addons. I've also implemented the missing setpoint stuff as I want to be able to bind it to HomeKit.

I rebased off of openhab2's master branch, so you may need to cherry-pick the changes if you want to submit them into your original PR.

Let me know if this is useful or not - it'd be great to get this into the main distro..

Nebula83 commented 6 years ago

Hi @magnayn, what an awesome initiative! I think you are referring to this commit:https://github.com/Nebula83/openhab2-addons/pull/15/commits/65341c80fa4eb7a67af708cde165a470145c9a72? (there are 239 commits in your PR ;-) ) I'm trying to get it merged as fast as possible indeed, so help is appreciated.

magnayn commented 6 years ago

Yeah, so I'd forgotten that I'd rebased the evohome patches on top of the current master of openhab2-addons, which is why it has hundreds of commits (it's really the last two or three, if I remember correctly).

quickest way to add them is probably to cherry-pick them out of my repository - I don't know how familiar with git you are?

On Mon, Feb 19, 2018 at 3:28 PM, Nebula83 notifications@github.com wrote:

Hi @magnayn https://github.com/magnayn, what an awesome initiative! I think you are referring to this commit:65341c8 https://github.com/Nebula83/openhab2-addons/commit/65341c80fa4eb7a67af708cde165a470145c9a72? (there are 239 commits in your PR ;-) ) I'm trying to get it merged as fast as possible indeed, so help is appreciated.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Nebula83/openhab2-addons/pull/15#issuecomment-366725824, or mute the thread https://github.com/notifications/unsubscribe-auth/AADRlc9Ncu_5h0P1HpdjJc0-oPjpH5KLks5tWZMDgaJpZM4SJv-Q .

Nebula83 commented 6 years ago

I think I'll manage :-) But if I have questions I'll ask you ;-)

Thing is, I was about to restructure some stuff to hopefully get the PR moving along a bit faster (chopping it in smaller pieces making the footprint of an iteration smaller and refactor the stuff out I wasn't too happy with). It probaly means that I'l copy-paste some of your improvements over to the code base. Please give me your email adres so I can provide the proper also-by in the PR.

Thanks again for the help!

magnayn commented 6 years ago

Sure - nigel.magnay@gmail.com. Let me know if there's anything I can help with.

It's a shame that openhab has a bunch of plugins that it seems are left a bit to rot by not being in the main distribution -- I personally find thinge like "need extra space at the end of this file" to be needlessly picky in a PR (but my bias is I'd rather have a non-pretty but working plugin rather than none at all). Splitting out the generally-useful evohome stuff into a completely separate project might be one way to go..

On Mon, Feb 19, 2018 at 6:32 PM, Nebula83 notifications@github.com wrote:

I think I'll manage :-) But if I have questions I'll ask you ;-)

Thing is, I was about to restructure some stuff to hopefully get the PR moving along a bit faster (chopping it in smaller pieces making the footprint of an iteration smaller and refactor the stuff out I wasn't too happy with). It probaly means that I'l copy-paste some of your improvements over to the code base. Please give me your email adres so I can provide the proper also-by in the PR.

Thanks again for the help!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Nebula83/openhab2-addons/pull/15#issuecomment-366772790, or mute the thread https://github.com/notifications/unsubscribe-auth/AADRlZH7HrtvD0IGyjU5h08vs5qSB6dbks5tWb4xgaJpZM4SJv-Q .

Nebula83 commented 6 years ago

Hiya magnayn, I recently updated the pull request on Openhab and made some substantial changes in the process. I'm afraid I did not look at your changes yet. Maybe I have already implemented enough for you in the latest release to make it work. I think the chances are pretty good of having it merged now.