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

Generic udev Configuration #87

Merged

Conversation

kate-goldenring
Copy link
Contributor

@kate-goldenring kate-goldenring commented Oct 29, 2020

What this PR does / why we need it:
We currently only have documentation and a sample Configuration for how to use the udev discovery handler to discover usb cameras. This adds a generic sample udev Configuration and documentation on how to use it to find any device discoverable by udev. It also modifies the name of the ONVIF configuration from onvif-video to onvif. This pr creates a standard of providing a generic configuration for each protocol that can be applied to a cluster with helm via --set <protocol>-enabled=true. Additionally, it sets the standard that a broker image must be specified on installation, else Akri will advertise the resources but not deploy brokers. Finally, it creates a standard of creating a <protocol>-configuration.md document for each generic configuration provided.

Special notes for your reviewer:
We've received questions on how to use udev to discover audio devices (ref #33). Modifying the udevVideo configuration seems illogical and starting from an empty yaml is a big barrier to entry.
closes #68
ref #69

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • added code adheres to standard Rust formatting (cargo fmt)
  • code builds properly (cargo build)
  • code is free of common mistakes (cargo clippy)
  • all Akri tests succeed (cargo test)
  • inline documentation builds (cargo doc)
  • version has been updated appropriately (./version.sh)

@kate-goldenring kate-goldenring added the enhancement New feature or request label Oct 29, 2020
@kate-goldenring
Copy link
Contributor Author

I think it would make sense at this point to remove the udevVideo configuration. Instead in the udev video sample documentation, we can walk through the steps of setting the environment variables (udev values would need to modified to be able to configure this), broker image, and Configuration name. Thoughts?

deployment/helm/templates/udev.yaml Outdated Show resolved Hide resolved
docs/udev-configuration.md Outdated Show resolved Hide resolved
docs/udev-configuration.md Outdated Show resolved Hide resolved
@kate-goldenring
Copy link
Contributor Author

I think it would make sense at this point to remove the udevVideo configuration. Instead in the udev video sample documentation, we can walk through the steps of setting the environment variables (udev values would need to modified to be able to configure this), broker image, and Configuration name. Thoughts?

I wonder if we should phase out the udevVideo configuration, since it is part of our e2e demo. Some people may be currently running it. I could change the e2e to use just udev and then maybe we remove udevVideo entirely for our next release. That bodes the question: do be make onvifVideo onvif?

docs/udev-configuration.md Outdated Show resolved Hide resolved
docs/onvif-configuration.md Outdated Show resolved Hide resolved
docs/onvif-configuration.md Outdated Show resolved Hide resolved
@kate-goldenring kate-goldenring merged commit 01874e2 into project-akri:main Nov 3, 2020
@kate-goldenring kate-goldenring deleted the udev-generic-configuration branch November 3, 2020 17:52
bfjelds pushed a commit that referenced this pull request Nov 11, 2020
* add generic udev configuration and documentation

* wrap text at 120 chars

* make service deployment and env vars configurable and more docs

* make generic onvif configuration and associated docs

* specify use of development Helm chart

* increase version

* explain instance naming

* update version.sh to check the new Configuration templates

* explain broker image and grammatical fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add documentation on how to use udev outside of udevvideo
3 participants