-
Notifications
You must be signed in to change notification settings - Fork 3
receivers for http/tcp/web socket communication #152
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
base: release/2.0.0
Are you sure you want to change the base?
Conversation
@DamienGilliard , @9and3 |
You can flag the line with an in-line comment param = ghenv.Component.Params.Input[indx] # noqa: F821 see diffCheck/src/gh/components/DF_csv_exporter/code.py Lines 30 to 42 in a73bc9d
The linter should stop complaining about it :) |
Thanks @9and3! it seems to have done the trick ;) |
All three are working now! Remains to be verified if I successfully added websockets to the python package or it just works on my side because I had it already installed. :) |
I'll try and let you know before the meeting tomorrow |
@eleniv3d Works fine on my side ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small questions here and there, but overall thanks a lot for this great addition!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if I'm being picky, but it seems to me that calling the file df_gh_canvas_utils.py would be more explicit. Otherwise thanks for this new module !
df_gh_canvas.add_button( | ||
ghenv.Component, label, idx, x_offset=60) # noqa: F821 | ||
df_gh_canvas.add_panel(ghenv.Component, "Host", "127.0.0.1", 3, 60, 20) # noqa: F821 | ||
df_gh_canvas.add_panel(ghenv.Component, "Port", "5000", 4, 60, 20) # noqa: F821 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clean ;)
sc.sticky.setdefault(f'{prefix}_prev_load', False) | ||
|
||
# Client handler | ||
def handle_client(conn): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add doctrings (here and elsewhere) in your functions that describe the types of data needed ? It is not clear to me what "conn" is :( . For example:
def handle_client(conn: <type_of_conn>):
"""
reads the incoming bytes from a single client socket and stores valid data in a shared buffer
:param conn: <what conn is in pain english>
:returns: <what it returns>
"""
Like what you did for some of the functions in the new df_gh_canvas.py
module.
I noticed that I had not done it myself so I'll fix it and put full docstring everywhere too ;)
with conn: | ||
while sc.sticky.get(f'{prefix}_server_started', False): | ||
try: | ||
chunk = conn.recv(4096) | ||
if not chunk: | ||
break | ||
buf += chunk | ||
while b'\n' in buf: | ||
line, buf = buf.split(b'\n', 1) | ||
try: | ||
raw = json.loads(line.decode()) | ||
except Exception: | ||
continue | ||
if isinstance(raw, list) and all(isinstance(pt, list) and len(pt) == 6 for pt in raw): | ||
sc.sticky[f'{prefix}_cloud_buffer_raw'] = raw | ||
except Exception: | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my experience, having infinite loops in separate threads that run "full gas" can lead to issues with the cpu, I would add a small time.sleep(0.1)
for 100ms sleep before re-running the loop, for example.
Although here on my computer this loop without pause seems to have no negative impact
if i_start and not sc.sticky[f'{prefix}_prev_start']: | ||
start_server() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When testing your components I made the mistake to first try to send data to the TCP socket (before starting the server, dumb of me) and it lead me to think that maybe we could add after this if
something like:
else:
self.AddRuntimeMessage(RML.Warning, "Please start server here before trying to send data from remote device.")
Maybe it is because I'm dumb but I think it could improve slightly the user-friendliness
cnt = len(sc.sticky[f'{prefix}_loaded_pcd']) if sc.sticky[f'{prefix}_loaded_pcd'] else 0 | ||
logs.append(f"Loaded pcd with {cnt} pts") | ||
ghenv.Component.ExpireSolution(True) # noqa: F821 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here too, as for the TCP listener, I would add a small warning to start server in gh before trying to send data. It sounds obvious but I think it's a nice-to-have feature. Let us know what you think ;)
Dynamic: author | ||
Dynamic: author-email | ||
Dynamic: classifier | ||
Dynamic: description | ||
Dynamic: description-content-type | ||
Dynamic: home-page | ||
Dynamic: requires-dist | ||
Dynamic: summary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why have these been deleted? is it intentional ?
prefix = 'ws' | ||
|
||
# Persistent state across runs | ||
sc.sticky.setdefault(f'{prefix}_ws_server', None) | ||
sc.sticky.setdefault(f'{prefix}_ws_loop', None) | ||
sc.sticky.setdefault(f'{prefix}_ws_thread', None) | ||
sc.sticky.setdefault(f'{prefix}_last_pcd', None) | ||
sc.sticky.setdefault(f'{prefix}_loaded_pcd', None) | ||
sc.sticky.setdefault(f'{prefix}_ws_logs', []) | ||
sc.sticky.setdefault(f'{prefix}_ws_thread_started', False) | ||
sc.sticky.setdefault(f'{prefix}_prev_start', False) | ||
sc.sticky.setdefault(f'{prefix}_prev_stop', False) | ||
sc.sticky.setdefault(f'{prefix}_prev_load', False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
having ws_ws_server
and other ws_ws names seems strange to me. Is it necessary ?
This PR includes:
(1) an http listener that retrieves .ply files from the internet
https://github.com/user-attachments/assets/59ce5c8d-adad-4164-9414-49cebac1f652
(2) a TCP listener that receives pcd color data
https://github.com/user-attachments/assets/a088ad9c-712a-49ea-90fb-4b6deb1e583e
(3) a Websocket listener that receives pcd color data
https://github.com/user-attachments/assets/8310f586-0c51-4769-babe-9453556af1a6
To test the files you can use the senders that are available here and here
The workflow now is:
I added automatic btns and panels for standard inputs like we had for the visualization_settings component and moved these utilities to a new df_gh_canvas.py file so that they are available to use in components in the future. The components look like this when dropped in the carvas :