Lane Follower Systems into dev #211

Vaibhav-Hariani · 2024-09-16T22:59:51Z

Post-Comp merge of all lane follower code into dev, with atomic commits. Cleaning up systems to make them more legibile and more clear.

jacobkoziej

Please split the code into smaller functions so that it's easier to follow. The use of einops might also be nice. I would also add type annotations to variables so that we have a better idea of how the data is getting permuted.

jacobkoziej · 2024-09-17T04:34:08Z

components/lane_follower/src/individual_follower.py

+# Lane Follower for a single camera
+# Applies a perspective transform to an image, and isolates lane lines


Please create a component.toml with this information.

jacobkoziej · 2024-09-17T04:36:17Z

components/lane_follower/src/individual_follower.py

+import numpy as np
+import math
+import cv2


Imports should be sorted alphabetically and have whitespace between a standard lib imports and 3rd-party improts. They should also come before 3rd-party imports.

jacobkoziej · 2024-09-17T04:36:45Z

components/lane_follower/src/individual_follower.py

+    # Constants for how we process images
+    # These should be parametrized and better defined at a later date: as part of testing


Suggested change

# Constants for how we process images

# These should be parametrized and better defined at a later date: as part of testing

jacobkoziej · 2024-09-17T04:37:27Z

components/lane_follower/src/individual_follower.py

+    NWINDOWS = 9
+    SEARCH_WIDTH = 100
+    MINPIXELS = 40
+    LANE_POLY_SIZE = 2
+
+    # Rendering Parameters
+    BOX_COLOR = (0, 255, 0)
+    LANE_COLOR = (255, 0, 0)
+
+    DRAW_THICKNESS = 2


I would make a configuration dataclass so that we can easily pass them to a lane-follower. I would also give them the same defaults.

Created a .toml

jacobkoziej · 2024-09-17T04:38:47Z

components/lane_follower/src/individual_follower.py

+    # Need to determine if these should remain properties
+    # Or if they should be passed as arguments to Plot_Line
+    @property
+    def _binary_warped(self, value: np.ndarray):
+        self._binary_warped = value
+        self.histogram = np.sum(
+            self.binary_warped[self.binary_warped.shape[0] // 2 :, :], axis=0
+        )


I'm not sure I follow why you're making this a property? If this is a getter, shouldn't it be a public member?

jacobkoziej · 2024-09-17T04:39:12Z

components/lane_follower/src/individual_follower.py

+
+    def plot_line(self):
+
+        if self.binary_warped is None:


Suggested change

if self.binary_warped is None:

if not self.binary_warped:

components/lane_follower/src/individual_follower.py

jacobkoziej · 2024-09-17T04:44:42Z

components/lane_follower/src/individual_follower.py

+import cv2
+
+
+class IndividualFollower:


Suggested change

class IndividualFollower:

class LaneFollower:

Not making this change as lane follower is an overarching class that creates 2 individual followers; one for each camera.

Seriously need to reconsider structure of line_window1 & line_window2: These lines are only for rendering, but seem to be half running in the loop and half out. This doesn't make a lot of sense? I think they can be fully replaced with the resultant np.polyval

…r better readability

… full module to dev

jacobkoziej · 2024-10-17T20:36:57Z

components/lane_follower/src/individual_follower.py

+import toml
+from einops import repeat


Suggested change

import toml

from einops import repeat

import toml

from einops import repeat

jacobkoziej · 2024-10-17T20:37:16Z

components/lane_follower/src/individual_follower.py

+from einops import repeat
+
+
+class ifResult:


Please follow PEP8 for naming. This can also be made into a dataclass.

jacobkoziej · 2024-10-17T20:39:36Z

components/lane_follower/src/individual_follower.py

+    def __init__(self, toml_path='Follower_Consts.toml'):
+
+        self.consts = toml.load(toml_path)["Individual_Follower"]


I like the vision, but the execution can be better. IMO TOML should be unmarshaled outside the class and passed as a constants struct.

jacobkoziej · 2024-10-17T20:40:38Z

components/lane_follower/src/individual_follower.py

+        # self.fit is set in Plot_Line
+        self.fit = None
+        # These two are set below
+        self._binary_warped = None
+        self._histogram = None


I don't think these comments add much value to the reader. I'd also separate public and private members of the class.

jacobkoziej · 2024-10-17T20:41:43Z

components/lane_follower/src/individual_follower.py

+    # Need to determine if these should remain properties
+    # Or if they should be passed as arguments to Plot_Line
+    @property
+    def _binary_warped(self, value: np.ndarray):


@property should only get used for public members and without arguments so that they just act as a variable access.

jacobkoziej · 2024-10-17T20:50:20Z

components/lane_follower/src/individual_follower.py

+            )
+        return out_img
+
+    def plot_line(self) -> ifResult:


