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.

Sign up for GitHub

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

Jump to bottom

Remove all instances of MessageToDict #3405

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Remove all instances of MessageToDict #3405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7df478c
Removed all instances of messagetodict
AmyKawa Nov 9, 2024
99e7f6a
Merge branch 'master' of https://github.com/UBC-Thunderbots/Software …
AmyKawa Nov 9, 2024
8aa2dd0
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Nov 9, 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
22 changes: 14 additions & 8 deletions src/software/thunderscope/common/proto_parameter_tree_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from pyqtgraph.Qt.QtWidgets import *
from pyqtgraph import parametertree
from google.protobuf.json_format import MessageToDict
from thefuzz import fuzz
from proto.import_all_protos import *


def __create_int_parameter_writable(key, value, descriptor):
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be nice to add type annotation

def __create_int_parameter_writable(key, value, descriptor): to def __create_int_parameter_writable(key:str, value:int, descriptor:FieldDescriptor):

Also, FieldDescriptor could be found here: from google.protobuf.descriptor import FieldDescriptor

Expand All @@ -16,10 +16,13 @@ def __create_int_parameter_writable(key, value, descriptor):
"""
# Extract the options from the descriptor, and store it
# in the dictionary.
options = MessageToDict(descriptor.GetOptions(), preserving_proto_field_name=True)
options = descriptor.GetOptions()

try:
min_max = options["[TbotsProto.bounds]"]
minimum, maximum = (
options.Extensions[bounds].min_double_value,
options.Extensions[bounds].max_double_value,
)
except KeyError:
raise KeyError("{} missing ParameterRangeOptions".format(key))

Expand All @@ -28,7 +31,7 @@ def __create_int_parameter_writable(key, value, descriptor):
"type": "int",
"value": value,
"default": value,
"limits": (int(min_max["min_int_value"]), int(min_max["max_int_value"])),
"limits": (int(minimum), int(maximum)),
"step": 1,
}

Expand All @@ -44,10 +47,13 @@ def __create_double_parameter_writable(key, value, descriptor):
"""
# Extract the options from the descriptor, and store it
# in the dictionary.
options = MessageToDict(descriptor.GetOptions(), preserving_proto_field_name=True)
options = descriptor.GetOptions()

try:
min_max = options["[TbotsProto.bounds]"]
minimum, maximum = (
options.Extensions[bounds].min_double_value,
options.Extensions[bounds].max_double_value,
)
except KeyError:
raise KeyError("{} missing ParameterRangeOptions".format(key))

Expand All @@ -57,8 +63,8 @@ def __create_double_parameter_writable(key, value, descriptor):
"value": value,
"default": value,
"limits": (
min_max["min_double_value"],
min_max["max_double_value"],
minimum,
maximum,
),
"step": 0.01,
}
Expand Down
15 changes: 5 additions & 10 deletions src/software/thunderscope/gl/layers/gl_tactic_layer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

import textwrap

from google.protobuf.json_format import MessageToDict

from proto.import_all_protos import *
from software.py_constants import *
from software.thunderscope.constants import Colors, DepthValues
Expand Down Expand Up @@ -38,20 +36,17 @@ def __init__(self, name: str, buffer_size: int = 5) -> None:
def refresh_graphics(self) -> None:
"""Update graphics in this layer"""
self.cached_world = self.world_buffer.get(block=False)
play_info = self.play_info_buffer.get(block=False)
play_info_dict = MessageToDict(play_info)
play_info = self.play_info_buffer.get(block=False).GetOptions()
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to call GetOptions() here. I believe you'll only need this if you define specific bounds for the proto definitions (like if you defined this proto:

Software/src/proto/parameters.proto

Line 61 in 10bded3

required double ball_is_kicked_m_per_s_threshold = 1
and you needed to access the "bounds" values). We don't use this pattern in most of other protos, including this one.


self.__update_tactic_name_graphics(
self.cached_world.friendly_team, play_info_dict
)
self.__update_tactic_name_graphics(self.cached_world.friendly_team, play_info)

def __update_tactic_name_graphics(self, team: Team, play_info_dict) -> None:
def __update_tactic_name_graphics(self, team: Team, play_info) -> None:
"""Update the GLGraphicsItems that display tactic data

:param team: The team proto
:param play_info_dict: The dictionary containing play/tactic info
:param play_info: The dictionary containing play/tactic info
"""
tactic_assignments = play_info_dict["robotTacticAssignment"]
tactic_assignments = play_info.robotTacticAssignment()
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't quite work. The correct way access the robot tactic assignment field is: play_info.robot_tactic_assignment().

This will return a MessageMapContainer which has this API: https://googleapis.dev/python/protobuf/3.17.0/google/protobuf/internal/containers.html#google.protobuf.internal.containers.MessageMap

The function call will match the field from the definition of the proto parameters:

Software/src/proto/play_info_msg.proto

Line 18 in 10bded3

map<uint32, Tactic> robot_tactic_assignment = 1;


# Ensure we have the same number of graphics as robots
self.tactic_fsm_info_graphics.resize(
Expand Down
21 changes: 10 additions & 11 deletions src/software/thunderscope/play/playinfo_widget.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from proto.play_info_msg_pb2 import PlayInfo
from google.protobuf.json_format import MessageToDict
from pyqtgraph.Qt.QtWidgets import *
from proto.import_all_protos import *
from software.thunderscope.common.common_widgets import set_table_data
Expand Down Expand Up @@ -35,42 +34,42 @@ def __init__(self, buffer_size: int = 1) -> None:

def refresh(self) -> None:
"""Update the play info widget with new play information"""
playinfo = self.playinfo_buffer.get(block=False, return_cached=False)
playinfo = self.playinfo_buffer.get(
block=False, return_cached=False
).GetOptions()
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to call GetOptions here


# Updating QTableWidget could be expensive, so we only update if there is new data
if playinfo is None or playinfo == self.last_playinfo:
return

self.last_playinfo = playinfo

play_info_dict = MessageToDict(playinfo)

robot_ids = []
tactic_fsm_states = []
tactic_names = []
play_name = []

if "robotTacticAssignment" not in play_info_dict:
if "robotTacticAssignment" not in playinfo:
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you can do if not playinfo.robot_tactic_assignment()

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe try if not playinfo.robot_tactic_assignment if that doesn't work. I may be wrong, but I feel like robot_tactic_assignment would return a type of dict which is not callable?

return

num_rows = max(
len(play_info_dict["robotTacticAssignment"]),
len(play_info_dict["play"]["playState"]),
len(playinfo.robotTacticAssignment()),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you meant len(playinfo.robot_tactic_assignment)

len(playinfo.play.playState()),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be len(playinfo.play.play_state), you should check these two comments.

)

# setting table size dynamically
self.play_table.setRowCount(num_rows)

for state in play_info_dict["play"]["playState"]:
for state in playinfo.play().playstate():
play_name.append(state)

for robot_id in sorted(play_info_dict["robotTacticAssignment"]):
for robot_id in sorted(playinfo.robotTacticAssignment()):
robot_ids.append(robot_id)
tactic_fsm_states.append(
play_info_dict["robotTacticAssignment"][robot_id]["tacticFsmState"]
playinfo.robotTacticAssignment().robot_id().tacticFsmState()
)
tactic_names.append(
play_info_dict["robotTacticAssignment"][robot_id]["tacticName"]
playinfo.robotTacticAssignment().robot_id().tacticName()
Comment on lines +56 to +72
Copy link
Contributor

Choose a reason for hiding this comment

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

these function calls should be updated. The field names should match the proto field names.

You should take a look at the proto definition:

Software/src/proto/play_info_msg.proto

Line 5 in 10bded3

message PlayInfo

And take a look at the Python generated code reference guide: https://protobuf.dev/reference/python/python-generated/

Copy link
Contributor

Choose a reason for hiding this comment

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

for your reference, it should be something like this:

for robot_id in sorted(playinfo.robot_tactic_assignment):
          robot_ids.append(robot_id)
          tactic_fsm_states.append(
              playinfo.robot_tactic_assignment[robot_id].tactic_fsm_state
          )

          tactic_names.append(
              playinfo.robot_tactic_assignment[robot_id].tactic_name
          )

playinfo has type is a google protobuf type. playinfo.robot_tactic_assignment returns a dictionary. So to access this we need an id like robot_id. playinfo.robot_tactic_assignment[robot_id] is this type:

Software/src/proto/play_info_msg.proto

Lines 12 to 16 in 10bded3

message Tactic
{
string tactic_name = 1;
string tactic_fsm_state = 2;
}

To get the tactic_name we do playinfo.robot_tactic_assignment[robot_id].tactic_name

)

set_table_data(
Expand Down
33 changes: 15 additions & 18 deletions src/software/thunderscope/play/refereeinfo_widget.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from google.protobuf.json_format import MessageToDict
from pyqtgraph.Qt.QtWidgets import *
from proto.import_all_protos import *
from software.py_constants import SECONDS_PER_MICROSECOND, SECONDS_PER_MINUTE
Expand Down Expand Up @@ -36,15 +35,13 @@ def __init__(self, buffer_size: int = 1) -> None:

def refresh(self) -> None:
"""Update the referee info widget with new referee information"""
referee = self.referee_buffer.get(block=False, return_cached=False)
referee = self.referee_buffer.get(block=False, return_cached=False).GetOptions()

Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to call GetOptions here

# Updating QTableWidget could be expensive, so we only update if there is new data
if referee is None:
return

referee_msg_dict = MessageToDict(referee)

if not referee_msg_dict:
if not referee:
return

Comment on lines -46 to 45
Copy link
Contributor

Choose a reason for hiding this comment

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

this check is duplicated above, you can delete this now

stage_time_left_s = (
Expand All @@ -54,27 +51,27 @@ def refresh(self) -> None:
f"Packet Timestamp: {round(float(referee_msg_dict['packetTimestamp']) * SECONDS_PER_MICROSECOND, 3)}\n"
+ f"Stage Time Left: {int(stage_time_left_s / SECONDS_PER_MINUTE):02d}"
+ f":{int(stage_time_left_s % SECONDS_PER_MINUTE):02d}\n"
+ f"Stage: {referee_msg_dict['stage']}\n"
+ f"Stage: {referee.stage()}\n"
+ "Command: "
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need a (), so just referee.stage

+ referee_msg_dict["command"]
+ referee.command()
+ "\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

+ f"Blue Team on Positive Half: {referee_msg_dict['blueTeamOnPositiveHalf']}\n"
+ f"Blue Team on Positive Half: {referee.blueTeamOnPositiveHalf()}\n"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

here as well

self.referee_info.setText(p)

team_info = []
blue = []
yellow = []

for team_info_name in referee_msg_dict["blue"]:
for team_info_name in referee.blue():
if team_info_name == "timeouts":
team_info.append("remainingTimeouts")
elif team_info_name == "goalkeeper":
team_info.append("goalkeeperID")
else:
team_info.append(team_info_name)

for team_info_name in referee_msg_dict["yellow"]:
for team_info_name in referee.yellow():
if team_info_name in team_info:
continue

Expand All @@ -87,17 +84,17 @@ def refresh(self) -> None:

for info in team_info:
if info == "yellowCardTimes":
blue.append(self.parse_yellow_card_times(referee_msg_dict["blue"]))
yellow.append(self.parse_yellow_card_times(referee_msg_dict["yellow"]))
blue.append(self.parse_yellow_card_times(referee.blue()))
yellow.append(self.parse_yellow_card_times(referee.yellow()))
elif info == "remainingTimeouts":
blue.append(referee_msg_dict["blue"]["timeouts"])
yellow.append(referee_msg_dict["yellow"]["timeouts"])
blue.append(referee.blue().timeouts())
yellow.append(referee.yellow().timeouts())
elif info == "goalkeeperID":
blue.append(referee_msg_dict["blue"]["goalkeeper"])
yellow.append(referee_msg_dict["yellow"]["goalkeeper"])
blue.append(referee.blue().goalkeepers())
yellow.append(referee.yellow().goalkeeper())
else:
blue.append(referee_msg_dict["blue"][info])
yellow.append(referee_msg_dict["yellow"][info])
blue.append(referee.blue().info())
yellow.append(referee.yellow().info())

Comment on lines 86 to +97
Copy link
Contributor

Choose a reason for hiding this comment

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

you actually don't need the () here.

set_table_data(
{
Expand Down
Loading