-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Next gen lidar parts #918
Next gen lidar parts #918
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, only some minor comments.
@DocGarbanzo thank you very very much for the code review. It made me go back and look more deeply at the code. If found a couple of bugs while doing so. |
donkeycar/parts/lidar.py
Outdated
""" | ||
angle = angle | ||
min_angle = min_angle | ||
max_angle = max_angle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 3 lines are obsolete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I remove the redundant lines
donkeycar/parts/lidar.py
Outdated
self.full_scan_index += 1 | ||
|
||
except serial.serialutil.SerialException: | ||
logger.info('SerialException from RPLidar.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably should be a
logger.error(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And so it should be, and so it is.
donkeycar/parts/lidar.py
Outdated
|
||
# background | ||
draw.rectangle(bounds, fill=self.background_color) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additional blank line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eliminated. Thanks.
- added a new part that streams measurements rather than scans in order to reduce the latency between retrieving full scans. - The part also will filter measurements by angle and distance. - The part will also adjust angles so that they increase in a counter-clockwise direction as is expected generally. The default is still angles increasing in a clockwise direction, since that is what the RPLidar does naturally. - The part will also adjust the zero-heading. This will adjust for different mounting of the lidar such that the physical front of the lidar is not longer pointing in the logical front direction. By default this is zero and the most common mounting of the RPlidar on a vehicle would use the default. Also added LidarPlot2, which fixes a few issues in the old plotter - It now assumes cartesian coordinates where y increases going up. The old LidarPlot (which remains untouched) used the pixels coordinate system where y increased going down. This combined witht the fact that the RPLidar was producing angles that increased in a clockwise direction, allowed the plotting of RPLidar work correctly. However, since we are now assuming that angles increase in a counter-clockwise direction, we need to plot in cartesian coordinates. - This will also take an argument for rotating the plot. By default the zero heading is on the positive x axis, which is 'East'. The rotating 90 degrees, the zero heading can point 'North' for instance. This is useful when aligning to an image take by a FPV camera. - This draws a line along the zero heading. Added a __main__ so the part can be run from the command line - This will draw the scans to a window. - Scan rate can be specified. - total number of scans can be specified - filtering for angle and distance can be specified. - The plot can be rotate. - In the future I hope to make this support other lidars.
- factored out funcs for drawing points and drawing a point - created funcs to draw polar bounds and zero heading
- for instance, if you want the 180 degrees in front of the lidar, then specify min -> max of 270 -> 90. That then is broken into between 270 -> 360 and 0 -> 90 for testing. - In comparison, specifying 90 -> 270 would be the 180 degrees behind the lidar
- shorten long lines - use one space between members of a class - use two spaces between classes
- refactored to create a poll() method that reads a single measurment. update() method calls poll in a loop. - added a run() method that will grab measurements in batches - refactored main to allow it to run in threaded or non-threaded mode. In non-threaded mode the user can quit by pressing the 'q' or escape key.
- single threaded operation used batch_ms to decide how long to loop getting measurements in the run() method.
- rebased against main 4.3.10
f56d0d3
to
b67066f
Compare
Rebased against main. |
setup.py
Outdated
@@ -24,7 +24,7 @@ def package_files(directory, strip_leading): | |||
long_description = fh.read() | |||
|
|||
setup(name='donkeycar', | |||
version="4.3.10", | |||
version="4.3.11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be 4.3.14 now - don't know why GitHub doesn't show it as conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merged from main and resolved the version conflict.
There is a desire to integrate 2D lidar data more completely into the donkeycar pipeline - see issue 910. In particular the idea is to integrate the lidar data with the front-facing camera image. In addition there are some issues with regarding accuracy and latency. This pull request endeavours to tackle both problems by creating new lidar parts for acquiring data from the RPLidar and plotting data.
There a few problems with the current RPLidar and LidarPlot parts.
These issues have been addressed by adding new versions of the parts (RPLidar2 and LidarPlot2) which presume mathematical angles that increase in a counter clockwise direction and are plotted on the Cartesian plane where y increases going up. This can be adjusted with parameters. In addition the new RPLidar2 streams measurements and makes them available immediately without waiting for a full scan. A main has been added to visualizing the output of the lidar without having to run a full donkeycar.
The current RPLidar and LidarPlot parts remain for those that have code that depends upon them.
Added RPLidar2 part for streaming measurements from an RPLidar.
Also added LidarPlot2, which fixes a few issues in the old plotter.
Added a main so the part can be run from the command line