add ICE to server #44
Conversation
commit 5b3a562 Author: Michael Sobrepera <[email protected]> Date: Mon Apr 27 13:04:01 2020 -0400 more stupid typos commit aa364a2 Author: Michael Sobrepera <[email protected]> Date: Mon Apr 27 13:02:19 2020 -0400 fixed the same typo in the rest of the file commit 4257019 Author: Michael Sobrepera <[email protected]> Date: Mon Apr 27 13:01:03 2020 -0400 fixed typo in python import of service commit 165c0d3 Author: Michael Sobrepera <[email protected]> Date: Mon Apr 27 12:59:02 2020 -0400 fixed launch file wrong name in param commit 9d7189b Author: Michael Sobrepera <[email protected]> Date: Mon Apr 27 12:58:19 2020 -0400 made python node runable commit 39a51e7 Author: Michael Sobrepera <[email protected]> Date: Mon Apr 27 11:38:35 2020 -0400 changed to using a service and made a python demo source
|
I think this is ready to review. It works for me, but I have admittedly not tested all edge cases. In fact, the only test I have run is through this launchfile with the setup described in that repository using coturn as the turn server and the remote endpoint programmed here. I did not add ICE servers to the example app since I can't imagine how that would be accessed in such a way as to need any sort of NAT traversal. But that wouldn't be hard to do if desired. I tested the TURN functionality using this trick |
|
Splitting that logic out allows someone else to easily replace the extra node with whatever they want and just make their own launch file. I implemented it using a rest api to get credentials. That is probably the most general purpose approach, but it is not the only approach that is viable. Someone else might have a database on their turn server with fixed usernames and passwords or a rest api that has a different authentication method, JWT for example. I didn't want to make people have to drop into the webrtc server to make those changes. |
|
Understood. Is this ready to merge, or do you still want to work on this PR? |
|
Ready to merge |
|
Thank you for your work! |
|
Happy to help out. Is it possible to update the built version installed via package managers? |
|
Yes, I will push a new release to ROS soon(ish). |
In reference to #39 and #24. This adds the default google stun servers. Combined with something like this You can get the system to work over the web in most situations.
I'm putting this in as a draft for now. Hardcoding in the stun servers is probably not what anyone wants. I also don't have TURN in there yet. I think for TURN what I am going to do is allow users to specify a service to call to get TURN credentials via whatever means the user asks for (REST, permanent, etc.). I will allow a flag to be passed into the webrtc server nodes whether to use that or not.
The alternative would be to pass all of the ICE info from the client to the server through the existing messaging architecture. That actually seems nicer for my applications, but quite a bit harder to implement and not as flexible for other users.
Let me know your thoughts.
A few important links: