Collision prevention: Option to enable flying outside FOV #13135

baumanta · 2019-10-09T07:07:35Z

This PR reacts to a feature request from the community. Since the latest changes in collision prevention it was not possible anymore to move into directions where there is no sensor coverage. Now I added a boolean MPC_CP_GO_NODATA which is per default set to false but enables people to explicitly allow such movements.

In the process I renamed the parameters of the collision prevention. Witch the naming before, we had only three letters available to actually describe the function of the parameter, which I thought was not enough. Open for discussions on the naming front if there are any better ideas/ objections to the proposed names.

baumanta · 2019-10-09T07:08:24Z

@hamishwillee any input on parameter naming? I would of course update the docs if that gets in.

baumanta · 2019-10-09T07:16:01Z

FYI @jordancoult

mrivi

I like the new naming but I'll let @hamishwillee comment on that

src/lib/CollisionPrevention/CollisionPrevention.cpp

julianoes · 2019-10-09T09:35:01Z

@baumanta can you give this PR a more descriptive name? It's all gobbledygook to me.

hamishwillee · 2019-10-09T22:27:39Z

@hamishwillee any input on parameter naming? I would of course update the docs if that gets in.

The new format is much better thank you. I'll look at the names individually.
But not sure the old group name is helpful "Multicopter Position Control". Should we group then under Multicopter Collision Prevention?

Also, have you guys thought about whether/how the UI needs to be informed about this case? [It might be that nothing changes]

hamishwillee · 2019-10-09T22:35:53Z

src/lib/CollisionPrevention/collisionprevention_params.c

@@ -73,4 +73,14 @@ PARAM_DEFINE_FLOAT(MPC_COL_PREV_DLY, 0.4f);
 * @unit [deg]
 * @group Multicopter Position Control
 */
-PARAM_DEFINE_FLOAT(MPC_COL_PREV_CNG, 30.f);
+PARAM_DEFINE_FLOAT(MPC_CP_GUIDE_ANG, 30.f);


Guide doesn't feel quite right. I guess you're thinking of it like when you go 10 pin bowling and put up the guide rails to prevent the ball going into the gutter? I can't think of a better word though.

Yes, I was struggling a bit with that one too. I was thinking of how to explain best what influence the parameter has. And what that feature does, is help you navigate tight spaces or guide/ adjust your input slightly to fit the obstacle map. But could not come up with anything better either...

Yep. Perhaps "adjust" is better - so MPC_CP_ADJUST_ANG. Not sure enough to say we should change it.

baumanta · 2019-10-10T08:49:35Z

@hamishwillee any input on parameter naming? I would of course update the docs if that gets in.

The new format is much better thank you. I'll look at the names individually.
But not sure the old group name is helpful "Multicopter Position Control". Should we group then under Multicopter Collision Prevention?

Also, have you guys thought about whether/how the UI needs to be informed about this case? [It might be that nothing changes]

I think it would be nice if we could ditch the MPC prefix, it would give us some more freedom with naming as we would have more letters. But I'm not sure whether this is possible from a parameter architecture point of view. Not sure who has insight in that, maybe @MaEtUgR ?

mhkabir · 2019-10-10T11:37:17Z

We can ditch the MPC prefix without issues.

hamishwillee · 2019-10-11T00:55:54Z

We can ditch the MPC prefix without issues.

Though as a reader is convenient to have immediate understanding that these are vehicle-specific parameters. That said, you get that from the group name. Did you think that group name is worth changing?

baumanta · 2019-10-11T06:21:12Z

I think it would be cool if we could group under "collision prevention" somehow. Maybe as you suggested MCP_DIST, MCP_DELAY, ... But maybe MCP is too close to MPC and confusing?

mhkabir · 2019-10-11T13:58:05Z

CPV_? And have the metadata group it under a more human readable "Collsiiom Prevention" group.

dagar · 2019-10-14T15:14:50Z

For the parameter naming one thing to consider is if this could potentially be broken out of the FlightTask/mc_pos_ctrl and used in rover or if it's going to be tightly integrated with MC (directly using MPC_XY_P, MPC_JERK_MAX, etc).

jkflying · 2019-10-16T13:11:16Z

For the parameter naming one thing to consider is if this could potentially be broken out of the FlightTask/mc_pos_ctrl and used in rover or if it's going to be tightly integrated with MC (directly using MPC_XY_P, MPC_JERK_MAX, etc).

Long term I see this as definitely happening, and for fixed wing too if the sensors support it. This could even be used for multi-vehicle collaborative avoidance if tuned/tweaked correctly. There's been a lot of churn in the parameters recently. Technically the MPC_JERK_MAX can be fed in as parameters from whatever module is calling it, no reason from the collision prevention to use them directly.

IMO: let's move the parameters to their own prefix, and get the pain over with now.

dagar · 2019-10-16T13:34:25Z

For the parameter naming one thing to consider is if this could potentially be broken out of the FlightTask/mc_pos_ctrl and used in rover or if it's going to be tightly integrated with MC (directly using MPC_XY_P, MPC_JERK_MAX, etc).

Long term I see this as definitely happening, and for fixed wing too if the sensors support it. This could even be used for multi-vehicle collaborative avoidance if tuned/tweaked correctly. There's been a lot of churn in the parameters recently. Technically the MPC_JERK_MAX can be fed in as parameters from whatever module is calling it, no reason from the collision prevention to use them directly.

IMO: let's move the parameters to their own prefix, and get the pain over with now.

One more slightly off topic note on this to think about for later is that we can afford to have collision prevention as a standalone module now.

