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

Plane: allow for different orientations for landing rangefinder #28014

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

tridge
Copy link
Contributor

@tridge tridge commented Sep 5, 2024

this is principally for tailsitters where rangefinders would be orientation with RNGFND1_ORIENT=12 (PITCH_180), but also allows for custom orientations which will be useful if the rangefinder is tilted forward

Tested in SITL using the quadplane-copter_tailsitter frame

@IamPete1
Copy link
Member

IamPete1 commented Sep 5, 2024

I do have a PR for this #20074 (it does need a rebase). That automatically selects the rangefinder closest to down for the current attitude. The potential problem there is that it will switch in flight.

Would the new set HAGL command work here too? It just seems a bit mad that we need a parameter to tell us which way down is. In the longer term I would like to use all rangefinders all the time using a average with weighting bases on how close to down they are.

@tridge
Copy link
Contributor Author

tridge commented Sep 6, 2024

I do have a PR for this #20074 (it does need a rebase). That automatically selects the rangefinder closest to down for the current attitude. The potential problem there is that it will switch in flight.

it also doesn't support custom orientations which I think will be useful on some vehicles. The switching in flight issue I think is a real problem, especially with the code we have for starting the use of a rangefinder.
We could use the HAGL method, but I think the parameter is warranted as tailsitters become more common. In the future we could support value -1 for "automatic", but that just complicates things for now

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Sep 6, 2024
ArduPlane/altitude.cpp Outdated Show resolved Hide resolved
ArduPlane/system.cpp Outdated Show resolved Hide resolved
// @Param: RNGFND_LND_ORNT
// @DisplayName: rangefinder landing orientation
// @Description: The orientation of rangefinder to use for landing detection. Should be set to Down for normal downward facing rangefinder and Back for rearward facing rangefinder for quadplane tailsitters. Custom orientation can be used with Custom1 or Custom2. The orientation must match at least one of the available rangefinders.
// @Values: 12:Back, 25:Down, 101:Custom1, 102:Custom2
Copy link
Member

Choose a reason for hiding this comment

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

Note that the rangefinder orient param does not list custom 1 and custom 2 as valid values.

Copy link
Member

Choose a reason for hiding this comment

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

In fact it also gives 4: Back. Here you have 12: Back. One is ROTATION_YAW_180 and the other ROTATION_PITCH_180. For a rangefinder it amounts to the same, but this should match the rangefinder params so selecting "Back" for both works as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the problem with using YAW_180 is the special handling of the yaw values in Aircraft::perpendicular_distance_to_rangefinder_surface() in SITL - YAW_180 is assumed to be a proximity sensor, so if you use YAW_180 you can't test in SITL
Using YAW_180 will work on a real aircraft, just not SITL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peterbarker looks like you added the special handling for the YAW values in SIM_Aircraft::perpendicular_distance_to_rangefinder_surface() ? does it really need to be that way or can we have some other way to hack in the proximity sensors?

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that the GCS may just give a drop down not the value, so if the user selects "Back" both this and the rangefinder orient it won't work. Maybe you could add "SITL back", or just don't have that case in the values, its not in the values for rangefinder either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unless we change that hack in SITL then we can't use YAW180 and make it testable in SITL
we could make the AP_RangeFinder library use pitch180 instead?

Copy link
Member

Choose a reason for hiding this comment

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

I'm only talking about the @values, the two "Backs" must be the same (4). You can still use a value that is not listed for in SITL and it should work fine. You were already having to set a value not in the @Values for the rangefinder, you will just have to do that too for the new param.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can change the meaning of "Back" in the rangefinder library to be 12, but I'd like to discuss that on the call
or we could rename "4: Back" to "4: Back(Yaw180)" in the rangefinder library and add "12: Back" there

ArduPlane/Parameters.cpp Show resolved Hide resolved
@tridge tridge force-pushed the pr-rangefinder-tailsitter branch 2 times, most recently from 6885df6 to 025ee96 Compare September 9, 2024 05:48
Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@IamPete1 IamPete1 left a comment

Choose a reason for hiding this comment

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

Just need to change the default, looks good apart from that.

ArduPlane/Parameters.cpp Outdated Show resolved Hide resolved
Copy link
Member

@IamPete1 IamPete1 left a comment

Choose a reason for hiding this comment

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

Looks good.

@peterbarker
Copy link
Contributor

Whoops. Killed the Rover RangeFinder test - we also use those posts in Europe to test Rover's horizontalk rangefinder, and that seems to be busted.

@tridge
Copy link
Contributor Author

tridge commented Sep 12, 2024

Killed the Rover RangeFinder test - we also use those posts in Europe to test Rover's horizontalk rangefinder, and that seems to be busted.

I've fixed it up

allows for quadplane tailsitter rangefinders
this is principally for tailsitters where rangefinders would be
orientation with RNGFND1_ORIENT=12 (PITCH_180), but also allows for
custom orientations which will be useful if the rangefinder is tilted
forward
test rearward rangefinder
@tridge tridge merged commit fc2f518 into ArduPilot:master Sep 13, 2024
95 checks passed
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.

6 participants