-
-
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
[tr064] reduce network load and improve XML handling #9693
Conversation
Signed-off-by: Jan N. Klug <[email protected]>
Yesterday I installed this version for testing. The reported issue in #9689 did not occur again. Unfortunately the timeout is still there, but not that often - it looks like the new timeout is not taken into account:
|
Signed-off-by: Jan N. Klug <[email protected]>
There's two different timeouts, one for SOAP requests and the other for all other HTTP requests. I have now adjusted bothto 5s. |
Signed-off-by: Jan N. Klug <[email protected]>
I also implementing caching for the SOAP requests. Especially for call lists there are many requests which are the same. |
...enhab.binding.tr064/src/main/java/org/openhab/binding/tr064/internal/soap/SOAPConnector.java
Show resolved
Hide resolved
...enhab.binding.tr064/src/main/java/org/openhab/binding/tr064/internal/soap/SOAPConnector.java
Show resolved
Hide resolved
Looks good so far. No messages in my log anymore for more than twelve hours. Thank you for looking into it. |
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.
LGTM. Thanks. And it is working fine. No more warnings since nearly two days.
private final Logger logger = LoggerFactory.getLogger(SOAPConnector.class); | ||
private final HttpClient httpClient; | ||
private final String endpointBaseURL; | ||
private final SOAPValueConverter soapValueConverter; | ||
|
||
private final ExpiringCacheMap<SOAPRequest, SOAPMessage> soapMessageCache = new ExpiringCacheMap<>( |
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.
Only a minor remark. Why not passing the time value directly like here ? Or use Duration.of()
to improve readability.
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 stateCache can probably be omitted. Sinc ethe SOAP request to fill it is cached anyway. WDYT?
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.
Yes, we probably could remove that one.
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.
OTOH, it bypasses the construction of the SOAP request and does not add much complexity now that it is here. So I would prefer to leave it in place.
...es/org.openhab.binding.tr064/src/main/java/org/openhab/binding/tr064/internal/util/Util.java
Show resolved
Hide resolved
Signed-off-by: Jan N. Klug <[email protected]>
I found another occurring message - is this handled differently?
|
That request is not cached because there is only one request per answering machine. I wonder if the problem is that two requests are made in the same time (e.g. the request for the TAM list and a call list). |
But there are two channels available for each TAM: |
True. But „enabled“ is from a regular (cached) soap request (like getting the URL) and only „newMessages“ is an additional HTTP call. |
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.
LGTM. Thanks.
* reduce network load * adjust soap timeout Signed-off-by: Jan N. Klug <[email protected]> Signed-off-by: John Marshall <[email protected]>
* reduce network load * adjust soap timeout Signed-off-by: Jan N. Klug <[email protected]>
BTW: I still get these messages on OpenHAB 3.2.0~M5 and Fritzbox software release 7.29 irregularly :(
|
* reduce network load * adjust soap timeout Signed-off-by: Jan N. Klug <[email protected]>
Fixes #9689