search

yuppity / unifi-video-api

Python API for UniFi Video
MIT License
59 stars 11 forks source link

handle width parameter same as camera.snapshot #29

Closed donaldrs closed 3 years ago

donaldrs commented 3 years ago

camera.snapshot has width=0 => whatever the server returns (max size). This change also implements that behavior for recording.snapshot

yuppity commented 3 years ago

Looks good to me, thank you!

This change may affect people who do not specify a width parameter as this changes the default from 600 to "the maximum the server can provide". I guess an option would be to make the default 600 again, but that kind of keeps the difference between the same methods on the 2 classes (Camera / Recording) which is a bit odd.

I had the same concern but ultimately consider the second point more convincing. Leave it as is.

I'll merge this soon.

donaldrs commented 3 years ago

Small other suggestion. Could you add “self.uuid = data[‘uuid’]” in load_data() for camera? I now have to access the _data to get the uuid and it would be helpful to just expose that value.

Greetings,

Richard

From: yuppity notifications@github.com Reply to: yuppity/unifi-video-api reply@reply.github.com Date: Tuesday, 9 February 2021 at 19:22 To: yuppity/unifi-video-api unifi-video-api@noreply.github.com Cc: donaldrs reg@hierzo.com, Author author@noreply.github.com Subject: Re: [yuppity/unifi-video-api] handle width parameter same as camera.snapshot (#29)

Looks good to me, thank you!

This change may affect people who do not specify a width parameter as this changes the default from 600 to "the maximum the server can provide". I guess an option would be to make the default 600 again, but that kind of keeps the difference between the same methods on the 2 classes (Camera / Recording) which is a bit odd.

I had the same concern but ultimately consider the second point more convincing. Leave it as is.

I'll merge this soon.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

donaldrs commented 3 years ago

Hi,

I know the answer is probably “no”, but I want to try anyway…. As Unifi Video is end-of-life, do you have any plans to support Unifi Protect on the Cloudkey (Gen2 plus specifically)?

There are some other libraries that seem to support that but not as nice as yours and they seem to lack functionality (like downloading video for a given event).

Kind regards,

Richard

From: yuppity @.> Reply to: yuppity/unifi-video-api @.> Date: Tuesday, 9 February 2021 at 19:22 To: yuppity/unifi-video-api @.> Cc: donaldrs @.>, Author @.***> Subject: Re: [yuppity/unifi-video-api] handle width parameter same as camera.snapshot (#29)

Looks good to me, thank you!

This change may affect people who do not specify a width parameter as this changes the default from 600 to "the maximum the server can provide". I guess an option would be to make the default 600 again, but that kind of keeps the difference between the same methods on the 2 classes (Camera / Recording) which is a bit odd.

I had the same concern but ultimately consider the second point more convincing. Leave it as is.

I'll merge this soon.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

yuppity commented 3 years ago

If I become a Protect user then I'll very likely look into writing a similar package for UniFi Protect. At the moment though, I haven't given any thought to switching so even if I do eventually end up doing so, it won't be any time soon.