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-9754] Remove webcam discovery logic #4720

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

randhid
Copy link
Member

@randhid randhid commented Jan 16, 2025

In an effort to make RSDK-9630 slightly smaller - I broke out the webcam Discovery removal into its own work since it is isolated.

I tested the app on my Mac - nothing broke. We get no response in the video_path field and it connects to the first camera it finds.

I will wait until we have a discovery card to merge this in.

The replacement find-cameras module

TODO:

  • Test on a pi.

@randhid randhid requested review from seanavery and hexbabe January 16, 2025 15:31
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jan 16, 2025
Copy link
Member

@hexbabe hexbabe left a comment

Choose a reason for hiding this comment

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

Is webcam_e2e_test ran anywhere in CI? Is this the one we saw didn't run a while back with Julie?

Also thanks for the @ I will note the merge conflicts that inevitably arise in #4604

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jan 29, 2025
@randhid
Copy link
Member Author

randhid commented Jan 29, 2025

Is webcam_e2e_test ran anywhere in CI? Is this the one we saw didn't run a while back with Julie?

Also thanks for the @ I will note the merge conflicts that inevitably arise in #4604

Nope! I Want me to remove the e2e test as well?

@hexbabe
Copy link
Member

hexbabe commented Jan 29, 2025

wtf happened here I tried to merge THESE changes into my branch but I think it went both ways

@randhid
Copy link
Member Author

randhid commented Jan 29, 2025

wtf happened here I tried to merge THESE changes into my branch but I think it went both ways

It is perfectly okay I can deal with the conflicts - still waiting for the discovery card to be in - which should be tomorrow!

@hexbabe
Copy link
Member

hexbabe commented Jan 29, 2025

It's ok I can revert it to the previous commit. (sigh time to go back to rand-rdk)

@hexbabe hexbabe force-pushed the remove-webcam-logic branch from 2d5cf42 to a00c1bc Compare January 29, 2025 21:42
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jan 29, 2025
@hexbabe
Copy link
Member

hexbabe commented Jan 29, 2025

OK I did what I set out to do, which is to merge these changes into #4604 and test & update aforementioned PR to be ready for review and merge after this one is merged (to avoid conflicts).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants