[intesis] Improve session handling #16476

fd0cwp · 2024-03-02T14:52:37Z

I have changed the login process so that the binding now only logs in at the beginning and logs out at the end. If a request fails due to a missing login or an invalid session ID, a new login attempt is made.

This ensures that

Several requests can be sent in direct succession (if I switch the air conditioning from cooling to heating, I also want to change the slats at the same time)
Less traffic is generated as there is not a constant logout and login again
Ideally, the login credentials should only be visible once at startup

I am unsure whether I have used the correct levels for the logging.

The updated binding has been running without any problems for a few weeks now in my environment. First under 4.1.0, and since a few days now under 4.1.1.

Signed-off-by: Christoph fd0cwp@gmx.de

I have changed the login process so that the binding now only logs in at the beginning and logs out at the end. If a request fails due to a missing login or an invalid session ID, a new login attempt is made. This ensures that - Several requests can be sent in direct succession (if I switch the air conditioning from cooling to heating, I also want to change the slats at the same time) - Less traffic is generated as there is not a constant logout and login again - Ideally, the login credentials should only be visible once at startup I am unsure whether I have used the correct levels for the logging. The updated binding has been running without any problems for a few weeks now in my environment. First under 4.1.0, and since a few days now under 4.1.1. Signed-off-by: Christoph <fd0cwp@gmx.de>

lsiepel

Left some comments, the build error has to be fixed (spotless), please also check if there are compile warnings.

Edit: Documentation about logging is here: https://www.openhab.org/docs/developer/guidelines.html#f-logging

...g.intesis/src/main/java/org/openhab/binding/intesis/internal/handler/IntesisHomeHandler.java

…binding/intesis/internal/handler/IntesisHomeHandler.java I think this is no code I changed, but I also prefer your suggestion. Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Christoph <36006493+fd0cwp@users.noreply.github.com>

I have changed the login process so that the binding now only logs in at the beginning and logs out at the end. If a request fails due to a missing login or an invalid session ID, a new login attempt is made. This ensures that - Several requests can be sent in direct succession (if I switch the air conditioning from cooling to heating, I also want to change the slats at the same time) - Less traffic is generated as there is not a constant logout and login again - Ideally, the login credentials should only be visible once at startup I am unsure whether I have used the correct levels for the logging. The updated binding has been running without any problems for a few weeks now in my environment. First under 4.1.0, and since a few days now under 4.1.1. Signed-off-by: Christoph <fd0cwp@gmx.de>

...g.intesis/src/main/java/org/openhab/binding/intesis/internal/handler/IntesisHomeHandler.java

I have changed the login process so that the binding now only logs in at the beginning and logs out at the end. If a request fails due to a missing login or an invalid session ID, a new login attempt is made. This ensures that - Several requests can be sent in direct succession (if I switch the air conditioning from cooling to heating, I also want to change the slats at the same time) - Less traffic is generated as there is not a constant logout and login again - Ideally, the login credentials should only be visible once at startup I am unsure whether I have used the correct levels for the logging. The updated binding has been running without any problems for a few weeks now in my environment. First under 4.1.0, and since a few days now under 4.1.1. Signed-off-by: Christoph <fd0cwp@gmx.de>

jlaur · 2024-03-05T22:18:09Z

...g.intesis/src/main/java/org/openhab/binding/intesis/internal/handler/IntesisHomeHandler.java

