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

Cannot Add More Than One Blink Doorbell #580

Open
GManOrG2C opened this issue May 28, 2022 · 7 comments
Open

Cannot Add More Than One Blink Doorbell #580

GManOrG2C opened this issue May 28, 2022 · 7 comments
Labels
bug There is an issue causing unexpected results help-wanted Assistance required due to lack of resources for testing or complexity

Comments

@GManOrG2C
Copy link

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
@fronzbot fronzbot added bug There is an issue causing unexpected results help-wanted Assistance required due to lack of resources for testing or complexity labels May 31, 2022
@fronzbot
Copy link
Owner

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.

@GManOrG2C
Copy link
Author

GManOrG2C commented Jun 23, 2022 via email

@fronzbot fronzbot mentioned this issue Jun 26, 2022
3 tasks
@dashrb
Copy link
Contributor

dashrb commented Jan 30, 2024

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",
                             }
                         }

@mkmer
Copy link
Contributor

mkmer commented Jan 31, 2024

@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
Copy link
Contributor

mkmer commented Jan 31, 2024

@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.

@dashrb
Copy link
Contributor

dashrb commented Feb 1, 2024

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.

@fronzbot
Copy link
Owner

fronzbot commented Feb 4, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug There is an issue causing unexpected results help-wanted Assistance required due to lack of resources for testing or complexity
Projects
None yet
Development

No branches or pull requests

4 participants