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

Erizo Controller refactoring to add Clients and Channels #1041

Merged
merged 8 commits into from
Sep 28, 2017

Conversation

jcague
Copy link
Contributor

@jcague jcague commented Sep 26, 2017

Description

Some code refactoring in Erizo Controller. Now we use the next models to improve code readability:

  • Stream: this one is not new, represents a published stream.
  • Channel: a basic transport channel that checks received tokens.
  • Client: represents an end client connection. Stores its room, streams, etc. Handles events received from the client side.
  • Room: represents a room, with clients and streams.
  • It needs and includes Unit Tests

Changes in Client or Server public APIs

Not needed. All changes are in the backend.

[] It includes documentation for these changes in /doc.

@kekkokk
Copy link
Contributor

kekkokk commented Sep 26, 2017

It would be nice to catch the socket disconnection event and pass it to clients just to be able to close p2p connections.
ex. atm if i'm in a p2p room and I'm publishing to a client and this client goes away, my peerconnection remains open until it goes to a failed state. this is not a desiderable behavior since
A) I'm sending udp packets for 10 seconds to no one
B) This causes false positive if I want to log for ex. streams failed

@jcague
Copy link
Contributor Author

jcague commented Sep 27, 2017

@kekkokk thanks for the heads up! will add a fix to this PR

@jcague
Copy link
Contributor Author

jcague commented Sep 27, 2017

@kekkokk done, can you check it please?

Copy link
Member

@aalonsog aalonsog left a comment

Choose a reason for hiding this comment

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

Great refactoring!

rpcPublic[body.message.method].apply(rpcPublic, body.message.args);
try {
rpcPublic[body.message.method].apply(rpcPublic, body.message.args);
} catch(e) {
Copy link
Member

Choose a reason for hiding this comment

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

👍

if (socket.room.streams[streamId]) {
delete socket.room.streams[streamId];
}
callback(true);
Copy link
Member

Choose a reason for hiding this comment

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

I will miss these lines here... ;)

@jcague jcague merged commit 3ddccc2 into lynckia:master Sep 28, 2017
@jcague jcague deleted the update/refactor_erizo_controller branch September 28, 2017 10:09
Arri98 pushed a commit to Arri98/licode that referenced this pull request Apr 6, 2021
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