-
Notifications
You must be signed in to change notification settings - Fork 18.5k
AP_Proximity: PROXIMITY is optional, so the number of devices can be specified #29873
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -28,7 +28,9 @@ | |||
|
|||
#include <AP_HAL/Semaphores.h> | |||
|
|||
#define PROXIMITY_MAX_INSTANCES 5 // Maximum number of proximity sensor instances available on this platform | |||
#ifndef PROXIMITY_MAX_INSTANCES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thoughts, I think we really should rename this before making it something someone can reasonably put in their hwdefs:
#ifndef PROXIMITY_MAX_INSTANCES | |
#ifndef AP_PROXIMITY_MAX_INSTANCES |
What do you think, @muramura ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the prefix "AP_" at the beginning.
72a2b9d
to
f08ce7a
Compare
f08ce7a
to
73335d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks!
PROXIMITY devices are optional.
Recently, the number of devices increased from 4 to 5.
Therefore, the number of devices will be configurable.
AFTER

BEFORE
