-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[velbus] New functionality and bug fix #15661
Conversation
New functionnality : - Add output channel for modules : VMBEL1, VMBEL2, VMBEL4, VMBELO, VMBELPIR. (openhab#14179) - Add module VMBDALI. (openhab#14654) Fix bug : - No access to sub-address modules when adding manually the modules : VMBELO,VMBGPO, VMBGPOD, VMBGPOD-2 (openhab#12702) - Thermostat event wrongly fired when using push button on sub-address for modules VMBELO, VMBGPO, VMBGPOD, VMBGPOD_2. - Wrong typeId for bridge alarms management. Signed-off-by: Daniel Rosengarten <github@praetorians.be>
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/velbus-vmbpirm-only-ch3-supported/147834/4 |
Changed the management of VMBDALI white channel. Signed-off-by: Daniel Rosengarten <github@praetorians.be>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial pass, still didn’t look at VMBDALI and need more time to grok subaddress changes.
...binding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusBindingConstants.java
Outdated
Show resolved
Hide resolved
...binding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusBindingConstants.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.velbus/src/main/resources/OH-INF/config/config.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.velbus/src/main/resources/OH-INF/config/config.xml
Outdated
Show resolved
Hide resolved
...ng.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBELOHandler.java
Outdated
Show resolved
Hide resolved
...ing.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBELHandler.java
Outdated
Show resolved
Hide resolved
....velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBMeteoHandler.java
Outdated
Show resolved
Hide resolved
...ing.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusThingHandler.java
Show resolved
Hide resolved
...src/main/java/org/openhab/binding/velbus/internal/discovery/VelbusThingDiscoveryService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the rest of this. Thanks for going through the implementation effort here (I’m sure people will appreciate it when this finally lands.)
As a broader comment, I truly wonder if having VirtualLight be configured through a parameter on VMBDALI is the right thing. My view of this is that making it a proper thing (perhaps a child of the VMBDALI Thing
, much like VMBDALI
is itself a child of the network bridge) would make it much much easier to expand the concept in the future if the need arised. Now with VL=CH1,CH2,CH3,CH4
configuration that's pretty much the extent that can exist. If we wanted to add additional functionality that requires configuration, we wouldn’t be able to do so without introducing a breaking change.
...ding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusVirtualColorChannel.java
Outdated
Show resolved
Hide resolved
...ding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusVirtualColorChannel.java
Outdated
Show resolved
Hide resolved
...ding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusVirtualColorChannel.java
Outdated
Show resolved
Hide resolved
...ding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusVirtualColorChannel.java
Outdated
Show resolved
Hide resolved
...ding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusVirtualColorChannel.java
Outdated
Show resolved
Hide resolved
...hab.binding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusColorChannel.java
Outdated
Show resolved
Hide resolved
...hab.binding.velbus/src/main/java/org/openhab/binding/velbus/internal/VelbusColorChannel.java
Show resolved
Hide resolved
One thing to keep in mind as you peruse my review comments: I haven’t contributed to OpenHAB code in the past (so e.g. my knowledge of conventions in this project is non-existent), and the last time I have written any Java was also many, many years ago (so my knowledge of conventions in this entire ecosystem are also non-existent.) If the comments don’t make sense or they suggest to do something the wrong way around, feel free to simply resolve that comment and forget ever seeing it :) |
…inding/velbus/internal/VelbusBindingConstants.java Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/VelbusBindingConstants.java Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/discovery/VelbusThingDiscoveryService.java Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/VelbusVirtualColorChannel.java Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/VelbusVirtualColorChannel.java Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/VelbusVirtualColorChannel.java Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
* VelbusVirtualColorChannel class Permits different values (CH0 or A0) to configure the virtual light's channels. * VelbusColorChannel class Makes color transformation easy with the rescale function. * onPacketReceived interface Stops processing packets when handler is disposed. Signed-off-by: Daniel Rosengarten <github@praetorians.be>
...src/main/java/org/openhab/binding/velbus/internal/discovery/VelbusThingDiscoveryService.java
Outdated
Show resolved
Hide resolved
This looks really nice so far. Unfortunately I haven’t the capabilities to help with driving the PR further. Hopefully my reviews have been helpful and whenever somebody with merge button powers comes along, they’ll be happy enough with the changes to be able to press it without further questions :) |
Signed-off-by: Daniel Rosengarten <github@praetorians.be>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the thing configuration has been altered, i think it would be needed to add thing upgrade instructions:
https://www.openhab.org/docs/developer/bindings/thing-xml.html#updating-thing-types
Thing upgrade instructions for VMBEL1, VMBEL2, VMBEL4, VMBELO. Signed-off-by: Daniel Rosengarten <github@praetorians.be>
Signed-off-by: Daniel Rosengarten <github@praetorians.be>
Added missing bundle reference in channel type. Signed-off-by: Daniel Rosengarten <github@praetorians.be>
@cedricboon is there any chance you find time to check the code and accept the pull request ? |
It seems that @cedricboon has no time to do the review. |
Disable push button events on sensor address for new modules *-20. Signed-off-by: Daniel Rosengarten <github@praetorians.be>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, left some comments. Overall it looks good, thanks for the fixes. Hopefully future PR's are a bit smaller :-)
...ing.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBELHandler.java
Outdated
Show resolved
Hide resolved
...ng.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBELOHandler.java
Outdated
Show resolved
Hide resolved
...ing.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusThingHandler.java
Show resolved
Hide resolved
...g.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBDALIHandler.java
Outdated
Show resolved
Hide resolved
...g.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBDALIHandler.java
Show resolved
Hide resolved
...g.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBDALIHandler.java
Show resolved
Hide resolved
...g.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBDALIHandler.java
Show resolved
Hide resolved
...g.velbus/src/main/java/org/openhab/binding/velbus/internal/handler/VelbusVMBDALIHandler.java
Show resolved
Hide resolved
Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/handler/VelbusVMBELHandler.java Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/handler/VelbusVMBELOHandler.java Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/handler/VelbusVMBDALIHandler.java Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
…inding/velbus/internal/handler/VelbusVMBDALIHandler.java Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Daniel Rosengarten <rosengarten01@hotmail.com>
Signed-off-by: Daniel Rosengarten <github@praetorians.be>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM
* [velbus] New functionnality & Bug fix Signed-off-by: Daniel Rosengarten <github@praetorians.be> Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Co-authored-by: lsiepel <leosiepel@gmail.com>
* [velbus] New functionnality & Bug fix Signed-off-by: Daniel Rosengarten <github@praetorians.be> Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Paul Smedley <paul@smedley.id.au>
* [velbus] New functionnality & Bug fix Signed-off-by: Daniel Rosengarten <github@praetorians.be> Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
* [velbus] New functionnality & Bug fix Signed-off-by: Daniel Rosengarten <github@praetorians.be> Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Co-authored-by: lsiepel <leosiepel@gmail.com>
* [velbus] New functionnality & Bug fix Signed-off-by: Daniel Rosengarten <github@praetorians.be> Co-authored-by: Simonas Kazlauskas <github@kazlauskas.me> Co-authored-by: lsiepel <leosiepel@gmail.com>
New functionality :
Fix bug :
Signed-off-by: Daniel Rosengarten github@praetorians.be