blinkpy icon indicating copy to clipboard operation
blinkpy copied to clipboard

Cannot Add More Than One Blink Doorbell

Open GManOrG2C opened this issue 3 years ago • 7 comments

Describe the bug When adding more than one Blink Doorbell no additional doorbell is added. The existing doorbell takes on the name of the new doorbell. It appears that a unique_id for the new doorbell may not be created.

To Reproduce Steps to reproduce the behavior:

  1. Add additional Blink Doorbell to existing Blink location.
  2. Reload the Blink Integration in HA.

Expected behavior Should be additional device and associated entities added to the Blink integration after reload.

Home Assistant version (if applicable): <core-2022.6.0b2>

**blinkpy version 17.93

Log Output/Additional Information

Logger: homeassistant.components.camera Source: helpers/entity_platform.py:598 Integration: Camera (documentation, issues) First occurred: 5:44:12 PM (2 occurrences) Last logged: 5:49:20 PM

Platform blink does not generate unique IDs. ID None-camera already exists - ignoring camera.blink_pagosa_doorbell

Logger: aiohttp.server Source: components/blink/camera.py:91 First occurred: 5:46:05 PM (1 occurrences) Last logged: 5:46:05 PM

Error handling request Traceback (most recent call last): File "/usr/local/lib/python3.9/site-packages/aiohttp/web_protocol.py", line 435, in _handle_request resp = await request_handler(request) File "/usr/local/lib/python3.9/site-packages/aiohttp/web_app.py", line 504, in _handle resp = await handler(request) File "/usr/local/lib/python3.9/site-packages/aiohttp/web_middlewares.py", line 117, in impl return await handler(request) File "/usr/src/homeassistant/homeassistant/components/http/security_filter.py", line 60, in security_filter_middleware return await handler(request) File "/usr/src/homeassistant/homeassistant/components/http/forwarded.py", line 94, in forwarded_middleware return await handler(request) File "/usr/src/homeassistant/homeassistant/components/http/request_context.py", line 28, in request_context_middleware return await handler(request) File "/usr/src/homeassistant/homeassistant/components/http/ban.py", line 79, in ban_middleware return await handler(request) File "/usr/src/homeassistant/homeassistant/components/http/auth.py", line 220, in auth_middleware return await handler(request) File "/usr/src/homeassistant/homeassistant/components/http/view.py", line 137, in handle result = await result File "/usr/src/homeassistant/homeassistant/components/camera/init.py", line 741, in get return await self.handle(request, camera) File "/usr/src/homeassistant/homeassistant/components/camera/init.py", line 759, in handle image = await _async_get_image( File "/usr/src/homeassistant/homeassistant/components/camera/init.py", line 179, in _async_get_image if image_bytes := await camera.async_camera_image( File "/usr/src/homeassistant/homeassistant/components/camera/init.py", line 586, in async_camera_image return await self.hass.async_add_executor_job( File "/usr/local/lib/python3.9/concurrent/futures/thread.py", line 58, in run result = self.fn(*self.args, **self.kwargs) File "/usr/src/homeassistant/homeassistant/components/blink/camera.py", line 91, in camera_image return self._camera.image_from_cache.content AttributeError: 'NoneType' object has no attribute 'content'

PASTE LOG OUTPUT HERE

GManOrG2C avatar May 28 '22 22:05 GManOrG2C

There are two potential problems here. The first MAY have been fixed with a recent PR. The second may be related to #581 (and a distant third: a totally new bug altogether).

What I plan on doing is adding a test where I add multiple doorbells. That should help me pinpoint the problem. I've been quite busy so it's taken me awhile to get to these issues, but I finally have some free time around the corner so I'm hoping I can get through some of these.

fronzbot avatar Jun 23 '22 01:06 fronzbot

Thanks, Kevin.

On Wednesday, June 22, 2022, 07:03:08 PM MDT, Kevin Fronczak @.***> wrote:

There are two potential problems here. The first MAY have been fixed with a recent PR. The second may be related to #581 (and a distant third: a totally new bug altogether).

What I plan on doing is adding a test where I add multiple doorbells. That should help me pinpoint the problem. I've been quite busy so it's taken me awhile to get to these issues, but I finally have some free time around the corner so I'm hoping I can get through some of these.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

GManOrG2C avatar Jun 23 '22 13:06 GManOrG2C

I personally only have one doorbell, and one "owl" (mini). They are both on the same sync module as each other (as is required in order for door chimes to work). Looking at the code, I'm observing that these devices are being added to the internal data model with the ID of the sync_module network, rather than the ID of the camera device itself. This seems wrong to me, and while it works for me because I only have one, I can see that if there were multiple doorbells or minis on the same sync_module, blinkpy might become confused by them. It effectively adds the second doorbell to the all_cameras list, with the same ID that the first doorbell had.

When I saw the code doing this, I searched for issues where people have reported problems with multiple doorbells or mini cameras, to see if anyone was experiencing symptoms that would fit my theory. I thus found this ticket.

I became confused though, because in the class BlinkOwl (and also the doorbell's class BlinkLotus), the method sync_initialize() and the self.summary data properly reflects the mini's own ID and network_id (separately). However, in contrast, the method network_info() returns a structure which contains id but explicitly uses the network_id value there. And the method comment says Format owl response to resemble sync module. But without knowing how this is used, it's not safe for me to change it without asking for help from the experts here.

My initial step toward supporting multiple doorbells and multiple mini's on the same sync_module was the following. Please note, it's not even remotely complete, because I don't know how to handle the stuff mentioned above, and I have no way to test it.

root@4b086fd7b7a9:/dashrb/blinkpy/blinkpy# diff -u -w OLD/blinkpy.py NEW/blinkpy.py
--- OLD/blinkpy.py	2024-01-16 23:34:29.328945048 +0000
+++ NEW/blinkpy.py	2024-01-30 20:38:15.644847855 +0000
@@ -184,9 +184,10 @@
             for owl in self.homescreen["owls"]:
                 name = owl["name"]
                 network_id = str(owl["network_id"])
+                cam_id = str(owl["id"])
                 if network_id in self.network_ids:
                     camera_list.append(
-                        {network_id: {"name": name, "id": network_id, "type": "mini"}}
+                        {network_id: {"name": name, "id": cam_id, "type": "mini"}}
                     )
                     continue
                 if owl["onboarded"]:
@@ -208,12 +209,13 @@
             for lotus in self.homescreen["doorbells"]:
                 name = lotus["name"]
                 network_id = str(lotus["network_id"])
+                cam_id = str(lotus["id"])
                 if network_id in self.network_ids:
                     camera_list.append(
                         {
                             network_id: {
                                 "name": name,
-                                "id": network_id,
+                                "id": cam_id,
                                 "type": "doorbell",
                             }
                         }

dashrb avatar Jan 30 '24 20:01 dashrb

~~@GManOrG2C - looking at your log file, you are running a VERY old version of HA and blinkpy. I think the first step would be to update to a 2024 version of HA which will get the latest version of blinkpy. Let's start from there :)~~ Sorry - didn't realize this was such an old issue being reopened :)

mkmer avatar Jan 31 '24 12:01 mkmer

@dashrb - When I worked on the async conversion, I too became lost in the sync module vs camera and the whole init path. I believe the sync module was tacked on sometime after the initial release and the whole thing could use some optimizing :)

You can test, that's what the tests are for! Write a test case the will cause this situation to "fail" using the data you have, then fix the code to allow the test case to pass. Use tox -e py311 as described here: https://github.com/fronzbot/blinkpy/blob/dev/CONTRIBUTING.rst) or the test environment found in vscode.

mkmer avatar Jan 31 '24 13:01 mkmer

No, I can't test--as I said, I do not own more than one doorbell. I just see that the code is flawed in that the network's id is used to track the individual doorbell, which means multiple doorbells on the same network id (sync_module) will collide. I have no way to test it. @fronzbot mentioned he owns multiple doorbells, possibly he could put them on the same sync_module and see how they act.

dashrb avatar Feb 01 '24 23:02 dashrb

@dashrb I own zero doorbells.

What @mkmer is saying is that you can test that the code is doing what you expect. The tests we have in the library are run to ensure nothing critical breaks. So if you make a code change, even if you can't physically test the feature, you'll be able to see if it had no impact on the existing tests (which means everything is behaving as expected) or if the tests fail you know that your change impacted the library in such a way that requires further consideration.

fronzbot avatar Feb 04 '24 19:02 fronzbot