ArduPilot / ardupilot

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

RangeFinder: Using existing Analog offset for other interfaces (I2C or Serial) #9873

Open patrickpoirier51 opened 5 years ago

patrickpoirier51 commented 5 years ago

Feature request

Expand rangefinder offset

Is your feature request related to a problem? Please describe. Need some easy fix to adjust sensor offset. For example, the Garmin Lidar Lite, has an offset of approx. 25 cm on low range, and users do not have a mean to compensate.

Describe the solution you'd like Just tested using the existing analog offset within this code https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_RangeFinder/AP_RangeFinder_PulsedLightLRF.cpp#L94

uint16_t _distance_cm = be16toh(val);
float offset  = state.offset;
// remove momentary spikes
if (abs(_distance_cm - last_distance_cm) < 100) {
    state.distance_cm = (_distance_cm-offset);

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

trilusa commented 5 years ago

Hi, I think I can do that. So basically what you are asking for is that the "offset" parameter be accounted for in every sensor driver? I'm not sure if this is the best approach, but I can just go through each one and subtract "state.offset" from the distance measurement before it is assigned to "state.distance_cm" like you did in your example. Thanks

patrickpoirier51 commented 5 years ago

Yes that’s it, and I was questioning the method of implementing in each drivers as well.. Could it be processed within rangefinder.cpp or in the header ?

trilusa commented 5 years ago

yes I was wondering that as well, the whole offset implementation seems a bit clunky.

Im still getting used to the codebase, but it seems that I could just account for the offsest in the rangefinder_backend.h before it returns: RangeFinder_Backed.h, line 39 enum Rotation orientation() const { return (Rotation)state.orientation.get(); } uint16_t distance_cm() const { return state.distance_cm - state.offset; }

However, it seems that would negatively effect the analog sensor functionality because it is treated as a voltage in that code.

I could add another offset parameter for a distance offset, use that in the header and then change the analog sensor backed to reflect that change, which probably would make more sense to the user anyway. I'm not sure if there would any unintended consequences of that but we could always figure that out in a PR.

patrickpoirier51 commented 5 years ago

Well I let you check that with @OXINARF and please take note to get the wiki and Mission Planner being updated with the new information once completed

Thanks