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

Orbit approach transition improvements #18988

Merged
merged 9 commits into from
Jan 25, 2022
Merged

Orbit approach transition improvements #18988

merged 9 commits into from
Jan 25, 2022

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Jan 10, 2022

Describe problem solved by this pull request
It's possible to have steps when entering and exiting the circle approach:
image-20220105-120441

Describe your solution

  1. @ThomasDebrunner removed switching to a circle approach if the radius is just slightly adjusted using the stick.
  2. I found a typo and did some variable name improvement refactoring.
  3. I checked how the position smoothing is initialized and made sure it's using the previous setpoint rather than the current vehicle state. This should give better results because the resulting error should then be continuous while if the setpoint jumps to the current vehicle state the error jumps to zero.

Describe possible alternatives
We should further improve the interface for using the jerk trajectory library to make it easier to reuse.

Test data / coverage
So far only parts are tested. I still have to test the latest state of this which I'll do the day after tomorrow.

@ThomasDebrunner
Copy link
Member

Thanks for cleaning up, looks good to me!

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Jan 13, 2022

Something is sadly broken in the reapproach 😞 I'll fix it and report back.

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Jan 21, 2022

Took much longer to figure all the problems out but now it seems ok to me but my learning is that we can't take care of all smooth transitions individually there are too many errors to be made. I suggest we try to have a jerk trajectory generator running more or less all the time and adjust the parameters or skip it only for when it's necessary.

Problems were:

  • Invalid setpoints could lead to flyaway: Multicopter position control: Invalid setpoint can windup integrator #19041
  • There were invalid altitude acceleration setpoints because I initialized the altitude smoothing for the next circle approach with the altitude setpoints from the circle smoothing but acceleration was not properly set.
  • In the circle the stick-based smoothed altitude change did not work properly, setpoints like the acceleration could get stuck, there were jumps. I used the inheritance from FlightTaskManualAltitudeSmoothVel again to solve this and made sure the altitude change command works.
  • In the vertical circle approach it would always use maximum downwards speed also for upwards.

The only remaining thing I could find is that the altitude given in an orbit command is not exactly reached because when close enough it switches back to stick altitude steering and to get to the exact commanded altitude smoothly is currently not possible without changing everything. This can be improved once we can easily give up the dependency on FlightTaskManualAltitudeSmoothVel.

@ThomasDebrunner
Copy link
Member

Tested in SITL, all working fine. As a further improvement, we could limit the radial speed on orbit radius change

ThomasDebrunner and others added 9 commits January 24, 2022 17:47
the parameters are anyways sanitized on every update so even if activate()
sets unfeasible one's they get adjusted on the first update.
…ublishing

I think they were forgotten and it leads to side effects:
- The acceleration feed-forward does not get executed
- The acceleration setpoint is NAN when initializing the altitude
  smoothing when arriving at the circle
This is done by inheriting from FlightTaskManualAltitudeSmoothVel again.
The altitude change by command is taken care of by switching
to the apporach when the altitude difference is big enough and
switching back once the altitude is close enough.

The altitude of the command is not perfectly reached but this can
only be done smoothly when the Orbit has full control over the
altitude smoothing. The independent altitude smoothing is not kept
because it was lacking stick handling like altitude lock and smooth
transitions when opening and closing the vertical position loop.
…and velocity changes

instead of arbitrary fractions of the maximum radius and velocity.
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Jan 24, 2022

As a further improvement, we could limit the radial speed on orbit radius change

Like discussed I just added this (after a rebase on master) in 75445b4

Before After
Velocity for radius change Maximum radius / 8 = 12m/s Cruise speed default = 5m/s
Acceleration for velocity change Maximum velocity / 4 = 2.5m/s^2 Manual horizontal acceleration default = 3m/s^2

@MaEtUgR MaEtUgR merged commit 42404ad into master Jan 25, 2022
@MaEtUgR MaEtUgR deleted the orbit-cleanup branch January 25, 2022 10:45
@hamishwillee
Copy link
Contributor

@MaEtUgR Are there any docs updates required for this in http://docs.px4.io/master/en/flight_modes/orbit.html#orbit-mc ?

(and the other PRS in PX4 1.13 release notes?

Specifically, I don't think we need to talk about improvements in the experience unless they have associated parameters that need to be changed, or if they change anything we say in the above doc.

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.

None yet

3 participants