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

fix headset on/off #251

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bgbraga
Copy link

@bgbraga bgbraga commented Mar 23, 2020

Description

Fix for bug #234 - Headset used to mark if person is in meeting is not visible outside the meeting

How to test?

  1. Open two browsers with two users to test
  2. Join a meeting in a browser
  3. See your profile photo with headset inside the meeting (it is ok)
  4. See your profile photo with headset at second browser.

Implementation

I could fix it but the code is not easy to understand.

I don't know if I'm fixing it in the better point of the code. But I know here we have an event handle for join / left the meet.

      events.onParticipantStartedMeet((user, roomId) => {
        user.inMeet = true;
        onUserEnterMeeting(user, roomId);
      });
      events.onParticipantLeftMeet((user, roomId) => {
        user.inMeet = false;
        onUserLeftMeeting(user, roomId);
      });

Possible point of bug: there is no documentation or comments explain the methods. So I don't know what onDisconnect meaning. Is it detecting an user that disconnecting closing the browser? If so, should I user.inMeet = false before disconnect? I tested and could not reproduce a problem of have a user always with headset due a disconnect. But I don't know the right behavior.

      events.onDisconnect(userId => {
        onRemoveUser(userId);
      });

@bgbraga
Copy link
Author

bgbraga commented Mar 23, 2020

I don't know if it solve 100% the issue but at least in almost cases the headset is there now.
There are some random cases that the users are not receiving the event to add the headset (I'm still testing), but it could be due a websocket issue like #231 or other problem.

The architecture is using localstorage and websocket to manage that kind of data, right?
What use Firestore? https://firebase.google.com/docs/firestore
It is a database (NoSQL) with events (publish / subscribe).

Each room could be an object (as today) with users. Any change at Firestore object will propagate a event to all users that are subscribing that object.

So it solve the problem of room setup (initially using Firestore UI), websocket issue / limitation at some application servers and also it is a database. So if use lost some comunication, a refresh could load the updated objects.

@juliemar
Copy link
Contributor

Hi @bgbraga. thanks for your contribution :)

The architecture is using localstorage and websocket to manage that kind of data, right?
What use Firestore? https://firebase.google.com/docs/firestore
It is a database (NoSQL) with events (publish / subscribe).

Firebase is a good solution, but we are taking care of not couples with an exclusive CLOUD. For example, today's #matrix only supports login with Google, we are working to remove this hight couple.

The main principle of #matrix is KISS and we are taking care to maintain the project easy busy to run.

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.

3 participants