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

Gamecontroller can Bind to a Different Port Based on Command Line Argument #3207

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
bacfacf
chnage port depending on argument
Mr-Anyone May 20, 2024
d620120
added docs
Mr-Anyone May 20, 2024
1557448
remove stuff
Mr-Anyone May 20, 2024
5478f15
updated entry points
Mr-Anyone May 20, 2024
8d394cd
fixed stuff
Mr-Anyone May 21, 2024
0b5d2a6
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] May 21, 2024
5d5e130
copied code from nima
Mr-Anyone May 22, 2024
53b3120
added some new port to fix
Mr-Anyone May 22, 2024
c052a97
change bind address and stuff like that
Mr-Anyone May 23, 2024
ace67d6
added new stuff to robot communication
Mr-Anyone May 23, 2024
730753a
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] May 23, 2024
9b8d59d
added new stuff for robot communication
Mr-Anyone May 23, 2024
9453411
Merge branch 'gamecontroller_port' of github.com:Mr-Anyone/Thunderbot…
Mr-Anyone May 23, 2024
ebcc8d6
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] May 23, 2024
6ad2b47
merged arune's changes
Mr-Anyone May 31, 2024
e730860
added new feature that arune requested
Mr-Anyone Jun 11, 2024
60dad81
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Jun 11, 2024
b44cdcf
added requested feature
Mr-Anyone Jun 26, 2024
0c984eb
added comments and remove necessary print statements
Mr-Anyone Jun 26, 2024
749fb3f
remove code that are non necessary
Mr-Anyone Jun 26, 2024
f3a42e7
removed one line of legacy code
Mr-Anyone Jun 26, 2024
bed3109
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Jun 26, 2024
51f42e2
Merge branch 'gamecontroller_port' of github.com:Mr-Anyone/Thunderbot…
Mr-Anyone Jun 26, 2024
1e8fcba
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Jun 26, 2024
3218a32
refactored function
Mr-Anyone Jun 28, 2024
b147096
Merge branch 'gamecontroller_port' of github.com:Mr-Anyone/Thunderbot…
Mr-Anyone Jun 28, 2024
cf668bf
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Jun 28, 2024
036da38
added comments to make things make more sense
Mr-Anyone Jul 2, 2024
4f1fbe5
Merge branch 'gamecontroller_port' of github.com:Mr-Anyone/Thunderbot…
Mr-Anyone Jul 2, 2024
9a3ef5f
address some comments
Mr-Anyone Jul 3, 2024
82e6f87
resolved some comments
Mr-Anyone Jul 5, 2024
c51c412
made new changes
Mr-Anyone Jul 5, 2024
e616430
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Jul 5, 2024
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
26 changes: 25 additions & 1 deletion src/software/field_tests/field_test_fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,34 @@ def load_command_line_arguments():
help="Estop Baudrate",
)

parser.add_argument(
"--gamecontroller_port",
type=int,
default=None,
help="The port number for the game controller.",
)

parser.add_argument(
"--run_yellow",
action="store_true",
default=False,
help="Run the test with friendly robots in yellow mode",
)

parser.add_argument(
"--sslvision_multicast_address",
type=str,
default=SSL_VISION_ADDRESS,
help="the multicast address for ssl vision",
)

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this arg, we can default to SSL_VISION_ADDRESS and just change the port

parser.add_argument(
"--sslreferee_multicast_address",
type=str,
default=SSL_REFEREE_ADDRESS,
help="the multicast address for referee (game controller likely)",
)

estop_group = parser.add_mutually_exclusive_group()
estop_group.add_argument(
"--keyboard_estop",
Expand Down Expand Up @@ -380,9 +401,12 @@ def field_test_runner():
estop_mode=estop_mode,
estop_path=estop_path,
enable_radio=args.enable_radio,
referee_address=args.sslreferee_multicast_address,
sslvision_address=args.sslvision_multicast_address,
) as rc_friendly:
with Gamecontroller(
supress_logs=(not args.show_gamecontroller_logs)
supress_logs=(not args.show_gamecontroller_logs),
gamecontroller_port=args.gamecontroller_port,
) as gamecontroller:
friendly_fs.setup_proto_unix_io(friendly_proto_unix_io)
rc_friendly.setup_for_fullsystem()
Expand Down
11 changes: 8 additions & 3 deletions src/software/networking/ssl_proto_communication.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,14 @@ def __init__(self, port: int) -> None:

:param port the port to bind to
"""
# bind to all local interfaces, TCP
self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
self.socket.connect(("", port))
try:
# bind to all local interfaces, TCP
self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
self.socket.connect(("", port))
except ConnectionRefusedError:
raise ConnectionRefusedError(
f"SSL Socket connection refused on port {port}. Is binary already running in a separate process?"
)

def send(self, proto: protobuf_message.Message) -> None:
"""
Expand Down
10 changes: 9 additions & 1 deletion src/software/simulated_tests/simulated_test_fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,13 @@ def load_command_line_arguments():
default=False,
help="Use realism in the simulator",
)

parser.add_argument(
"--gamecontroller_port",
type=int,
default=None,
help="The port number for the game controller.",
)
return parser.parse_args()


Expand Down Expand Up @@ -542,7 +549,8 @@ def simulated_test_runner():
should_restart_on_crash=False,
) as yellow_fs:
with Gamecontroller(
supress_logs=(not args.show_gamecontroller_logs)
supress_logs=(not args.show_gamecontroller_logs),
gamecontroller_port=args.gamecontroller_port,
) as gamecontroller:

blue_fs.setup_proto_unix_io(blue_full_system_proto_unix_io)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,33 @@ class Gamecontroller(object):
REFEREE_IP = "224.5.23.1"
CI_MODE_OUTPUT_RECEIVE_BUFFER_SIZE = 9000

def __init__(self, supress_logs: bool = False) -> None:
def __init__(
self,
supress_logs: bool = False,
gamecontroller_port: int = None,
referee_addresse: int = None,
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
referee_addresse: int = None,
referee_address: int = None,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor

Choose a reason for hiding this comment

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

bump: looks like it didn't get committed

) -> None:
"""Run Gamecontroller

:param supress_logs: Whether to suppress the logs
:param gamecontroller_port: the port that the gamecontroller would bind to
:param referee_address: the address the referee binds to
"""
self.supress_logs = supress_logs

# We need to find 2 free ports to use for the gamecontroller
# so that we can run multiple gamecontroller instances in parallel
self.referee_port = self.next_free_port()
self.ci_port = self.next_free_port()

self.REFEREE_IP = referee_addresse
if referee_addresse is None:
self.REFEREE_IP = "224.5.23.1"

Copy link
Contributor

Choose a reason for hiding this comment

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

REFEREE_IP is supposed to be a constant, we shouldn't be modifying it inside here

Copy link
Contributor Author

@Mr-Anyone Mr-Anyone May 31, 2024

Choose a reason for hiding this comment

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

On the other hand, #3160 discusses the problem we faced during Robocup where most teams connect their computer to the same network which has access to the internet. Furthermore, during a game or field testing, all teams on the field connect to the same LAN. Our goal is to basically almost always launch the gamecontroller on some non-official port/multicast IP address that other teams or other members of our team aren't listening to for gamecontroller commands. This will allow us to send gamecontroller commands that only effects the fullsystem being ran locally. This should be the case UNLESS we're playing an actual game where we're not launching our own gamecontroller locally and we're using the provided field gamecontroller. Hence #3161 discussing a new "robocup" mode used during actual games with a non-local gc. To achieve this, we will probably need to update RobotCommunication aswell to not always listen to the official gamecontroller/referee port/IP

Doesn't this contradicts what nima said? Am I misunderstanding something? Are we setting the port instead then but keeping the multicast address constant?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nima's right but in the implementation, don't modify REFEREE_IP. That is a constant (which is why it's all capitalized) we use during real games for connecting to the Gamecontroller and we should leave it so that it always point to that ip address. By convention, we never modify constants during runtime.

Instead, use a new local variable:

Suggested change
self.REFEREE_IP = referee_addresse
if referee_addresse is None:
self.REFEREE_IP = "224.5.23.1"
multicast_address = referee_address if referee_address else Gamecontroller.REFEREE_IP

Copy link
Contributor Author

@Mr-Anyone Mr-Anyone Jun 1, 2024

Choose a reason for hiding this comment

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

Shouldn't this be self.multicast_address instead of multicast_address, since we are accessing this variable in line 180?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, you're correct

self.ci_port = gamecontroller_port
if self.ci_port is None:
self.ci_port = self.next_free_port(40000)
else:
self.ci_port = self.next_free_port(self.ci_port)

# this allows gamecontroller to listen to override commands
self.command_override_buffer = ThreadSafeBuffer(
Expand Down Expand Up @@ -102,24 +118,35 @@ def refresh(self):
)
manual_command = self.command_override_buffer.get(return_cached=False)

def next_free_port(self, port: int = 40000, max_port: int = 65535) -> None:
def is_valid_port(self, port: int) -> bool:
"""
Check if a port is available

:param port: the port we are checking
:return: True if the port is available, False otherwise
"""
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

try:
sock.bind(("", port))
sock.close()
return True
except OSError:
return False

def next_free_port(self, start_port: int = 40000, max_port: int = 65535) -> int:
"""Find the next free port. We need to find 2 free ports to use for the gamecontroller
so that we can run multiple gamecontroller instances in parallel.

:param port: The port to start looking from
:param start_port: The port to start looking from
:param max_port: The maximum port to look up to
:return: The next free port

"""
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

while port <= max_port:
try:
sock.bind(("", port))
sock.close()
return port
except OSError:
port += 1
while start_port <= max_port:
if self.is_valid_port(start_port):
return start_port
start_port += 1

