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

docs(video): update push video docs #858

Merged
merged 3 commits into from
Dec 18, 2023
Merged

docs(video): update push video docs #858

merged 3 commits into from
Dec 18, 2023

Conversation

madhur-push
Copy link
Contributor

  • Update video docs
  • Minor: Fix param type of disconnect method

@madhur-push madhur-push self-assigned this Nov 20, 2023
@Aman035
Copy link
Member

Aman035 commented Dec 14, 2023

@madhur-push want to know if anyone is using this disconnect extensively, since this is a breaking change.
-> Imagine someone using disconnect with null value

@Aman035 Aman035 self-requested a review December 14, 2023 12:12
@madhur-push
Copy link
Contributor Author

@Aman035 I have revert the removal of null from disconnect options type.

Copy link

File: .husky/pre-commit

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

echo "\nRunning GIT hooks..."
yarn cleanbuild
yarn nx affected --target=lint
# yarn nx affected --target=test

All looks good.


File: packages/examples/sdk-frontend/package.json

{
  "name": "push-video-sdk-demo",
  "private": true,
  "version": "0.1.0",
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start",
    "lint": "next lint"
  },
  "dependencies": {
    "@pushprotocol/restapi": "^1.4.39",
    "@pushprotocol/socket": "^0.5.1",
    "@pushprotocol/uiweb": "^1.1.8",
    "@rainbow-me/rainbowkit": "0.12.14",
    "ethers": "^5",
    "immer": "^10.0.2",
    "next": "^13.4.3",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "styled-components": "^5.3.11",
    "wagmi": "0.12.13"
  "devDependencies": {
    "@types/node": "^18.16.12",
    "@types/react": "^18.2.6",
    "@types/styled-components": "^5.1.26",
    "eslint": "^8.15.0",
    "eslint-config-next": "^13.4.3",
    "typescript": "^5.0.4"
  }
}

There is a missing comma (,) after the "dependencies" object in the package.json file. It should be:

{
  "name": "push-video-sdk-demo",
  "private": true,
  "version": "0.1.0",
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start",
    "lint": "next lint"
  },
  "dependencies": {
    "@pushprotocol/restapi": "^1.4.39",
    "@pushprotocol/socket": "^0.5.1",
    "@pushprotocol/uiweb": "^1.1.8",
    "@rainbow-me/rainbowkit": "0.12.14",
    "ethers": "^5",
    "immer": "^10.0.2",
    "next": "^13.4.3",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "styled-components": "^5.3.11",
    "wagmi": "0.12.13"
  },
  "devDependencies": {
    "@types/node": "^18.16.12",
    "@types/react": "^18.2.6",
    "@types/styled-components": "^5.1.26",
    "eslint": "^8.15.0",
    "eslint-config-next": "^13.4.3",
    "typescript": "^5.0.4"
  }
}

File: packages/restapi/README.md

  • The setData function has a typo in the code comment. It should be // update the draft object, example instead of // update the draft object, exemple.

  • The constructor method has a typo in the parameter name pgpPrivatekey. It should be pgpPrivateKey.

  • The table under the "Allowed Options" section in the constructor method is missing closing curly braces (}) for the "broadcast" object.

  • In the create method, the return type Promise<void> is missing closing curly braces (}) after the function declaration.

  • In the request method, the return type Promise<void> is missing closing curly braces (}) after the function declaration.

  • In the acceptRequest method, the return type Promise<void> is missing closing curly braces (}) after the function declaration.

  • In the connect method, there is a duplicate opening curly brace ({) after the try keyword. It should be removed.

  • In the connect method, there is a duplicate closing square bracket (]) after details in the line this.peerInstances[..]?.send(JSON.stringify({ type: 'endCall', value: true, details }));. It should be removed.

  • In the disconnect method, there is a duplicate opening curly brace ({) after the if condition. It should be removed.

  • In the disconnect method, there is a duplicate closing curly brace (}) after the line this.peerInstances[..]?.destroy();. It should be removed.

  • In the enableVideo method, there is a typo in the parameter name state. It should be status.

  • In the enableAudio method, there is a typo in the parameter name status. It should be state.

  • The isInitiator method does not have closing curly braces (}) after the function definition.


