-
-
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] Add new functionality PRESSED and LONG PRESSED #10664
Conversation
New functionnality : Add the the possibility to simulate the PRESSED and LONG PRESSED message of an input. Module supported with button simulation : VMB1RYS (button : CH6) VMB6IN (buttons : CH1 ... CH6) VMB2PBN, VMB6PBN, VMB7IN, VMB8IR, VMB8PB, VMB8PBU, VMBEL1, VMBEL2, VMBEL4, VMBGP1, VMBGP1-2, VMBGP2, VMBGP2-2, VMBGP4, VMBGP4-2, VMBGP4PIR, VMBGP4PIR-2 (buttons : CH1 ... CH8) VMBELO, VMBGPOD, VMBGPOD-2 (buttons : CH1 ... CH32) Fix bug : The channels names were not correctly assigned to the thing properties. The last channel had the default name, not the one retrieved from the module. Signed-off-by: Daniel Rosengarten <github@praetorians.be>
@cedricboon Can you take a look? |
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.
Hi @Rosen01, thank you for the enhancements!
I've reviewed the code and made a few suggestions.
The most important one is probably of the usage of Thread.sleep in a few places, which I would replace by the use of the scheduler.
try { | ||
packet.Pressed(); | ||
velbusBridgeHandler.sendPacket(packet.getBytes()); | ||
Thread.sleep(20); |
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.
I don't think you are allowed to let the thread sleep here.
But you could use the scheduler, like here
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.
I d'idn't check this part of your code, the 20 ms delay is useless then. I've remove this from my code.
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.
You're absolutely right. I didn't thought of that when reviewing the code :-)
if (stringTypeCommand.equals(LONG_PRESSED)) { | ||
packet.LongPressed(); | ||
velbusBridgeHandler.sendPacket(packet.getBytes()); | ||
Thread.sleep(20); |
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.
I don't think you are allowed to let the thread sleep here.
But you could use the scheduler, like here
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.
I d'idn't check this part of your code, the 20 ms delay is useless then. I've remove this from my code.
try { | ||
packet.Pressed(); | ||
velbusBridgeHandler.sendPacket(packet.getBytes()); | ||
Thread.sleep(20); |
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.
I don't think you are allowed to let the thread sleep here.
But you could use the scheduler, like here
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.
I d'idn't check this part of your code, the 20 ms delay is useless then. I've remove this from my code.
if (stringTypeCommand.equals(LONG_PRESSED)) { | ||
packet.LongPressed(); | ||
velbusBridgeHandler.sendPacket(packet.getBytes()); | ||
Thread.sleep(20); |
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.
I don't think you are allowed to let the thread sleep here.
But you could use the scheduler, like here
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.
I d'idn't check this part of your code, the 20 ms delay is useless then. I've remove this from my code.
|
||
packet.Released(); | ||
velbusBridgeHandler.sendPacket(packet.getBytes()); | ||
} catch (InterruptedException e) { |
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.
This empty catch clause will probably become obsolete if you use the scheduler instead of a Thread.sleep.
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.
I've remove this from my code.
|
||
packet.Released(); | ||
velbusBridgeHandler.sendPacket(packet.getBytes()); | ||
} catch (InterruptedException e) { |
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.
This empty catch clause will probably become obsolete if you use the scheduler instead of a Thread.sleep.
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.
I've remove this from my code.
Co-authored-by: cedricboon <cedric.boon@hotmail.com> Signed-off-by: Daniel Rosengarten <github@praetorians.be>
Co-authored-by: cedricboon <cedric.boon@hotmail.com> Signed-off-by: Daniel Rosengarten <github@praetorians.be>
Co-authored-by: cedricboon <cedric.boon@hotmail.com> Signed-off-by: Daniel Rosengarten <github@praetorians.be>
Remove uneeded Thread.sleep in code. Trigger the events PRESSED, LONG_PRESSED, RELEASED on the linked trigger channel when using the button simulation. 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.
LGTM
Redundant superinterface DiscoveryService for the type VelbusThingDiscoveryService, already defined by AbstractDiscoveryService. 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!
* [velbus] Add new functionality PRESSED and LONG PRESSED and fix bug New functionality: Add the the possibility to simulate the PRESSED and LONG PRESSED message of an input. Module supported with button simulation : VMB1RYS (button : CH6) VMB6IN (buttons : CH1 ... CH6) VMB2PBN, VMB6PBN, VMB7IN, VMB8IR, VMB8PB, VMB8PBU, VMBEL1, VMBEL2, VMBEL4, VMBGP1, VMBGP1-2, VMBGP2, VMBGP2-2, VMBGP4, VMBGP4-2, VMBGP4PIR, VMBGP4PIR-2 (buttons : CH1 ... CH8) VMBELO, VMBGPOD, VMBGPOD-2 (buttons : CH1 ... CH32) Fix bug: The channels names were not correctly assigned to the thing properties. The last channel had the default name, not the one retrieved from the module. Also-by: cedricboon <cedric.boon@hotmail.com> Signed-off-by: Daniel Rosengarten <github@praetorians.be>
* [velbus] Add new functionality PRESSED and LONG PRESSED and fix bug New functionality: Add the the possibility to simulate the PRESSED and LONG PRESSED message of an input. Module supported with button simulation : VMB1RYS (button : CH6) VMB6IN (buttons : CH1 ... CH6) VMB2PBN, VMB6PBN, VMB7IN, VMB8IR, VMB8PB, VMB8PBU, VMBEL1, VMBEL2, VMBEL4, VMBGP1, VMBGP1-2, VMBGP2, VMBGP2-2, VMBGP4, VMBGP4-2, VMBGP4PIR, VMBGP4PIR-2 (buttons : CH1 ... CH8) VMBELO, VMBGPOD, VMBGPOD-2 (buttons : CH1 ... CH32) Fix bug: The channels names were not correctly assigned to the thing properties. The last channel had the default name, not the one retrieved from the module. Also-by: cedricboon <cedric.boon@hotmail.com> Signed-off-by: Daniel Rosengarten <github@praetorians.be>
Thanks @Rosen01 one "tiny" detail, is there any way you can get the channels to show the live state of the button, or return to null or "released" after a while? I have been suggesting people do this, but I know that is a hack.
|
* [velbus] Add new functionality PRESSED and LONG PRESSED and fix bug New functionality: Add the the possibility to simulate the PRESSED and LONG PRESSED message of an input. Module supported with button simulation : VMB1RYS (button : CH6) VMB6IN (buttons : CH1 ... CH6) VMB2PBN, VMB6PBN, VMB7IN, VMB8IR, VMB8PB, VMB8PBU, VMBEL1, VMBEL2, VMBEL4, VMBGP1, VMBGP1-2, VMBGP2, VMBGP2-2, VMBGP4, VMBGP4-2, VMBGP4PIR, VMBGP4PIR-2 (buttons : CH1 ... CH8) VMBELO, VMBGPOD, VMBGPOD-2 (buttons : CH1 ... CH32) Fix bug: The channels names were not correctly assigned to the thing properties. The last channel had the default name, not the one retrieved from the module. Also-by: cedricboon <cedric.boon@hotmail.com> Signed-off-by: Daniel Rosengarten <github@praetorians.be>
The messages PRESSED/LONG_PRESSED and RELEASED are sent in less than 1 sec, it won't show up in the interface. |
Hi @Rosen01 and @cedricboon ... Just checking if org.openhab.binding.velbus-3.1.0.2105092215-SNAPSHOT.jar is the latest and greatest available Velbus .jar file for OpenHAB 3.2.0 - Release Build (running in docker). Unfortunately broke my own rule (never touch a working setup), hence facing below errors when trying to load the file. Next to the fact I like the great new feature of "long pressed" (as I use this trigger to execute rules as "hidden button"), the current official Velbus release in OpenHAB is no longer supporting the channel triggers from vmbmeteo (CH1->CH8) This was working perfectly in org.openhab.binding.velbus-2.5.10-20201017.033442-19.jar but I think it skipped the lease/rebuild for openhab 3 unfortunately. (unless maybe included in the .jar releases on github) I tried to run the 2.5 version unsuccessfully (logically), but also didn't get the release from Rosen01 to work... Can you help me please as my knowledge is limited to put the .jar files in addons folder ;-) Running version: Log details with custom .jar files:
12:23:48.235 [ERROR] [Events.Framework ] - FrameworkEvent ERROR
12:23:48.235 [ERROR] [Events.Framework ] - FrameworkEvent ERROR 14:37:06.010 [WARN ] [org.apache.felix.fileinstall ] - Error while starting bundle: file:/openhab/addons/org.openhab.binding.velbus-3.1.0.2105092215-SNAPSHOT.jar
|
Good morning If it makes you feel any better, I broke that golden rule yesterday too... I don't know what I've done wrong, but I just can't get Chromium to start on this machine. Oh well, I'll just re-flash the card and load the backup configuration. Regarding your issue, I'm no expert in these matters, looking at your log file, it would seem that you haven't installed the serial binding.
|
You cannot use a package for OpenHAB 2.5 with OpenHAB 3.x. I just look in the code in the repository and the trigger on channels CH1 -> CH8 is still implemented for the VMBMETEO in the official 3.2 release. But I'm a littlebit surprised by the identifier of your module. You can check in the OpenHAB event logs if the the channel is triggered : |
Hi both, Thx for the feedback and sorry we are going off-topic on this topic. I will use the official 3.2 release for my purpose. As per request, I recreated the module. Remark, the "scan" only give back a few modules, hence needed to manually add the module providing the address ... which might explain the naming. note: there is a workaround by just trigger alarms in openhab using the wind & temperature readouts ... hence non-blocking for me. If no quick win, just let me know. ---Recreated the module vmbmeteo--- ---initiating --- ---ItemStateChangedEvents---- --- ChannelTriggeredEvent (for 1TS) ---- We aren't at openhab rule level yet, but this is the test rule to capture the channel event |
hi @Rosen01 --- SOLVED --- velbus:vmbmeteo:179718b801:9A:input#CH1 triggered PRESSED Thanks a lot for the golden tip ! Lessons learned: I'm thinking to make my things also text based (.things) instead of creation through UI. 21:22:48.259 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'VMBMETEO_CH10OutdoorTemp' changed from 3.3125 °C to 3.0625 °C |
* [velbus] Add new functionality PRESSED and LONG PRESSED and fix bug New functionality: Add the the possibility to simulate the PRESSED and LONG PRESSED message of an input. Module supported with button simulation : VMB1RYS (button : CH6) VMB6IN (buttons : CH1 ... CH6) VMB2PBN, VMB6PBN, VMB7IN, VMB8IR, VMB8PB, VMB8PBU, VMBEL1, VMBEL2, VMBEL4, VMBGP1, VMBGP1-2, VMBGP2, VMBGP2-2, VMBGP4, VMBGP4-2, VMBGP4PIR, VMBGP4PIR-2 (buttons : CH1 ... CH8) VMBELO, VMBGPOD, VMBGPOD-2 (buttons : CH1 ... CH32) Fix bug: The channels names were not correctly assigned to the thing properties. The last channel had the default name, not the one retrieved from the module. Also-by: cedricboon <cedric.boon@hotmail.com> Signed-off-by: Daniel Rosengarten <github@praetorians.be>
New functionnality :
Add the the possibility to simulate the PRESSED and LONG PRESSED message of an input.
Module supported with button simulation :
VMB1RYS (button : CH6)
VMB6IN (buttons : CH1 ... CH6)
VMB2PBN, VMB6PBN, VMB7IN, VMB8IR, VMB8PB, VMB8PBU, VMBEL1, VMBEL2, VMBEL4, VMBGP1, VMBGP1-2, VMBGP2, VMBGP2-2, VMBGP4, VMBGP4-2, VMBGP4PIR, VMBGP4PIR-2 (buttons : CH1 ... CH8)
VMBELO, VMBGPOD, VMBGPOD-2 (buttons : CH1 ... CH32)
Fix bug :
The channels names were not correctly assigned to the thing properties. The last channel had the default name, not the one retrieved from the module.
Signed-off-by: Daniel Rosengarten github@praetorians.be