Skip to content

Refactoring for optional #101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Jun 1, 2020
Merged

Conversation

yfre
Copy link
Contributor

@yfre yfre commented Mar 16, 2020

a major refactoring based on discussion in #63 and work done by @ccutrer

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

@J-N-K
Copy link

J-N-K commented Mar 16, 2020

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
Copy link
Contributor Author

yfre commented Mar 16, 2020

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
Copy link
Collaborator

ccutrer commented Apr 1, 2020

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
Copy link
Contributor Author

yfre commented Apr 1, 2020

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
Copy link

J-N-K commented Apr 12, 2020

@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
Copy link

blafois commented Apr 14, 2020

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

@ccutrer
Copy link
Collaborator

ccutrer commented Apr 14, 2020

@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
Copy link

blafois commented Apr 14, 2020

Thanks ! So if my understanding is correct - this PR should bring support for Door & Window and as such I might remove mine (#103) ?

@yfre
Copy link
Contributor Author

yfre commented Apr 25, 2020

@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 openhab/openhab-addons#7495

feedback is welcome. here or directly there.

Eugen Freiter added 6 commits April 26, 2020 13:23
Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
…accessories

Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
Eugen Freiter added 5 commits May 3, 2020 12:37
Signed-off-by: Eugen Freiter <freiter@gmx.de>
…characteristic

Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
@yfre
Copy link
Contributor Author

yfre commented May 12, 2020

@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.

Eugen Freiter added 3 commits May 17, 2020 00:23
Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
…atures

Signed-off-by: Eugen Freiter <freiter@gmx.de>
Eugen Freiter added 2 commits May 24, 2020 17:11
Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
@ccutrer ccutrer merged commit 7145c94 into hap-java:master Jun 1, 2020
@ccutrer
Copy link
Collaborator

ccutrer commented Jun 1, 2020

@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
Copy link
Contributor Author

yfre commented Jun 2, 2020

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

@yfre
Copy link
Contributor Author

yfre commented Jun 2, 2020

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

@yfre yfre mentioned this pull request Jun 10, 2020
@yfre yfre deleted the refactoring_for_optional branch February 12, 2022 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants