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

add nerf viewer #40

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions viewers/nerf_viewer/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Nerf Viewer Quick Start
Copy link
Member

Choose a reason for hiding this comment

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

Nerf -> NeRF

Copy link
Author

@WYK96 WYK96 Jun 6, 2023

Choose a reason for hiding this comment

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

Glitch fixed: Nerf Viewer -> XRNeRF Viewer


## 1 Client

### 1.1 Install Dependencies

#### 1.1.1 Install NVM
Copy link
Contributor

Choose a reason for hiding this comment

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

NVM is not a requirement but a recommendation.

Will it be better for users to choose their own method to install node.js?

And NVM could be an optional choice like:

Install node.js 18.15.

It's suggested to use `NVM` to install and manage `node.js` installations.

Copy link
Author

Choose a reason for hiding this comment

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

The original version of README.md is for debug purpose. In the lastest doc, the installation guide has been replaced to:

The package.json contains dependencies to build the viewer, which can be installed using Node Package Manager(npm). It's suggested to manage Node.js installations with Node Version Manager(nvm). The installation instructions of nvm can be found here.


Please follow the instructions in:

https://juejin.cn/post/7120267871950733320/
Copy link
Contributor

Choose a reason for hiding this comment

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

This a blog in Chinese not suitable for non-Chinese users and only for windows platform. For i18n, post official installation instructions.

Copy link
Author

Choose a reason for hiding this comment

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

The original version of README.md is for debug purpose. In the lastest doc, the link for nvm installation guidelines has been replaced from https://juejin.cn/post/7120267871950733320/ to https://heynode.com/tutorial/install-nodejs-locally-nvm/.

Copy link
Member

Choose a reason for hiding this comment

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

Do we have any installation instruction in English? Say https://github.com/nvm-sh/nvm#installing-and-updating.

Copy link
Author

Choose a reason for hiding this comment

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

The original version of README.md is for debug purpose. In the lastest doc, the installation instructions has been updated so that it is suitable for non-chinese users.


#### 1.1.2 Install Node

Once you have nvm correctly configured, install node 18.15.0 using the commands below:

```shell
# install node 18.15.0
nvm install 18.15.0
Copy link
Contributor

Choose a reason for hiding this comment

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

nodejs 18.15 could be precise enough. Don't pin the patch version.

Copy link
Author

@WYK96 WYK96 Jun 8, 2023

Choose a reason for hiding this comment

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

The patch version has been removed: 18.15.0 -> 18.15


# activate node 18.15.0
nvm use 18.15.0
```

#### 1.1.3 Install Node Packages


```shell
# make sure that your current working directory is 'client'
cd ./client

# install node packages using configuration in client/package.json
npm install
```

### 1.2 Start Viewer

Build and run the viewer using:

```shell
# make sure that your current working directory is 'client'
cd ./client

# start the viewer
npm start
```

Then, check whether viewer is successfully compiled:

![alt viewer_compile_success](./doc/viewer_compile_success.png)

Visit http://localhost:3000/ in the browser:

![alt viewer](./doc/viewer.png)
Copy link
Member

Choose a reason for hiding this comment

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

Shall we replace it with new vierwer screenshot? The coordinate axis on upper right corner is tilted.

Copy link
Author

@WYK96 WYK96 Jun 6, 2023

Choose a reason for hiding this comment

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

The original version of README.md is for debug purpose. In the lastest doc, the screenshots have been removed.


## 2 Bridge Server

### 2.1 Install Dependencies

```shell

cd ./bridge_server

# create a virtual environment
conda create -n BridgeServer python=3.7

conda activate BridgeServer

# install dependencies
pip install -r ./requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

We have a requirements folder in the root directory of the repository, where many different requirements files are named according to their functionality and placed here, such as requirements/synbody.txt. You can also consider renaming your requirements.txt to something like bridge_server.txt. In addition, most of the relative paths in our documentation are relative to the root directory of the repository, not to subdirectories like viewers.

Copy link
Author

Choose a reason for hiding this comment

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

For requriements, the dependencies list of server has been moved from
python/xrprimer/services/xrnerf/requirements.txt to requirements/service_xrnerf.txt.

For paths, they are now relative to the root directory of the repository, e.g., ./run.py to xrprimer/python/xrprimer/services/xrnerf/run.py.

Appreciate for the suggestion.

```

#### 2.2 Start Server
Copy link
Member

Choose a reason for hiding this comment

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

"###" would be enough

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the typo. The title hierarchy issue has been fixed.


```
python ./run_viewer.py
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
python ./run_viewer.py
python .python/tools/un_viewer.py

We often put files with a main function program entry point in the tools folder to distinguish them from files that are only called and not used as entry points.

```

Check whether server is successfully deployed:

![alt run_viewer](./doc/run_viewer.png)

Open the browser and visit http://localhost:3000/. Then, check whether the server is connected with the viewer:

![alt viewer_connected](./doc/viewer_connected.png)
1 change: 1 addition & 0 deletions viewers/nerf_viewer/bridge_server/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.idea
Copy link
Member

Choose a reason for hiding this comment

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

What if we merge this file with the .gitignore file in the root directory?

Copy link
Author

Choose a reason for hiding this comment

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

The original .gitignore file is for debug purpose only. It has been removed now.

23 changes: 23 additions & 0 deletions viewers/nerf_viewer/bridge_server/actions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
"""
Actions are a series of keys that determine how data flows
"""

"""for viewer"""

UPDATE_CAMERA_TRANSLATION = "UPDATE_CAMERA_TRANSLATION"

UPDATE_CAMERA_ROTATION = "UPDATE_CAMERA_ROTATION"

UPDATE_CAMERA_FOV = "UPDATE_CAMERA_FOV"

UPDATE_RESOLUTION = "UPDATE_RESOLUTION"

UPDATE_RENDER_TYPE = "UPDATE_RENDER_TYPE"


"""for nerf backend"""

# update the scene state
UPDATE_STATE = "UPDATE_STATE"

UPDATE_RENDER_RESULT = "UPDATE_RENDER_RESULT"
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend to refactor those constants to an Enum class.

Suggested change
"""for viewer"""
UPDATE_CAMERA_TRANSLATION = "UPDATE_CAMERA_TRANSLATION"
UPDATE_CAMERA_ROTATION = "UPDATE_CAMERA_ROTATION"
UPDATE_CAMERA_FOV = "UPDATE_CAMERA_FOV"
UPDATE_RESOLUTION = "UPDATE_RESOLUTION"
UPDATE_RENDER_TYPE = "UPDATE_RENDER_TYPE"
"""for nerf backend"""
# update the scene state
UPDATE_STATE = "UPDATE_STATE"
UPDATE_RENDER_RESULT = "UPDATE_RENDER_RESULT"
from enum import Enum
class ViewerActionEnum(str, Enum)
"""for viewer"""
UPDATE_CAMERA_TRANSLATION = "UPDATE_CAMERA_TRANSLATION"
UPDATE_CAMERA_ROTATION = "UPDATE_CAMERA_ROTATION"
UPDATE_CAMERA_FOV = "UPDATE_CAMERA_FOV"
UPDATE_RESOLUTION = "UPDATE_RESOLUTION"
UPDATE_RENDER_TYPE = "UPDATE_RENDER_TYPE"
class BackendActions(str, Enum):
"""for nerf backend"""
# update the scene state
UPDATE_STATE = "UPDATE_STATE"
UPDATE_RENDER_RESULT = "UPDATE_RENDER_RESULT"

Copy link
Author

Choose a reason for hiding this comment

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

Wrapped actions into enums.

85 changes: 85 additions & 0 deletions viewers/nerf_viewer/bridge_server/bridge_server_subprocess.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import atexit
Copy link
Member

Choose a reason for hiding this comment

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

Shall we rename it to bridge_server.py? As there is no conflict, it is not necessary to add _subprocess suffix.

Copy link
Author

Choose a reason for hiding this comment

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

  1. Renamed bridge_server_subprocess.py to bridge_server.py
  2. Renamed function run_bridge_server_as_subprocess to start_bridge_server

import os
import signal
import socket
import subprocess
import sys
import threading
import time

import server

from typing import Optional


def is_port_open(port: int):
# check whether the given port is open
try:
sock = socket.socket()
sock.bind(("", port))
sock.close()

return True
except OSError:
return False


