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.

Sign up for GitHub

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

Jump to bottom

[yamahareceiver] connect timed out since 2.5M4 fix #6835

Merged
merged 1 commit into from
Jan 16, 2020
Merged

[yamahareceiver] connect timed out since 2.5M4 fix #6835

merged 1 commit into from
Jan 16, 2020

Conversation

zarusz
Copy link
Member

@zarusz zarusz commented Jan 14, 2020

Fixes #6328

As per #6328 and thread here community tested on few devices and it seems to resolve the issue.

The jar build for testing is available here.

This is rebased against the 2.5.x branch, I hope this can get in as a 2.5.2 release (the bug is critical).

@TravisBuddy
Copy link

Travis tests were successful

Hey @zarusz,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

1 similar comment
@TravisBuddy
Copy link

Travis tests were successful

Hey @zarusz,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@Hilbrand Hilbrand added the bug An unexpected problem or unintended behavior of an add-on label Jan 14, 2020
martinvw
martinvw reviewed Jan 15, 2020
View reviewed changes
Copy link
Member

@martinvw martinvw left a comment

Choose a reason for hiding this comment

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

Some tiny remarks, looks good. Thanks!

.../org/openhab/binding/yamahareceiver/internal/protocol/xml/InputWithNavigationControlXML.java
throw new IOException(
"Menu still not ready after " + YamahaReceiverBindingConstants.MENU_MAX_WAITING_TIME + "ms");
logger.info("Menu still not ready after " + YamahaReceiverBindingConstants.MENU_MAX_WAITING_TIME + "ms. The menu state will be out of sync.");
// ToDo: this needs to redesigned to allow for some sort of async update
Copy link
Member

Choose a reason for hiding this comment

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

There is a TODO, what should be done and can it be completed before merging the PR?

Copy link
Member Author

@zarusz zarusz Jan 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I put that ToDo for another time. I am not using menus myself and not sure how it works, but when you look at the code it would require a rework to avoid delaying and a looping to fetch updates until the devices applies changes.

Too big refactor for now.
With this PR I am trying to fix the rather critical issue introduced last time.

...er/src/main/java/org/openhab/binding/yamahareceiver/internal/protocol/xml/XMLConnection.java Outdated
@@ -74,7 +74,7 @@ public XMLConnection(String host) {
connection.setRequestMethod("POST");
connection.setRequestProperty("Content-Length", Integer.toString(message.length()));

connection.setConnectTimeout(5); // set a timeout in case the device is not reachable (went offline)
connection.setConnectTimeout(5000); // set a timeout in case the device is not reachable (went offline)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be constant, that could be a good place to include the unit in the name, because I hope that is milliseconds?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is milliseconds. This is the crux of the fix. Before I thought these were seconds (wanted 5 seconds).

Sure, a constant could be done added.

@zarusz
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328)
d42e634 
Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
@TravisBuddy
Copy link

Travis tests were successful

Hey @zarusz,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@zarusz
Copy link
Member Author

zarusz commented Jan 15, 2020

Hi @martinvw I incorporated the feedback.

Please let me know if I need to do anything particular for this to ends up in the 2.5.2 release.
Also, do I have to cherry pick that into the master at some later point?

@martinvw martinvw merged commit 11072a2 into openhab:2.5.x Jan 16, 2020
@zarusz zarusz deleted the bugfix/6328 branch January 16, 2020 18:03
@zarusz zarusz self-assigned this Jan 16, 2020
@kaikreuzer kaikreuzer added this to the 2.5.2 milestone Jan 19, 2020
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jan 19, 2020
@zarusz @J-N-K
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328) (openha…
4a8c82b 
…b#6835)

Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
tmrobert8 pushed a commit to tmrobert8/openhab-addons that referenced this pull request Jan 21, 2020
@zarusz @tmrobert8
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328) (openha…
a0c5584 
…b#6835)

Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
Signed-off-by: Tim Roberts <timmarkroberts@gmail.com>
leluna pushed a commit to leluna/openhab2-addons that referenced this pull request Mar 21, 2020
@zarusz @leluna
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328) (openha…
0516a97 
…b#6835)

Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
Signed-off-by: leluna <hengrui.jiang@googlemail.com>
Hans-Reiner pushed a commit to Hans-Reiner/openhab2-addons that referenced this pull request Apr 11, 2020
@zarusz @Hans-Reiner
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328) (openha…
c66561e 
…b#6835)

Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@zarusz @andrewfg
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328) (openha…
52bb10a 
…b#6835)

Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@zarusz @andrewfg
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328) (openha…
f8e2831 
…b#6835)

Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@zarusz @andrewfg
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328) (openha…
bddc864 
…b#6835)

Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@zarusz @andrewfg
[yamahareceiver] connect timed out since 2.5M4 (openhab#6328) (openha…
f7c356c 
…b#6835)

Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinvw martinvw martinvw approved these changes

@davidgraeff davidgraeff Awaiting requested review from davidgraeff davidgraeff is a code owner

@markus7017 markus7017 Awaiting requested review from markus7017

Assignees

@zarusz zarusz

Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Milestone
2.5.2
Development

Successfully merging this pull request may close these issues.

[yamahareceiver] connect timed out since 2.5M4
5 participants
@zarusz @TravisBuddy @martinvw @Hilbrand @kaikreuzer