@@ -109,6 +112,9 @@ public void initialize() {
            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Password not set");
            return;
        } else {
+            logger.trace("trying to log in - current session ID: {}", sessionId);
+            login();


initialize() should return fast and not do any blocking network calls. This is the reason for background initialization below.

jlaur · 2024-03-05T22:18:24Z

...g.intesis/src/main/java/org/openhab/binding/intesis/internal/handler/IntesisHomeHandler.java

@@ -129,6 +135,8 @@ public void dispose() {
            refreshJob.cancel(true);
            this.refreshJob = null;
        }
+
+        logout(sessionId);


lsiepel · 2024-03-06T19:21:45Z

@fd0cwp can you come up with a folow up PR as i missed these comment jlaur mentioned.

fd0cwp · 2024-03-07T15:07:41Z

I am not sure if this is reasonable.

What is the purpose of the initialize function? I would say that the binding shall be set up so that it is functional. In my view, this also includes the login.
initialize should only be run when openHAB is started. Does the two seconds for the login matter? Or am I missing something?
From my point of view, the same applies to dispose.

I could remove the login from the initialize. This would result in the first call failing, then the login is made and the call is tried again.
I could also remove the logout from dispose. But then openHAB no longer shuts down cleanly.

@lsiepel, @jlaur: What do you think?

jlaur · 2024-03-07T16:03:59Z

@lsiepel, @jlaur: What do you think?

I already stated what I think. 🙂 Please have a look at https://www.openhab.org/docs/developer/bindings/#startup:

Furthermore, the framework expects initialize() to be non-blocking and to return quickly. For longer running initializations, the implementation has to take care of scheduling a separate job which must guarantee to set the status eventually.

So this means you need to move the login(); into the scheduler.submit block just below, as mentioned. Similarly, you can also schedule the logout - like it was previously also done here:
20f306f#diff-94852e57bc82cccaead7afd2af8297593bd11fd5cee89fa0957a41a57677494eL176-L182

fd0cwp · 2024-03-19T13:41:40Z

Hi @jlaur,

in https://www.openhab.org/docs/developer/bindings/#lifecycle this is noted:
The initialize method is called when the handler is started and dispose just before the handler is stopped. Therefore these methods can be used to allocate and deallocate resources.
I was the opinion that a valid connection is also some kind of resource.

However, I found several threads telling what you stated.
@lsiepel: So, I updated the source code again. Hopefully, it is now OK.

* [intesis] SESSION ID HANDLING IMPROVED Signed-off-by: Christoph <fd0cwp@gmx.de> Signed-off-by: Jørgen Austvik <jaustvik@acm.org>

fd0cwp requested a review from hmerk as a code owner March 2, 2024 14:52

fd0cwp mentioned this pull request Mar 2, 2024

[intesis] SESSION ID HANDLING IMPROVED #16475

Closed

lsiepel requested changes Mar 3, 2024

View reviewed changes

...g.intesis/src/main/java/org/openhab/binding/intesis/internal/handler/IntesisHomeHandler.java Outdated Show resolved Hide resolved

...g.intesis/src/main/java/org/openhab/binding/intesis/internal/handler/IntesisHomeHandler.java Outdated Show resolved Hide resolved

lsiepel added the enhancement An enhancement or new feature for an existing add-on label Mar 3, 2024

lsiepel changed the title ~~[intesis] SESSION ID HANDLING IMPROVED~~ [intesis] Improve session handling Mar 3, 2024

fd0cwp and others added 4 commits March 4, 2024 17:37

Merge branch 'openhab:main' into main

35f5f2a

Merge branch 'main' of https://github.com/fd0cwp/openhab-addons

56b77c7

fd0cwp requested a review from lsiepel March 4, 2024 19:30

lsiepel reviewed Mar 4, 2024

View reviewed changes

...g.intesis/src/main/java/org/openhab/binding/intesis/internal/handler/IntesisHomeHandler.java Outdated Show resolved Hide resolved

fd0cwp requested a review from lsiepel March 5, 2024 11:06

lsiepel merged commit 9987215 into openhab:main Mar 5, 2024
3 checks passed

lsiepel added this to the 4.2 milestone Mar 5, 2024

jlaur reviewed Mar 5, 2024

View reviewed changes

This was referenced Mar 19, 2024

[intesis] Session Handling improved #16549

Closed

[intesis] Session Handling improved #16551

Closed

[intesis] Session Handling improved #16555

Closed

austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024

[intesis] Improve session handling (openhab#16476)

75c5e03

* [intesis] SESSION ID HANDLING IMPROVED Signed-off-by: Christoph <fd0cwp@gmx.de> Signed-off-by: Jørgen Austvik <jaustvik@acm.org>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[intesis] Improve session handling #16476

[intesis] Improve session handling #16476

fd0cwp commented Mar 2, 2024

lsiepel left a comment •

edited

jlaur Mar 5, 2024

jlaur Mar 5, 2024

lsiepel commented Mar 6, 2024

fd0cwp commented Mar 7, 2024

jlaur commented Mar 7, 2024

fd0cwp commented Mar 19, 2024

[intesis] Improve session handling #16476

[intesis] Improve session handling #16476

Conversation

fd0cwp commented Mar 2, 2024

lsiepel left a comment • edited

Choose a reason for hiding this comment

jlaur Mar 5, 2024

Choose a reason for hiding this comment

jlaur Mar 5, 2024

Choose a reason for hiding this comment

lsiepel commented Mar 6, 2024

fd0cwp commented Mar 7, 2024

jlaur commented Mar 7, 2024

fd0cwp commented Mar 19, 2024

lsiepel left a comment •

edited