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

Enable Orbit Flight Mode #10907

Merged
merged 4 commits into from
Dec 8, 2018
Merged

Enable Orbit Flight Mode #10907

merged 4 commits into from
Dec 8, 2018

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Nov 25, 2018

Orbit
Fly a perfect circle with a multicopter which is not achievable with waypoints. The circle can be commanded and controlled in center position size and speed using the MAVLink command also speed and size can be adjusted using the remote sticks if they are present. Maximum total acceleration is limited for feasibility resons. The vehicle currently always yaws such that the front "looks at" the center point.

History

Describe problem solved by the proposed pull request
The first try to enable orbit by at the same time relieving the commander of certain duties and growing totally uncontrolled with #10199 failed. I discussed the novel principle of hirarchical command and error handling e.g. by flight tasks handling their respective commands and reporting problems/failsafes to commander with @julianoes, @Stifael, @ChristophTobler. But #10199 was rejected

This isn't safe, it ignores nearly all failsafes and current system conditions.

After lengthy discussions with @dagar and no follow up on my side due to time constraints I'm giving up and am just adding an orbit commander state. I'm not blaming @dagar, he might very well be right and I imagined the solution too easy. Still we should improve the commander situation, I'm happy to join the discussion again. Bottom line of this pr is: Orbit doesn't have to wait anymore for any architecture changes.

Describe your preferred solution
There is a commander main and vehicle navigation state now specifically for orbit. The command DO_ORBIT does just switch to the orbit main state because switching the flight task and applying the parameters was already implemented a long time ago in the flight task architecture.

Test data / coverage
Windows SITL

FYI @DonLakeFlyer (asked when Firmware will finally support what the UI promises)

EDIT: CI problems seem to be unrelated...

@dagar
Copy link
Member

dagar commented Nov 25, 2018

CI problems appear to be docker hub connectivity issues. Restarting individual jobs for now. @lamping7 I wonder if this is another case we can catch and retry.
image

@hamishwillee hamishwillee added the Documentation 📑 Anything improving the documentation of the code / ecosystem label Nov 25, 2018
@MaEtUgR MaEtUgR changed the title Enable Orbit Flight Mode [WIP] Enable Orbit Flight Mode Nov 25, 2018
Stifael
Stifael previously approved these changes Nov 26, 2018
@@ -3365,6 +3369,20 @@ set_control_mode()

break;

case vehicle_status_s::NAVIGATION_STATE_ORBIT:
control_mode.flag_control_manual_enabled = false;
control_mode.flag_control_auto_enabled = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Orbit is a good example where the flag_control_-flags breaks

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I totally agree. I certainly want to iterate on this. It's not too late to change this state to the custom flight task state that covers multiple features in the classic commander structure with feedback on feasability of the tasks themselves. But as of right now I want orbit to get usable without loosing too much extra time, it's waiting for too long.

@MaEtUgR MaEtUgR changed the title [WIP] Enable Orbit Flight Mode Enable Orbit Flight Mode Dec 3, 2018
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 3, 2018

rebased and fixed the failsafe handling more based on loiter requirements. I tested correct data link loss reaction.

@dagar Could you rereview?

@dagar
Copy link
Member

dagar commented Dec 4, 2018

Looks good. Would it be safer for now to also require manual?

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 4, 2018

It's not required in terms of the flight mode. It will continue the orbit like before with 0 or no RC input. There's a verified timeout check in flight tasks here https://github.com/PX4/Firmware/blob/master/src/lib/FlightTasks/tasks/Manual/FlightTaskManual.cpp#L75 which means it will not apply unupdated RC values. The most important thing for safety is not RC but that the pilot can stop via switching mode.

@dagar
Copy link
Member

dagar commented Dec 5, 2018

Actually it looks like we should be covered by the existing logic copied from loiter. RC loss is still respected if data link loss isn't configured.

dagar
dagar previously approved these changes Dec 5, 2018
@LorenzMeier LorenzMeier requested a review from a team December 5, 2018 06:32
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 5, 2018

@PX4/testflights To test the orbit:

  • Have an up to date QGC that supports the command (was introduced ~July/August this year).
  • Takeoff with a multicopter that has GPS and hover in an open area e.g. Position or Hold mode.
  • Press on the map in QGC like you want to command a go to action
  • Choose Orbit at the spot from the popup menu
  • You see a circle on the map (with default radius 30m)
  • Drag and resize the circle to where there are no obstacles
  • Adjust the altitude on the right side bar if necessary
  • Swipe on the bottom to start
  • It will go as fast as possible to the commanded circle and do a slow (1m/s) Orbit exactly on the planned circle looking always with the front to the center
    To stop just switch to another flight mode again using RC or QGC
    You can influence the Orbit altitude, radius, speed and direction with the RC sticks
    Left stick: up/down for altitude change like in position mode
    Right stick: right/left is more/less speed counter-/clockwise, up down is radius smaller/bigger
    Be gentle with the right stick input maximum velocity is limited by the radius, maximum radius is 100m

Telemetry to show the current, maybe adjusted commanded circle on the map is on its way, only command is working until now.

@Avysuarez
Copy link

@MaEtUgR
The PR does not work, does not initialize the ESC's and does not recognize the radio

@santiago3dr
Copy link

needs to be rebased due to #10983

@mrpollo
Copy link
Contributor

mrpollo commented Dec 5, 2018

What are your thoughts on getting this in for v1.9 @MaEtUgR ?

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 6, 2018

@santiago3dr Thanks for the hint, I rebased without conflicts.
@mrpollo I would include it. More context in PX4/PX4-user_guide#397 (comment).

@Avysuarez
Copy link

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 8, 2018

Nice tests, thanks!!
orbits

@MaEtUgR MaEtUgR merged commit e979d24 into master Dec 8, 2018
@simonegu simonegu deleted the pr-enable-orbit branch December 10, 2018 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin: Enhancement (improvement) 💡 Documentation 📑 Anything improving the documentation of the code / ecosystem Multirotor 🛸
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants