Skip to content
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

Clean up lidar flip param, and re-introduce CI testing for ros1 #94

Merged
merged 17 commits into from
Jan 27, 2023

Conversation

josephduchesne
Copy link
Member

No description provided.

@@ -239,7 +239,7 @@ void Laser::ComputeLaserRanges() {

// Unqueue all of the future'd results
const auto reflectance = reflectance_layers_bits_;
if (flipped_) {
if (scan_clockwise_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In order for the resultant scan to be equivalent when the scan direction is flipped, we also have to update the angle metrics on the published scans:

  • new_angle_increment = -1 * og_angle_increment (since we're now winding the opposite direction)
  • new_angle_min = og_angle_min + og_angle_increment * ranges.size()
  • new_angle_max = og_angle_min

@@ -345,7 +337,7 @@ void Laser::ParseParameters(const YAML::Node& config) {
rng_ = std::default_random_engine(rd());
noise_gen_ = std::normal_distribution<float>(0.0, noise_std_dev_);

ROS_DEBUG_NAMED("LaserPlugin",
ROS_INFO(//"LaserPlugin",
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to print the value of upside_down here too.

/**
* Test the laser plugin's ability to flip the scan direction to clockwise works as expected
*/
TEST_F(LaserPluginTest, scan_direction_test) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the names of these tests should be updated to reflect the new name of the parameter (upside_down vs flipped)

Copy link

@nightingale0131 nightingale0131 left a comment

Choose a reason for hiding this comment

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

Looks good! I tested it on Neo2 and Switch, and everything seems to be in order. I also tried flipping and reversing the scans (not together though). Unfortunately there's a bug in the sector mapper that prevents autonomy from running when using a negative increment...I'll have to look into that.

@josephduchesne josephduchesne merged commit 4c71e1a into master Jan 27, 2023
@josephduchesne josephduchesne deleted the clean-up-lidar-flip-param branch January 27, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants