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

SF45: Adjust _handle_missed_bins() logic #24248

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

mahimayoga
Copy link
Contributor

@mahimayoga mahimayoga commented Jan 23, 2025

Solved Problem

When flying with the SF45 rotary lidar, the _handle_missed_bins function in lightware_sf45_serial.cpp can fill bins in the obstacle map that are outside the sensor's FOV. This has negative consequences for the setpoint constraints that are set when flying with CollisionPrevention. (For example: losing ability to move in direction outside of FOV)

Data being added to bins outside the FOV & consequenctly not being able to pitch-up (move backwards):
image

Solution

When filling the missed bins, we loop thruough the indices always starting from the first bin within the FOV. (see figure). We loop through these indices, but fill the obstacle map at the original bin indices.

This way we can handle cases in which the current_bin (in this illustration bin 6) and previous_bin (in this illustration bin 10) are near the bins which are outside the FOV. It also simplifies the logic such that we don't need to consider the wrap-around case.

image

Relevant functions added to ObstacleMath library.

Test coverage

  • Unit/integration test: Unit tests added in ObstacleMathTest.cpp
  • Bench tests:
  1. Wrap around case.
    image

  2. Bin jumps from 24 to 47. Shortest path to fill the missed bins would be through the bins not in the FOV (~ bins 32 - 40). But we can see that now we fill in the opposite direction. (Not all bins shown as it's messy)

image

@mahimayoga
Copy link
Contributor Author

@MaEtUgR do you think this is conceptually better logic? If you have a better idea let me know! Otherwise I can flight test it in the next days.

Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

You're onto something. I have to continue reviewing through, I didn't get through all the logic yet.

Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

Good find, and nice cleanup along the way!

When filling the missed bins, the bin indices are temporarily re-numbered such that the 0th bin corresponds to the first bin which is within the FOV

That confused me for a moment, maybe you could instead just say that you loop from the indices always starting from the first bin within the FOV.

Minor comments. Please use const wherever applicable.

@mahimayoga mahimayoga requested a review from sfuhrer February 3, 2025 14:54
Copy link
Contributor

@sfuhrer sfuhrer 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 now. Please clean up the commits into stand-alone functional changes or refactors, then good to go!

These functions help simplify repeated calculations accross driver and collision prevention files that are computing bins, angles and sensor offsets in obstacle maps.
To simplify logic for wrap-around cases and cases in which bins outside the FOV may be filled. Bin indices are offset such that the 0th bin is the first bin within the sensor FOV. This way we can always fill bins from smallest to largest index.
… to incoming sensor data.

yaw_cfg is now read into the obstacle_distance message as the angle_offset. The offset is computed once at init and applied to each measurement.
@mahimayoga mahimayoga force-pushed the pr-collision_prevention_position_lock branch from 7f5a6e1 to 2164a48 Compare February 3, 2025 15:45
@mahimayoga mahimayoga requested a review from sfuhrer February 3, 2025 16:01
@sfuhrer sfuhrer merged commit 48c0992 into main Feb 3, 2025
60 checks passed
@sfuhrer sfuhrer deleted the pr-collision_prevention_position_lock branch February 3, 2025 16:17
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