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

Treat UAVS diffrently from manned aviation #13159

Merged
merged 22 commits into from
Jan 17, 2020
Merged

Treat UAVS diffrently from manned aviation #13159

merged 22 commits into from
Jan 17, 2020

Conversation

LowOrbitIonCannon
Copy link
Contributor

@LowOrbitIonCannon LowOrbitIonCannon commented Oct 11, 2019

Using ADSB transmitter data existing or not.

A seperation distance of 10 Meters should suffice for D2D Collision avoidance. This is needed to test the collision avoidance of UAVs. 500 Meters is an unnecessary Distance for UAVs.

Describe problem solved by the proposed pull request
500 Meters is an unnecessary Distance for UAV to UAV Collision avoidance

Describe your preferred solution
A Flag that decides if the encountered entity is Manned or an UAV

Additional context
Collision avoidance uses UTM_Global_Position as per this PR
Testing Collision avoidance would mean a seperation distance of 500 meters.

@hamishwillee rightly informed me of the docs changes needed with this request:
http://docs.px4.io/master/en/advanced_features/traffic_avoidance_adsb.html#implementation

#11262

@LowOrbitIonCannon LowOrbitIonCannon changed the title ready 4 test Treat UAVS diffrently from manned aviation Oct 11, 2019
@hamishwillee
Copy link
Contributor

@ItsTimmy Don't know if you have interest in this since you looked at this code recently.

@LorenzMeier
Copy link
Member

Hey! Sorry, I've been traveling (and still are). What is your testing state with this PR?

@LowOrbitIonCannon
Copy link
Contributor Author

I wasnt yet able to test, I hope I can do that this week.

@LowOrbitIonCannon
Copy link
Contributor Author

I was able to Test today:

Tested was wether the Drone will Warn in QGC if there is another UAV nearby:

Testing Drone Traffic Warning with a Drone and a Rover

Test Setup:
Drone with D2X Module and QGC Groundstation.
Rover with D2X Module and D2X Groundstation.
Modified Firmware running only on the Drone
"NAV_TRAFFIC_AVOID" in Warn Mode
MavLink Messages where send over Telem. 2 port to D2X.

Drone on NXP building terrace one and Rover on terrace two, distance between both is greater than 10 Meters.

Setup Pictures:
20191029_124824
Colli Test Setup Terrace 2 29 10 2019

Stage 2:
Colli Test Setup Stage 2 29 10 2019

Test:
Stage 1:
Drone and Rover where switched on and started sending and receiving MavLink Messages.
Rover was emitting Collision warnings. Drone did not send those Messages:
Colli Test No Warning 29 10 19
Colli Test Rover Groundstation Warning 29 10 2019

Stage 2:
Rover moved over to terrace one, upon regain of GPS lock, the Drone was emitting Traffic Warnings:
Colli Test Warning 29 10 19
Colli Test Rover Groundstation Warnings 29 10 2019

Test was therefore successful.

@dk7xe
Copy link
Contributor

dk7xe commented Nov 4, 2019

Hey! Sorry, I've been traveling (and still are). What is your testing state with this PR?

Hi @LorenzMeier testing was successful. See #13159 (comment)
Who can take care on moving that forward?

@ItsTimmy may you please review, comment and merge if ok. Thank you!

@AmeliaEScott AmeliaEScott self-requested a review November 7, 2019 09:39
julianoes
julianoes previously approved these changes Nov 8, 2019
@LowOrbitIonCannon
Copy link
Contributor Author

LowOrbitIonCannon commented Nov 11, 2019

I'll test this with @ItsTimmy suggestions again on Wednesday.

Build Error on Nxp_fmuk66-v3: Cast from 'Constant Char*' to 'constant uint16_t*'
@LowOrbitIonCannon
Copy link
Contributor Author

LowOrbitIonCannon commented Nov 12, 2019

I referenced the Mavlink Variable "ADSB_EMITTER_TYPE" in Navigator_Main, (I Included Mavlink in the Cmake.txt) Px4 builds correctly but i get this Error when i Build nxp_fmuk66-v3:
image

@davids5 could you take a look at this?

