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

AP_RangeFinder: initial support for the DFRobot07 lidar #28366

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

landswellsong
Copy link
Contributor

Support for DFRobot07 which is also sold as VSemi Atom

Documentation: https://dfimg.dfrobot.com/nobody/wiki/1840a7b7b14e02f3566e0cef5b51e9ba.pdf

Implementation caveats:

  • UART only for now;
  • 50 ms fixed data rate, can be synced up with update() but it involves waiting for the result since the sensor does take a bit of time to process commands;
  • there are a bunch of values coming from the sensor that don't have a clear mapping to anything in AP so they are ignored for now, although potentially can be used to estimate the signal quality;

Code quality and style questions that I'd love to have a senior developer feedback:

  • Should the initialization be done like I did in update() or should I rather have overriden init_serial? Given how sensor works it would mean the thread that runs it needs to be waiting for a while.
  • Is the offset macros fine or should I rather make structures/unions for a more clear picture of the data layout?
  • The CRC32 in the sensor seems to work in opposite bit order reflection mode in comparison to the AP implementation (and frankly any standard implementation). Is it okay to put extra CRC32 utils hidden like I did or should I move that to AP_Math / optimize it etc?

@landswellsong
Copy link
Contributor Author

SITL test in the room conditions:
image

@landswellsong
Copy link
Contributor Author

https://github.com/ArduPilot/ardupilot/actions/runs/11257361975/job/31301422237?pr=28366#step:9:1282

Right, guess I'll rewrite the offset part to work with ARM and test that locally too.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Sorry, very brief review only.

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Oct 10, 2024
- Only the distance and the temperature are extracted
- 50 ms fixed reading interval
- Only UART mode implemented
@landswellsong
Copy link
Contributor Author

Resolved all the issues, removed any sleeping whatsoever. Will test on the weekend on the sensor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WikiNeeded needs wiki update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants