Feature/multi port - #151
Conversation
e88228f to
2d5030c
Compare
|
Forgot to push a fix |
JordanYates
left a comment
JordanYates
left a comment
There was a problem hiding this comment.
Basic argparse testing for the helper type should be added in tests/util/test_argparse.py
| parser.add_argument( | ||
| '--server-port', | ||
| dest='server_sock', | ||
| default=[default_multicast_address()] if multi_port else default_multicast_address(), |
There was a problem hiding this comment.
This is sometimes a list of list[tuple[addr, port]], and sometimes just a tuple[addr, port]. It's passed directly into LocalServer, which only support the later.
There was a problem hiding this comment.
The parser outputs either a single port -> single socket pair, or a list of ports-> list of socket pairs depending on multi_port.
If a tool is designed to support multiple apps like how ota_upgrade was adapted in
, the tool can indicate this by setting
multi_port to true, and then it's up to the tool to handle the socket list using list comprehension.
| dest='server_sock', | ||
| default=[default_multicast_address()] if multi_port else default_multicast_address(), | ||
| type=server_port_parser, | ||
| nargs= '+' if multi_port else None, |
There was a problem hiding this comment.
nargs = '+' means one or more. Is this intended? What would be the meaning if 4 port numbers are supplied?
There was a problem hiding this comment.
Yes, when a tool specifies it supports multiple ports, it can use as many ports. I've had a reference sample of ota_upgrade using multiple gateways (tested with 2) where I tried entering 2 or 3 ports. The ports are converted to a list of socket tuples. In the case of the ota_upgrade, it setups comms with each of the gateways on the other side of the port (if it's available). Providing 4 port numbers, means connecting to as many as 4 gateways/scripts to use as workers to perform updates.
noting this only occurs when a tool specifies it opts-in for multiple ports. By default it will only allow 1.
Does it make sense to also move |
No strong opinions either way, its very specific to the socket comms abstraction, unlike the other helpers in |
2d5030c to
ba40e95
Compare
|
Changed Added tests for both |
| return default_multicast_address(port) | ||
|
|
||
| def add_server_port_parser(parser: argparse.ArgumentParser, multi_port: bool = False): | ||
| parser.add_argument( |
There was a problem hiding this comment.
Add a docstring to this function please
Add an argparse helper to parse port numbers and preprocess them into a socket (addr/port tuple). Add an argparse helper to add a `--server-port` argument to tools that need it. This can be used to specify a different UDP port to allow for multiple gateway scripts and corresponding connections to run simultaneously. Signed-off-by: Aeyohan Furtado <aeyohan@embeint.com>
Added tests for the `add_server_port_parser` function for registering the `--server-port` argument. Tests include checking correct single and multi-port handling and default values for each. Added test for the `ServerPort` type to ensure that it correctly parses valid and invalid port numbers. Signed-off-by: Aeyohan Furtado <aeyohan@embeint.com>
Added the `--server-port` argument to any tools that act or interact with a gateway. Signed-off-by: Aeyohan Furtado <aeyohan@embeint.com>
ba40e95 to
f560e6d
Compare
|
Added missing doc string |
2 participants
Support multiple gateways and specifying which gateway a tool should connect to.
Updated all tools with the
--server-portoption.Ports must be odd (since they are used in pairs).