Skip to content
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

[RSDK-9931] Let the config have an optional maximum bounding box #31

Merged
merged 12 commits into from
Feb 13, 2025

Conversation

penguinland
Copy link
Contributor

@penguinland penguinland commented Feb 12, 2025

The intention here is that if the camera provides a corrupted frame and everything suddenly goes gray for a moment, that shouldn't count as motion. This also means that if someone bumps the camera and the entire scene shifts to the left, that also shouldn't count as motion.

Tried on an Orin Nano: seems to work! edit: if I don't mention it in the config, the behavior seems unchanged, and if I do mention it, large bounding boxes do not appear, while small ones still do. I was unable to get a corrupted image, so can't ensure that those are no longer considered noise.

I've also made some other cleanup changes:

  • Remove trailing whitespace. Ideally we'd never have any to begin with
  • Don't raise raw Exceptions, because the only way to catch them is to catch all exceptions. Instead, raise something specific (in these cases, I went with ValueError, but could be convinced of other types, too).

My hope is that reviewing commit-by-commit is easy, and makes it clear why each change occurred.

@penguinland penguinland requested a review from bhaney February 12, 2025 18:35
@penguinland
Copy link
Contributor Author

...this time with not just the old tests passing, but new tests that also pass!

@penguinland penguinland requested review from kharijarrett and removed request for bhaney February 12, 2025 20:21
@penguinland penguinland force-pushed the maximum_motion_amount branch from 693cf20 to ad05127 Compare February 12, 2025 22:35
Copy link
Collaborator

@kharijarrett kharijarrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@penguinland penguinland merged commit 17a28a5 into viam-modules:main Feb 13, 2025
2 checks passed
@penguinland penguinland deleted the maximum_motion_amount branch February 13, 2025 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants