Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Refactore/bbt position control #1403

Open
wants to merge 10 commits into
base: development
Choose a base branch
from

Conversation

mamiksik
Copy link
Contributor

Comments for the reviewer

Pre pull request checklist:

Code Quality
  • Is the code is understandable and easy to read
  • Changes to the code comply with set clang-format rules
  • No use of manual memory control (e.g new/malloc/colloc etc)
  • Are (only) smart pointers used?
Testing
  • All tests are passing.
  • I added new / changed existing tests to reflect code changes (state why not otherwise!)
  • I tested my changes manually (Describe how, to what extent etc.)
Commit Messages
  • Commit message is saying what has been changed, why it was changed? Remember other developers might not know
    what the problem you are fixing was. Note also negative decision (e.g., why did you not do particular thing)
    TLDR: Commit message are comprehensive
  • Commit messages follows the rules of https://chris.beams.io/posts/git-commit/

Since BBT was implemented there was a lot of dead code in the repo. Note
that method setRobotPositions was not removed yet. Removing it
causes linking issues (wtf?)
- Since old collision detector is removed, world objects can be renamed to collision detector
  (a way better name imho)
- Collision Detector now precomputes the positions of all robots at the start of each tick,
  to (theoretically) save time when performing collision checks (lookup vs computation)

Next the Position Control has to be refactored!
There are 2 main reasons for this change:
1) The previous path planning was slow
2) The previous implementation was spaghetti

The new implementation speeds up the position control significantly (60 ticks per second with -0g)

All the details are recorded in docstrings, but main idea of the new implementation was to separate
better separate concerts(path tracking x path planning x position control). For example path tracking
is now stateful and there is instance for each robot separately.
@mamiksik mamiksik force-pushed the refactore/bbt-position-control branch from da1ed20 to 0f1902c Compare June 23, 2022 12:51
@mamiksik mamiksik changed the title [draft] Refactore/bbt position control Refactore/bbt position control Jun 24, 2022
Small change to remove unnecessary variable
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant