Vberenz/o80 robot ball#19
Open
vincentberenz wants to merge 2 commits intomasterfrom
Open
Conversation
luator
reviewed
Jan 12, 2023
Member
luator
left a comment
luator
left a comment
There was a problem hiding this comment.
I have a few comments but nothing critical.
Apart from this, there are some flake8 and mypy errors in the new code (see annotations in the diff). I think it would be good to fix them before merging.
| return stamps, positions | ||
|
|
||
|
|
||
| def _p(l): |
Member
There was a problem hiding this comment.
This function could have a more descriptive name.
| The number of trajectories added to the file. | ||
| """ | ||
|
|
||
| def _read_trajectory(o80_robot_ball_file: pathlib.Path) -> StampedTrajectory: |
Member
There was a problem hiding this comment.
The amount of nested functions here makes it a bit difficult to follow the code. Maybe it would make sense to move them to a separate module?
| List all the file in tennicam_path that have the tennicam_ prefix, | ||
| parse them and returns the corresponding list of stamped trajectories. | ||
| """ | ||
| files = _list_files(o80_robot_ball_path, prefix="o80_robot_ball_") |
Member
There was a problem hiding this comment.
There seems to be a mismatch between the prefix mentioned in the docstring and the one actually used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
So far, ball trajectories could be added to a hdf5 file only from "older" json files (recorded in the past by Dieter and Simon) or by tennicam files (record of ball trajectories by tennicam).
This pull requests allows to add ball and robot trajectories recorded at the same time and dumped into files by o80_pam / o80_robot_ball_logger
How I Tested
Recorded ball / robot trajectories are replayed them
Do not merge before
I fulfilled the following requirements