baumanta · 2019-10-17T09:48:42Z

I renamed the parameters so they now live under the Collision Prevention group CP. They are now called:
CP_DIST
CP_DELAY
CP_GUIDE_ANG
CP_GO_NO_DATA
Decided to stick with GUIDE for now, was not that convince by ADJUST...

mrivi

let's get this in before 1.10. Already a couple of people asked about this feature

src/lib/CollisionPrevention/CollisionPrevention.cpp

Co-Authored-By: Martina Rivizzigno <martina@rivizzigno.it>

jkflying · 2019-10-17T11:57:16Z

One more slightly off topic note on this to think about for later is that we can afford to have collision prevention as a standalone module now.

This will also allow it to be decoupled from FMUv2 flash space issues.

jkflying

Just a missing test from my side.

Also, this CP_GO_NO_DATA is pretty unsafe, because I think it might affect the failsafe behavior if one of multiple sensors fails. But I don't see a reasonable way around it, unless we tracked whether a bin had ever received data and then ignore the CP_GO_NO_DATA in that bin.

src/lib/CollisionPrevention/collisionprevention_params.c

mrivi reviewed Oct 9, 2019

View reviewed changes

src/lib/CollisionPrevention/CollisionPrevention.cpp Outdated Show resolved Hide resolved

baumanta mentioned this pull request Oct 9, 2019

rename colprev params PX4/PX4-Avoidance#501

Merged

baumanta changed the title ~~Colprev enable mv no data~~ Collision prevention: Option to enable flying outside FOV Oct 9, 2019

hamishwillee reviewed Oct 9, 2019

View reviewed changes

weekly-digest bot mentioned this pull request Oct 13, 2019

Weekly Digest (6 October, 2019 - 13 October, 2019) #13171

Closed

TSC21 requested review from jkflying and mrivi October 15, 2019 15:08

baumanta added 3 commits October 17, 2019 11:16

rename parameters to allow more descriptive names

703c488

add option to enable moving where there is no data

0ef5a94

group collision prevention params under CP group

c7b385f

baumanta force-pushed the colprev-enable-mv-no-data branch from cab63a9 to c7b385f Compare October 17, 2019 09:23

mrivi reviewed Oct 17, 2019

View reviewed changes

src/lib/CollisionPrevention/CollisionPrevention.cpp Outdated Show resolved Hide resolved

make bool const

66fa0ce

Co-Authored-By: Martina Rivizzigno <martina@rivizzigno.it>

jkflying requested changes Oct 17, 2019

View reviewed changes

src/lib/CollisionPrevention/collisionprevention_params.c Show resolved Hide resolved

add test for param CP_GO_NO_DATA

f3450a2

jkflying approved these changes Oct 17, 2019

View reviewed changes

dagar merged commit be1f966 into PX4:master Oct 17, 2019

weekly-digest bot mentioned this pull request Oct 20, 2019

Weekly Digest (13 October, 2019 - 20 October, 2019) #13236

Closed

baumanta mentioned this pull request Oct 21, 2019

update collision prevention description PX4/PX4-user_guide#591

Merged

mrivi mentioned this pull request Oct 23, 2019

rename collision prevention parameters to match Firmware mavlink/qgroundcontrol#7944

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collision prevention: Option to enable flying outside FOV #13135

Collision prevention: Option to enable flying outside FOV #13135

baumanta commented Oct 9, 2019 •

edited

Loading

baumanta commented Oct 9, 2019 •

edited

Loading

baumanta commented Oct 9, 2019

mrivi left a comment

julianoes commented Oct 9, 2019

hamishwillee commented Oct 9, 2019

hamishwillee Oct 9, 2019

baumanta Oct 10, 2019

hamishwillee Oct 11, 2019

baumanta commented Oct 10, 2019

mhkabir commented Oct 10, 2019

hamishwillee commented Oct 11, 2019

baumanta commented Oct 11, 2019

mhkabir commented Oct 11, 2019

dagar commented Oct 14, 2019

jkflying commented Oct 16, 2019

dagar commented Oct 16, 2019

baumanta commented Oct 17, 2019

mrivi left a comment •

edited

Loading

jkflying commented Oct 17, 2019

jkflying left a comment •

edited

Loading

Collision prevention: Option to enable flying outside FOV #13135

Collision prevention: Option to enable flying outside FOV #13135

Conversation

baumanta commented Oct 9, 2019 • edited Loading

baumanta commented Oct 9, 2019 • edited Loading

baumanta commented Oct 9, 2019

mrivi left a comment

Choose a reason for hiding this comment

julianoes commented Oct 9, 2019

hamishwillee commented Oct 9, 2019

hamishwillee Oct 9, 2019

Choose a reason for hiding this comment

baumanta Oct 10, 2019

Choose a reason for hiding this comment

hamishwillee Oct 11, 2019

Choose a reason for hiding this comment

baumanta commented Oct 10, 2019

mhkabir commented Oct 10, 2019

hamishwillee commented Oct 11, 2019

baumanta commented Oct 11, 2019

mhkabir commented Oct 11, 2019

dagar commented Oct 14, 2019

jkflying commented Oct 16, 2019

dagar commented Oct 16, 2019

baumanta commented Oct 17, 2019

mrivi left a comment • edited Loading

Choose a reason for hiding this comment

jkflying commented Oct 17, 2019

jkflying left a comment • edited Loading

Choose a reason for hiding this comment

baumanta commented Oct 9, 2019 •

edited

Loading

baumanta commented Oct 9, 2019 •

edited

Loading

mrivi left a comment •

edited

Loading

jkflying left a comment •

edited

Loading