ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
11.05k stars 17.61k forks source link

AP_RangeFinder/EKF: add rangefinder delay parameter #12020

Open chobitsfan opened 5 years ago

chobitsfan commented 5 years ago

There are _DELAY parameters for barometer, GPS, and various sensor, but there seems no delay parameter for rangefinder.

It seems rangefinder delay is hard coded as 25ms https://github.com/ArduPilot/ardupilot/blob/92783bccfaa720f6a1de8ee5718a0d5fb3d27c6c/libraries/AP_NavEKF2/AP_NavEKF2_Measurements.cpp#L53

It would be helpful to a EK2_RNG_DELAY parameter.

Platform [ ] All [ ] AntennaTracker [X] Copter [ ] Plane [ ] Rover [ ] Submarine

rmackay9 commented 5 years ago

Yes, I agree. One question is whether this delay should be part of the EKF or part of the range finder driver. I'd say that it probably belongs in the range finder driver although I'm not sure we need one per lidar. Anyway, putting it into the EKF would be fine for now I think because only the EKF would use it.

chobitsfan commented 5 years ago

Thank you @rmackay9

We have EK2_HGT_DELAY for barometer, EK2_FLOW_DELAY for optical flow, etc. And GPS_DELAY_MS for gps. Maybe it would be better to be consistent?

WickedShell commented 5 years ago

It should be in the sensor library. The problem with putting it in the EKF, is that enforces an assumption that all the sensors have the same delay, which is not a good general assumption. GPS used to be in the EKF, and we moved it out for that reason. This also let us provide the delay value automatically from the different GPS drivers, or let the user override it, but it provided a more felxibile solution, while requiring less setup, and giving more accurate results.

chobitsfan commented 5 years ago

Thank you @WickedShell

May I ask a question? Why rangefinder did not have delay parameter like other sensors? I think it is hard coded in AP_NavEKF2_Measurements.cpp after tracing related code. But I am not really 100% sure. Maybe delay parameter is not needed for rangefinder, so author do not add it? Thank you very much