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.

Sign up for GitHub

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

Jump to bottom

Add AP_Periph support for Proximity sensors #21822

Merged
merged 4 commits into from
Dec 14, 2022
Merged

Add AP_Periph support for Proximity sensors #21822

merged 4 commits into from
Dec 14, 2022

Conversation

rishabsingh3003
Copy link
Contributor

This needs: dronecan/DSDL#15 to be merged first

@rishabsingh3003 rishabsingh3003 marked this pull request as draft September 27, 2022 19:53
@patrickpoirier51
Copy link

patrickpoirier51 commented Sep 28, 2022 •
edited
Loading

Tested with 3 Cygbot connected to 3 Matek L431 and ARKFlow on Cube Orange = Logs are available

@rishabsingh3003 rishabsingh3003 force-pushed the prx_dronecan branch 2 times, most recently from afae7cb to 2ada4ae Compare November 8, 2022 19:28
@rishabsingh3003 rishabsingh3003 marked this pull request as ready for review November 8, 2022 19:28
peterbarker
peterbarker previously requested changes Nov 9, 2022
View reviewed changes
libraries/AP_Proximity/AP_Proximity_Backend.cpp
}
}

// update Object Avoidance database with Earth-frame point
// pitch can be optionally provided if needed
void AP_Proximity_Backend::database_push(float angle, float pitch, float distance, uint32_t timestamp_ms, const Vector3f &current_pos, const Matrix3f &body_to_ned)
{

#if !APM_BUILD_TYPE(APM_BUILD_AP_Periph)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#if !APM_BUILD_TYPE(APM_BUILD_AP_Periph)
#if AP_OADATABASE_ENABLED

and all related changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's okay I would like to do this as another PR to not increase the scope of this one

tridge
tridge requested changes Nov 9, 2022
View reviewed changes
Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

should be in build_options.py

Tools/AP_Periph/can.cpp Outdated
static uint32_t last_probe_ms;
if (now - last_probe_ms >= 1000) {
last_probe_ms = now;
proximity.init();
Copy link
Contributor

Choose a reason for hiding this comment

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

how long does init() take? could stall other tasks on the main thread

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Init takes about 400-500 microseconds on my L431, depending on the sensor selected

libraries/AP_Proximity/AP_Proximity.h Outdated Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_Boundary_3D.cpp Outdated Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_DroneCAN.cpp Outdated Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_DroneCAN.cpp Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_DroneCAN.cpp Outdated Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_DroneCAN.cpp Outdated Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_DroneCAN.cpp Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_DroneCAN.h Outdated Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_Params.cpp Show resolved Hide resolved
rmackay9
rmackay9 reviewed Nov 14, 2022
View reviewed changes
libraries/AP_Proximity/AP_Proximity_DroneCAN.cpp Outdated

if (is_zero(prx.params[i].max_m) && is_zero(prx.params[i].min_m)) {
// GCS reporting will be incorrect if min/max are not set
gcs().send_text(MAV_SEVERITY_CRITICAL, "Configure PRX%u_MIN and PRX%u_MAX",
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this message is unlikely to be seen. An alternative would be to make it appear when prearm_healthy() is called somehow. This is a bit optional though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but I think I will do this as part of another PR perhaps

@rmackay9
Copy link
Contributor

I've added some comments mostly around naming (as per usual) but in any case, this looks pretty good to me.

@rishabsingh3003 rishabsingh3003 force-pushed the prx_dronecan branch 2 times, most recently from 56e1cb9 to f2d4920 Compare November 21, 2022 19:07
@rishabsingh3003
Copy link
Contributor Author

I think the build_options.py bit needs to be done in another PR. We haven't listed any Proximity devices in that script

@tridge tridge removed the DevCallEU label Nov 23, 2022
@rishabsingh3003
Copy link
Contributor Author

PR autotest fails because of bug solved here: dronecan/DSDL#19

@rishabsingh3003 rishabsingh3003 force-pushed the prx_dronecan branch 2 times, most recently from 0dc8225 to e23b3fd Compare December 13, 2022 21:39
@peterbarker peterbarker dismissed their stale review December 14, 2022 07:06

LAter PR for the define thing

tridge
tridge requested changes Dec 14, 2022
View reviewed changes
libraries/AP_Proximity/AP_Proximity_DroneCAN.cpp Outdated Show resolved Hide resolved
libraries/AP_Proximity/AP_Proximity_DroneCAN.cpp Outdated Show resolved Hide resolved
@tridge tridge removed the DevCallEU label Dec 14, 2022
@tridge tridge merged commit 650f0ff into ArduPilot:master Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmackay9 rmackay9 rmackay9 left review comments

@tridge tridge tridge approved these changes

@peterbarker peterbarker peterbarker left review comments

Assignees
No one assigned
Labels
AP_Periph MergeOnCIPass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rishabsingh3003 @patrickpoirier51 @rmackay9 @tridge @peterbarker @andyp1per