[lgwebos] Wake-on-Lan Integration #7103
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
Contributor
sprehn
commented
Mar 5, 2020
•
sprehn
commented
- Integrates Wake-on-Lan functionality in power channel, so that there is no need anymore for wol1 binding.
- Removes special ssdp search request for lge-com:service:webos-second-screen:1
Travis tests were successfulHey @sprehn, |
lolodomo
requested changes
Mar 6, 2020
...ab.binding.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/PowerControlPower.java
Show resolved
Hide resolved
...hab.binding.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/WakeOnLanUtility.java
Outdated
Show resolved
Hide resolved
...hab.binding.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/WakeOnLanUtility.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/openhab/binding/lgwebos/internal/discovery/LGWebOSUpnpShutdownDetector.java
Outdated
Show resolved
Hide resolved
|
I have the stange feeling that the problem is in your discovery code that would not consider the right UPnP device/service, or maybe not all. |
|
I think the proper way to do it would be to have a class that implements UpnpIOParticipant. Of course, this class has to be registered as UPnP participant in order to be called by the core framework and has to return the appropriate UDN. Normally the method onStatusChanged will then be called by the core framework each time there is a change detected by JUPnP. I have clearly the feeling that you have everything already implemented in the openHAB core framework. You just need to implement UpnpIOParticipant with the appropriate UDN and you will be notified of any status change by the core framework. I own a LG TV since recently, I will start UPnP Spy to understand this stuff with UPnP devices/services from LG and then I can help you. By the way, this will be the occasion to give you a feedback on your binding. |
|
I see that you have the UDN set in the deviceId property of your thing but only for discovered things. That means, things created manually will not have this property set and so you don't know what UDN to return in your thing handler in case you want to implement UpnpIOParticipant. Forcing the user to set this value would be a too much big constraint. So implementing DiscoveryListener is probably not a bad idea and could help retrieving this UDN if not yet set. I will think about that. Regarding the two root devices you are interested in and their respective UDN, if there is a way to deduce one for the other, this would help a lot. |
|
It even exists in the core framework the method |
|
I installed your binding and tested most of the features and everything is working very well (even if I discovered few issues). Then I updated the code to test what I was talking about, that is having the thing handler implementing UpnpIOParticipant. Bingo, I can see an immediate call to onStatusChanged with true or false just after I switch on or off the TV with the TV remote control. When I switch ON my TV, I see in app UPnP Device Spy 4 UPnP devices appearing immediately:
They immediately disappear when I switch off the TV with the TV remote control. All this means that the only difficulty is to get the UDN of the UPnP device. When the thing is created from the Paper UI inbox, that is easy because this UDN is a property of the thing. When the thing is created manually (my current case), this property is unset. I will let you know a soon as I have something finsihed. |
@lolodomo cool, help is welcome. My Upnp knowledge is not so deep as yours. Maybe we can change the device classes on which the binding listens to org:device:MediaRenderer and then DiscoveryListener:thingRemoved would be called on shutdown. |
Travis tests were successfulHey @sprehn, |
|
Could you please suppress the shutdown detection from this PR ? |
lolodomo
requested changes
Mar 7, 2020
...nding.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/handler/LGWebOSHandler.java
Outdated
Show resolved
Hide resolved
I think this is not very important at this step what UPnP device we consider because they all appear and disappear at the same time and because UPnP is used at its minimum by the binding, just for thing discovery and to help detecting if the TV is off or on.. So the one you consider is OK for me. If UPnP was used for something else (run UPnP actions), in this case, we should consider the device |
|
@sprehn : I have the feeling we could detect the TV OFF easily, even without UPnP. |
|
empty application was already discussed in the community. however it can also be empty when switching between apps takes slightly longer. so this was ruled out |
Zut, it looked like a good solution and I already have pushed the change. I just reverted it now. |
Travis tests have failedHey @sprehn, 1st BuildExpand here |
Travis tests were successfulHey @sprehn, |
lolodomo
reviewed
Mar 7, 2020
...nding.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/handler/LGWebOSHandler.java
Outdated
Show resolved
Hide resolved
...nding.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/handler/LGWebOSHandler.java
Show resolved
Hide resolved
sprehn
requested review from
bobadair,
cdjackson,
kaikreuzer,
kgoderis and
vbier
as code owners
|
OK, are we good to proceed with this PR? |
Travis tests have failedHey @sprehn, 1st BuildExpand here |
Travis tests were successfulHey @sprehn, |
Travis tests have failedHey @sprehn, 1st BuildExpand here |
lolodomo
approved these changes
Mar 8, 2020
sprehn
changed the title
|
My last option leads to current ON => OFF => ON => OFF when sending a OFF command. Not good. |
|
less (channel updates) may be more. :-) |
|
My previous proposal remains the best I think. It does not produce unnexpected switch toggling. The only little problem, only when We still have to recommend setting |
|
agree. will apply that proposal a bit later today |
I reduced the number of channel update to 2 in the last proposed code. I checked again the documentation and |
|
@sprehn : just for my information, do you get this 5 minutes timeout or something shorter in your case ? |
|
No, normally the device disconnects properly when the TV shuts down. it takes a few seconds. WebOs runs on linux. we simply wait until all services shut down. The 5min timeout exists indeed. without any activity on the websocket for 5mins it will close. that is why we have the keepalivejob |
… CONNECTING and REGISTERING still count as OFF states. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de>
…ding. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de>
|
@lolodomo applied your patch good to go now? |
Travis tests were successfulHey @sprehn, |
|
The following discussion should not block the merge of this PR ;)
That means the behaviour is different with models or maybe depends on a TV setting ? |
@sprehn : for your information, I have an enabled TV setting named "Démarrage rapide +". It could be translated into "Fast startup +". Maybe with this setting enabled, the TV is not really shutting down, like it is for your TV. Do you have this setting on your model ? I will check later what is the behaviour if I disable this setting. |
yes, indeed
yes, probably depends on model. I don't think I have it, but cannot check anymore. Left my apartment in the city again this morning - no access to TV anymore. But that would perfectly explain it. TV only turns off screen, but keeps on running in background and does not actively close the websocket connection in this case. We stop the keepAlive job, so 5mins later OH's http client decides to kill the connection due to timeout. |
yfre
pushed a commit
to yfre/openhab-addons
that referenced
this pull request
Apr 27, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com> Signed-off-by: Eugen Freiter <freiter@gmx.de>
markus7017
pushed a commit
to markus7017/openhab-addons
that referenced
this pull request
May 29, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com>
LoungeFlyZ
pushed a commit
to LoungeFlyZ/openhab2-addons
that referenced
this pull request
Jun 8, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com>
J-N-K
pushed a commit
to J-N-K/openhab-addons
that referenced
this pull request
Jul 14, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com>
CSchlipp
pushed a commit
to CSchlipp/openhab-addons
that referenced
this pull request
Jul 26, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com> Signed-off-by: CSchlipp <christian@schlipp.de>
andrewfg
pushed a commit
to andrewfg/openhab-addons
that referenced
this pull request
Aug 31, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com>
andrewfg
pushed a commit
to andrewfg/openhab-addons
that referenced
this pull request
Aug 31, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com>
andrewfg
pushed a commit
to andrewfg/openhab-addons
that referenced
this pull request
Aug 31, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com>
andrewfg
pushed a commit
to andrewfg/openhab-addons
that referenced
this pull request
Aug 31, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com>
DaanMeijer
pushed a commit
to DaanMeijer/openhab-addons
that referenced
this pull request
Sep 1, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@gmail.com> Signed-off-by: Daan Meijer <daan@studioseptember.nl>
markus7017
pushed a commit
to markus7017/openhab-addons
that referenced
this pull request
Sep 19, 2020
* Adding WOL Implementation to PowerControl channel with ability to determine MAC (best effort via arp) and send WOL natively. Removed Search, as second screen service does actually show up in regular scans now. * Addressing review comments in WakeOnLanUitility and config.xml. * Add mac and windows commands to discover MAC. * Break once MAC is found in result. * Using org.eclipse.smarthome.io.net.exec.ExecUtil instead of implementing Commandline Execution in the binding itself. * Detecting which linux tool to detect MAC exists, arp or arping. * Fixed typo in debug message. * Addressing review comments on formatting. * MacAddress parameter added to README demo item and Power Control Handler updated. * Handle power on off commands in all possible LGWebOSTVSocket states. * Reformatting debug message. * Fix whitespace. * Moved If statement cases into switch statement. * Adding comments. * Applying code review recommendations. Reducing power channel updates. CONNECTING and REGISTERING still count as OFF states. * Inlining Socket's isConnected method. The method name was also misleading. Signed-off-by: Sebastian Prehn <sebastian.prehn@gmx.de> Co-authored-by: cpmeister <mistercpp2000@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.