Bump reolink-aio to 0.9.1 #118655
Bump reolink-aio to 0.9.1 #118655
Conversation
home-assistant
bot
added
cla-signed
dependency
integration: reolink
small-pr
|
I would like to get this into the 2024.6.0 (beta) release because otherwise I will be blocked from doing version bumps of reolink-aio in the 2024.6.x release cycle. (Due to adjustments needed in HA code for reolink > 0.9 releases) |
| self._update_cmd_list_count[cmd_key].setdefault(self._channel, 0) | ||
| self._update_cmd_list_count[cmd_key][self._channel] += 1 | ||
| if self._channel not in self._host.update_cmd_list[cmd_key]: | ||
| self._host.update_cmd_list[cmd_key].append(self._channel) |
There was a problem hiding this comment.
If you are going to do ref counting and channel is hashable it might make this a bit simpler to make update_cmd_list a defaultdict of dict and do
self._host.update_cmd[cmd_key][self._channel] += 1 and self._host.update_cmd[cmd_key][self._channel] -= 1 (and than delete the entry when it reaches 0)
There was a problem hiding this comment.
Thank you for the suggestion, I have implemented that now (I think).
There was a problem hiding this comment.
I now have two lists (update_cmd_list and update_cmd_list_count), I do that because I want to keep update_cmd_list as simple as possible since it is looked up many times for each update that happens every 1 minute.
There was a problem hiding this comment.
In the upstream reolink-aio library:
https://github.com/starkillerOG/reolink_aio/blob/714e87f05669c1b0889ba5c0718297ce6a06a10c/reolink_aio/api.py#L1533-L1537
For each command a check is made if it needs to be included for that channel:
https://github.com/starkillerOG/reolink_aio/blob/714e87f05669c1b0889ba5c0718297ce6a06a10c/reolink_aio/api.py#L1558-L1598
There was a problem hiding this comment.
In the future you should be able to optimize https://github.com/starkillerOG/reolink_aio/blob/714e87f05669c1b0889ba5c0718297ce6a06a10c/reolink_aio/api.py#L1546 a bit more by having all the commands in a set and than using intersection on the dict to find the supported commands
There was a problem hiding this comment.
That is very instresting, will keep this in mind.
You are so good in runtime optimazations (I read the members discord and I am amazed by your work).
|
@bdraco I tested the code and it is working perfectly. |
| self._host.update_cmd_list_count[cmd_key].setdefault("host", 0) | ||
| self._host.update_cmd_list_count[cmd_key]["host"] += 1 | ||
| self._host.update_cmd_list.setdefault(cmd_key, []) |
There was a problem hiding this comment.
Do you need a seperate list and count. Can you use the same container where the value is the reference count, and the key is the cmd_key?
There was a problem hiding this comment.
well I also need the channel in there.
The upstream library needs to now which commands it needs to send to which channels.
(the library talks to a NVR which has multiple camera's connected using a build-in POE switch, so each channel is a seperate camera)
Currently the structure for the upstream library is:
{cmd_key: [channels]}
So for instance if only camera's on channel 2 and 5 support Zoom and those entities are enabled:
{"GetZoomFocus": [2, 5]}
For commands that talk to the NVR itself and not to a channel, for instance the hard drive status of the NVR, it looks like:
{"GetHddInfo": []}
There was a problem hiding this comment.
The update_cmd_list_count looks like:
{"GetZoomFocus": {"Host": 6, 2: 3, 5: 3}}
and
{"GetHddInfo": {"Host": 1}}
But that added complexity of knowing how many entities are using the command is not needed in the upstream library (reolink-aio)
There was a problem hiding this comment.
Maybe something like this
Co-authored-by: J. Nick Koston <nick@koston.org>
|
@bdraco thank you very much for your code suggestion. |
Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: J. Nick Koston <nick@koston.org>
Breaking change
Proposed change
Breaking changes:
Additions:
Bug fixes:
Optimizations:
Full Changelog: starkillerOG/reolink_aio@0.8.11...0.9.1
Due to a breaking change in reolink-aio regarding the structure of the cmd_list and the doorbell LED, some small adjustments are needed in the bump PR. The rest of the changes will be done in split out PRs to make Battery cameras fully compatible (and not drain their batteries).
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: