Skip to content

fix: refactor state to remove duplication and improve draw performance - #132

Open
Cid-oe wants to merge 2 commits into
ShisatoYano:mainfrom
Cid-oe:fix-issue-130
Open

Cid-oe wants to merge 2 commits into
ShisatoYano:mainfrom
Cid-oe:fix-issue-130

Conversation

@Cid-oe

@Cid-oe Cid-oe commented Oct 4, 2026

Copy link
Copy Markdown

Closes #130

@ShisatoYano

Copy link
Copy Markdown
Owner

Hi @Cid-oe, thank you for picking up #130! The PR covers every point of the issue in a small, focused diff: the speed limiting now lives in one _limit_speed() with the original order kept, input is renamed to motion_input (docstring included), update_by_localizer() has its docstring, and the history plot is updated with set_data instead of being re-created. I checked it locally: all 96 tests pass, and State.draw cumulative time in pure_pursuit_path_tracking drops from 0.27 s to 0.12 s over 251 frames.

Please address the following before merge:

  1. The trajectory is now drawn under the course. Because the history line is created once and kept, the course line (re-created every frame) is drawn on top of it, and the black tracking-error dots are mostly hidden. On the final frame of pure_pursuit_path_tracking, 5,310 px differ from main. Adding zorder=2.5 to the axes.plot(...) call brings it back to main's look (11 px difference).
  2. Please add a short comment explaining that hist_plot is intentionally not appended to elems (it is kept so set_data can update it). Every other draw() appends to elems, so readers may "fix" it otherwise.
  3. Small style points: rename it to _hist_plot since it is internal, and add a one-line docstring to _limit_speed().
  4. (Optional) To stay safe if the same State is ever drawn on a new axes, you could re-create the line when self._hist_plot.axes is not axes.

Thanks again, looking forward to the update!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Clean up State and keep trajectory drawing fast

2 participants