raise IOError("no free ports")

Expand Down Expand Up @@ -150,7 +177,7 @@ def __send_referee_command(data: Referee) -> None:
autoref_proto_unix_io.send_proto(Referee, data)

self.receive_referee_command = tbots_cpp.SSLRefereeProtoListener(
Gamecontroller.REFEREE_IP, self.referee_port, __send_referee_command, True,
self.REFEREE_IP, self.referee_port, __send_referee_command, True,
Copy link
Contributor

Choose a reason for hiding this comment

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

you can keep the old code

)

blue_full_system_proto_unix_io.register_observer(
Expand Down
15 changes: 13 additions & 2 deletions src/software/thunderscope/robot_communication.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ def __init__(
estop_path: os.PathLike = None,
estop_baudrate: int = 115200,
enable_radio: bool = False,
referee_address: str = None,
sslvision_address: str = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this param

):
"""Initialize the communication with the robots

Expand All @@ -39,6 +41,15 @@ def __init__(
:param enable_radio: Whether to use radio to send primitives to robots

"""
# setting up the ssl vision and referee addresses
self.ssl_vision_address = sslvision_address
if self.ssl_vision_address is None:
self.ssl_vision_address = SSL_VISION_ADDRESS
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this code


self.referee_address = referee_address
if self.referee_address is None:
self.referee_address = SSL_REFEREE_ADDRESS

self.receive_ssl_referee_proto = None
self.receive_ssl_wrapper = None
self.sequence_number = 0
Expand Down Expand Up @@ -105,14 +116,14 @@ def setup_for_fullsystem(self) -> None:
Sets up a listener for SSL vision and referee data, and connects all robots to fullsystem as default
"""
self.receive_ssl_wrapper = tbots_cpp.SSLWrapperPacketProtoListener(
SSL_VISION_ADDRESS,
self.ssl_vision_address,
Copy link
Contributor

Choose a reason for hiding this comment

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

keep the old code

SSL_VISION_PORT,
lambda data: self.__forward_to_proto_unix_io(SSL_WrapperPacket, data),
True,
Copy link
Contributor

Choose a reason for hiding this comment

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

line 125 should be self.referee_port if it isn't None or SSL_REFEREE_PORT

)

self.receive_ssl_referee_proto = tbots_cpp.SSLRefereeProtoListener(
SSL_REFEREE_ADDRESS,
self.referee_address,
SSL_REFEREE_PORT,
lambda data: self.current_proto_unix_io.send_proto(Referee, data),
True,
Expand Down
28 changes: 27 additions & 1 deletion src/software/thunderscope/thunderscope_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from software.thunderscope.binary_context_managers.full_system import FullSystem
from software.thunderscope.binary_context_managers.simulator import Simulator
from software.thunderscope.binary_context_managers.game_controller import Gamecontroller

from software.thunderscope.binary_context_managers.tigers_autoref import TigersAutoref


Expand Down Expand Up @@ -218,6 +219,27 @@
help="Whether to populate with default robot positions (False) or start with an empty field (True) for AI vs AI",
)

parser.add_argument(
"--gamecontroller_port",
type=int,
default=None,
help="The port number for the game controller.",
)

parser.add_argument(
"--sslvision_multicast_address",
type=str,
default=SSL_VISION_ADDRESS,
help="the multicast address for ssl vision",
)

parser.add_argument(
"--sslreferee_multicast_address",
type=str,
default=SSL_REFEREE_ADDRESS,
help="the multicast address for referee (game controller likely)",
)

args = parser.parse_args()

# Sanity check that an interface was provided
Expand Down Expand Up @@ -317,6 +339,8 @@
estop_mode=estop_mode,
estop_path=estop_path,
enable_radio=args.enable_radio,
referee_address=args.sslreferee_multicast_address,
sslvision_address=args.sslvision_multicast_address,
Copy link
Contributor

Choose a reason for hiding this comment

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

if args.launch_gc exists, then we should have a gamecontroller.get_referee_port() call somewhere here

) as robot_communication:
nimazareian marked this conversation as resolved.
Show resolved Hide resolved

if estop_mode == EstopMode.KEYBOARD_ESTOP:
Expand Down Expand Up @@ -428,7 +452,9 @@ def __ticker(tick_rate_ms: int) -> None:
should_restart_on_crash=False,
run_sudo=args.sudo,
) as yellow_fs, Gamecontroller(
supress_logs=(not args.verbose)
supress_logs=(not args.verbose),
gamecontroller_port=args.gamecontroller_port,
referee_addresse=args.sslreferee_multicast_address,
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
referee_addresse=args.sslreferee_multicast_address,
referee_address=args.sslreferee_multicast_address,

) as gamecontroller, (
# Here we only initialize autoref if the --enable_autoref flag is requested.
# To avoid nested Python withs, the autoref is initialized as None when this flag doesn't exist.
Expand Down
Loading