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

Remove the lock on apply_action #93

Merged
merged 9 commits into from
Jul 2, 2020
Merged

Remove the lock on apply_action #93

merged 9 commits into from
Jul 2, 2020

Conversation

vlebourl
Copy link
Collaborator

@vlebourl vlebourl commented Jun 29, 2020

Add two functions to select first existing command or state from a list.
Implement in cover.py
As an async apply_action doesn't prevent the lock, I propose to add
an execution queue to tahoma_device entities, and only perform the update
once the queue is empty.
Having `update` instead of `schedule_update` line 97 doesn't prevent the lock...

Related Github issues: #92
Also clear the exec_queue if it's not executing anymore.

Related Github issues: #92
@vlebourl vlebourl added bug Something isn't working Needs Testing This needs extensive testing labels Jun 29, 2020
@vlebourl
Copy link
Collaborator Author

If accepted, I think this would deserve a beta release.

@vlebourl vlebourl marked this pull request as ready for review July 1, 2020 07:08
@@ -60,6 +60,12 @@ def device_class(self):

def update(self):
"""Update the state."""
exec_queue = self.controller.get_current_executions()
Copy link
Owner

Choose a reason for hiding this comment

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

This code is the same in all implementations? Should we move it to the tahoma_device and call this function from here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It could take this form:

    def should_wait(self):
        """Wait for actions to finish (this would be in tahoma_device"""
        exec_queue = self.controller.get_current_executions()
        self._exec_queue = [e for e in self._exec_queue if e in exec_queue]
        return True if self._exec_queue else False

    def update(self):
        """Update the state."""
        if self.should_wait():
            self.schedule_update_ha_state(True)
            return
        # continue update

@vlebourl vlebourl merged commit 75f3e8e into master Jul 2, 2020
@vlebourl vlebourl deleted the async_apply_action branch July 2, 2020 08:49
@rabesocke
Copy link

I test with the 1.5-alpha2. My covers reacts with several seconds delay to the click in the HA frontend.

@vlebourl
Copy link
Collaborator Author

vlebourl commented Jul 2, 2020

How reactive is it when interacting through the TaHoma web dashboard?
Also, when clicking in the HA frontend, can you keep the Tahomalink website open and visible, you should get something like this:
image
Is the message on TaHoma dashboard appearing with the same delay?

@vlebourl
Copy link
Collaborator Author

vlebourl commented Jul 2, 2020

It's very reactive at my place... Not sure why it would add any delay.

@rabesocke
Copy link

There is no delay through the TaHoma Dashobard. I can change the position, wait a little bit and click on stop and the cover reacts immediately.

Through HA it starts in general directly but not always. When there is a delay then you see the delay also on the TaHoma dashboard.

One interesting point: The stop button is not working while moving. When I click the button on TaHoma dashboard it stops immediately. (Please wait some seconds after you start the movement and then try to click on stop - for me it is not working)

@vlebourl
Copy link
Collaborator Author

vlebourl commented Jul 2, 2020

"When there is a delay then you see the delay also on the TaHoma dashboard."
Do you mean there is the delay for the message in my screen capture to appear?
If so, the delay is in the communication between HA and TaHoma, not between TaHoma and the Cover.
Be aware that the api is an unofficial one, not intended by Somfy to be used by HA. Hence sending lots of requests in a row might get you kicked out, forcing a re-log that might take few seconds.

I don't know about the stop button, but I suspect it's not related to the delay, so I propose to keep it away from this issue and open one of its own.

@rabesocke
Copy link

The delay is completely new to me. Before I had to use the current version (because of the login issue) there were no such delays. It appears the first timt 30 minutes ago after the HA update to 0.112 and the component update to 1.5-alpha2

@vlebourl
Copy link
Collaborator Author

vlebourl commented Jul 2, 2020

The delay might be related to the login issue. You probably still get logged out a lot, then the api re-logs you in automatically causing the delay. We would need to figure out why you get logged out, but I have no idea how to do that...

@jurgen272
Copy link

Overhere it is far beter then in the last versions. Before the stop button wasn't reacting, and now it is. will test again in few hours.
0.112 en alpha2

@rabesocke
Copy link

rabesocke commented Jul 2, 2020

The delay might be related to the login issue. You probably still get logged out a lot, then the api re-logs you in automatically causing the delay. We would need to figure out why you get logged out, but I have no idea how to do that...

Ok, that could be the reason for the delay. It would be great to get this fixed - but I also have no idea, how to find the reason

Overhere it is far beter then in the last versions. Before the stop button wasn't reacting, and now it is. will test again in few hours.

Interesting ... for me all versions including 1.3 were much better than the current version and the stop button was working for me without problems. The problems for me start with version 1.4 (which I skipped until the login issue)

@vlebourl
Copy link
Collaborator Author

vlebourl commented Jul 2, 2020

Can you open a new issue with details on your cover regarding the stop button?

I'll try to put more log messages to find why you get kicked out of the api.

@rabesocke
Copy link

I tested it again and I would say that the reason is the login issue. I clicked on the move-down-button and it had a delay of around 2 seconds. The same happens when I clicked on the stop-button some seconds later. So I would say it relates not only to the stop-button.

@vlebourl
Copy link
Collaborator Author

vlebourl commented Jul 2, 2020

can you edit the file tahoma_api.py and add the following line 157, just before the if not self.login()

_LOGGER.debug("not authenticated: login again.")

Restart HA a try again with debug log setup for the component (see here)

@vlebourl
Copy link
Collaborator Author

vlebourl commented Jul 2, 2020

By the way, conversation should be moved to #86

@rabesocke rabesocke mentioned this pull request Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Needs Testing This needs extensive testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Actions are applied sequentialy Covers move step by step and can't be stopped while moving
4 participants