Closed IGx89 closed 5 months ago
An alternative implementation I think might work as well, though wouldn't be as automatic, is adding settings for every supported event code, with VideoMotion being the only one enabled by default. Then if someone wanted to use smart motion events, they'd uncheck VideoMotion and check SmartMotionHuman/SmartMotionVehicle. Though the fact that they're already manually enabling them in the Dahua admin UI makes that somewhat redundant, so my first proposed implementation would probably be preferable.
I agree that it would be ideal if this plugin supported other alerts than VideoMotion
however, I have an older NVR with no plans to get a newer Dahua. If I'm supporting this plugin, it's going to be difficult for me to support these newer smart motion alerts. It requires a lot of back and forth zip files and debug logs to somewhat non-technical (sometimes very non-technical) people.
So I guess, if you want this feature merged in upstream, I'm open to it however you would be volunteering to support it longterm.
As far as implementation details go, I massively prefer your latter approach with settings (checkboxes) for each type of alert that the user wants to receive. This way we can easily add alerts that more universally supported too, such as:
You should know that my goal for this plugin has always been to support as many cameras/NVRs as possible without alienating anyone. Personally, I would like to keep it simple to just VideoMotion since that's really all I can support and it casts the widest net possible. All that being said, I don't want to stand in front of progress, because that's how plugins die.
By the way, thank you for the thoughtful tickets with the in-depth analysis in each of them. Sorry I'm getting back to you so late. I will attempt to be more prompt in the future.
Definitely understand! I just changed codes
to [SmartMotionHuman%2CSmartMotionVehicle]
and that's been working great for me, so a checkbox route would be just fine for my scenario. Even simpler maybe, a text field, that defaults to VideoMotion if blank and allows comma separated lists of events to be added (I'd put SmartMotionHuman,SmartMotionVehicle
myself)?
No problem! Sorry for creating a bunch of issues at a time :). Just glad to hear someone's still giving this some love and support!
You know, I just realized that this doesn't require payload parsing changes, it's just a url param addition. I didn't realize this when I initially read your issue.
I'm down with this change if you want to add 3 checkbox options with VideoMotion
enabled by default. Then users can pick and choose the type of alerts. Also it creates an established pattern on adding new alert types (as long as they don't require payload parsing changes). The reason I prefer static/ridged checkbox options rather than textbox is because the majority of the users for this plugin are non technical and I don't want users to have to understand Dahua's api.
If you prefer checkboxes please consider also the following events:
I changed codes
to [CrossRegionDetection]
, as I need only a specific area (IVS, Intrusion, Action: Appears) of human detection.
@tunip I'm happy if someone with the hardware adds in the changes so they can test them. I have an old Lorex DVR with very basic features.
Smart motion event alerts works with LOREX N841A8 NVR by changing codes from [VideoMotion] to [CrossRegionDetection].
Smart motion event alerts works with LOREX N841A8 NVR by changing codes from [VideoMotion] to [CrossRegionDetection].
Same here, same NVR model
FYI you can use Codes=[All]
;)
Locking this conversation as there has been no productive comments added.
Once again like I mentioned in comment #15, I will not be adding in this feature. Someone with the appropriate hardware can add this feature in and create a pull request. Alternatively I’m happy to implement this feature if someone sends me some hardware with smart motion detection.
For any bugs with the plugin please open a new isssue, thanks!
Would you be open to me submitting a PR to add support for smart motion events? My initial idea is that if a VideoMotion event came in with
"SmartMotionEnable" : true
it would be ignored, and SmartMotionHuman/SmartMotionVehicle events would trigger motion alerts instead.Here's what the admin UI looks like for it:
And here's what the events look like:
I get false positive motions alerts when it's really windy or my outdoor lights turn off, even after reducing sensitivity a lot, so this would be helpful for me. Long term I may look into the CrossLineDetection/CrossRegionDetection events to further narrow the focus, but smart motion support would be a big first step forward.
Let me know your thoughts!