Conversation
There was a problem hiding this comment.
Functionality works great, I tested the UDP and TCP functionality as well as the changing the frequency. Most requested changes are style-related or just comments pointing out helpful python features that could help.
There are some important comments about the blocking behaviour of the sendUDP method, that's the most important one I'd say.
Let me know if you have any questions about these comments!
Footnote: In the future, there is a branch naming convention to follow that can be found on the website
Tools/boringNetworkTool/main.py
Outdated
| s.sendto(bytes(message.toString(), 'utf-8'), (host, port)) | ||
| print("message sent:" + message.toString()) | ||
| # receive data from client (data, addr) | ||
| d = s.recvfrom(1024) |
There was a problem hiding this comment.
This blocks, with UDP I don't think we will always want to wait for a reply from the server before sending the next packet. Potentially, we could have an option in the config to await a reply.
Where it is a debug tool, we will usually just want it to send data continuously as well. This could be another option in the config (bool sendContinuous), that if true the script just continuously calls sendUDP or sendTCP.
I tried a while loop, it worked fine for UDP but there was an error when calling send TCP in a while loop (the server reset the connection or something)
Burke-Daniel
left a comment
Burke-Daniel
left a comment
There was a problem hiding this comment.
Couple things to look at, but this is super nice and really clean. Great work!
| sys.exit() | ||
|
|
||
|
|
||
| def send_with_tcp(msgconfig, host, port): |
There was a problem hiding this comment.
It might be nice for TCP to have the option for the network tool to act as either the server or the client
There was a problem hiding this comment.
If you did this, you might be able to eliminate the tcp_server.py script altogether and use 2 instances of main.py, one configured as server and one configured as client, to test
| "config": { | ||
| "ip": "localhost", | ||
| "port": 8888, | ||
| "useTCP": false, |
There was a problem hiding this comment.
I think it might be better if this is commsType, or something similar, and then it parses for TCP or UDP as the value. If you make the option to act as the tcp server, you could then have a setting called servePort which would determine that.
| class Message: | ||
| def __init__(self, data): | ||
| self.data = data | ||
|
|
||
| def to_string(self) -> str: | ||
| return "%s \n" % str(self.data) |
There was a problem hiding this comment.
I like this a lot, very clean
| import socket | ||
| import sys | ||
| import time | ||
| import json |
There was a problem hiding this comment.
Probably want to order these in alphabetical order
| s.sendto(bytes(message.to_string(), 'utf-8'), (host, port)) | ||
| print("message sent: %s" % message.to_string()) | ||
| # receive data from client (data, addr) | ||
| if msgconfig["config"]["sendContinuous"] is False: |
There was a problem hiding this comment.
We are probably gonna want to properly send the messages repetitively indefinitely
4 participants
No description provided.