-
-
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
[homematic] Fixed for two (re)connection problems #9692
Conversation
Signed-off-by: Martin Herbst <develop@mherbst.de>
They were already marked as deprecated since several versions. Signed-off-by: Martin Herbst <develop@mherbst.de>
- simplified coding for the communication with the gateway - buffer size for communication is now configurable to avoid problems with too small buffers - Previous solution for #6963 was not sufficient. Should be finally done with these changes Signed-off-by: Martin Herbst <develop@mherbst.de>
ping requests could therefore be safely removed problems with the automatic reconnection were solved. Fixes #8808 Signed-off-by: Martin Herbst <develop@mherbst.de>
} catch (UnsupportedEncodingException | ExecutionException | TimeoutException | InterruptedException e) { | ||
} catch (Exception 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.
Please keep the catch explicit as it was before. I'd rather not have RuntimeExceptions get treated as IOExceptions.
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.
OK, accepted.
I have changed the catch clause and explicitly added IllegalArgumentException because this is throws by Jetty if the buffer is too small.
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 the buffer is too small, then why is sendMessage
retrying the request when that happens?
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 agree, that it does not really make sense, but this would mean some additional changed in another place and I wanted to avoid to it together with these changes. It's on my to-do list for a larger refactoring.
.../main/java/org/openhab/binding/homematic/internal/communicator/AbstractHomematicGateway.java
Show resolved
Hide resolved
Signed-off-by: Martin Herbst <develop@mherbst.de>
} | ||
if (logger.isTraceEnabled()) { | ||
String result = new String(ret, config.getEncoding()); | ||
logger.trace("Client XmlRpcResponse (port {}):\n{}", port, result); | ||
} | ||
} catch (UnsupportedEncodingException | ExecutionException | TimeoutException | InterruptedException e) { | ||
} catch (InterruptedException | ExecutionException | TimeoutException | IllegalArgumentException 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.
I noticed that code that calls the send
method will attempt to retry a request anytime an IOException
is thrown. This should not be the case when an InterruptedException
is thrown since the thread should exit as soon as possible when an interrupt occurs. So I suggest instead of catching the InterruptedException here, you instead throw it as far up the stack as possible.
But such a change is probably out of scope for this PR so feel free to make another PR with the changes.
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.
Exactly, see my comment above.
Signed-off-by: Martin Herbst <develop@mherbst.de>
Signed-off-by: Martin Herbst <develop@mherbst.de>
Sorry, I had to edit the file in the github editor because of some git problems in my Eclipse workspace. Unfortunately forgot to add two imports. |
@MHerbst This also fixes my reconnect problem, thank you! |
* Replace deprecated constructors * Removed no longer existing settings from the documentation. They were already marked as deprecated since several versions. * Refactored communication with the HM gateway - simplified coding for the communication with the gateway - buffer size for communication is now configurable to avoid problems with too small buffers - Previous solution for openhab#6963 was not sufficient. Should be finally done with these changes * Retrieving the duty cycle is sufficient to check connection - ping requests could therefore be safely removed problems with the automatic reconnection were solved. * Changed to explicit list of Exception Fixes openhab#8808 Signed-off-by: Martin Herbst <develop@mherbst.de> Signed-off-by: John Marshall <john.marshall.au@gmail.com>
* Replace deprecated constructors * Removed no longer existing settings from the documentation. They were already marked as deprecated since several versions. * Refactored communication with the HM gateway - simplified coding for the communication with the gateway - buffer size for communication is now configurable to avoid problems with too small buffers - Previous solution for openhab#6963 was not sufficient. Should be finally done with these changes * Retrieving the duty cycle is sufficient to check connection - ping requests could therefore be safely removed problems with the automatic reconnection were solved. * Changed to explicit list of Exception Fixes openhab#8808 Signed-off-by: Martin Herbst <develop@mherbst.de>
* Replace deprecated constructors * Removed no longer existing settings from the documentation. They were already marked as deprecated since several versions. * Refactored communication with the HM gateway - simplified coding for the communication with the gateway - buffer size for communication is now configurable to avoid problems with too small buffers - Previous solution for openhab#6963 was not sufficient. Should be finally done with these changes * Retrieving the duty cycle is sufficient to check connection - ping requests could therefore be safely removed problems with the automatic reconnection were solved. * Changed to explicit list of Exception Fixes openhab#8808 Signed-off-by: Martin Herbst <develop@mherbst.de>
This PR focusses primarily on the solution of two connection problems:
Two additional commits are removing deprecations from the doc and replaced deprecated Java constructors.