-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Acceleration based control/feed-forward #14212
Conversation
e7ff219
to
78c3f1d
Compare
78c3f1d
to
bb48644
Compare
I rebased on master and removed the draft since the configuration default changes are also there. |
@MaEtUgR I tried offboard acceleration feedforward with this PR, but couldn't enter offboard mode. Am I missing something? I was sending on
Same offboard setpoint works on master, but enabling acceleration setpoints introduces hunge tracking errors. To benchmark the performance I am sending |
@Jaeyoung-Lim Thanks for testing offboard!! I don't know why you can't enter offboard mode, maybe we can have a quick look together to get a clue? |
@MaEtUgR Ah, sorry for not being clear. That was the logs from master since I couldn't make it go in offboard mode with this PR. Would be great if we can dig in! |
bb48644
to
e0b9e5a
Compare
tested on pixhawk4 v5 f-450 Position Mode: Good.
Notes: Log: |
Tested on NXP FMUK66 v3Modes Tested
Procedure Notes Logs Tested on Pixhawk 3 v4ProModes Tested
Procedure Notes Logs |
Tested on PixRacer V4Tested Modes Position Mode Notes Logs https://review.px4.io/plot_app?log=6aa51b7b-f999-4aea-bcdd-1a4cb0bde0ac Tested on CUAV nano V5Tested Modes Position Mode Notes Logs https://review.px4.io/plot_app?log=d12827ac-cfd1-48d1-babe-b85b10c54264 |
e0b9e5a
to
9e84122
Compare
@dagar As we discussed I rebased (without conflict) and removed the in my eyes deprecated
EDIT: I think now CI is showing the offboard problem that @Jaeyoung-Lim saw before. |
9e84122
to
09681c7
Compare
I rebased on master and tried to reproduce the offboard issues by using the MAVSDK offboard integration tests: What do the two different errors I had come from, could they be glitches? Now also jenkins seems to cancel my build NuttX builds only one shows in the UI and whole test fails even though the GitHub workflow builds succeed and "dronecode-macmini" cannot be contacted for the Mac build. |
09681c7
to
5f0a4a0
Compare
@MaEtUgR Any docs impact to all this? |
@hamishwillee Good point. Sport mode was never documented since it wasn't too different. But the control diagram https://dev.px4.io/master/en/flight_stack/controller_diagrams.html#multicopter-position-controller needs to be adjusted a little bit. Time to look into how these diagrams 😬 |
I was able to reproduce what CI is flying and it found a real issue that I'm currently looking into. |
4aaad24
to
7131d7c
Compare
@bresch Here's the plot of the same scenario including velocity tracking. Note that it's not tracking anymore because it's hitting the maximum tilt limit and the integrator is still not winding up: Rebased on master and lowered the maximum jerk in the VTOL defaults based on @fury1895's valuable VTOL testing feedback 🎉 . That concludes testing and fixes from my side and I consider it ready to merge 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Before #14212 the velocity control gains used in the multicopter position controller were defined as a scale between velocity error in one axis (or it's integral and derivative respectively) and the unit thrust vector. The problem with this is that the normalization of the unit thrust vector changes per vehicle or even vehicle configuration as 0 and 100% thrust get a different physical response. That's why the gains are now defined as scale between velocity error (integral/derivative) and the output acceleration in m/s².
Before #14212 the velocity control gains used in the multicopter position controller were defined as a scale between velocity error in one axis (or it's integral and derivative respectively) and the unit thrust vector. The problem with this is that the normalization of the unit thrust vector changes per vehicle or even vehicle configuration as 0 and 100% thrust get a different physical response. That's why the gains are now defined as scale between velocity error (integral/derivative) and the output acceleration in m/s².
Before #14212 the velocity control gains used in the multicopter position controller were defined as a scale between velocity error in one axis (or it's integral and derivative respectively) and the unit thrust vector. The problem with this is that the normalization of the unit thrust vector changes per vehicle or even vehicle configuration as 0 and 100% thrust get a different physical response. That's why the gains are now defined as scale between velocity error (integral/derivative) and the output acceleration in m/s².
Before #14212 the velocity control gains used in the multicopter position controller were defined as a scale between velocity error in one axis (or it's integral and derivative respectively) and the unit thrust vector. The problem with this is that the normalization of the unit thrust vector changes per vehicle or even vehicle configuration as 0 and 100% thrust get a different physical response. That's why the gains are now defined as scale between velocity error (integral/derivative) and the output acceleration in m/s².
@MaEtUgR Maybe this is too late, but I have tried this with offboard control, but this doesn't seem to work with offboard control. |
@Jaeyoung-Lim It's never too late. Is there an easy way to reproduce such that we can find the issue? |
@MaEtUgR You can use the ROS node I created, but if this is too much overhead I can also create a simple mavsdk app for testing |
I think that would be useful not just for me but also to put in CI to make sure it does not break again. |
Before #14212 the velocity control gains used in the multicopter position controller were defined as a scale between velocity error in one axis (or it's integral and derivative respectively) and the unit thrust vector. The problem with this is that the normalization of the unit thrust vector changes per vehicle or even vehicle configuration as 0 and 100% thrust get a different physical response. That's why the gains are now defined as scale between velocity error (integral/derivative) and the output acceleration in m/s².
Before #14212 the velocity control gains used in the multicopter position controller were defined as a scale between velocity error in one axis (or it's integral and derivative respectively) and the unit thrust vector. The problem with this is that the normalization of the unit thrust vector changes per vehicle or even vehicle configuration as 0 and 100% thrust get a different physical response. That's why the gains are now defined as scale between velocity error (integral/derivative) and the output acceleration in m/s².
Before #14212 the velocity control gains used in the multicopter position controller were defined as a scale between velocity error in one axis (or it's integral and derivative respectively) and the unit thrust vector. The problem with this is that the normalization of the unit thrust vector changes per vehicle or even vehicle configuration as 0 and 100% thrust get a different physical response. That's why the gains are now defined as scale between velocity error (integral/derivative) and the output acceleration in m/s².
The acceleration setpoint gets implicitly inherited from the altitude flight task since #14212. This feed-forward adds an unwanted acceleration when the right stick is deflected. Instead I'm using it to command the expected centripetal acceleration when flying in a circle for better orbit tracking.
The acceleration setpoint gets implicitly inherited from the altitude flight task since #14212. This feed-forward adds an unwanted acceleration when the right stick is deflected. Instead I'm using it to command the expected centripetal acceleration when flying in a circle for better orbit tracking.
Before #14212 the velocity control gains used in the multicopter position controller were defined as a scale between velocity error in one axis (or it's integral and derivative respectively) and the unit thrust vector. The problem with this is that the normalization of the unit thrust vector changes per vehicle or even vehicle configuration as 0 and 100% thrust get a different physical response. That's why the gains are now defined as scale between velocity error (integral/derivative) and the output acceleration in m/s².
The acceleration setpoint gets implicitly inherited from the altitude flight task since PX4#14212. This feed-forward adds an unwanted acceleration when the right stick is deflected. Instead I'm using it to command the expected centripetal acceleration when flying in a circle for better orbit tracking.
The acceleration setpoint gets implicitly inherited from the altitude flight task since PX4#14212. This feed-forward adds an unwanted acceleration when the right stick is deflected. Instead I'm using it to command the expected centripetal acceleration when flying in a circle for better orbit tracking.
Describe problem solved by this pull request
More or less last step to finish splitting up and QAing #12072. It's based on #245 and introduces processing acceleration setpoints. This enables acceleration feed-forward which the jerk optimized flight tasks by @bresch immediately make use of. It also heavily reduces dependence between collective thrust and vehicle tilt making the demanded tilt less jerky when the altitude control output is changing.
Describe your solution
I make the velocity control output acceleration in m/s² which in theory would make the controller tuning more independent of the vehicle's thrust to weight ratio (~hover thrust) but for the initial introduction I introduce a scaling factor using the hover thrust to make the existing gains compatible. To separate the high-frequency altitude control changes from the collective thrust from the demanded tilt I'm switching control strategy to apply horizontal acceleration setpoints assuming hover/gravitational acceleration in the vertical direction effectively directly mapping it to the vehicle's tilt. The vertical acceleration gets projected onto the planned tilt and then executed by setting the collective thrust accordingly. Only in the limit case of more than the maximum thrust tilt gets coupled by being limited using vertical acceleration demand to ensure prioritizing altitude control over horizontal navigation.
Test data / coverage
For the basic limit cases, input combinations, failsafe and zero thrust inputs there are unit tests now. I did a lot of SITL testing and multiple outdoor tests with a 250 size, one test on a big multicopter.
Additional context
Refactorings that made this change possible: #13247, #13248, #13262, #13347, #13701, #14017