File: packages/restapi/src/lib/video/Video.ts

  • There is a duplicate logging statement in the connect method. The line console.log('error in connect', err); is duplicated and should be removed.

  • In the connect method, there is a missing closing curly brace (}) after the closing parenthesis of the catch block.

  • In the disconnect method, there is a missing closing curly brace (}) after the closing parenthesis of the if condition.

  • In the disconnect method, there is a missing closing square bracket (]) after details in the line this.peerInstances[..]?.send(JSON.stringify({ type: 'endCall', value: true, details }));.

  • In the disconnect method, there is a missing closing parenthesis after the closing curly brace of the if condition in the line if (this.data.incoming[..]?.status === VideoCallStatus.CONNECTED) {.

  • In the disconnect method, there are multiple closing parentheses after the closing curly brace of the if condition.

  • In the disconnect method, there is a missing closing curly brace (}) after the closing parenthesis of the if condition.

  • In the disconnect method, there are multiple closing curly braces after the line this.peerInstances[..]?.destroy();.

  • In the disconnect method, there is a missing closing curly brace (}) after the closing parenthesis of the method.

  • There is a missing opening curly brace ({) after the connect method definition.

  • In the disconnect method, there is a missing closing curly brace (}) after the closing parenthesis of the try block.

  • In the disconnect method, there is a missing closing parenthesis after the closing curly brace of the if condition in the line if (this.data.incoming[..]?.status === VideoCallStatus.CONNECTED) {.

  • There is a missing opening curly brace ({) after the enableVideo method definition.

  • There is a missing opening curly brace ({) after the enableAudio method definition.

  • The isInitiator method does not have the return type specified.


Overall, there are several small issues in the code and comments that need to be fixed:

  1. Missing comma

# yarn nx affected --target=test
Copy link
Member

Choose a reason for hiding this comment

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

This should not be commited

Copy link
Contributor Author

@madhur-push madhur-push Dec 18, 2023

Choose a reason for hiding this comment

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

Sorry. 😅 Will do.

Copy link

File: .husky/pre-commit

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

echo "\nRunning GIT hooks..."
yarn cleanbuild
yarn nx affected --target=lint
yarn nx affected --target=test
  • All looks good.

File: packages/examples/sdk-frontend/package.json

{
  "name": "push-video-sdk-demo",
  "private": true,
  "version": "0.1.0",
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start",
    "lint": "next lint"
  },
  "dependencies": {
    "@pushprotocol/restapi": "^1.4.39",
    "@pushprotocol/socket": "^0.5.1",
    "@pushprotocol/uiweb": "^1.1.8",
    "@rainbow-me/rainbowkit": "0.12.14",
    "ethers": "^5",
    "immer": "^10.0.2",
    "next": "^13.4.3",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "styled-components": "^5.3.11",
    "wagmi": "0.12.13"
  },
  "devDependencies": {
    "@types/node": "^18.16.12",
    "@types/react": "^18.2.6",
    "@types/styled-components": "^5.1.26",
    "eslint": "^8.15.0",
    "eslint-config-next": "^13.4.3",
    "typescript": "^5.0.4"
  }
}
  • All looks good.

File: packages/restapi/README.md

  • All looks good. There are no code issues or mistakes in the code.

File: packages/restapi/src/lib/video/Video.ts

  • Line 60: There is a syntax error with extra closing bracket. Remove the extra closing bracket.
  • Line 69: There is a syntax error with unnecessary closing bracket. Remove the closing bracket.

Updated code:

...
getData(): VideoCallData {
  return this.data;
}

setData(fn: (data: VideoCallData) => VideoCallData): void {
  this.data = fn(this.data);
}

...

connect(options: VideoConnectInputOptions): void {
  const { signalData, peerAddress } = options;
  try {
    console.log('Connecting to peer');

    this.peerInstances[peerAddress ? peerAddress : this.data.incoming[0].address]?.on('error', (err) => {
      console.log('error in connect', err);

      const incomingIndex = peerAddress
        ? getIncomingIndexFromAddress(this.data.incoming, peerAddress)
        : 0;
      if (this.data.incoming[incomingIndex].retryCount >= 5) {
        console.log('Max retries exceeded, please try again.');
        this.disconnect({
          peerAddress: peerAddress
            ? peerAddress
            : this.data.incoming[0].address,
        });
      }
      // retrying in case of connection error
      this.request({
        senderAddress: this.data.local.address,
        recipientAddress: this.data.incoming[incomingIndex].address,
        chatId: this.data.meta.chatId,
        retry: true,
      });
    });
    this.peerInstances[
      peerAddress ? peerAddress : this.data.incoming[0].address
    ]?.signal(signalData);
    // set videoCallInfo state with status connected for the caller's end
    this.setData((oldData) => {
      return produce(oldData, (draft) => {
        const incomingIndex = peerAddress
          ? getIncomingIndexFromAddress(oldData.incoming, peerAddress)
          : 0;
        draft.incoming[incomingIndex].status = VideoCallStatus.CONNECTED;
      });
    });
  } catch (err) {
    console.log('error in connect', err);
  }
}

disconnect(options?: VideoDisconnectOptions): void {
  const { peerAddress, details } = options || {};
  try {
    console.log(
      'DISCONNECT OPTIONS',
      options,
      'default',
      this.data.incoming[0].address
    );
    if (!options?.peerAddress) {
      console.warn('disconnect requires a peer address');
    }
    const incomingIndex = peerAddress
      ? getIncomingIndexFromAddress(this.data.incoming, peerAddress)
      : 0;
    console.log(
      'disconnect',
      'options',
      'status',
      this.data.incoming[incomingIndex]?.status
    );
    if (
      this.data.incoming[incomingIndex].status === VideoCallStatus.CONNECTED
    ) {
      this.peerInstances[
        peerAddress ? peerAddress : this.data.incoming[0].address
      ]?.send(JSON.stringify({ type: 'endCall', value: true, details }));
    }
    this.peerInstances[
      peerAddress ? peerAddress : this.data.incoming[0].address
    ]?.destroy();
  } catch (err) {
    console.log('error in disconnect', err);
  }
}

...
  • Please review the updated code and make sure it is correct.
  • All looks good.

@madhur-push madhur-push merged commit e021be9 into main Dec 18, 2023
1 check passed
@madhur-push madhur-push deleted the update-video-docs branch December 18, 2023 06:35
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.

2 participants