Why do we pass a datachannel to `RtcPeerConnection::new` and `create_data_channel`?

[D1plo1d]

I see it in both the examples and I feel like I'm missing something. I see the second datachannel gets initialized with tx_ready but what would happen if I initialized a data channel once with tx_ready in RtcPeerConnection::new? Would it be functionally equivalent or is there an datachannel-rs internal reason we need to always initialize a second data channel via create_data_channel?

1st DataChannel: https://github.com/lerouxrgd/datachannel-rs/blob/019d9357d65282d2bccb27d315310d4f2d6ecc84/tests/local.rs#L120 2nd DataChannel: https://github.com/lerouxrgd/datachannel-rs/blob/019d9357d65282d2bccb27d315310d4f2d6ecc84/tests/local.rs#L140

[D1plo1d]

I'm specifically referring to pipe1 and pipe in the test.

[lerouxrgd]

So actually in both examples DataPipe is not a datachannel per se. It is a struct that implements the trait DataChannel which just defines callbacks. The real datachannels are instances of RtcDataChannel and there are only 2 ways to obtain them:

1. Explicitely call create_data_channel on an instance of RtcPeerConnection (initiator side).
2. Passively through PeerConnection::on_data_channel (receiver side, it means your peer called create_data_channel on his side to initiate a datachannel.

Maybe I could rename DataChannel trait to DataChannelCallbacks if it is not clear enough (I didn't do it because I found it a bit long).

So in the end the idea is to give to RtcPeerConnection::new an instance of a struct that defines all the usual callbacks you have in the corresponding javascript API. It makes it simpler (regarding types) than letting the user define those callbacks on the instantiated RtcDataChannel, like it's the case in javascript (and in the underlying C++ library). The alternative would be to box all the callbacks, which I wanted to avoid as it has a small overhead and callbacks aren't usually defined more than once.

Then, the reason to initialize a DataPipe with tx_ready in the examples is only synchronization, in the sense that dc.send(format!("Hello from {}", id1).as_bytes()).ok(); must be called only once the datachannel is ready, and when that's the case DataPipe::on_open is called (and signals the it through tx_ready).

So this is essentially changing callback (hell?) synchro to rx/tx (a bit less hellish?) synchro.

[lerouxrgd]

In the test local.rs, you have 2 peers, each in a dedicated thread.

The thread1 calls create_data_channel and waits for it to be opened (through tx_ready) before sending its actual message (that's in its loop { ... }).

The thread2 simply receive the datachannel through PeerConnection::on_data_channel (as he is not the initiator), so there is no need to have a tx_ready synchronization for him. Then, insides the on_data_channel callback it will call dc.send(...) to send its answer message. Note that this callback is never used in by thread1 (as he is the initator).

This is really just a quick test to showcase basic exchange between 2 peers.

[D1plo1d]

Ok, that makes a lot more sense now. DataChannelCallback is verbose but feels less confusing - RtcDataChannel and DataChannel are just too semantically the same IMO.

If RtcPeerConnection and DataChannel are one-to-one we could avoid creating the callbacks twice - function names very much stolen from/inspired by std::net::Tcp* but something like this might work:

PeerConnection::accept(config, pc, dc) PeerConnection::connect(config, pc, label, dc)

But that's only if they are one-to-one.

https://doc.rust-lang.org/std/net/struct.TcpStream.html

[D1plo1d]

Actually, even if PeerConnection and DataChannel are not one-to-one it might be ergonomic to have those methods. Eg. creating a connection that establishes 2 data channels to a remote peer.

let pc = PeerConnection::connect(config, pc, "channel1", dc1);
pc.create_data_channel("channel2", dc2);

Edit: channel names

[lerouxrgd]

I renamed PeerConnection and DataChannel traits to PeerConnectionHandler and DataChannelHandler respectively. I also simplify the RtcPeerConnection::new API a bit, now it takes just a conf and a struct implementing PeerConnectionHandler, I hope it's less confusing.

I updated the local.rs test by now defining two distinct structs Ping and Pong instead of DataPipe which will hopefully help understand the data flow.

All those changes are on the branch renaming, let me know what you think.

As for accept/connect methods, it can be misleading as they are not part of the original API so I think they shouldn't be implemented here.

[D1plo1d]

Looks good!

[lerouxrgd]

This has been released in 0.3.0