Navigation Menu

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: TeraRangerI2C redefining the output distance logic with OutOfRange cases #17050

Merged
merged 2 commits into from Apr 6, 2021
Merged

AP_RangeFinder: TeraRangerI2C redefining the output distance logic with OutOfRange cases #17050

merged 2 commits into from Apr 6, 2021

Conversation

PYBrulin
Copy link
Contributor

This PR is intended to correct the output distances and status that the TeraRanger sensor should return when used under the I2C protocol.

It is related to this previous PR: #13376

To add to what @rmackay9 said in the above issue, the way this is currently implemented triggers the sensor as being "unhealthy" whenever the sensor is unable to measure. And this, whether it is "too close" OR "too far" OR "not seeing anything".

The main reason for this PR is that the TeraRangerI2C driver cannot be used as a simple proximity sensor, and it is expected to use the auxiliary module TeraRangerTower Hub (UART) when one wants to use a single rangefinder as a proximity sensor. Because the sensor is considered "unhealthy", the proximity library blocks (actually, it's more the rangefinder library here) the sensor from being read. Since being out of range is actually quite common when a rangefinder is used as a proximity sensor, the rangefinder constantly triggers the proximity failsafe... (The vehicle even refuses to arm when there is no obstacle seen by the sensor)

  • For the 0x0000 error code: I followed what rmackay9 suggested in the previous PR by setting the distance to 0cm. This sets the sensor status to OutOfRange_Low. (I understand that it is a regression to what was decided for the above mentioned PR, so I apologize if it is unwanted);
  • Concerning the error code 0x0001: It is unreliable because "being unable to measure" can also imply that the sensor does not see anything. This problem occurs very often when the TeraRangerI2C looks at the horizon or the sky when used as a proximity sensor. For this reason, I think this error code should include the OutOfRange_High case so that the rangefinder remains active when looking at the sky. I based the output distance logic the same way it is implemented for the Benewake driver, by adding an arbitrary value to max_distance() (cf. AP_RangeFinder: Benewake sends max-range+1m when out of range #9727);
  • Finally, regarding the error code 0xFFFF. I didn't touch this case, but I think defining the OutOfRange_High should be appropriate here. However, I am unsure why it is considered to be unreliable in the first place, as I couldn't get the sensor to return this error code. Only the error code 0x0001 was triggered when the obstacle was too far. I would really appreciate an external opinion on this, so I could add the OutOfRange_High case if it is deemed appropriate. (My guess is that it was probably meant for the older version "TeraRanger One").

I consider that for the function "process_raw_measure()", each error code should output a value. Whether the sensor is "healthy" or not should be checked in the above function "collect_raw()" but not when processing the value itself. I originally only wanted to modify the 0x0001 error code, but having found the previous mentioned PR, I decided to modify the whole function, which is probably contraindicated..

@rmackay9
Copy link
Contributor

In general this looks good to me and fixes the issue introduced by PR #13376.

Re 0xFFFF, if we think the rangerfinder will never send this value then treating it either as out-of-range-high or as a distance should be fine.

Copy link
Contributor

@rmackay9 rmackay9 left a comment

Choose a reason for hiding this comment

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

I'm happy with this although I think @PYBrulin will update it slightly to change the handling of the 0xFFFF case.

@PYBrulin
Copy link
Contributor Author

Thank you for your review, Mr. Mackay. I have now added the modification you suggested for the 0xFFFF case so that it can trigger the OutOfRange_High status as well.

@rmackay9
Copy link
Contributor

@PYBrulin,

Great, I've added it to the weekly dev call agenda so we should review it next Tuesday and hopefully merge it after that. It's also possible another dev will review it before then but let's see. Txs again!

@tridge tridge merged commit 240bfeb into ArduPilot:master Apr 6, 2021
@PYBrulin PYBrulin deleted the TeraRanger_OutOfRange branch April 6, 2021 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants