Skip to content

Refactoring for optional - #101

Merged
ccutrer merged 25 commits into
hap-java:masterfrom
yfre:refactoring_for_optional
Jun 1, 2020
Merged

ccutrer merged 25 commits into
hap-java:masterfrom
yfre:refactoring_for_optional

Conversation

@yfre

@yfre yfre commented Mar 16, 2020

Copy link
Copy Markdown
Contributor

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

J-N-K commented Mar 16, 2020

Copy link
Copy Markdown

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

yfre commented Mar 16, 2020

Copy link
Copy Markdown
Contributor Author

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

ccutrer commented Apr 1, 2020

Copy link
Copy Markdown
Member

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

yfre commented Apr 1, 2020

Copy link
Copy Markdown
Contributor Author

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

J-N-K commented Apr 12, 2020

Copy link
Copy Markdown

@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

blafois commented Apr 14, 2020

Copy link
Copy Markdown

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

@ccutrer

ccutrer commented Apr 14, 2020

Copy link
Copy Markdown
Member

@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

blafois commented Apr 14, 2020

Copy link
Copy Markdown

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

@yfre

yfre commented Apr 25, 2020 •

Copy link
Copy Markdown
Contributor Author

@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
Comment thread pom.xml Resolved
@yfre

yfre commented May 12, 2020 •

Copy link
Copy Markdown
Contributor Author

@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 2 commits May 24, 2020 17:11
Signed-off-by: Eugen Freiter <[email protected]>
@ccutrer
ccutrer merged commit 7145c94 into hap-java:master Jun 1, 2020
@ccutrer

ccutrer commented Jun 1, 2020

Copy link
Copy Markdown
Member

@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

yfre commented Jun 2, 2020

Copy link
Copy Markdown
Contributor Author

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

@yfre

yfre commented Jun 2, 2020

Copy link
Copy Markdown
Contributor Author

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