Skip to content
This repository has been archived by the owner on Jun 19, 2023. It is now read-only.

Implement Connection #14

Merged
merged 2 commits into from
Aug 12, 2022
Merged

Implement Connection #14

merged 2 commits into from
Aug 12, 2022

Conversation

ckousik
Copy link
Collaborator

@ckousik ckousik commented Aug 11, 2022

Implement connection interface for RTCPeerConnection.

@@ -9,28 +9,35 @@
"scripts": {
"build": "aegir build",
"clean": "rm -rfv node_modules dist *.lock *-lock.json ",
"test": "aegir test",
"test": "aegir test --target browser",
Copy link
Contributor

Choose a reason for hiding this comment

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

The example suggested targets for test, test:browser, and test:node.
But I suppose this project only targets a browser, right, so maybe we should only do that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is because we don't intend to support node at the moment.

timeline: { open: 0 },
status: 'CLOSED',
direction: init.direction,
status: 'OPEN',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it always open immediately?

this.peerConnection.createDataChannel(genUuid());
throw new Error('not implemented');
private wrapMsStream(rawStream: WebRTCStream, stream: Duplex<Uint8ArrayList, Uint8ArrayList | Uint8Array, Promise<void>>): ic.Stream {
return {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this wrapping just to be able to set the ID? Because if so... we could add that as an optional thing to StreamInit

throw new Error('not implemented');
let protocol = stream.stat.protocol!;
let direction = stream.stat.direction;
if (this.countStream(protocol, direction) === this.findStreamLimit(protocol, direction)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Although I understand it would be an incredibly bizarre implementation that did this, it seems to me to be at least possible that the data being fetched by findStreamLimit could theoretically change at runtime. So I'd be inclined to test with something like >=

let metrics = this.components.getMetrics();
let openError: Error | undefined;

log.trace(`opening new stream with protocols: ${protocols}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that we're not testing stream limits here. So those limits only apply to incoming streams, not the sum of the two?

log('new outbound connection %s', rawConn, genUuid());
throw new Error('not implemented');
const rawConn = await this._connect(ma, options);
log(`dialing address - ${ma}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Given this happens after _connect, shouldn't it be "dialed" address?


export const echoHandler: StreamHandler = ({ stream }) => pipe(stream.source, stream.sink);

export async function createConnectedRTCPeerConnectionPair(): Promise<RTCPeerConnection[]> {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@John-LittleBearLabs John-LittleBearLabs merged commit d9cbc83 into develop Aug 12, 2022
@ddimaria ddimaria linked an issue Aug 12, 2022 that may be closed by this pull request
17 tasks
@ddimaria ddimaria changed the title [CON-431][CON-430][CON-438] Implement Connection Implement Connection Aug 12, 2022
@github-actions
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement webrtc in js-libp2p
2 participants