Added fourth fake_traffic message, added additional info to Mavlink warnings
these wont be displayed in QGC due to Message Link Limitation but are written to log.
@LowOrbitIonCannon
Copy link
Contributor Author

I Added a simulated drone to fake_traffic and used that to validate my changes.
Here ist the flight Log.

What i tested:

Standard Params
Start with Warn only.
NAV_TRAFF_AVOID =1
NAV_TRAFF_A_RADM= 500
NAV_TRAFF_A_RADM= 10

No warnings
Start with Warn only.
NAV_TRAFF_AVOID =1
NAV_TRAFF_A_RADM= 40
NAV_TRAFF_A_RADM= 5

All Warnings
Start with Warn only.
NAV_TRAFF_AVOID =1
NAV_TRAFF_A_RADM= 15000
NAV_TRAFF_A_RADM= 50

RETURN TO LAUNCH
NAV_TRAFF_AVOID =2
NAV_TRAFF_A_RADM= 15000
NAV_TRAFF_A_RADM= 50

LAND MODE
NAV_TRAFF_AVOID =3
NAV_TRAFF_A_RADM= 15000
NAV_TRAFF_A_RADM= 50

NO WARNING
NAV_TRAFF_AVOID =0
NAV_TRAFF_A_RADM= 15000
NAV_TRAFF_A_RADM= 50

All Messages where cut of at 50 Digits, in QGC, due to this limitation.
The log displays the Full message.
MAVLINK STATUSTEXT

NAV_TRAFFIC_AVOID = 3 is not implemented yet. So won't do anything.
(Land Mode)

image

This shows that the Parameter works correctly and Changes the distance PX4 Considers a Collision warning.

