Skip to content
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

[wemo] Optimize port detection #12651

Merged
merged 2 commits into from
Apr 25, 2022
Merged

[wemo] Optimize port detection #12651

merged 2 commits into from
Apr 25, 2022

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Apr 25, 2022

Additional optimization after #12648.

For each request to be made to a WeMo device, up to seven requests are made prior to the actual request in order to determine the correct port. In most cases the port from the descriptor is the right one, so by verifying this one first, we can avoid some unneeded traffic and perform the real call faster.

The port scanning approach origins from #6253 (later revised in #7874) and is left intact as only the port order is changed.

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur requested a review from hmerk as a code owner April 25, 2022 14:16
Copy link
Contributor

@hmerk hmerk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the optimization!

@wborn wborn merged commit e659cee into openhab:main Apr 25, 2022
@wborn wborn added this to the 3.3 milestone Apr 25, 2022
@wborn wborn added the enhancement An enhancement or new feature for an existing add-on label Apr 25, 2022
@jlaur
Copy link
Contributor Author

jlaur commented Apr 25, 2022

@roman-76, I just discovered #6694 after this. If you are still interested, you might give this a try as it seems similar to what you wanted to achieve. Only actual port from descriptor is used though, not default protocol port (80).

@roman-76
Copy link

thanks @jlaur! I'm glad to see that after 2 years the change I wanted to achieve finally made into OpenHab :) Hope that will help to other smart home enthusiasts. I was using my private build to overcome the issue. As of today, I've moved off OpenHab to Home Assistant so I do not have a platform anymore to give it a try.

@jaywiseman1971
Copy link

I have 32 wemos. I'd be glad to try it once it gets finalized.

@jlaur
Copy link
Contributor Author

jlaur commented Apr 25, 2022

@jaywiseman1971 - it's merged, so it will be included in next snapshot as well as next milestone release. Or you can get the JAR right here: https://drive.google.com/file/d/1BCu--JkVFNh_qfSKzL1NReqd82eMfbWj/view?usp=sharing

@jlaur
Copy link
Contributor Author

jlaur commented Apr 25, 2022

As of today, I've moved off OpenHab to Home Assistant so I do not have a platform anymore to give it a try.

I'm sad to hear that this change was too late, I hope this issue didn't push you in that direction.

NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Apr 27, 2022
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Nick Waterton <n.waterton@outlook.com>
@jaywiseman1971
Copy link

@jaywiseman1971 - it's merged, so it will be included in next snapshot as well as next milestone release. Or you can get the JAR right here: https://drive.google.com/file/d/1BCu--JkVFNh_qfSKzL1NReqd82eMfbWj/view?usp=sharing

I finally tried this WeMo binding after deleting 38 Wemo Devices and uninstalling the original Wemo binding given to me from @hmerk hmerk back in January 2022. After installing the new binding, it discovered all of them quickly BUT Wemo devices are going into communication errors off/on. This was the trait when the many rewrite's of it started earlier this year.

2022-05-08 17:07:23.885 [INFO ] [openhab.event.ThingStatusInfoChangedEvent ] - Thing 'wemo:lightswitch:Lightswitch-1_0-221602K1301B70' changed from OFFLINE (COMMUNICATION_ERROR): URL for the WeMo device cannot be created. to ONLINE
2022-05-08 17:07:24.081 [INFO ] [openhab.event.ThingStatusInfoChangedEvent ] - Thing 'wemo:socket:Socket-1_0-221749K01027A4' changed from ONLINE to OFFLINE (COMMUNICATION_ERROR): URL for the WeMo device cannot be created.
2022-05-08 17:07:24.089 [INFO ] [openhab.event.RuleStatusInfoEvent ] - default-276 updated: RUNNING
2022-05-08 17:07:24.091 [INFO ] [openhab.event.ThingStatusInfoChangedEvent ] - Thing 'wemo:lightswitch:Lightswitch-1_0-221736K1300F26' changed from ONLINE to OFFLINE (COMMUNICATION_ERROR): URL for the WeMo device cannot be created.

WeMo States

My Insight for the washer completely goes OFFLINE and stays that way for a long time with this version.

I'm going to roll this binding back to @hmerk version.

Best, Jay

@jlaur
Copy link
Contributor Author

jlaur commented May 9, 2022

I finally tried this WeMo binding after deleting 38 Wemo Devices and uninstalling the original Wemo binding given to me from @hmerk hmerk back in January 2022. After installing the new binding, it discovered all of them quickly BUT Wemo devices are going into communication errors off/on. This was the trait when the many rewrite's of it started earlier this year.

Hi @jaywiseman1971, thanks for reporting this. If you create a new issue with detailed logs (DEBUG should be sufficient), I'll have a look at it.

andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Wemo] unable to control devices with Tasmota firmware in Wemo emulation mode
5 participants