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

Add new mode AUTO_PRECLAND #9261

Merged
merged 1 commit into from
Apr 11, 2018
Merged

Conversation

okalachev
Copy link
Contributor

@okalachev okalachev commented Apr 6, 2018

This adds support of a new mode for precision landing, so it can be set and viewed using MAVLink.

Corresponding mavros PR: mavlink/mavros#994.

Tested in SITL.

@dagar
Copy link
Member

dagar commented Apr 7, 2018

Looks right, although to be honest I was still hoping we'd find a good way to make this functional without having yet another flight mode.

The custom flight mode header needs to be copied over to QGC. https://github.com/mavlink/qgroundcontrol/blob/master/src/FirmwarePlugin/PX4/px4_custom_mode.h

@okalachev
Copy link
Contributor Author

okalachev commented Apr 7, 2018

@dagar , actually this is already functional using auto missions.

But, I also can do commander mode auto:precland, and switch to the precision landing. But then the vehicle mode becomes broken in mavros and QGroundControl.

So, yes, QGC needs to be updated also.

@mhkabir
Copy link
Member

mhkabir commented Apr 7, 2018

@okalachev This should be good to go. Can you make a QGC PR too? :)

@okalachev
Copy link
Contributor Author

@DonLakeFlyer
Copy link
Contributor

How does this work? I assume the user does not manually change to precision land flight mode. It's just some vehicle setup which changes any landing to precision landing?

@okalachev
Copy link
Contributor Author

okalachev commented Apr 7, 2018

In mission, it's a MAV_CMD_NAV_LAND parameter. Internally, it's a commander and navigator states (like TAKEOFF, LAND and FOLLOW_TARGET). All of the above are also a MAVLink states, so they can be viewed and set (with restrictions) through QGroundControl and mavros.

So user can manually switch the vehicle to, for example, FOLLOW_TARGET and TAKEOFF modes.

@DonLakeFlyer
Copy link
Contributor

How does a mission using MAV_CMD_NAV_LAND change from regular landing to precision landing?

@okalachev
Copy link
Contributor Author

It's the second parameter of MAV_CMD_NAV_LAND: 0 = normal landing, 1 = opportunistic precision landing, 2 = required precision landing.

@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Apr 7, 2018

So that is a change to mavlink spec which QGC knows nothing about. When mavlink mission item spec changes QGC needs to be notified/modified as well otherwise users will not be able to set this value in MAV_CMD_NAV_LAND mission items.

Also when QGC commands the vehicle to land using a guided action (left side buttons) it is always going to set param2=0.

Is this new flight mode really something the user can manually change to by changing flight mode? The LAND flight mode is currently just used as a state indicator. The user does not change to LAND flight mode. Is this new flight mode different in that the user should be able to select it manually (not from a mission).

It seems a bit strange that this isn't a parameter setting on the vehicle to attempt precision land as opposed to having to specify precision land in the command you are using. What happens when you RTL?

@okalachev
Copy link
Contributor Author

okalachev commented Apr 7, 2018

So that is a change to mavlink spec which QGC knows nothing about. When mavlink mission item spec changes QGC needs to be notified/modified as well otherwise users will not be able to set this value in MAV_CMD_NAV_LAND mission items.

Yes, this changes to spec were made, when PX4 precision landing was introduced, see the docs, http://mavlink.org/messages/common. Although I'm not sure that this is the final list of the precision landing variants.

And yes, there some options have to be added to MAV_CMD_NAV_LAND in mission editor.

Also when QGC commands the vehicle to land using a guided action (left side buttons) it is always going to set param2=0.

This is clear. Maybe, this button should contain some options.

Is this new flight mode really something the user can manually change to by changing flight mode? The LAND flight mode is currently just used as a state indicator. The user does not change to LAND flight mode. Is this new flight mode different in that the user should be able to select it manually (not from a mission).

Yes, with this PR code, the user can perform a precision landing using regular MAV_CMD_DO_SET_MODE. And, LAND is actually also a regular flight mode, so it's can be turned on, using regular mode switching (with mavros or anything else).

It seems a bit strange that this isn't a parameter setting on the vehicle to attempt precision land as opposed to having to specify precision land in the command you are using. What happens when you RTL?

Actually, this architecture was chosen, when precision landing feature was developed. But, I think, that you may have several landing points in your mission, and some of them can be precise, and some of them not.

@DonLakeFlyer
Copy link
Contributor

It would seem to me that this would be used mostly in missions. So not super interested in cluttering the QGC UI with additional user selectable modes for landing when not flying a mission. Whether that be in the flight mode drop down or the Guided Land button. Regular Land is not currently a selectable flight mode by the user. Doesn't seem like precision land would be either. Does that make sense to everyone?

@okalachev
Copy link
Contributor Author

I'm okay, if this mode can't be selected in drop-down menu. But if it would be a MAVLink mode, it should be displayed in QGC correctly, agreed?

As an alternative, the internal PRECLAND mode can be mapped to regular LAND MAVLink mode. But, in this case we cannot turn on precision landing using modes switching (but we can, using COMMAND_LONG).

@DonLakeFlyer
Copy link
Contributor

But if it would be a MAVLink mode, it should be displayed in QGC correctly, agreed?

Yes. It just need to be marked as not user selectable in your pull. Can you add an Issue to QGC to add the new param support to NAV_LAND for missions?

@okalachev
Copy link
Contributor Author

It just need to be marked as not user selectable in your pull.

OK, did it.

Can you add an Issue to QGC to add the new param support to NAV_LAND for missions?

May be I consider to do a PR about this, but note, that this feature isn't released yet.

@okalachev
Copy link
Contributor Author

@ndepal , what do you think?

@ndepal
Copy link
Contributor

ndepal commented Apr 9, 2018

Thanks @okalachev .

I agree with @DonLakeFlyer that this mode will usually be used within a mission, so being able to select it in QGC is not crucial.

It seems a bit strange that this isn't a parameter setting on the vehicle to attempt precision land as opposed to having to specify precision land in the command you are using. What happens when you RTL?

The reason for specifying it for in the land waypoint itself is so that you can have normal and precise landings, as @okalachev said. Precision landing in RTL mode is not currently supported, but I would like to add this.

Also when QGC commands the vehicle to land using a guided action (left side buttons) it is always going to set param2=0.

Perhaps setting it to 1 (opportunistic precision landing) would be a good option?

@DonLakeFlyer
Copy link
Contributor

Perhaps setting it to 1 (opportunistic precision landing) would be a good option?

It's unclear what affect that will have on other firmwares and/or mavlink generic vehicles. Usually not a good idea to use non-default command values for generic capabilities like Land.

@okalachev
Copy link
Contributor Author

QGroundControl already adopted this mode, so I think this should be merged.

@mhkabir mhkabir merged commit 1b1617b into PX4:master Apr 11, 2018
@TSC21
Copy link
Member

TSC21 commented Apr 11, 2018

@okalachev
Copy link
Contributor Author

okalachev commented Apr 11, 2018

I'm checking...

@mhkabir
Copy link
Member

mhkabir commented Apr 11, 2018

#9290

@mhkabir
Copy link
Member

mhkabir commented Apr 11, 2018

Not sure how it got through CI. @dagar ideas?

@okalachev
Copy link
Contributor Author

#8582 this PR has changed custom_mode to pointer, it recently has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants