Skip to content
This repository has been archived by the owner on Jan 23, 2024. It is now read-only.

Feature: refactor and test team comm #127

Merged
merged 17 commits into from
Feb 23, 2023
Merged

Feature: refactor and test team comm #127

merged 17 commits into from
Feb 23, 2023

Conversation

texhnolyze
Copy link
Contributor

@texhnolyze texhnolyze commented Jan 16, 2023 •
edited
Loading

Proposed changes

Major refactoring of team communication including the following:

  • split of conversion State <-> Protobuf <-> TeamData into own converters
  • extract server/connection setup
  • introduce full typing
  • unit tests of the conversion with pytest and syrupy
  • a lot of bugfixes (concerning both logic, as well as ros2 issues)
  • fixing of scripts for testing
  • move to soccer 3d vision RobotArray msgs on the /robots_relative topic, as we never publish ObstacleRelativeArray

Related issues

Necessary checks

  • Update package version
  • Run colcon build
  • Write documentation
  • Create issues for future work
  • Test on your machine
  • Test on the robot
  • Put the PR on our Project board

@texhnolyze
Copy link
Contributor Author

I'm still doing some testing and there are also still @TODOs in the code, but as this is a big PR I think it makes sense, that you already have a look over the general stuff.
For most of the TODOs I could also use your feedback.

@texhnolyze texhnolyze force-pushed the feature/refactor-and-test-team-comm branch from f2d8938 to d867aef Compare January 19, 2023 17:10
Flova
Flova reviewed Jan 20, 2023
View reviewed changes
Copy link
Member

@Flova Flova left a comment

Choose a reason for hiding this comment

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

It looks really good! I left some comments addressing hopefully all the TODOs as well as some minor stuff.

humanoid_league_team_communication/humanoid_league_team_communication/communication.py Outdated Show resolved Hide resolved
humanoid_league_team_communication/humanoid_league_team_communication/communication.py Outdated Show resolved Hide resolved
humanoid_league_team_communication/humanoid_league_team_communication/communication.py Outdated Show resolved Hide resolved
humanoid_league_team_communication/humanoid_league_team_communication/communication.py Outdated Show resolved Hide resolved
humanoid_league_team_communication/humanoid_league_team_communication/communication.py Show resolved Hide resolved
..._team_communication/humanoid_league_team_communication/humanoid_league_team_communication.py Outdated Show resolved Hide resolved
..._team_communication/humanoid_league_team_communication/humanoid_league_team_communication.py Show resolved Hide resolved
..._team_communication/humanoid_league_team_communication/humanoid_league_team_communication.py Show resolved Hide resolved
..._team_communication/humanoid_league_team_communication/humanoid_league_team_communication.py Outdated Show resolved Hide resolved
humanoid_league_team_communication/scripts/test_team_comm.py Outdated Show resolved Hide resolved
@texhnolyze texhnolyze force-pushed the feature/refactor-and-test-team-comm branch from d867aef to 1dd18b3 Compare January 31, 2023 20:31
@texhnolyze texhnolyze changed the title WIP: Feature: refactor and test team comm Feature: refactor and test team comm Jan 31, 2023
@texhnolyze texhnolyze force-pushed the feature/refactor-and-test-team-comm branch from 1dd18b3 to cfab407 Compare February 13, 2023 22:59
@Flova
Copy link
Member

Flova commented Feb 15, 2023

But isn't the target configurable and thus dynamic? Or what do you mean

But semantically it should still be the map frame. Maybe prefixed by a robot name or something like that. tf_transform sounds very generic and not like a function that only transforms into the map frame.

@texhnolyze
style(team_comm): apply google python styleguide
e086aab
@texhnolyze
fix(team_comm): correctly pass args on thread setup
5d0d0bb
@texhnolyze
refactor(team_comm): extract UDP socket communication
c134c75
@texhnolyze
refactor(team_comm): add typing and helper methods
8480c2d
@texhnolyze
fix(team_comm): checking for expired msg data
c9698fd 
as timestamps from headers now need to be converted as the times in
messages are not `rclpy.time.Time` classes
@texhnolyze
refactor(team_comm): fix/improve scripts for ros2 usage
7f29d05
@texhnolyze
refactor(team_comm): extract conversion between ros/protobuf
80f40eb
@texhnolyze
test(team_comm): unit and syrupy snapshot tests
42837fb 
for conversion between ros state <-> protobuf <-> team data
@texhnolyze
test(team_comm): add tmux testing script
dcf2176 
opening multiple panes and launching the TeamComm, while also listening
and showing messages from other robots
@texhnolyze
fix(team_comm): switch to RobotArray msgs on robots_relative
cbf04a2 
topic, instead of ObstaclRelativeArray on `obstacles_relative` topic,
where we do not publish any msgs.

Currently the vision produces RobotArray msgs on `robots_in_image`,
which are then transformed relative to the `base_footprint` map frame to
have relative coordinates based on the running robot's location.
This is done in the ipm (inverse projection mapping), which in turn
publishes `robots_relative`.
For now we totally humans and other obstacles, which could be handled
with ObstaclRelative msgs.

Currently the TeamData msgs also still contains an ObstaclRelativeArray,
because change it would most likely also require changes in the behavior
module.
This will be done at a later stage.

Fixes: #125
@texhnolyze
refactor(team_comm): communication code review
472c66e 
intoducing handling of truncated messages if a protobuf message should
be larger than 1024 bytes, we then double the buffer size for the next
iteration
@texhnolyze
refactor(team_comm): code review optional typings
06c2432 
and other smaller fixes
@texhnolyze texhnolyze force-pushed the feature/refactor-and-test-team-comm branch 2 times, most recently from 49e6154 to ce0894b Compare February 21, 2023 18:34
This was referenced Jan 23, 2024
Closed
Closed
@texhnolyze texhnolyze force-pushed the feature/refactor-and-test-team-comm branch from 6755953 to 798dfdb Compare February 21, 2023 19:00
@texhnolyze
fix(team_comm): transform to "map" frame in test script
bee6fb3 
and improve error messages for transform errors
@texhnolyze texhnolyze force-pushed the feature/refactor-and-test-team-comm branch from 798dfdb to a5f3cf8 Compare February 21, 2023 19:09
@texhnolyze texhnolyze merged commit 5c6811c into master Feb 23, 2023
@texhnolyze texhnolyze deleted the feature/refactor-and-test-team-comm branch February 23, 2023 15:55
@Flova Flova mentioned this pull request Mar 8, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Flova Flova Flova approved these changes

@timonegk timonegk Awaiting requested review from timonegk

@jaagut jaagut Awaiting requested review from jaagut

@SammyRamone SammyRamone Awaiting requested review from SammyRamone

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Misc
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

team communication should use soccer_vision_3d_msgs for obstacles
2 participants
@texhnolyze @Flova