[velux] Velux Binding initial contribution #2531
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martinvw
added
the
new binding
|
Thanks! Note that our queue is rather big so it might take some time, but your efforts are greatly appreciated! |
martinvw
requested changes
Aug 7, 2017
addons/binding/org.openhab.binding.velux/doc/allclasses-frame.html
Outdated
Show resolved
Hide resolved
addons/binding/org.openhab.binding.velux/ESH-INF/i18n/velux_de.properties
Outdated
Show resolved
Hide resolved
addons/binding/org.openhab.binding.velux/ESH-INF/i18n/velux_en.properties
Outdated
Show resolved
Hide resolved
...org.openhab.binding.velux/src/main/java/org/openhab/binding/velux/VeluxBindingConstants.java
Outdated
Show resolved
Hide resolved
|
Thanks, now you added only 3500+ lines instead of the previous 35.000+ lines 😆 |
|
Summary of (rebased) changes according to review and other user feedback: |
htreu
previously requested changes
Sep 15, 2017
There was a problem hiding this comment.
Hi @gs4711, thanks for this binding contribution. I´m happy to see an attempt to include Velux io-homecontrol into openHAB!
I left some inline remarks in the code but didn´t make it all through the end.
In addition I have some more high-level comments which we need to discuss:
From the code it looks like the bridge actually is the only device you are able to communicate with. It will expose the configured scenes (and devices but not really used) which then may be triggered ON by the gateway. Therefore the modelling with one bridge and several things is not necessary: You may as well define only one thing (the KLF200) which in turn has several channels, one channel for every scene which is available on the gateway. This channels may then be added dynamically by discovery or manually in the *.things file like
Thing velux:klf200:home [ host="velux.bridge" ] {
Channel V_DG_Window_Mitte_100
Channel V_DG_Window_Mitte_90
Channel V_DG_Window_Mitte_80
...
}
This way you also avoid the illegal notation of ...ACTION#V_DG_Window_Mitte_100 which is meant to address a channel-group-type (ACTION) and a channel-type V_DG_Window_Mitte_100 inside this group. You currently misuse this to pass the scene name to the channel in case it is not defined in the configuration.
Another advantage is you dont need to remember all the available scenes in their own data structure because the Thing will handle this for you with channels.
When you put all the system information into properties this will then make for clean and sleek binding
I´m personally very interested in your work since I have some KLF50 devices which I intended to use for Velux roller shutter control. It seems like I have to trade them in for a KLF200 now :-)
Let me know what you think.
Cheers,
Henning
addons/binding/org.openhab.binding.velux/ESH-INF/i18n/velux_de.properties
Outdated
Show resolved
Hide resolved
addons/binding/org.openhab.binding.velux/ESH-INF/i18n/velux_de.properties
Outdated
Show resolved
Hide resolved
addons/binding/org.openhab.binding.velux/ESH-INF/i18n/velux_de.properties
Outdated
Show resolved
Hide resolved
addons/binding/org.openhab.binding.velux/ESH-INF/thing/bridge.xml
Outdated
Show resolved
Hide resolved
addons/binding/org.openhab.binding.velux/ESH-INF/thing/bridge.xml
Outdated
Show resolved
Hide resolved
...penhab.binding.velux/src/main/java/org/openhab/binding/velux/bridge/VeluxBridgeFirmware.java
Outdated
Show resolved
Hide resolved
...penhab.binding.velux/src/main/java/org/openhab/binding/velux/bridge/VeluxBridgeFirmware.java
Outdated
Show resolved
Hide resolved
...enhab.binding.velux/src/main/java/org/openhab/binding/velux/bridge/VeluxBridgeLANConfig.java
Outdated
Show resolved
Hide resolved
...nhab.binding.velux/src/main/java/org/openhab/binding/velux/bridge/comm/BCdetectProducts.java
Outdated
Show resolved
Hide resolved
...hab.binding.velux/src/main/java/org/openhab/binding/velux/bridge/comm/BCidentifyProduct.java
Outdated
Show resolved
Hide resolved
|
@htreu: thanks for your comments. Now, after vacation, I will address the different points!
Best regards,
Guenther
|
|
hi @gs4711, this is just a ping :-) It would be awesome to have your contribution merge ready.Let me know if you need any help on this. Cheers |
|
Hi Henning,
am still alive… made good experiences with the running code
and have to include some code changes according to your comments.
Unfortunately I haven’t been able to find the appropriate starting point
for adapting the binding to be suitable for v1 as well … some responses
lead me to the conclusion that there are for the time being some
preferences to the more stable version one.
Best regards,
Guenther
Von: Henning Treu [mailto:notifications@github.com]
Gesendet: Donnerstag, 9. November 2017 11:51
An: openhab/openhab2-addons <openhab2-addons@noreply.github.com>
Cc: Guenther Schreiner <guenther.schreiner@smile.de>; Mention <mention@noreply.github.com>
Betreff: Re: [openhab/openhab2-addons] [velux] Initial contribution of Velux binding (#2531)
hi @gs4711 <https://github.com/gs4711> , this is just a ping :-) It would be awesome to have your contribution merge ready.Let me know if you need any help on this.
Cheers
Henning
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <https://github.com/openhab/openhab2-addons/pull/2531#issuecomment-343118374> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AMbkaOmGfGH51clapY_FO1mPsEW7lFoUks5s0tkOgaJpZM4Ovm_v> . <https://github.com/notifications/beacon/AMbkaEv8ViSNqncwS9OeBlATo4aMEHszks5s0tkOgaJpZM4Ovm_v.gif>
|
|
Hi @gs4711 imho it would be probably okay to have an initial version of your binding ready and officially in the repo and add additional version support later on. If its working for you this is a good start. Maybe other community members will then step up and help with further version support. |
|
Done with changes. |
|
Hello team,
currently I have come to a dead end as Jenkins and Travis/CI complain about failures
not only on the current pull request (https://github.com/openhab/openhab2-addons/pull/2531 <https://github.com/openhab/openhab2-addons/pull/2531>)
but also on the official Github repository.
Can you confirm that there is a general problem?
//original/openhab2-addons/addons/binding/org.openhab.binding.feed.test/target/work/data/.metadata/.log.
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] openHAB 2 Add-ons .................................. SUCCESS [ 0.411 s]
[INFO] openHAB Add-ons .................................... SUCCESS [ 0.017 s]
[INFO] openHAB Bindings ................................... SUCCESS [ 0.018 s]
[INFO] AirQuality Binding ................................. SUCCESS [ 16.104 s]
[INFO] AllPlay Binding .................................... SUCCESS [ 7.804 s]
[INFO] Amazon Dash Button Binding ......................... SUCCESS [ 7.009 s]
[INFO] Atlona Binding ..................................... SUCCESS [ 8.563 s]
[INFO] Autelis Binding .................................... SUCCESS [ 5.993 s]
[INFO] AVM FRITZ! Binding ................................. SUCCESS [ 7.922 s]
[INFO] AVM FRITZ! Binding Tests ........................... SUCCESS [ 9.012 s]
[INFO] BigAssFan Binding .................................. SUCCESS [ 7.518 s]
[INFO] BoschIndego Binding ................................ SUCCESS [ 5.201 s]
[INFO] Chromecast Binding ................................. SUCCESS [ 11.086 s]
[INFO] CM11A Binding ...................................... SUCCESS [ 11.339 s]
[INFO] CoolMasterNet Binding .............................. SUCCESS [ 5.478 s]
[INFO] D-Link Smart Home Binding .......................... SUCCESS [ 5.594 s]
[INFO] DSCAlarm Binding ................................... SUCCESS [ 10.571 s]
[INFO] Exec Binding ....................................... SUCCESS [ 6.500 s]
[INFO] openHAB I/O Add-Ons ................................ SUCCESS [ 0.009 s]
[INFO] Transport Feed bundle .............................. SUCCESS [ 1.392 s]
[INFO] Feed Binding ....................................... SUCCESS [ 5.115 s]
[INFO] Feed Binding Tests ................................. FAILURE [ 8.930 s]
[INFO] Feican Binding ..................................... SKIPPED
Best regards,
Guenther
|
|
Jenkins failed due to memory issues, Cloudbees stumbled over a failing test in the feed binding, which is unrelated to your code - this test seems to be unstable, see also https://github.com/openhab/openhab2-addons/pull/3238#issuecomment-365574418. So please simply ignore those problems as they are not related to your PR. |
|
Thanks, Kai, for the feedback. But, really *memory issues*?
Don’t know what happens …. let’s take a look when accessing the plain GitHub repository
% git clone git@github.com:openhab/openhab2-addons.git
% cd openhab2-addons/
% mvn clean install
leads to …
An error has occurred. See the log file openhab2-addons/addons/binding/org.openhab.binding.feed.test/target/work/data/.metadata/.log.
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] openHAB 2 Add-ons .................................. SUCCESS [ 0.411 s]
[INFO] openHAB Add-ons .................................... SUCCESS [ 0.017 s]
[INFO] openHAB Bindings ................................... SUCCESS [ 0.018 s]
[…]
[INFO] Transport Feed bundle .............................. SUCCESS [ 1.392 s]
[INFO] Feed Binding ....................................... SUCCESS [ 5.115 s]
[INFO] Feed Binding Tests ................................. FAILURE [ 8.930 s]
[INFO] Feican Binding ..................................... SKIPPED
[INFO] Folding Binding .................................... SKIPPED
[…]
[INFO] openHAB 2 Feature Verification ..................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:47 min
[INFO] Finished at: 2018-02-14T12:19:49+01:00
[INFO] Final Memory: 136M/891M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.eclipse.tycho:tycho-surefire-plugin:1.0.0:test (default-test) on project org.openhab.binding.feed.test: An unexpected error occured while launching the test runtime (return code 13). See log for details. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn <goals> -rf :org.openhab.binding.feed.test
This behavior is independent of the platforms where we're developing. IMHO this did not occur in the past.
… Am 14.02.2018 um 13:08 schrieb Kai Kreuzer ***@***.***>:
Jenkins failed due to memory issues, Cloudbees stumbled over a failing test in the feed binding, which is unrelated to your code - this test seems to be unstable, see also #3238 (comment) <https://github.com/openhab/openhab2-addons/pull/3238#issuecomment-365574418>.
So please simply ignore those problems as they are not related to your PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <https://github.com/openhab/openhab2-addons/pull/2531#issuecomment-365588111>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMbkaF_su1QDN3kM-vgner4MC-KqTbm0ks5tUsyigaJpZM4Ovm_v>.
|
Yes, I can confirm. Working on it. |
|
Hello Henning,
is there anything I can provide in addition? I’d like to start some enhancements after the 1st release...
Cheers, Guenther
… Am 11.01.2018 um 15:08 schrieb Henning Treu ***@***.***>:
Hi @gs4711 <https://github.com/gs4711> imho it would be probably okay to have an initial version of your binding ready and officially in the repo and add additional version support later on. If its working for you this is a good start. Maybe other community members will then step up and help with further version support.
Please don't hesitate to bring a first version live.
Cheers,
Henning
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <https://github.com/openhab/openhab2-addons/pull/2531#issuecomment-356944185>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMbkaNAF-ULXeOKEjKL1hyCgDCDpPValks5tJhXqgaJpZM4Ovm_v>.
|
|
Hi Guenther (ping @gs4711), I just wanted to point your attention again to https://github.com/openhab/openhab2-addons/pull/2531#pullrequestreview-62976881 where I addressed some modelling issues:
What do you think? |
|
Hi Henning.
Thanks for the pointer to #2531 (review). Let me try a brief answer:
should all be properties, […]we will provide other mechanisms to do that.
Perhaps I've missed something, but I cannot follow: what kind of mechanism do you think of?
Do you have an example for this kind of properties?
The bridge seems to be the only addressable thing and your things are mainly for grouping purposes.
According to the guidelines ("Things represent devices or services that can be individually added to, configured or removed from the system",
"A bridge is a specific type of thing as it can additionally provide access to other Things as well.") I assume that the binding exactly follows
the definition.
Just some question to make sure that we're discussing the same issue:
- Do you agree with the velux device as bridge? BTW there seem to be other bridging device in the release queue.
- If you look at the complete set of velux components (ignoring for a moment that there is a bridge which interfaces to the actors),
the three smarthome (thing!) categories blinds/PowerOutlet/Window are fully matching the functionality where as the channel
categories lead to ignore the set of windows ...
Best regards,
Guenther
|
|
Sorry for the late answer. I had to make up my mind about the modelling first.
Unfortunately not. Right now thing properties are only shown in Paper UI in the thing details view. But in case there is a strong need to display them to the End-User we should discuss this in the Eclipse SmartHome issue tracker.
The velux device is not a 100% fit for a bridge. Bridges tend to control an arbitrary number of physical devices which each of them being addressable from the bridge point of view.
May be I'm totally off, so here are some questions which are not answered by the manual:
Thanks and cheers. |
|
Hi @gs4711, I know my questions are not easy going and would eventually mean a rewrite of some parts of the binding. However I would be happy if we could go on with the discussion and bring the binding forward. Did you have time to read my last comment? Let my know what you think. |
|
Hello Henning, thanks for your message. Having read your recent comments, my personal feeling is that the OH2 environment has a lot of open issues like undocumented ways of implementing stories. Therefore, I had decided to focus on releasing a OH1 binding which is now already included in the recent build. Coming back to your comments: the KLF device is designed to control an arbitrary number of physical devices which can be addressed from the bridge individually. The limitation of five scenes you had mentioned does not exist in real life. In fact, the currently officially published firmware is lacking ONE important fact (which IMHO essentially limits the naming as „bridge“ device): it cannot return the actual state of the different devices – but this is a confirmed firmware limitation (btw which has been proven to be overcome by a second snooping KLF200 which can return the different states originated by the first KLF200. To put it in a nutshell: I’m not convinced to go into the direction of five channels. Of course, if there is any sample of working code, I’d like to change the property handling. Best regards, Guenther |
htreu
reviewed
Aug 31, 2018
There was a problem hiding this comment.
Thanks @gs4711 for coming back to me so quickly. I did another quick scan of the code and have some more questions. Again, its not my plan to bother you with nonrelevant things. During review we try to clean up the binding architecture so we end up with potential candidates for Eclipse SmartHome.
Thanks again for your patience and I'm happy to discuss any issues.
...penhab.binding.velux/src/main/java/org/openhab/binding/velux/handler/VeluxBridgeHandler.java
Outdated
Show resolved
Hide resolved
addons/binding/org.openhab.binding.velux/ESH-INF/thing/bridge.xml
Outdated
Show resolved
Hide resolved
addons/binding/org.openhab.binding.velux/ESH-INF/thing/bridge.xml
Outdated
Show resolved
Hide resolved
wborn
changed the title
Hilbrand
approved these changes
Feb 22, 2020
martinvw
approved these changes
Feb 23, 2020
Co-Authored-By: J-N-K <J-N-K@users.noreply.github.com> Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
Co-Authored-By: J-N-K <J-N-K@users.noreply.github.com> Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
Travis tests were successfulHey @gs4711, |
|
@J-N-K if you fine can you then approve and merge? |
Travis tests have failedHey @gs4711, 1st BuildExpand here |
Shall I take care about the
or can it be ignored for now? |
J-N-K
approved these changes
Feb 25, 2020
|
Since java11 succeeded it seems to be a temporary failure. I have restarted Travis. |
|
Not sure why the status is not reported, Travis succeeded. Thanks for the huge amount of work you put into this one. |
leluna
pushed a commit
to leluna/openhab2-addons
that referenced
this pull request
Mar 21, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de> Signed-off-by: leluna <hengrui.jiang@googlemail.com>
Hans-Reiner
pushed a commit
to Hans-Reiner/openhab2-addons
that referenced
this pull request
Apr 11, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de> Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>
markus7017
pushed a commit
to markus7017/openhab-addons
that referenced
this pull request
May 29, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
LoungeFlyZ
pushed a commit
to LoungeFlyZ/openhab2-addons
that referenced
this pull request
Jun 8, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
J-N-K
pushed a commit
to J-N-K/openhab-addons
that referenced
this pull request
Jul 14, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
J-N-K
pushed a commit
to J-N-K/openhab-addons
that referenced
this pull request
Jul 14, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
andrewfg
pushed a commit
to andrewfg/openhab-addons
that referenced
this pull request
Aug 31, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
andrewfg
pushed a commit
to andrewfg/openhab-addons
that referenced
this pull request
Aug 31, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
andrewfg
pushed a commit
to andrewfg/openhab-addons
that referenced
this pull request
Aug 31, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
andrewfg
pushed a commit
to andrewfg/openhab-addons
that referenced
this pull request
Aug 31, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
DaanMeijer
pushed a commit
to DaanMeijer/openhab-addons
that referenced
this pull request
Sep 1, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de> Signed-off-by: Daan Meijer <daan@studioseptember.nl>
markus7017
pushed a commit
to markus7017/openhab-addons
that referenced
this pull request
Sep 19, 2020
Initial contribution Signed-off-by: Guenther Schreiner <guenther.schreiner@smile.de>
Flole998
pushed a commit
to Flole998/openhab-addons
that referenced
this pull request
Dec 30, 2021
Fixes openhab#760 Signed-off-by: Jan Vybíral <jan.vybiral1@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[velux] Initial contribution of Velux binding
This binding integrates the Velux devices with help of a special gateway, the Velux Bridge KLF200.
The Velux Binding interacts via the Velux Bridge with the Velux devices like controlling window openers, shutters and others.
For details about the features, see the following website
.
Signed-off-by: Guenther Schreiner guenther.schreiner@smile.de (github: gs4711)