Conversation
AadarshHegde123
assigned
SimonKato,
maoyifei2013605,
dazhenc2,
AadarshHegde123 and
jyl2017
krishauser
left a comment
krishauser
left a comment
There was a problem hiding this comment.
Can you explain why you took out goal and added parking_goal? This seems to conflict with summoning / inspection
Oh, I did not realize they also make use of the same goal variable. The rename was just for clarity on our end. I can change this back. |
|
Changed parking_goal back to goal to avoid conflicts. |
|
@krishauser All good on our end for re-review. I see that there are a couple merge conflicts so I will note the differences and give explanations here: gem_e4_dynamics.yaml: The gem e4 dynamics model we chose is selected because it prevents some obscure behavior in both simulator and on-board for our tests. longitudinal_planning: The code is largely the same, but we have refactored such that hard-coding has been eliminated so you can now specify different arguments via the launch file. route_planning_component.py: This conflict seemed pretty trivial since it was mainly complaining about a difference in imports and some helper functions from inspection team that were missing in our file. I fixed this in the web editor and marked as resolved. |
krishauser
left a comment
krishauser
left a comment
There was a problem hiding this comment.
Need coordination with control team. Either @Jugthegreat can comment or message me to approve these changes.
There was a problem hiding this comment.
Was this cleared with the control team?
There was a problem hiding this comment.
I spoke with Jugal. We had an out outdated pid_p value that we have now updated to match Controls. We do however need crosstrack_gain set to 0.5 and @Jugthegreat has agreed that this is an acceptable change.
There was a problem hiding this comment.
Was this cleared with the control team?
There was a problem hiding this comment.
Control Team is using kris model so this may create some conflicts. I am talking with parking team to sort this out
There was a problem hiding this comment.
It seemed we had an outdated pid_p value that seemed to affect the kris model. We have re-tested with the kris_v1 model and performance is as expected. We have changed this file to match control team, so there should no longer be any issue.
There was a problem hiding this comment.
Did you clear this with the control team?
There was a problem hiding this comment.
Yes, I spoke with Jugal and he has said that Controls is fine with the changes made here.
|
@krishauser This should be ready again for re-review, we have talked everything over with controls team. In regards to the merge conflicts, I have resolved resolve parking_detection.py and route_planning_component.py, but the existing longitudinal_planning.py on s2025 from my tests seems non-functional (see attached video). I propose having two separate motion_planning files, the current existing one on s2025 since I am guessing this is what worked for inspection team if they merged it, and this parking_motion_planning file that I have created. These files have diverged significantly to the point where I think its reasonable to have them separate. I have made this change to this branch. Let me know if you have any concerns with this. long_unfunctional.MOV |
|
7 participants
This PR builds atop the recently merged Parking + Perception PR: #174
We add our planning logic on top of this PR to support the full end to end pipeline of perpendicular parking.