search

CMU-Robotics-Club / RoboBuggy2

A complete re-write of the old RoboBuggy.
GNU General Public License v3.0
2 stars 0 forks source link

Rk4 sim #41

Closed PatXue closed 4 months ago

PatXue commented 4 months ago

What type of PR is this? (check all applicable)

Description

Change physics simulator step function to use RK4 instead of midpoint rule for improved simulation accuracy.

QA Instructions, Screenshots, Recordings

Ran simulator on Gabriel's laptop and buggy appeared to travel as expected. Simulator ran very slowly (10Hz), but doesn't seem to be related to these changes, since even after undoing all the changes it ran at the same speed.

PatXue commented 4 months ago

Why does pylint think this commit changed every file in rb_ws?

christianvluu commented 4 months ago

Why does pylint think this commit changed every file in rb_ws?

Can you try pulling in/merging in main? Also which branch did you guys start from?

PatXue commented 4 months ago

Why does pylint think this commit changed every file in rb_ws?

Can you try pulling in/merging in main? Also which branch did you guys start from?

Ah, we started from main 2 weeks ago, and there's been some changes so that's probably why

christianvluu commented 4 months ago

Okay I’m feeling line endings change? @Jackack had this issue before so perhaps he can diagnose if that’s what causing this.

PatXue commented 4 months ago

Okay I’m feeling line endings change? @Jackack had this issue before so perhaps he can diagnose if that’s what causing this.

Is that the LF/CRLF thing in the bottom right? I have it set to LF

Jackack commented 4 months ago

If you look at the pylint output for each of the commits in this branch, pylint only complained about engine.py until the "fix pylint" commit, which caused pylint to complain about a bunch of other files.

Jackack commented 4 months ago

Just a hunch, might be merging the SW/Ui velocity PR from main that caused this

Jackack commented 4 months ago

also what did you do in the merge branch main commit? This branch is still behind main. Did you pull main from origin first when you merged?

Jackack commented 4 months ago

I took a look at the git log of this branch.

Since pylint checks the files that are different from main, I think what happened is:

  1. main got updated because PRs were merged
  2. you pushed this morning, without merging main into your branch
  3. because your branch has files from an older version of main, pylint sees them and thinks: these files are different from the current main, let's check them
  4. because these files are from before pylint existed and don't have proper style, pylint complains.
  5. you merge main into your code, but without pulling main first. Your local main branch has the ui velocity PR, but not the auton overtake PR.
  6. the files that pylint were mad about are still behind main, so pylint still complains

The fix would be to pull from main, and then merge main into your branch again.

PatXue commented 4 months ago

I took a look at the git log of this branch.

Since pylint checks the files that are different from main, I think what happened is:

  1. main got updated because PRs were merged
  2. you pushed this morning, without merging main into your branch
  3. because your branch has files from an older version of main, pylint sees them and thinks: these files are different from the current main, let's check them
  4. because these files are from before pylint existed and don't have proper style, pylint complains.
  5. you merge main into your code, but without pulling main first. Your local main branch has the ui velocity PR, but not the auton overtake PR.
  6. the files that pylint were mad about are still behind main, so pylint still complains

The fix would be to pull from main, and then merge main into your branch again.

Ah ok it seems to have worked now after pulling and merging again

Jackack commented 4 months ago

Tested on my laptop, sim frequency is good. Approved.