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

add empty cred #77

Merged
merged 1 commit into from
Jan 29, 2025
Merged

add empty cred #77

merged 1 commit into from
Jan 29, 2025

Conversation

JohnN193
Copy link
Contributor

@JohnN193 JohnN193 commented Jan 28, 2025

I missed that the new discovery function still needs a credential to be passed in to run. by adding an empty credential it ensures that insecure cameras on the network are found

@JohnN193 JohnN193 requested a review from randhid January 28, 2025 17:06
@@ -20,6 +20,7 @@ import (
var (
Model = viamrtsp.Family.WithModel("onvif")
errNoCamerasFound = errors.New("no cameras found, ensure cameras are working or check credentials")
emptyCred = device.Credentials{}
Copy link
Collaborator

Choose a reason for hiding this comment

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

not blocking, but add test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I currently cannot test this, since I need to be able to add camera mocks to these tests.

Once the circular dependencies can be removed(after discovercomponents is deleted) we can move the discovery service into the viamonvif folder. then I can use the camera mocks defined in those tests and add a way to use them with onvif discovery

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, thanks for bringing that up again.

@randhid randhid merged commit 5a4daa3 into main Jan 29, 2025
15 checks passed
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