search

AnishShr / autoware_mini_practice

Practice Sessions and Labs for the Autonomous Vehicles Project (Autoware mini)
MIT License
0 stars 0 forks source link

practice 6 - simple_local_planner - code comments #10

Closed geopimik closed 4 months ago

geopimik commented 5 months ago

Please fix the following:

  1. No need to assign parameter values that are not about to change to local variables. You can use them directly in the callback using the class (global) variable name https://github.com/AnishShr/autoware_mini_practice/blob/21d92c582d289c5f9fc24b477ca34f7889b10b66/practice_6/nodes/planning/local/simple_local_planner.py#L102-L116

  2. I would bring these out of the self.lock group. Important is that all the things coming from path callback stay consistent, so they are in the lock group. But current pose and velocity come from separate topics and with different frequency, so no point having them in that group. https://github.com/AnishShr/autoware_mini_practice/blob/21d92c582d289c5f9fc24b477ca34f7889b10b66/practice_6/nodes/planning/local/simple_local_planner.py#L95-L100

  3. This is wrong and it is not used in this callback later. Not necessary! https://github.com/AnishShr/autoware_mini_practice/blob/21d92c582d289c5f9fc24b477ca34f7889b10b66/practice_6/nodes/planning/local/simple_local_planner.py#L134

  4. Important is to publish empty local path! Then follower will get it empty and will stop the go vehicle if it is not stopped already! https://github.com/AnishShr/autoware_mini_practice/blob/21d92c582d289c5f9fc24b477ca34f7889b10b66/practice_6/nodes/planning/local/simple_local_planner.py#L141-L143

  5. You should not return from here! If we don't get the transform, then we set it to None and we still need to detect the obstacles publish the local path and calculate the target velocity. Now we will just assume that objects have 0 velocity since we don't have a reliable transform. https://github.com/AnishShr/autoware_mini_practice/blob/21d92c582d289c5f9fc24b477ca34f7889b10b66/practice_6/nodes/planning/local/simple_local_planner.py#L154-L163

AnishShr commented 5 months ago

1&2. Put only relevant variables inside the self.lock block https://github.com/AnishShr/autoware_mini_practice/blob/db7de2e51eb2ac93c232ab33cd755905c6f50521/practice_6/nodes/planning/local/simple_local_planner.py#L93-L98

  1. Removed this variable
  2. published empty local path when local_path is None before returning
  3. Removed the return line an later assigned zero velocity if transform is not there https://github.com/AnishShr/autoware_mini_practice/blob/db7de2e51eb2ac93c232ab33cd755905c6f50521/practice_6/nodes/planning/local/simple_local_planner.py#L153-L161 https://github.com/AnishShr/autoware_mini_practice/blob/db7de2e51eb2ac93c232ab33cd755905c6f50521/practice_6/nodes/planning/local/simple_local_planner.py#L181-L185
AnishShr commented 5 months ago

latest commit: https://github.com/AnishShr/autoware_mini_practice/commit/7d19ddadcd79e106cadf593cf4e87f63427ab2fd

geopimik commented 4 months ago

OK