-
Notifications
You must be signed in to change notification settings - Fork 136
Per camera snooze #1135
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
base: dev
Are you sure you want to change the base?
Per camera snooze #1135
Changes from all commits
90a1a58
a002e90
2d649f6
50cc35e
267909f
5554686
85d7c66
456d0f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -177,6 +177,31 @@ async def async_set_night_vision(self, value): | |||||||||||||||||||||||||||
| return await res.json() | ||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||
| async def snooze_till(self): | ||||||||||||||||||||||||||||
| """Return snooze_till status.""" | ||||||||||||||||||||||||||||
| res = await api.request_get_config( | ||||||||||||||||||||||||||||
| self.sync.blink, | ||||||||||||||||||||||||||||
| self.network_id, | ||||||||||||||||||||||||||||
| self.camera_id, | ||||||||||||||||||||||||||||
| product_type=self.product_type, | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
| if res is None: | ||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||
| return res.get("camera", [{}])[0].get("snooze_till") | ||||||||||||||||||||||||||||
|
Comment on lines
+189
to
+191
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no indication to the user if this fails. I think a better approach would be to either let it raise an exception OR catch the exception and log an error/warning. I tend to favor the latter, but could be convinced either way.
Suggested change
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| async def async_snooze(self, snooze_time=240): | ||||||||||||||||||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what are the units of |
||||||||||||||||||||||||||||
| """Set camera snooze status.""" | ||||||||||||||||||||||||||||
| data = dumps({"snooze_time": snooze_time}) | ||||||||||||||||||||||||||||
| res = await api.request_camera_snooze( | ||||||||||||||||||||||||||||
| self.sync.blink, | ||||||||||||||||||||||||||||
| self.network_id, | ||||||||||||||||||||||||||||
| self.camera_id, | ||||||||||||||||||||||||||||
| product_type=self.product_type, | ||||||||||||||||||||||||||||
| data=data, | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
| return res | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| async def record(self): | ||||||||||||||||||||||||||||
| """Initiate clip recording.""" | ||||||||||||||||||||||||||||
| return await api.request_new_video( | ||||||||||||||||||||||||||||
|
|
@@ -540,3 +565,30 @@ def __init__(self, sync): | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| async def get_sensor_info(self): | ||||||||||||||||||||||||||||
| """Get sensor info for blink doorbell camera.""" | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| async def get_liveview(self): | ||||||||||||||||||||||||||||
| """Get liveview link.""" | ||||||||||||||||||||||||||||
| url = ( | ||||||||||||||||||||||||||||
| f"{self.sync.urls.base_url}/api/v1/accounts/" | ||||||||||||||||||||||||||||
| f"{self.sync.blink.account_id}/networks/" | ||||||||||||||||||||||||||||
| f"{self.sync.network_id}/doorbells/{self.camera_id}/liveview" | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
| response = await api.http_post(self.sync.blink, url) | ||||||||||||||||||||||||||||
| await api.wait_for_command(self.sync.blink, response) | ||||||||||||||||||||||||||||
| server = response["server"] | ||||||||||||||||||||||||||||
| link = server.replace("immis://", "rtsps://") | ||||||||||||||||||||||||||||
| return link | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| async def async_snooze(self): | ||||||||||||||||||||||||||||
| """Set camera snooze status.""" | ||||||||||||||||||||||||||||
| data = dumps({"snooze_time": 240}) | ||||||||||||||||||||||||||||
| res = await api.request_camera_snooze( | ||||||||||||||||||||||||||||
| self.sync.blink, | ||||||||||||||||||||||||||||
| self.network_id, | ||||||||||||||||||||||||||||
| self.camera_id, | ||||||||||||||||||||||||||||
| product_type="doorbell", | ||||||||||||||||||||||||||||
| data=data, | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
|
Comment on lines
+582
to
+591
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have this method in the
Suggested change
|
||||||||||||||||||||||||||||
| if res and res.status == 200: | ||||||||||||||||||||||||||||
| return await res.json() | ||||||||||||||||||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm actually a little confused. Why is this line also not needed in the |
||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |||||
| import logging | ||||||
| import string | ||||||
| import datetime | ||||||
| from json import dumps | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this.
Suggested change
|
||||||
| import traceback | ||||||
| import asyncio | ||||||
| import aiofiles | ||||||
|
|
@@ -127,6 +128,30 @@ async def async_arm(self, value): | |||||
| return await api.request_system_arm(self.blink, self.network_id) | ||||||
| return await api.request_system_disarm(self.blink, self.network_id) | ||||||
|
|
||||||
| @property | ||||||
| async def snooze_till(self): | ||||||
| """Return snooze_till status.""" | ||||||
| res = await api.request_sync_snooze( | ||||||
| self.blink, | ||||||
| self.network_id, | ||||||
| ) | ||||||
| if res is None: | ||||||
| return None | ||||||
| res = res.get("snooze_till") | ||||||
| return res | ||||||
|
|
||||||
| async def async_snooze(self, snooze_time=240): | ||||||
| """Set sync snooze status.""" | ||||||
| data = dumps({"snooze_time": snooze_time}) | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| res = await api.request_sync_snooze( | ||||||
| self.blink, | ||||||
| self.network_id, | ||||||
| data=data, | ||||||
| ) | ||||||
| if res and res.status == 200: | ||||||
| return await res.json() | ||||||
| return None | ||||||
|
|
||||||
| async def start(self): | ||||||
| """Initialize the system.""" | ||||||
| _LOGGER.debug("Initializing the sync module") | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -205,3 +205,25 @@ async def test_wait_for_command(self, mock_resp): | |
|
|
||
| response = await api.wait_for_command(self.blink, None) | ||
| self.assertFalse(response) | ||
|
|
||
| async def test_request_camera_snooze(self, mock_resp): | ||
| """Test request_camera_snooze.""" | ||
| mock_resp.return_value = mresp.MockResponse({}, 200) | ||
| response = await api.request_camera_snooze( | ||
| self.blink, "network", "camera_id", "owl", {} | ||
| ) | ||
| self.assertEqual(response.status, 200) | ||
| response = await api.request_camera_snooze( | ||
| self.blink, "network", "camera_id", "catalina", {} | ||
| ) | ||
| self.assertEqual(response.status, 200) | ||
| response = await api.request_camera_snooze( | ||
| self.blink, "network", "camera_id", "doorbell", {} | ||
| ) | ||
| self.assertEqual(response.status, 200) | ||
|
|
||
| async def test_request_sync_snooze(self, mock_resp): | ||
| """Test sync snooze update.""" | ||
| mock_resp.return_value = mresp.MockResponse({}, 200) | ||
| response = await api.request_sync_snooze(self.blink, "network", {}) | ||
| self.assertEqual(response.status, 200) | ||
|
Comment on lines
+208
to
+229
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What exactly are you testing here?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I think you want to test the the function appropriately fail when given bad data. For example, a test that sends a product_type of "foo" to ensure that the API logs an error. Testing need to look at both expected and unexpected inputs to ensure the functions are behaving appropriately. |
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |||||
| import datetime | ||||||
| from unittest import mock | ||||||
| from unittest import IsolatedAsyncioTestCase | ||||||
| from blinkpy import api | ||||||
| from blinkpy.blinkpy import Blink | ||||||
| from blinkpy.helpers.util import BlinkURLHandler | ||||||
| from blinkpy.sync_module import BlinkSyncModule | ||||||
|
|
@@ -222,6 +223,30 @@ async def test_night_vision(self, mock_resp): | |||||
| mock_resp.return_value = mresp.MockResponse({"code": 400}, 400) | ||||||
| self.assertIsNone(await self.camera.async_set_night_vision("on")) | ||||||
|
|
||||||
| async def test_snooze_till(self, mock_resp): | ||||||
| """Test snooze_till property.""" | ||||||
| mock_resp = {"camera": [{"snooze_till": 1234567890}]} | ||||||
| with mock.patch.object( | ||||||
| api, | ||||||
| "request_get_config", | ||||||
| return_value=mock_resp, | ||||||
| ): | ||||||
| result = await self.camera.snooze_till | ||||||
| self.assertEqual(result, {"camera": [{"snooze_till": 1234567890}]}) | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The So this should be the following, assuming it was your code intent:
Suggested change
|
||||||
|
|
||||||
| async def test_async_snooze(self, mock_resp): | ||||||
| """Test async_snooze function.""" | ||||||
| mock_resp = mresp.MockResponse({}, 200) | ||||||
| with mock.patch("blinkpy.api.request_camera_snooze", return_value=mock_resp): | ||||||
| response = await self.camera.async_snooze() | ||||||
| self.assertEqual(response, {}) | ||||||
| mock_resp = mresp.MockResponse({}, 200) | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is failing and I think it's related to my confusion earlier. I believe the |
||||||
| with mock.patch("blinkpy.api.request_camera_snooze", return_value=mock_resp): | ||||||
| response = await self.camera.async_snooze() | ||||||
| self.assertEqual(response, {}) | ||||||
| response = await self.camera.async_snooze("invalid_value") | ||||||
| self.assertIsNone(response) | ||||||
|
|
||||||
| async def test_record(self, mock_resp): | ||||||
| """Test camera record function.""" | ||||||
| with mock.patch( | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||
| """Tests camera and system functions.""" | ||||||
|
|
||||||
| import datetime | ||||||
| from json import dumps | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| import logging | ||||||
| from unittest import IsolatedAsyncioTestCase | ||||||
| from unittest import mock | ||||||
|
|
@@ -653,3 +654,42 @@ async def test_download_delete(self, mock_prepdl, mock_del, mock_dl, mock_resp): | |||||
| mock_del.return_value = mock.AsyncMock() | ||||||
| mock_dl.return_value = False | ||||||
| self.assertFalse(await item.download_video_delete(self.blink, "filename.mp4")) | ||||||
|
|
||||||
| async def test_async_snooze(self, mock_resp): | ||||||
| """Test successful snooze.""" | ||||||
| with mock.patch( | ||||||
| "blinkpy.api.request_sync_snooze", new_callable=mock.AsyncMock | ||||||
| ) as mock_resp_local: | ||||||
| mock_resp_local.return_value.status = 200 | ||||||
| mock_resp_local.return_value.json.return_value = {"status": 200} | ||||||
| snooze_time = 240 | ||||||
| expected_data = dumps({"snooze_time": snooze_time}) | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| expected_response = {"status": 200} | ||||||
|
|
||||||
| self.assertEqual( | ||||||
| await self.blink.sync["test"].async_snooze(snooze_time), | ||||||
| expected_response, | ||||||
| ) | ||||||
| mock_resp_local.assert_called_once_with( | ||||||
| self.blink, | ||||||
| self.blink.sync["test"].network_id, | ||||||
| data=expected_data, | ||||||
| ) | ||||||
|
|
||||||
| mock_resp_local.return_value.status = 400 | ||||||
| mock_resp_local.return_value.json.return_value = None | ||||||
| expected_response = None | ||||||
|
|
||||||
| self.assertEqual( | ||||||
| await self.blink.sync["test"].async_snooze(snooze_time), | ||||||
| expected_response, | ||||||
| ) | ||||||
|
|
||||||
| async def test_snooze_till(self, mock_resp) -> None: | ||||||
| """Test snooze_till method.""" | ||||||
| mock_resp.return_value = {"snooze_till": "2022-01-01T00:00:00Z"} | ||||||
| self.assertEqual( | ||||||
| await self.blink.sync["test"].snooze_till, "2022-01-01T00:00:00Z" | ||||||
| ) | ||||||
| mock_resp.return_value = None | ||||||
| self.assertIsNone(await self.blink.sync["test"].snooze_till) | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tad messy with a lot of duplicated information, I'd recommend changing to the following: