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

Auto switch stream when device is unplugged #186

Merged
merged 5 commits into from
Jan 12, 2024

Conversation

thyal
Copy link
Member

@thyal thyal commented Jan 9, 2024

Description

Sorry for the big PR on this small bugfix, but we didn't really handle switching of streams at all, so I had to add a bunch of logic. Also did some refactoring.

Summary:

  • Handle unplugging of external devices (Auto-switch stream to next device)
  • Handle busy devices, store a list of them in the state, and don't try to switch to a busy device
  • Refactor local media slice. Aligning it with the rest of the slices, action creator naming, reducer structure etc
  • Add a new slice for local screenshare. Moving the screenshare stuff out of local media and in to a dedicated slice.

Related Issue:

Testing

  1. Test with storybook.
  2. Verify that all local media functionality works as before
  3. Change the camera device to an external one
  4. Unplug it
  5. Verify that the stream switches to the next device in the list
  6. Plug the camera back
  7. Verify that the stream does not switch automatically, but that the camera becomes available for manual switching

Screenshots/GIFs (if applicable)

Checklist

  • My code follows the project's coding standards.
  • I have written unit tests (if applicable).
  • I have updated the documentation (if applicable).
  • By submitting this pull request, I confirm that my contribution is made
    under the terms of the MIT license.

Dependency Updates

Reviewers

@havardholvik
@kevinwhereby
@nandito
@thyal

Additional Information

@thyal thyal force-pushed the thomas/PAN-501-auto-switch-device branch 7 times, most recently from 263d27c to c1188dd Compare January 10, 2024 13:15
@thyal thyal marked this pull request as ready for review January 10, 2024 13:22
@thyal thyal requested a review from a team January 10, 2024 13:22
Copy link
Collaborator

@havardholvik havardholvik left a comment

Choose a reason for hiding this comment

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

There is a conflict you might want to resolve before this is reviewed @thyal 👍

@thyal thyal force-pushed the thomas/PAN-501-auto-switch-device branch from c1188dd to 0403e21 Compare January 11, 2024 08:04
@thyal
Copy link
Member Author

thyal commented Jan 11, 2024

There is a conflict you might want to resolve before this is reviewed @thyal 👍

Done 👍

Copy link
Collaborator

@havardholvik havardholvik left a comment

Choose a reason for hiding this comment

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

Nice work!

Ive just reviewed the code now. Might be one thing for you to look at while I do manual testing before approval. Also remember to bump to the next beta version 👍

src/lib/core/redux/slices/localMedia.ts Show resolved Hide resolved
Copy link
Collaborator

@havardholvik havardholvik left a comment

Choose a reason for hiding this comment

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

Tested, works as expected ✅

@thyal thyal mentioned this pull request Jan 12, 2024
4 tasks
@thyal thyal force-pushed the thomas/PAN-501-auto-switch-device branch from 0403e21 to b7db2f1 Compare January 12, 2024 13:45
@thyal
Copy link
Member Author

thyal commented Jan 12, 2024

Nice work!

Ive just reviewed the code now. Might be one thing for you to look at while I do manual testing before approval. Also remember to bump to the next beta version 👍

Bumped the version in #188

@thyal thyal force-pushed the thomas/PAN-501-auto-switch-device branch from b7db2f1 to fe342aa Compare January 12, 2024 14:18
@thyal thyal force-pushed the thomas/PAN-501-auto-switch-device branch from fe342aa to e035012 Compare January 12, 2024 14:31
@thyal thyal merged commit 940601e into main Jan 12, 2024
2 checks passed
@thyal thyal deleted the thomas/PAN-501-auto-switch-device branch January 12, 2024 14:41
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