Can you search online how other projects might add code to generate plots for certain things? There has to be a cleaner way to do this.

jacobkoziej · 2024-10-17T20:52:14Z

components/lane_follower/src/lane_follower.py

Dead code should just not be committed as it's accessible in Git history.

jacobkoziej · 2024-10-17T20:52:47Z

components/lane_follower/src/pyproject.toml

I know we haven't discussed this yet, but here's the current schema for a component:

selfdrive/site_scons/site_tools/Component.py

Line 9 in 2f99103

schema = Schema(

jacobkoziej · 2024-10-17T20:53:57Z

components/lane_follower/src/pyproject.toml

+  {name = "Vaibhav Hariani"},
+  {name = "Isaiah Rivera"},
+]
+description = "IGVC lane follower module for a single camera"


Suggested change

description = "IGVC lane follower module for a single camera"

description = "lane follower for a video feed"

Something along those lines.

jacobkoziej · 2024-10-17T20:54:53Z

components/lane_follower/src/Follower_Consts.toml

It's probably better to just have these as dataclasses that have "sane" defaults.

Vaibhav-Hariani added 4 commits September 15, 2024 12:39

added individual follower class definition

a9212b8

added constants and property for binary warp

f8c5ad7

added window iteration and drawing

d0b6223

full refactoring of individual follower

187bd1b

Vaibhav-Hariani self-assigned this Sep 16, 2024

jacobkoziej requested changes Sep 17, 2024

View reviewed changes

Vaibhav-Hariani and others added 8 commits September 28, 2024 20:40

created lf_const.toml, a pyproject.toml, and added type hinting

8c363c8

Merge branch 'dev' into user/Vaibhav-Hariani/lane-follower

473134b

Tried removing some of the numpy vomit

c7d7692

Added .toml, piping constants into individual folllower from there fo…

a4a018f

…r better readability

Building indermediary function for code readability

20d1e0b

Broke individual follower up into loop & not loop

e5fd3d1

Added Lane Follower constants to .toml, beginning effort to move this…

d0c80ed

… full module to dev

jacobkoziej requested changes Oct 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lane Follower Systems into dev #211

Lane Follower Systems into dev #211

Vaibhav-Hariani commented Sep 16, 2024

jacobkoziej left a comment

jacobkoziej Sep 17, 2024

jacobkoziej Sep 17, 2024

Vaibhav-Hariani Oct 14, 2024

jacobkoziej Sep 17, 2024

jacobkoziej Sep 17, 2024

Vaibhav-Hariani Oct 14, 2024

jacobkoziej Sep 17, 2024

jacobkoziej Sep 17, 2024

jacobkoziej Sep 17, 2024

Vaibhav-Hariani Oct 14, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

jacobkoziej Oct 17, 2024

		# Lane Follower for a single camera
		# Applies a perspective transform to an image, and isolates lane lines

		# Constants for how we process images
		# These should be parametrized and better defined at a later date: as part of testing

		def __init__(self, toml_path='Follower_Consts.toml'):

		self.consts = toml.load(toml_path)["Individual_Follower"]

	description = "IGVC lane follower module for a single camera"
	description = "lane follower for a video feed"

Lane Follower Systems into dev #211

Are you sure you want to change the base?

Lane Follower Systems into dev #211

Conversation

Vaibhav-Hariani commented Sep 16, 2024

jacobkoziej left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment