jaak-peterson / autoware_mini_practice

MIT License
0 stars 0 forks source link

Practice 3 #2

Closed geopimik closed 4 months ago

geopimik commented 6 months ago

Please fix the following:

  1. Just a matter of taste, but I would move this group below the #Parameters and add an explanatory title, like global variables or similar. Parameters are the first because the values come from outside and they determine the node behaviour. Global variables are internal to that node. https://github.com/jaak-peterson/autoware_mini_practice/blob/0377df857d517db02bcb1db841aabd2f651e890c/practice_3/nodes/control/pure_pursuit_follower.py#L15-L16

  2. You have exactly the same for loop over msg.waypoints two times. Reorganize the code that the loop is done only once. https://github.com/jaak-peterson/autoware_mini_practice/blob/0377df857d517db02bcb1db841aabd2f651e890c/practice_3/nodes/control/pure_pursuit_follower.py#L31-L32

  3. As discussed in the lecture, these two assignments:

import threading

# inside class init
self.lock = threading.Lock()

# in the callback
with self.lock:
    self.path_linestring = path_linestring
    self.distance_to_velocity_interpolator = distance_to_velocity_interpolator
geopimik commented 5 months ago
  1. OK
  2. OK
  3. Some comments:

path_callback

https://github.com/jaak-peterson/autoware_mini_practice/blob/8136687018f903b9cee09250bd1b6d785b69cf3b/practice_3/nodes/control/pure_pursuit_follower.py#L47-L49

I would first create interp1d into local variable and in the self.lock group do only the assignment to global (class) variable.

current_pose_callback

https://github.com/jaak-peterson/autoware_mini_practice/blob/8136687018f903b9cee09250bd1b6d785b69cf3b/practice_3/nodes/control/pure_pursuit_follower.py#L51-L57

geopimik commented 4 months ago

OK