-
-
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
[upb] UPB Binding initial contribution #1620
Conversation
@cvanorman Since it worked well to sort out bugs last time, would you mind providing a JAR again for testing? |
File attached. |
Thanks. It doesn't work for me. I dropped the file in
|
It requires that nrjavaserial be provided by the runtime. This has historically been provided by the openhab-transport-serial bundle, but is being refactored to include it directly from the runtime (see openhab/openhab-core#101) So, you can either run the following in the karaf console:
or you can download the nrjavaserial file attached in the the previously mentioned issue and drop it in your addons folder. |
OK, I assume that that is a dependency and will be automatically installed via PaperUI. I installed it via Karaf just fine. However, I haven't specified any devices, and I have another error: upb.things:
log:
|
I'll be honest with you, I've never gotten the "things" files to work with any binding. Just use the Paper UI. |
Actually, if you remove the quotes from around the numbers it should parse it correctly. |
OK. I used the quotes because that's how it was stated in the doc file in the commit. That fixed the error. |
Working with things files should just as well work. In the community forum one of the main sources among discussed problems is the not always clear and consistent way bindings and add-ons are configured and how they behave. With the step-by-step evolution of PaperUI and the database to be a full replacement for manual file based configurations the significance of files might drop but they are still important. My point being: For the sake of conformity, please also pay attention to the file based configuration aspect :) |
@ThomDietrich All is well! The config files work! I prefer the file-based configs because I can backup and restore my OH configuration very easily. The doc file in this commit simply needs an adjustment to remove the quotation marks. However, I will note that this binding is configured slightly differently. Most other bindings seem to follow this model:
Though I can't really see a situation where you would need more than one modem (assuming your house is wired properly with repeaters/phase couplers), and the current config method seems to support that anyway. @cvanorman I successfully got the binding to control my lights! Thank you! However, it doesn't seem to respond to status updates. Also, I've noticed that the config option for how often the lights are queried is now gone. And, although it makes no difference to me (I use BasicUI and HABpanel), there are no items in the Control section of PaperUI. Current config:
upb.items:
|
I've also just noticed an odd delay 10-15 second every third or fourth time a light is adjusted. It could just be my network/machine though. |
One final thought: It would definitely be possible to "discover" devices on the UPB network. UPStart does it by simply getting the info of every device in the network and showing the IDs that respond. This could be for a later update though. |
There are a few differences in regards to refreshing as OH2 doesn't have the same polling refresh process as OH1. The items should refresh when they are initialized and they will properly respond to a REFRESH command. So, you could create a cron rule that refreshes them all if you like. I would recommend keeping that to a minimum though as the powerline bus has limited throughput and flooding it can cause missed messages. Links will ignore the REFRESH command. Also, I've put in a "LOW" priority queue for the refresh messages and they block for 1 second to ensure a response is received prior to sending a new command. |
Yes, discovery is definitely possible, I just haven't worked on it. |
As for the layout and / or structure of the config files, I don't do anything particular there. My binding does not parse any information from it directly. It interacts only with the openHAB api so it should work the same as it does for any other binding. I think for your items files you are not binding to the correct channel. It should look like this: The last part of the channel is the channel name which should be either brightness or switch depending on the item type. |
I see. I updated my config to use
|
Is there any activity in the debug logs? Like, is the modem receiving data? |
It receives and logs both status updates, but it does not update the state:
|
Those messages are link messages for a link with id=15, but I didn't see that in your config. I should also mention that it is now more accurate in identifying things. Previously it just checked the source or destination and flagged those items. This could result in incorrect items being updated and so you may be expecting those messages to update your LightDenOuterCans, but that is not what the UPB message actually indicated. |
Ummm, there is no ambiguity here. The message in that log is a deactivate command sent to link 15 from device 5 on network 135. That is what it is. The binding will interpret that as a status update for a link with id 15. If no such thing is configured, it will be ignored. |
It seems to be decoding the command wrong. This is what I get after switching Device 8 to 100. I am dead sure that this is device 8 and that it is emitting device 8. It can't possibly also be emitting link 15:
However, when I decode it, I also get link 15. Is it possible that the PIM is being read incorrectly? The first two commands say that it is sending to device 15 and the source is device 8, but the next two commands say that it is sending a null command to no particular device and is from device 8. |
I don't think so, this is very normal traffic. If it was decoding wrong it would likely not produce a meaningful message at all. These messages are all coming from device 8 which matches up with what you said. There are two sets which is also normal. The first, PU8904870F0820FFFFB7, is an activate command for link 15 sent from device 8. The second, PU080487000886647B, is a device status report of 100% for device 8. Each message is duplicated twice and this is also normal. I'm also realizing that the earlier message, PU8904870E0521FFFFBA, is actually link 14 (coming from device 5) and not 15 like I had indicated. |
Well, it's been a few days, and it seems that some devices send updates properly and others don't. I'm not sure why. They're all Simply Automated switches, and UPStart grabs their updates every time. |
Can you provide me with your configuration and the debug log from when you are pressing a switch? |
It seems that my switches only broadcast their change if they are manually pressed. They do not broadcast if another switch tells it what to do. There doesn't appear to be an option for this. It makes sense too, because the switch broadcasting at that point would be redundant. Being able to control another light from another switch is a feature of any smart switch, and I would hate to give that up. |
Devices do not broadcast via a link. Only the device that is pressed. The powerline bus simply cannot support it as it would just result in a lot of collisions. There is not a lot I can do about that. Ideally, I'd like to be able to store the links themselves (in OH) and update a device based upon the activation of a link. I haven't really had time to look into what my options are in OH for that. If the system is updating when you manually press a switch, then it is working as it was intended. |
Of course, you can do this manually via a rule. If you configure the link in OH, you will get updates for the link and can then update the status of devices based upon your knowledge of what that link is supposed to do. |
That config doesn't include link 66. Are none of the links or switches updating? |
I was just giving you an example. Making sure a Link is always a "brightness" channel no matter which item is used. A Dimmer item is always a "brightness" channel no matter if device or link. And a "switch" channel is only used for a device with a switch item. All seems to be working for me but need further test. I do have the link update problem that @DaAwesomeP talks about which seemed to work in OH1. |
@cvanorman @DaAwesomeP Just want to make sure my behavior is same as what you know to be. Group item=BasementNew label="Basement" icon="cellar" { upb:link:LightCAllbasement (upb:bridge:upbmodem) [ id=35 ] |
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.
@cvanorman thanks for your PR, I added some review comments. Questions, please let me know.
Bundle-Name: UPB Binding | ||
Bundle-SymbolicName: org.openhab.binding.upb;singleton:=true | ||
Bundle-Vendor: openHAB | ||
Bundle-Version: 2.0.0.qualifier |
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.
Can you bump the bundle and Java version to 2.1.0 and 1.8
<?xml version="1.0" encoding="UTF-8"?> | ||
<!-- | ||
|
||
Copyright (c) 2014-2016 by the respective copyright holders. |
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.
It seems that this file has a missing or invalid copyright header, can you fix all of them by running
mvn license:format
Please commit only the ones for your binding.
bin.includes = META-INF/,\ | ||
.,\ | ||
OSGI-INF/,\ | ||
ESH-INF/ |
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.
Can you include the about.html in the file (a new requirement :-) ) and make sure the file ends with a newline
<parent> | ||
<groupId>org.openhab.binding</groupId> | ||
<artifactId>pom</artifactId> | ||
<version>2.0.0-SNAPSHOT</version> |
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.
Please update the version
* used across the whole binding. | ||
* | ||
* @author Chris Van Orman - Initial contribution | ||
* @since 2.0.0 |
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.
Can you update all @since
-s
try { | ||
wait(ACK_TIMEOUT); | ||
} 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.
If we receive this InterruptedException, the interrupted flag was cleared and we most likely need to reset the flag:
http://www.yegor256.com/2015/10/20/interrupted-exception.html
https://www.ibm.com/developerworks/library/j-jtp05236/
/** | ||
* {@link Runnable} implementation used to write data to the UPB modem. | ||
* | ||
* @author cvanorman |
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.
Unexpected author name, please add it like you did at the other classes.
<module>org.openhab.binding.urtsi</module> | ||
<module>org.openhab.binding.vitotronic</module> | ||
<module>org.openhab.binding.yamahareceiver</module> | ||
<module>org.openhab.binding.zway</module> | ||
</modules> | ||
|
||
</project> | ||
</project> |
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.
Undo this change
@@ -65,10 +65,11 @@ | |||
<module>org.openhab.binding.tellstick</module> | |||
<module>org.openhab.binding.tesla</module> | |||
<module>org.openhab.binding.toon</module> | |||
<module>org.openhab.binding.upb</module> |
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.
To have the binding being picked up by the distro, you furthermore need to add it to the feature.xml, again at the alphabetically correct position. If you have a dependency on some transport bundle (e.g. upnp, mdns or serial), make sure to add a line for this dependency as well (see the other bindings as an example).
@@ -0,0 +1,7 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
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.
The about.html file seems missing, guidelines B.2
Hello @cvanorman how are you doing? Will you able to process the review comments? If you any questions please let us know. |
Sorry, I'm out of town right now, and am not available to do much with it
yet. I will get to it as soon as I am able.
…On Fri, Jun 16, 2017 at 6:02 AM, Martin van Wingerden < ***@***.***> wrote:
Hello @cvanorman <https://github.com/cvanorman> how are you doing? Will
you able to process the review comments? If you any questions please let us
know.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/openhab/openhab2-addons/pull/1620#issuecomment-309020570>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADuRbDJKoCAEtdk0P336yt9JIe_3FV89ks5sEnz5gaJpZM4LWy2z>
.
|
@cvanorman thanks for all your work till now. However I see quite some work ahead it seems to me that some comments are still open, besides that the build fails and you have a conflict. Give that you merged in the master (in openhab you should always rebase instead of merge) the proper way to go now is:
If you managed this, it's time to rebase on the openhab/master |
Hello @cvanorman , how are you doing, do you have plans to finalize this binding? Or would you prefer someone else to take it over? In that case we could maybe tag it and ask other developer to jump in? |
Sorry, yes, unfortunately my life has become occupied with other matters and I'm unlikely to find time to continue this anytime soon. |
Closing as no one is working on this anymore |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/altering-jar-file-for-manual-binding-installation/84155/8 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/altering-jar-file-for-manual-binding-installation/84155/9 |
1 similar comment
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/altering-jar-file-for-manual-binding-installation/84155/9 |
Another implementation here |
Can be closed as #6742 got merged. |
* Added expire functionality as core framework feature Closes openhab#1620 Signed-off-by: Kai Kreuzer <kai@openhab.org>
Signed-off-by: Markus Storm <markus.storm@gmx.net>
Signed-off-by: Chris Van Orman vanorman.chris@gmail.com
Fixes #1619