home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
72.72k stars 30.45k forks source link

Automatically lower Somfy screens at given time (sunset +00:40:00) - 10 out of 12 go down, 2 remain open #49300

Closed Wiggum127 closed 2 years ago

Wiggum127 commented 3 years ago

The problem

I've added integration Somfy to my HA installation using the Web UI. Created an automation to close all screens (12 in total) trigger: sunset +00:40:00 conditions: none

10 out of 12 close down, but 2 remain and don't move. They are the last in the list of the screens to close.

The behaviour is consistent each day, and so is the daily error log (see error log).

Perhaps the issue can be avoided using a scene instead of closing each individual screen. But still I consider this to be a bug.

What is version of Home Assistant Core has the issue?

core-2021.3.4

What was the last working version of Home Assistant Core?

core-2021.3.4

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Somfy

Link to integration documentation on our website

No response

Example YAML snippet

- id: '1618307358500'
  alias: Screens dicht na zonsondergang
  description: ''
  trigger:
  - platform: sun
    event: sunset
    offset: 00:40:00
  condition: []
  action:
  - device_id: 2951e4ed02c4275ec29aafc4d3af4461
    domain: cover
    entity_id: cover.bureau_voorkant
    type: set_position
    position: 0
  - device_id: 27caf25f6ff5294f950b94eea2af1f72
    domain: cover
    entity_id: cover.bureau_zijkant
    type: set_position
    position: 0
  - device_id: 9bb8ca5ab1cee081316bfb9c56b34ee5
    domain: cover
    entity_id: cover.linus
    type: set_position
    position: 0
  - device_id: 0616143b3a9993f33c3e43a0742444f3
    domain: cover
    entity_id: cover.lisa_voorkant
    type: set_position
    position: 0
  - device_id: bc9ba6f448e41d4681fd5e2a3449362e
    domain: cover
    entity_id: cover.lisa_zijkant
    type: set_position
    position: 0
  - device_id: 3c7ee591ee8f98401f5d61f7a4875124
    domain: cover
    entity_id: cover.mama_papa
    type: set_position
    position: 0
  - device_id: 10c769f2ad77298153066aed678f63e6
    domain: cover
    entity_id: cover.rena
    type: set_position
    position: 0
  - device_id: d46ab9b3b7f3b366162071d83f98dd08
    domain: cover
    entity_id: cover.tv_zone_1
    type: set_position
    position: 0
  - device_id: e2fe1c413c38715445ba8a8b76a15a82
    domain: cover
    entity_id: cover.tv_zone_2
    type: set_position
    position: 0
  - device_id: f65dd84c4277194ab6cb53d089113c64
    domain: cover
    entity_id: cover.tv_zone_3
    type: set_position
    position: 0
  - device_id: 0a54cb8a183b18b27f215855b9d4f77a
    domain: cover
    entity_id: cover.tv_zone_4
    type: set_position
    position: 0
  - device_id: 9ed65598d9007d48a12e616314c17d14
    domain: cover
    entity_id: cover.voordeur
    type: set_position
    position: 0
  mode: single

Anything in the logs that might be useful for us?

Logger: homeassistant.components.automation.screens_dicht_na_zonsondergang
Source: components/somfy/cover.py:121
Integration: Automatisering (documentation, issues)
First occurred: 14 april 2021 19:16:46 (4 occurrences)
Last logged: 15 april 2021 19:18:26

