Fix timing issue in Withings

[joostlek]

Proposed change

Fix timing issue in Withings

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes Withings - Showing as Unknown and/or Showing Detected even when not in bed #102359
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[allenporter]

Can register_webhook raise an exception e.g. while communicating with a cloud api? It may require setting/unsetting this with some exception handling. If so, then consider adding another wrapper function to manage the lock. (or maybe route all calls through manage_cloudhook and put the lock in that function.)

[joostlek]

Well the problem I have is that the user disconnects from cloud for a 3 seconds, so both the register and unregister process is running. I think I can solve this by putting the same lock on those functions, not sure what's bvest

[allenporter]

Well the problem I have is that the user disconnects from cloud for a 3 seconds, so both the register and unregister process is running. I think I can solve this by putting the same lock on those functions, not sure what's best

This doesn't seem to handle register and unregister running at the same time? Seems like it just prevents register from running twice at once?

There are some problematic cases to worry about for something like this:

  is_running: bool = False

  async def register(...)
    if is_running:
        return
    is_running = True
    ...
    is_running = False

  async def unregister(...)
    if is_running:
        return
    is_running = True
    ...
    is_running = False

That is problematic because an unregister task may arrive while register is running and it's not necessarily correct to just drop it on the floor.

You can put a lock on both operations like this:

  lock = asyncio.Lock()

  async def register(...)
    async with lock:
        ...

  async def unregister(...)
    async with lock:
        ...

Then if an unregister task arrives while the register task is running then it will queue and wait for the register task to finish. This may also mean that register/unregister tasks pile up though. I also am not sure if they necessarily run in a specific order (e.g. if multiple register tasks or unregister tasks are waiting, does the lock "wake up" in a specific order they arrived in? Probably not necessarily...)

Another approach can be any time a task is invoked it decides what to do once it actually runs based on the current state, or exits early or no-op if things are already in the proper state e.g.

  lock = asyncio.Lock()

  async def run(...)
    # Can be called for any reason, startup, shutdown, connection state change, etc. When running: What should i do? Look at
    # the current state of the system and decide
    async with lock:
        if is_stopping or cloud.is_disconnected:
          if is_already_registered:
              await unregister()
          return
       if not is_already_registered
          await register()

I may be missing some details here, but broadly those are approaches that come to mind.

[joostlek]

I will create tests for this in a follow up. Since the tests are encountering locks, I need some more time for this and don't want to hold up the fix. This has been tested with several people.

[frenck]

Let's take this route to get the "fix" out, but we should seriously considering extracting this to get rid of nonlocals and incoperate some feedback from Allen above.