def get_free_port(default_port: int = None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def get_free_port(default_port: int = None):
from typing import Union
def get_free_port(default_port: Union[int, None] = None):

Copy link
Author

@WYK96 WYK96 Jun 7, 2023

Choose a reason for hiding this comment

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

The default_port: int has been declared as default_port: Optional[int].

Copy link
Member

Choose a reason for hiding this comment

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

For users, there are two possible scenarios. In the first scenario, the user does not care which port is used as long as the service is started successfully and the port used is returned. The current function can meet this requirement. In the second scenario, the user may be required to use a specific port, such as port 80. If the target port is occupied, an exception should be thrown and there is no need to continue running. Considering these two scenarios, we can modify the parameter list of this function, for example:

  1. def bind_port(allow_random_port: bool, preferred_port: Union[int, None] = None) -> int:
  2. def bind_port(target_port: Union[int, None] = None) -> int:

In the first example, the allow_random_port parameter is used to determine which usage scenario to use, and preferred_port is the port number that is preferentially attempted. In the second example, the target_port parameter is used to determine the usage scenario. If it is provided, the target port must be used. If it is not provided, there are no requirements and any available port can be returned.

Copy link
Author

Choose a reason for hiding this comment

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

Updated port binding logics: if the user specified a port, the server will check whether the port is already in use and then create connections using zmq, otherwise the server will automatically find an available port.

if default_port:
if is_port_open(default_port):
return default_port
sock = socket.socket()
sock.bind(("", 0))
port = sock.getsockname()[1]

return port


def run_bridge_server_as_subprocess(
websocket_port: int,
zmq_port: Optional[int] = None,
Copy link
Member

Choose a reason for hiding this comment

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

ChatGPT:
In Python, Union[int, None] and Optional[int] are similar in that they both indicate that a variable can be either an integer or None. However, there is a subtle difference between them.

Union[int, None] explicitly specifies that the variable can be either an integer or None. It does not allow any other type to be assigned to it. On the other hand, Optional[int] is actually an alias for Union[int, None], which means it represents the same set of possible types. However, Optional[int] does not explicitly indicate that the variable cannot be assigned any other type.

Therefore, using Union[int, None] is more specific and explicit, and may help prevent type-related bugs in your code. However, both Optional[int] and Union[int, None] can be used interchangeably in most cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I doubt the response from ChatGPT. According to PEP484 which is the official definitions of python typing hints, Optional[...] is no more than a shorthand of Union[..., None]. It says in The typing module section:

Optional, defined by Optional[t] == Union[t, None]

So do the documentation of typing.Optional:

Optional[X] is equivalent to X | None (or Union[X, None]).

It's also recommended to use Optional[...] for readability. Since for complex types, we could combine Union with Optional. For example, from def foo(x: Optional[Union[str, pathlib.Path]] = None): ..., we know that foo accepts a path-like argument or None as default. And we can extract this complex type to be PathLike = Union[str, pathlib.Path] and def foo(x: Optional[PathLike] = None): .... Otherwise, we need two typings of Union[str, pathlib.Path, None] and Union[str, pathlib.Path].

There is a related disccusion at StackOverflow says why Optional is suggested:

https://stackoverflow.com/questions/51710037/how-should-i-use-the-optional-type-hint

Copy link
Contributor

Choose a reason for hiding this comment

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

from ChatGPT:

In python, is there any difference between Optional[int] and Union[int, None]`?

Yes, there is a difference, but it's mainly in how they are expressed. Both Optional[int] and Union[int, None] represent the same type, which is either an integer or None. The difference lies in how you express this type annotation.

Optional[int] is a more concise and readable way to express a type that can either be an integer or None. It is equivalent to Union[int, None]. The Optional type is just a shorthand for a Union with None as one of the alternatives.

In Python, you can use both Optional[int] and Union[int, None] to annotate a variable or a function argument that accepts either an integer or None. However, it's generally recommended to use the Optional type for readability and simplicity.

from typing import Optional, Union

def example_optional(x: Optional[int]) -> None:
    print(x)

def example_union(x: Union[int, None]) -> None:
    print(x)

example_optional(42)  # Prints 42
example_optional(None)  # Prints None
example_union(42)  # Prints 42
example_union(None)  # Prints None

In both cases, the functions example_optional and example_union accept either an integer or None as their argument, and both function calls produce the same output.


And I asked ChatGPT about the practical difference in static type checkers mypy.

Is there any difference between Optional[int] and Union[int, None] when using python type checkers like mypy?

No, there is no difference between Optional[int] and Union[int, None] when using Python type checkers like mypy.

Optional[int] is actually a shorthand for Union[int, None], and they are equivalent in terms of type checking. Both indicate that the annotated variable can either be an integer or None.

Using Optional[int] is generally preferred as it is more concise and easier to read. However, if you are using an older version of Python or a type checker that does not support Optional, you can use Union[int, None] instead.

Copy link
Author

Choose a reason for hiding this comment

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

Since Optional[int] is equivalent to Union[int, None] whereas more concise and readable, we're using Optional as the type annotation.

ip_address: str = "127.0.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

0.0.0.0 may allow access to a larger range than 127.0.0.1. Should we change the default value to 0.0.0.0?

Copy link
Author

Choose a reason for hiding this comment

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

Replace the default ip address to 0.0.0.0 makes zmq fail to connect with client.

TODO: figure out why this happens.

):
# run bridge server as a sub-process
args = [sys.executable, "-u", "-m", server.__name__]

# find an available port for zmq
if zmq_port is None:
zmq_port = get_free_port()
print(f"Using ZMQ port: {zmq_port}")

args.append("--zmq-port")
args.append(str(zmq_port))
args.append("--websocket-port")
args.append(str(websocket_port))
args.append("--ip-address")
args.append(str(ip_address))

process = subprocess.Popen(args, start_new_session=True)

def cleanup(process):
process.kill()
process.wait()

def poll_process():
"""
Continually check to see if the viewer bridge server process is still running and has not failed.
If it fails, alert the user and exit the entire program.
"""
while process.poll() is None:
time.sleep(0.5)

message = "The bridge server subprocess failed."
cleanup(process)
Copy link
Member

Choose a reason for hiding this comment

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

process in cleanup is passed by arg list, but in poll_process it looks like a global var.
Considering that the process variable has a clear lifetime and these functions are strongly related to it, would it be better to rewrite this code as a class and access it through self.sub_process?


# windows system do not have signal.SIGKILL
# os.kill(os.getpid(), signal.SIGKILL)
os.kill(os.getpid(), signal.SIGINT)

watcher_thread = threading.Thread(target=poll_process)
watcher_thread.daemon = True
watcher_thread.start()
# clean up process when it has shut down
atexit.register(cleanup, process)

return zmq_port
5 changes: 5 additions & 0 deletions viewers/nerf_viewer/bridge_server/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pyzmq==25.0.2
Copy link
Member

Choose a reason for hiding this comment

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

pyngrok==5.2.1
tornado
Copy link
Contributor

Choose a reason for hiding this comment

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

Pin tornado's major version to prevent incompatibility (i.e. tornado>=6,<7), since a major version update could probably break current codes.

umsgpack==0.1.0
opencv-contrib-python==4.5.3.56
14 changes: 14 additions & 0 deletions viewers/nerf_viewer/bridge_server/run_viewer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from viewer_state import ViewerState


def run_viewer():
"""
start the viewer
"""
viewer_state = ViewerState()
while True:
viewer_state.update_scene()


if __name__ == "__main__":
run_viewer()
173 changes: 173 additions & 0 deletions viewers/nerf_viewer/bridge_server/server.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
import pickle
import sys
from typing import List, Optional, Tuple

# for web networking
import tornado.gen
import tornado.ioloop
import tornado.web
import tornado.websocket

# for data conversion
import umsgpack

# ZeroMQ: for messaging
import zmq
import zmq.eventloop.ioloop
from zmq.eventloop.zmqstream import ZMQStream

from pyngrok import ngrok

from actions import \
UPDATE_CAMERA_FOV, UPDATE_CAMERA_ROTATION, UPDATE_CAMERA_TRANSLATION, \
UPDATE_RENDER_TYPE, UPDATE_RESOLUTION, UPDATE_STATE, UPDATE_RENDER_RESULT

from state import State


class WebSocketHandler(tornado.websocket.WebSocketHandler): # pylint: disable=abstract-method
"""for receiving and sending commands from/to the viewer"""

def __init__(self, *args, **kwargs):
self.bridge = kwargs.pop("bridge")
super().__init__(*args, **kwargs)

def check_origin(self, origin: str) -> bool:
return True

def open(self, *args: str, **kwargs: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

Are args of List[str] & kwargs of Dict[str, str]?
Since they are not used, their typings could be omitted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def open(self, *args: str, **kwargs: str):
def open(self, *args, **kwargs):

self.bridge.websocket_pool.add(self)
print("opened:", self, file=sys.stderr)

async def on_message(self, message: bytearray): # pylint: disable=invalid-overridden-method
"""parses the message from viewer and calls the appropriate function"""
data = message
unpacked_message = umsgpack.unpackb(message)

type = unpacked_message["type"]
data = unpacked_message["data"]
# type_ = m["type"]
# path = list(filter(lambda x: len(x) > 0, m["path"].split("/")))

if type == UPDATE_CAMERA_TRANSLATION:
Copy link
Contributor

Choose a reason for hiding this comment

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

use Enum class (in viewers/nerf_viewer/bridge_server/actions.py) to check type

Suggested change
if type == UPDATE_CAMERA_TRANSLATION:
if type == ViewerActionEnum.UPDATE_CAMERA_TRANSLATION:

Copy link
Author

Choose a reason for hiding this comment

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

Wrapped actions into enums.

self.bridge.state.camera_translation = data
elif type == UPDATE_CAMERA_ROTATION:
self.bridge.state.camera_rotation = data
elif type == UPDATE_RENDER_TYPE:
self.bridge.state.render_type = data
elif type == UPDATE_CAMERA_FOV:
self.bridge.state.camera_fov = data
elif type == UPDATE_RESOLUTION:
self.bridge.state.resolution = data
else:
# TODO: handle exception
pass

def on_close(self) -> None:
self.bridge.websocket_pool.remove(self)
print("closed: ", self, file=sys.stderr)


class ZMQWebSocketBridge:

context = zmq.Context() # pylint: disable=abstract-class-instantiated

def __init__(self, zmq_port: int, websocket_port: int, ip_address: str):
self.zmq_port = zmq_port
self.websocket_pool = set()
self.app = self.make_app()
self.ioloop = tornado.ioloop.IOLoop.current()

# zmq
zmq_url = f"tcp://{ip_address}:{self.zmq_port:d}"
self.zmq_socket, self.zmq_stream, self.zmq_url = self.setup_zmq(zmq_url)

# websocket
listen_kwargs = {"address" : "0.0.0.0"}
self.app.listen(websocket_port, **listen_kwargs)
self.websocket_port = websocket_port
self.websocket_url = f"0.0.0.0:{self.websocket_port}"

# state
self.state = State()

def __str__(self) -> str:
class_name = self.__class__.__name__
return f'{class_name} using zmq_port="{self.zmq_port}" and websocket_port="{self.websocket_port}"'

def make_app(self):
# create an application for the websocket server
return tornado.web.Application([(r"/", WebSocketHandler, {"bridge": self})])

def handle_zmq(self, frames: List[bytes]):

type_ = frames[0].decode("utf-8")
data = frames[1]

if type_ == UPDATE_RENDER_RESULT:
self.forward_to_websockets(frames)
self.zmq_socket.send(umsgpack.packb(b"ok"))
elif type_ == UPDATE_STATE:
serialized = pickle.dumps(self.state)
self.zmq_socket.send(serialized)
elif type_ == "ping":
self.zmq_socket.send(umsgpack.packb(b"ping received"))
else:
print("type: " + str(type_))
self.zmq_socket.send(umsgpack.packb(b"error: unknown command"))

def forward_to_websockets(
self,
frames: Tuple[str, str, bytes],
websocket_to_skip: Optional[WebSocketHandler] = None
):
"""forward a zmq message to all websockets"""
"""nerf backend -> viewer"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""forward a zmq message to all websockets"""
"""nerf backend -> viewer"""
"""forward a zmq message to all websockets
nerf backend -> viewer"""

Copy link
Author

Choose a reason for hiding this comment

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

The comment """nerf backend -> viewer""" is no longer useful and has been deleted.

_type, _data = frames # cmd, data
for websocket in self.websocket_pool:
if websocket_to_skip and websocket == websocket_to_skip:
pass
else:
websocket.write_message(_data, binary=True)

def setup_zmq(self, url: str):
"""setup a zmq socket and connect it to the given url"""
zmq_socket = self.context.socket(zmq.REP) # pylint: disable=no-member
zmq_socket.bind(url)
zmq_stream = ZMQStream(zmq_socket)
zmq_stream.on_recv(self.handle_zmq)

return zmq_socket, zmq_stream, url

def run(self):
"""starts and runs the websocet bridge"""
self.ioloop.start()


def run_bridge_server(
zmq_port: int = 6000,
websocket_port: int = 4567,
ip_address: str = "127.0.0.1",
use_ngrok: bool = False
):
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra indents

Suggested change
def run_bridge_server(
zmq_port: int = 6000,
websocket_port: int = 4567,
ip_address: str = "127.0.0.1",
use_ngrok: bool = False
):
def run_bridge_server(
zmq_port: int = 6000,
websocket_port: int = 4567,
ip_address: str = "127.0.0.1",
use_ngrok: bool = False
):

Copy link
Author

@WYK96 WYK96 Jun 8, 2023

Choose a reason for hiding this comment

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

Extra indents have been fixed by pre-commit.

# whether expose the zmq port
if use_ngrok:
http_tunnel = ngrok.connect(addr=str(zmq_port), proto="tcp")
print(http_tunnel)

bridge = ZMQWebSocketBridge(
zmq_port=zmq_port,
websocket_port=websocket_port,
ip_address=ip_address
)

print(bridge)

try:
bridge.run()
except KeyboardInterrupt:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why KeyboardInterrupts are ignored here? Should it be a comment here?

pass


if __name__ == "__main__":
run_bridge_server()
Loading