Skip to content
This repository was archived by the owner on Nov 16, 2023. It is now read-only.

Conversation

@timdreier
Copy link
Contributor

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #211

Time invested

Tim Dreier: 11h

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Does this PR introduce a breaking change?

No :)

Most important changes

If you start the simulation you will see the filtered PointCloud:

image

image

Additionally, you can check the output of the range messages (closest and fairest point) by running:

$ b5 shell
$ rostopic echo /carla/hero/LIDAR_range

You should then see the result in the following form:

---
header: 
  seq: 54
  stamp: 
    secs: 0
    nsecs:         0
  frame_id: ''
radiation_type: 0
field_of_view: 0.0
min_range: 13.809732437133789
max_range: 14.744861602783203
range: 13.809732437133789
---

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works (might be obsolete with CI later on)
  • New and existing unit tests pass locally with my changes (might be obsolete with CI later on)

@timdreier timdreier linked an issue Mar 16, 2023 that may be closed by this pull request
Copy link
Contributor

@erlbacsi erlbacsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR seems to work fine. There are tests missing with traffic participants. Maybe the range values have to be tuned after some traffic tests.
The code and the documentation are clear.
The extension for filtering points behind the car also seems to work.

Review time: 45min

@timdreier
Copy link
Contributor Author

Additional time: 1.5h for

  • Add rear left filter
  • Add rear right filter
  • fix a sign error which took some time to identify
  • create a image which describes the parameters better

Copy link
Contributor

@vogelnik vogelnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing it in combination with some planning components where we need to use the lidar, I ran in some issues which shouldn't be merged like that. That's why I'm leaving this review even though it already got reviewed.

The lidar seems to work perfectly, but I think you forgot to delete some things you wrote for testing reasons. Most importantly, please remove the rotation of the imu which seems to cause the car to constantly turn left. Furthermore, please resolve the merge conflicts and bring it up to date with the newest version of main.

Review time 30min

@timdreier
Copy link
Contributor Author

The problem was that I merged the old branch from @nylser where he placed the sensors. I resetted the config to main and just change the lidar rotation.

Did some testing, seems to work as intended now!

Additional time: 30 min

…-of-car' of github.com:ll7/paf22 into 211-feature-publish-distances-to-closest-point-in-front-of-car
…-of-car' of github.com:ll7/paf22 into 211-feature-publish-distances-to-closest-point-in-front-of-car
…nt-in-front-of-car

# Conflicts:
#	code/agent/config/rviz_config.rviz
#	code/agent/src/agent/agent.py
#	code/perception/launch/perception.launch
#	doc/06_perception/02_lidar_distance_utility.md
@ghost ghost requested a review from vogelnik March 31, 2023 14:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Publish distances to closest point in front of car

4 participants