Closed ircmaxell closed 2 years ago
Great spot and thanks for the PR. Looking at the issue, I wonder if it’s not better to actually invert the check and ignore everything that’s not a motion, smart detection or ring event. That’s how it use to work before support for ring events was added. I clearly borked the logic a bit.
Honestly, I think either way could work. On one hand, deny-listing known events without video would retain forward compatibility if they add a new event type, it would transparently work.
On the other hand, you'd still need to ship a release to allow for configuration of the new event type(s), so in the end, I'm inclined to agree that inverting the check is likely the best alternative.
Want to ship the change? or want me to update the PR to reflect?
Merging #32 (f2c9ee5) into dev (af8ca90) will not change coverage. The diff coverage is
0.00%
.
@@ Coverage Diff @@
## dev #32 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 3 3
Lines 328 330 +2
=====================================
- Misses 328 330 +2
Impacted Files | Coverage Δ | |
---|---|---|
unifi_protect_backup/unifi_protect_backup.py | 0.00% <0.00%> (ø) |
|
unifi_protect_backup/cli.py | 0.00% <0.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update af8ca90...f2c9ee5. Read the comment docs.
DISCONNECT events never have video associated with them, skip processing if we encounter one. This fixes #31 which prevents unnecessary fetch attempts.