@@ -34,6 +34,10 @@
px4_add_module(
MODULE modules__navigator
MAIN navigator
COMPILE_FLAGS
-Wno-cast-align # TODO: fix and enable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how it is Implemented in the CMakeLists.txt off mavlink_receiver.cpp.
This will need fixfin in the future

LowOrbitIonCannon added a commit to LowOrbitIonCannon/px4_user_guide that referenced this pull request Nov 20, 2019
@davids5
Copy link
Member

davids5 commented Nov 20, 2019

@mrpollo - Please add for dev call

COMPILE_FLAGS
-Wno-cast-align # TODO: fix and enable
INCLUDES
${PX4_SOURCE_DIR}/mavlink/include/mavlink
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to directly access mavlink outside of the mavlink module.

@dagar dagar self-requested a review November 20, 2019 17:11
Added Behavior: HOLD Position to collision avoidance mode.
and implemented Landmode to collision avoidance.
@LowOrbitIonCannon
Copy link
Contributor Author

Added Behavior: Hold Position to collision avoidance mode.
and implemented Landmode to collision avoidance.

Landmode was not implemented in PX4.
Hold Position should be an available option to avoid Collisions

The German DLR wanted the Hold Position function for a Demo

While testing I found out that Check_Traffic will respond to selfpublished Uorb messages of Transponder_report.

@hamishwillee
Copy link
Contributor

FYI only, last set of docs discussed for this feature are here: PX4/PX4-user_guide#527. Still waiting on clarification.

@hamishwillee
Copy link
Contributor

I like the PR, but I expect a 10m minimum separation is too small. Drones can be moving quite fast, and there won't be time to react unless the transponder publish rate is very high.

Collision avoidance ignores messages with the Uas_id identical to Board GUID
Changed location where Selfpublished UTM msg are ignored from Navigator_main to Mavlink_reciever
fixed mavlink critical warning wrong value being send
@LowOrbitIonCannon
Copy link
Contributor Author

LowOrbitIonCannon commented Dec 10, 2019

I had an interesting Testing day.

I changed where the UTM message is captured and ignored:
before it is send to Uorb to reduce Ressource usage.
These self published messages are reliably ignored now.

while Testing on the ground with 2 Vehicles.
@dk7xe and me found out that the Parameter set in QGC for nav_traff_radx is off by a factor of 10.

We had trouble getting Collision messages on the modified Drone.
When we set down Drone 2 at a Distanz of 4 meters we could only get Collision messages:
40<nav_traff_radx
In the Documentation setting this distanz to 40 should set the Collision Distance to 40 meters not 4 meters.

more testing Confirmed the issues we where having today.

Iam not sure why this happens. Is this due to the way the Parameter is Defined in QGC?

To warn the user I programmed the uas_id parameter into the mavlink_warning

Flight log

Boards where no Hardware GUID is defined will send 0 as GUID.
rightnow COllision avoidance for more then one FMU with out Hardware GUID is not possible.
We should consider adding a randomly generated HW GUID as a placeholder for legacy Boards
@LowOrbitIonCannon
Copy link
Contributor Author

Boards with no HW GUID will send 0 as their UAS_ID.

This means that Collision avoidance between two drones that dont have a HWGUID will not be processed.

This should be written down in Documentation.
And/Or maybe consider adding a Randomly generated HW UID as a Placeholder to Legacy Boards?
image
This is the solution for no HW UID. maybe replace this.

i removed the report of emitter_type to mavlink to reduce footprint
@LowOrbitIonCannon
Copy link
Contributor Author

LowOrbitIonCannon commented Dec 12, 2019

Succsessfully Tested.

LOG

Test was wether the Drone reacts correctly to changes in Nav_traff_RadU
and at what distance.

Observed could be that the Parameter defined in QGC changes the maximum Distance a Collision is called.
Self Published UTM_global_position messages where successfully ignored.

@hamishwillee
Copy link
Contributor

@julianoes FYI I am not tracking this discussion. Happy to rejoin when there are docs to be done.

@LowOrbitIonCannon
Copy link
Contributor Author

LowOrbitIonCannon commented Jan 9, 2020

LOG
This log Does not show the Mavlink Error messages anymore.
They where caused by missmatched Submodules to the newer build.
Updating the subs removed the Errors.

Everything works now. @dagar could you Review it please?

I already started with the Updates to the Documentation and will make a PR when this PR is approved.

// ask the commander to land
vehicle_command_s vcmd = {};
vcmd.command = vehicle_command_s::VEHICLE_CMD_NAV_LAND;
publish_vehicle_cmd(&vcmd);
Copy link
Member

Choose a reason for hiding this comment

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

This is okay as a test, but if we want it to exist long term it should really be tied into the state machine a little more directly.

@@ -898,7 +898,7 @@ Navigator::load_fence_from_file(const char *filename)
*
*/
void Navigator::fake_traffic(const char *callsign, float distance, float direction, float traffic_heading,
float altitude_diff, float hor_velocity, float ver_velocity)
float altitude_diff, float hor_velocity, float ver_velocity, int emitter_type)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
float altitude_diff, float hor_velocity, float ver_velocity, int emitter_type)
float altitude_diff, float hor_velocity, float ver_velocity, uint16_t emitter_type)

Type should match the size used in the message definition.

@dagar
Copy link
Member

dagar commented Jan 17, 2020

I didn't go through the specific utm_global_position handling in detail, but overall from PX4/Firmware perspective this change looks like safe to merge.

*
* @group Mission
*/
PARAM_DEFINE_INT32(NAV_TRAFF_AVOID, 1);

/**
* Set NAV TRAFFIC AVOID RADIUS MANNED
Copy link
Contributor

@hamishwillee hamishwillee Jan 30, 2020

Choose a reason for hiding this comment

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

@LowOrbitIonCannon Can we have a new PR to change this to more natural formatting?

Set avoidance radius/distance for manned vehicles (ADS-B/FLARM).

/**
* Set NAV TRAFFIC AVOID RADIUS MANNED
*
* Defines the Radius where NAV TRAFFIC AVOID is Called
Copy link
Contributor

Choose a reason for hiding this comment

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

@LowOrbitIonCannon Can we have a new PR to change this to more natural formatting?

Defines the avoidance radius/distance at which the avoidance action is triggered for for manned vehicles (ADS-B/FLARM).

PARAM_DEFINE_FLOAT(NAV_TRAFF_A_RADM, 500);

/**
* Set NAV TRAFFIC AVOID RADIUS
Copy link
Contributor

Choose a reason for hiding this comment

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

@LowOrbitIonCannon Can we have a new PR to change this to more natural formatting?

... following pattern as I put above

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.

8 participants