search

hap-java / HAP-Java

Java implementation of the HomeKit Accessory Protocol
MIT License
153 stars 82 forks source link

Refactoring for optional #101

Closed yfre closed 4 years ago

yfre commented 4 years ago

a major refactoring based on discussion in https://github.com/hap-java/HAP-Java/issues/63 and work done by @ccutrer

PR has a breaking changes and is not compliant to previous version.

J-N-K commented 4 years ago

I don‘t have enough knowledge about HAP or this code to really review this. But sounds good and I would really appreciate seeing a new release so we can get back to regular releases in OH instead of the forked/cherry-picked version we have now.

yfre commented 4 years ago

i have just upgraded OH2.5 homekit to this HAP lib on my local. it was pretty straightforward. i will push it to OH repo once this PR is merged. in meantime will add more accessories to it.

ccutrer commented 4 years ago

i have just upgraded OH2.5 homekit to this HAP lib on my local. it was pretty straightforward. i will push it to OH repo once this PR is merged. in meantime will add more accessories to it.

I'd be interested in seeing the OpenHAB PR, even if it can't be merged until this PR is in.

(And sorry I haven't had any time to look into any HAP stuff. Work has been insane the last month or so. I've been seeing notifications though, so I'm aware work is being done).

yfre commented 4 years ago

yes, openHab is a good validation for the Java-HAP API concept, e.g addOptionalCharacteristic methods approach work very well, additional interfaces for optional characteristics not. in any case, adding support for optional attributes required some major changes in OH addon. Im almost done there - only the most interesting items left, thermostat and valve. i will inform here once the OH PR is created.

J-N-K commented 4 years ago

@ccutrer, @timcharper Can we proceed here? I would like to see these improvements (and the fixes for windowcovering and window/door support) in openHAB 2.5.5 but I would prefer not to compile a own version again.

blafois commented 4 years ago

Hello! I discover your PR now ... I have also added support for Window / Door and a PR pending. Is the maintainer dead ?

ccutrer commented 4 years ago

@blafois not completely dead :). @J-N-K I'm +1 on this PR so far, I just wanted to see how it worked out on the OpenHAB side before fully approving it. Fortunately/unfortunately for me during this lockdown my work has gotten busier than I've ever been (I work in education technology), plus with lots of kids trying to keep up with their own schoolwork... any spare minute I happen to have gets spent trying to relieve a bit of stress, and not working on home automation stuff.

blafois commented 4 years ago

Thanks ! So if my understanding is correct - this PR should bring support for Door & Window and as such I might remove mine (https://github.com/hap-java/HAP-Java/pull/103) ?

yfre commented 4 years ago

@blafois yes, this PR adds support for door and window. as this PR includes a major refactoring it could still make sense to merge your PR in order to add door&window support to the last stable version.

regarding validation of this PR. it took a little bit longer to prepare a patch for openhab addon. not that much due the changes in java-hap but mainly due challenges to map openhab items to optional characteristics using labels. at the end, i decided to use metadata.

@ccutrer please take a look on OH PR here https://github.com/openhab/openhab-addons/pull/7495

feedback is welcome. here or directly there.

yfre commented 4 years ago

@ccutrer @timcharper i have added all HomeKit accessories except of few related to audio and video streaming. In total we have 37 out of 45 HomeKit accessories defined in the latest HAP non-commercial specification.

the non-official release of HAP-Java is already used in latest openHAB snapshot and we get good feedback.

it would be great if we could make an official release. please take a look and approve if it is ok.

ccutrer commented 4 years ago

@yfre : do you need anything else from me? I didn't look over everything, but it seems @J-N-K has done good code reviews, and the overall structure looks good with my original intentions. I don't know how to publish an official version. and it seems you've been getting things into OpenHAB as well. I've added as a contributor to this repo as well

yfre commented 4 years ago

@ccutrer thank you a lot! I will check whether i can publish an official version.

yfre commented 4 years ago

looks like i cannot create new releases. @timcharper could you please create a new release based on master