Screens dicht na zonsondergang: Error executing script. Unexpected error for device at pos 11: 500 Server Error: Internal Server Error for url: https://api.somfy.com/api/v1/device/[removed by Wiggum127]/exec
While executing automation automation.screens_dicht_na_zonsondergang
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 262, in _async_step
    await getattr(
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 474, in _async_device_step
    await platform.async_call_action_from_config(
  File "/usr/src/homeassistant/homeassistant/components/cover/device_action.py", line 189, in async_call_action_from_config
    await hass.services.async_call(
  File "/usr/src/homeassistant/homeassistant/core.py", line 1488, in async_call
    task.result()
  File "/usr/src/homeassistant/homeassistant/core.py", line 1523, in _execute_service
    await handler.job.target(service_call)
  File "/usr/src/homeassistant/homeassistant/helpers/entity_component.py", line 204, in handle_service
    await self.hass.helpers.service.entity_service_call(
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 642, in entity_service_call
    future.result()  # pop exception if have
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 681, in async_request_call
    await coro
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 679, in _handle_entity_call
    await result
  File "/usr/src/homeassistant/homeassistant/components/cover/__init__.py", line 280, in async_set_cover_position
    await self.hass.async_add_executor_job(
  File "/usr/local/lib/python3.8/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/src/homeassistant/homeassistant/components/somfy/cover.py", line 121, in set_cover_position
    self._cover.set_position(100 - kwargs[ATTR_POSITION])
  File "/usr/local/lib/python3.8/site-packages/pymfy/api/devices/roller_shutter.py", line 16, in set_position
    self.send_command(command)
  File "/usr/local/lib/python3.8/site-packages/pymfy/api/devices/base.py", line 19, in send_command
    self.api.send_command(self.device.id, command)
  File "/usr/local/lib/python3.8/site-packages/pymfy/api/somfy_api.py", line 56, in send_command
    response.raise_for_status()
  File "/usr/local/lib/python3.8/site-packages/requests/models.py", line 943, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 500 Server Error: Internal Server Error for url: https://api.somfy.com/api/v1/device/[removed by Wiggum127]/exec

Additional information

No response

probot-home-assistant[bot] commented 3 years ago

Hey there @tetienne, mind taking a look at this issue as its been labeled with an integration (somfy) you are listed as a codeowner for? Thanks! (message by CodeOwnersMention)

tetienne commented 3 years ago

This issue came from Somfy servers. When too many commands are sent in a short time, you got this error. You can complain to their support. You can also use https://github.com/iMicknl/ha-tahoma if you meet too many times this issue.

Wiggum127 commented 3 years ago

I'll do three things: inform Somfy, try a scene (which you can also set on Somfy level) and try the Tahoma integration (which resolves the issue of Somfy Terrace connections not showing up in the Somfy integration).

But perhaps the Somfy integration can throttle in order to avoid this issue? That would also avoid this issue for other users of Somfy integration. The threshold appears to be 10, based on this use case.

tetienne commented 3 years ago

Until now, I didn’t find a way to throttles the commands. An integration only expose the commands to HA. Then the Core call these commands. I tried also to add some retry, without any success: https://github.com/tetienne/somfy-open-api/pull/69/files About the scenario, the official API does not support it.

Wiggum127 commented 3 years ago

Too bad.

Any pointers where/how to inform Somfy about this unwanted behaviour? In case you already used a communication channel, I can use the same so that awareness grows at their side.

tetienne commented 3 years ago

I use to contact them here: https://developer.somfy.com/contact.

Wiggum127 commented 3 years ago

Somfy has been informed :-)

BTW, perhaps include a side note on the integration page of HA with known issues/shortcommings?

Added suggestion to the Integration page.

decairn commented 3 years ago

Had a similar issue on another integration. Got around it by putting in a 1s delay between each action to each cover.

Wiggum127 commented 3 years ago

Wouldn't that mean you need to create an automation for each individual screen? Seems a bit of overkill.

tetienne commented 3 years ago

You can create a script to do exactly like your scene and adding the 1s delay. Or you can also create a template cover for each of your cover where you add a delay after each command.

decairn commented 3 years ago

I just did a script and had the automation call the script. Super easy to do.

Wiggum127 commented 3 years ago

Changed implementation to an automation of two scripts. Each script lowering 6 screens, with 1 sec interval within the script between screens and the automation is adding a 5 sec interval between the two scripts. The behaviour remains the same, only 10 out of 12 go down.

Switched to the Tahoma integration (fixing the Terrace connection) and the behaviour remains the same. However, the error says:

Execution queue is full on gateway: #0823-7095-4654 (soft limit: 10)

This confirms what we already thought: there is a threshold of 10 actions in happening in parallel. It would be good to add this to the documentation page of the integration.

iMicknl commented 3 years ago

@Wiggum127 that would be a good improvement indeed. We are in the progress of adding ha-tahoma to core, but eventually I think the documentation for somfy, tahoma and velux can use some improvements to tell the differences and also list the limitations.

Wiggum127 commented 3 years ago

@iMicknl I can make suggestions to the documentation for somfy and tahoma. For velux, however, I wouldn't know.

Now changed the automation to leave 1min between the first 6 and second 6 screens. I didn't get any errors in the logs and all screens were closed. Will keep an eye on things, but it appears we've narrowed it down to the soft limit of 10 actions on the somfy/tahoma gateway.

bdraco commented 3 years ago

This could likely be solved with a queue in the underlying library which rate limits how much comes out of the queue.

I had the same issue but since all my devices are RTS I bought a bond (https://www.home-assistant.io/integrations/bond/) and now have local control of them

tetienne commented 3 years ago

@bdraco do you know any library that’s implement such behavior? For the Tahoma API we can access the execution list, but for the Somfy (Official) API there is no such thing.

iMicknl commented 3 years ago

This confirms what we already thought: there is a threshold of 10 actions in happening in parallel. It would be good to add this to the documentation page of the integration.

@Wiggum127 I added it to this PR https://github.com/home-assistant/home-assistant.io/pull/14920/files. Feel free to add your input.

Wiggum127 commented 3 years ago

@iMicknl The PR link shows the technical documentation will be updated to better inform the user. Looks fine for me.

The user can work his way around with timers etc., but perhaps this can be solved more elegantly.

@bdraco / @tetienne Isn't there a way to deal with this on behalf of the user? For example: on execution of a script/automation, allow adding items to the queue up to 10 and then throttle the next as long as the queue depth is not below 10. You could foresee a warning in the logs in that case instead of just throwing an error and killing the script/automation.

Should the soft limit ever disappear or change to another value, it would then also be a matter of changing this once in the core component.

bdraco commented 3 years ago

@bdraco do you know any library that’s implement such behavior? For the Tahoma API we can access the execution list, but for the Somfy (Official) API there is no such thing.

Since the goal is to limit the number of items going to the api you could wrap all api calls with a semaphore https://docs.python.org/3/library/asyncio-sync.html#semaphore

sem = asyncio.Semaphore(10)
async with sem:
    <code that calls the api here>

Then everything will wait until the semaphore counter < 10

bdraco commented 3 years ago

This functionality is actually built in already with the PARALLEL_UPDATES constant

bdraco commented 3 years ago

https://developers.home-assistant.io/docs/integration_fetching_data/#request-parallelism

bdraco commented 3 years ago

But since the cloud is doing the queuing and the goal isn't to overload the queue that probably won't be enough.

I think you would have to build a wrapper that dumps asyncio futures into a asyncio queue and then awaits for them to be processed

bdraco commented 3 years ago

Synchronization will be a bit of a problem though, as you have to know when the queue is full. If you don't you send the request and then back off and continue processing when the queue is not full

tetienne commented 3 years ago

@bdraco You think about something like this: https://github.com/tetienne/somfy-open-api/pull/69/files#diff-b8631135383c54e673570076cda74677c2f3c7146338b2c7a9bf4c9b9e47e08fR59

iMicknl commented 3 years ago

@bdraco You think about something like this: https://github.com/tetienne/somfy-open-api/pull/69/files#diff-b8631135383c54e673570076cda74677c2f3c7146338b2c7a9bf4c9b9e47e08fR59

For TaHoma / Overkiz integration, I did look into this, but backoff won't have a guaranteed order. Which could mean if you do multiple actions for the same device in a sequence, it could be that they are executed in the wrong order. I couldn't find an easy and quick fix yet..

bdraco commented 3 years ago

Besides the order issue that iMicknl mentioned, the downside to something like backoff with sync code is it will tie up the executor until the backoff completes which can end up starving the instance as everything waiting for a free executor thread has to wait for the call to complete.

If you use an asyncio queue (https://docs.python.org/3/library/asyncio-queue.html) and do the backoff in async you are only creating an executor job for the short period of time that it needs for the call to fail and the backoff waits in async efficiently.

bdraco commented 3 years ago

Some rough pseudo code:

from functools import partial
    async def set_cover_position(self, **kwargs):
        """Move the cover shutter to a specific position."""
        await self.queue.put(partial(self._cover.set_position, 100 - kwargs[ATTR_POSITION]))
async def queue_runner(hass, queue):
    while True:
        api_call = await queue.get()

        while not max_attempts:
           try:
             await hass.async_add_executor_job(api_call)
           except <somfy exception indicating queue full>
             await asyncio.sleep(<PROGRESSIVE_BACKOFF_TIME>) 
           except Exception:
             _LOGGER.warning("Unexpected exception ....")

queue_task = asyncio.create_task(queue_runner(hass, queue))

At unload/stop:

queue_task.cancel()
bdraco commented 3 years ago
class SomfyDataUpdateCoordinator(DataUpdateCoordinator):
    """Class to manage fetching somfy data."""

    def __init__(
        self,
        hass: HomeAssistant,
        *,
        api: api.ConfigEntrySomfyApi,
        queue: asyncio.Queue,
    ):
        """Initialize the somfy data updater."""
        self.api = api
        self.queue = queue
        super().__init__(
            hass,
            _LOGGER,
            name="somfy device update",
            update_interval=SCAN_INTERVAL,
        )

    async def _async_update_data(self):
        """Update all the devices."""
        devices = await self.hass.async_add_executor_job(self.api.get_devices)
        previous_devices = self.data
        # Sometimes Somfy returns an empty list.
        if not devices and previous_devices:
            raise UpdateFailed("No devices returned")
        return {dev.id: dev for dev in devices}

In async_setup_entry:


    coordinator = data[COORDINATOR] = SomfyDataUpdateCoordinator(
        hass,
        api=api.ConfigEntrySomfyApi(hass, entry, implementation),
    )

You could do something like the above to make the queue accessible to all the entities.

tetienne commented 3 years ago

@bdraco thx for the suggestion. But in your last comment, the first exemple talks about queue and the second one about a semaphore. I'm confused.

bdraco commented 3 years ago

At first I was hoping to be able to use a Semaphore since it makes it much simpler, but since you don't know the state of the remote queue you have to try and then retry which means a queue is needed.

Wiggum127 commented 3 years ago

For the record, developer@somfy.com just replied with following feedback:

The API allows a buffer of 10 executions. You need to set a delay between each batch of requests to make sure there are all taken into consideration.

This is just confirming what we already knew, but now coming from the owner of the API.

Wiggum127 commented 3 years ago

With the automation of script for 6screens, wait 1min, script for other 6screens things have been working for a while.

However, the past few days the automation no longer works and is throwing an error. This is weird since it used to work for 12 screens with the interval, but now not even a single screens is put in motion.

`Logger: custom_components.tahoma.tahoma_entity Source: custom_components/tahoma/tahoma_entity.py:177 Integration: Somfy TaHoma (documentation, issues) Last logged: 5 mei 2021 19:45:55

Execution queue is full on gateway: #0823-7095-4654 (soft limit: 10) Cannot connect to host tahomalink.com:443 ssl:default [Connect call failed ('178.32.15.131', 443)] [Errno 104] Connection reset by peer`

and

`Logger: custom_components.tahoma Source: helpers/update_coordinator.py:173 Integration: Somfy TaHoma (documentation, issues) Last logged: 5 mei 2021 19:46:01

Error fetching device events data: Cannot connect to host tahomalink.com:443 ssl:default [Connect call failed ('178.32.15.131', 443)] Error fetching device events data: [Errno 104] Connection reset by peer`

This to me seems to be another bug where either:

  1. the code claims it is hitting the soft limit, but in fact it's a false claim
  2. things are put on the queue without being processed and the queue is now full
  3. HA no longer respects the wait interval

Important to note:

tetienne commented 3 years ago

Now the soft limit error is thrown right from the first screen in the automation.

@Wiggum127 If you update your script to move only one cover you got the error too?

github-actions[bot] commented 3 years ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Wiggum127 commented 3 years ago

@tetienne

Now the soft limit error is thrown right from the first screen in the automation.

@Wiggum127 If you update your script to move only one cover you got the error too?

@tetienne I'm currently unable to debug the issue as I'm stuck with another bug, crippling the integration (@iMicknl see https://github.com/iMicknl/ha-tahoma/issues/476)

tetienne commented 3 years ago

@Wiggum127 I don’t see the link between the two integrations. They haven’t any interactions.

Wiggum127 commented 3 years ago

Indeed. I used to have the Somfy integration, but because of the unclear errors, unstable behavriour and not being able to control the Terrace connection or using scenes defined the Somfy app, I've switched to the Tahoma integration.

Which now fails completely with no sign of progress on the issue for weeks.

I guess I'll be switching back to the Somfy integration in the coming days.

Will let you know how it behaves.

github-actions[bot] commented 2 years ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.