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.

Sign up for GitHub

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

Jump to bottom

[WIP] purge legacy mixers (switch to control allocation) #19596

Closed
wants to merge 6 commits into from
Closed

[WIP] purge legacy mixers (switch to control allocation) #19596

wants to merge 6 commits into from

Conversation

dagar
Copy link
Member

@dagar dagar commented May 5, 2022 •
edited
Loading

This is still a bit premature, but lack of flash space is becoming a serious roadblock.

I'm opening this now so we can start to identify everything that will be needed to fully purge the old mixing system and replace it with control allocation.

Questions

  • how do we want to handle somewhat generic airframe defaults?
    • certain boards default to usable PWM_MAIN settings?

TODO

  • review and update all airframes
    • fixedwing
    • multicopter
    • vtol
      • standard
      • tailsitter
      • tiltrotor
    • rover
    • others
    • what can we delete?
  • dshot
  • pwm_out
    • crazyflie special case
  • px4io
  • pwm_out_sim
  • tap_esc (mantis)
  • review rev (PWM_MAIN_REV) and trim
  • introduce mechanism to limit airframes to specific boards or classes of board

@dagar
Copy link
Member Author

dagar commented May 5, 2022

Saves about 50 kB of flash on px4_fmu-v5x and around 30 kB on px4_fmu-v2_default.

@dagar dagar force-pushed the pr-legacy_mixers_purge branch 3 times, most recently from f9ce8e2 to 27d0e98 Compare May 5, 2022 23:18
@dagar
Copy link
Member Author

dagar commented May 5, 2022

Given the scale of this I think we should start incrementally converting airframes in master entirely to control allocation.

@sfuhrer @bkueng @MaEtUgR

@dagar dagar force-pushed the pr-legacy_mixers_purge branch 5 times, most recently from 90876ac to 7f52897 Compare May 6, 2022 02:38
@sfuhrer
Copy link
Contributor

sfuhrer commented May 6, 2022

I think we should take a moment to think about what the purpose of the airframe configs is and which ones we need to keep. Until now with, with the legacy mixing system, at least on the VTOL/FW side they were mainly there for supporting various actuation methods without having to re-compile. E.g. a lot of people selected the "FunCub" airframe because they also had a standard VTOL with a puller/pusher, 4 MC motors, 2 ailerons, 1 elevater, 1 rudder. With this selection they though also pulled in a lot customized parameters, that somebody determined a couple of years ago (on a much older PX4 version) on his custom vehicle, and since then was likely not maintained at all.
I would prefer if we only had one generic airframe per type (so 1 generic standard VTOL, 1 Tiltrotor etc), and then only allow non-generic airframe configs if the vehicle can be bought in this configuration or if there exist detailed build instructions. It should be easy enough for people to extend the generic one to the exact setup, e.g. adding an additional motor.

@dagar
Copy link
Member Author

dagar commented May 6, 2022

Thanks @sfuhrer, I think we're in agreement. You probably didn't see it in the noise, but I already started dropping some of the nearly empty airframes.

  • generic airframes for each category
  • specific airframes that correspond to widely available RTF models and kits
    • we need to be careful here that the airframe doesn't capture more than it should (eg don't set the board rotation if it doesn't come with a flight controller)
    • we probably shouldn't even bother carrying frames that don't come with specific motors, ESCs, props, etc unless it's some non-trivial geometry
  • airframes for one specific board (like the crazyflie)

One area we should talk about is handling the transition for things like the VTOL setups where many users expected to use MAIN + AUX in a specific way. Are we ready to pull all of that away and have them rely on the GUI to assign actuators to physical outputs or should we carry a few specific transition airframes where we have something like a standard VTOL with PWM MAIN and PWM AUX configured all the way through as it is now?

@dagar dagar mentioned this pull request May 6, 2022
Merged
@taileron
Copy link
Contributor

taileron commented May 7, 2022

Thanks @sfuhrer, I think we're in agreement. You probably didn't see it in the noise, but I already started dropping some of the nearly empty airframes.

  • generic airframes for each category

  • specific airframes that correspond to widely available RTF models and kits

    • we need to be careful here that the airframe doesn't capture more than it should (eg don't set the board rotation if it doesn't come with a flight controller)
    • we probably shouldn't even bother carrying frames that don't come with specific motors, ESCs, props, etc unless it's some non-trivial geometry

  • airframes for one specific board (like the crazyflie)

One area we should talk about is handling the transition for things like the VTOL setups where many users expected to use MAIN + AUX in a specific way. Are we ready to pull all of that away and have them rely on the GUI to assign actuators to physical outputs or should we carry a few specific transition airframes where we have something like a standard VTOL with PWM MAIN and PWM AUX configured all the way through as it is now?

Is it perhaps even possible with the redesign of the mixer structure to decouple middle positions from limits and only mix on a fixed percentage or per 1000 basis? Currently, changing the upper or lower PWM limit shifts both the center and the gain of the respective mix function.

@junwoo091400 junwoo091400 mentioned this pull request May 8, 2022
Open
19 tasks
@sfuhrer
Copy link
Contributor

sfuhrer commented May 9, 2022

Is it perhaps even possible with the redesign of the mixer structure to decouple middle positions from limits and only mix on a fixed percentage or per 1000 basis? Currently, changing the upper or lower PWM limit shifts both the center and the gain of the respective mix function.

Changing the PWM min/max still also has an effect on the middle position, but you can additionally set an "output trim" to get servos centered. Please try it out and provide feedback, though best in a separate issue report, as it's not directly linked to the purpose of this PR.

@sfuhrer
Copy link
Contributor

sfuhrer commented May 9, 2022 •
edited
Loading

One area we should talk about is handling the transition for things like the VTOL setups where many users expected to use MAIN + AUX in a specific way. Are we ready to pull all of that away and have them rely on the GUI to assign actuators to physical outputs or should we carry a few specific transition airframes where we have something like a standard VTOL with PWM MAIN and PWM AUX configured all the way through as it is now?

I would think the GUI + documentation + a walk-through video is enough and as safe as it gets.

@sfuhrer sfuhrer closed this May 9, 2022
@sfuhrer sfuhrer reopened this May 9, 2022
@dagar dagar mentioned this pull request May 20, 2022
Merged
@sfuhrer sfuhrer mentioned this pull request May 24, 2022
Merged
2 tasks
@dagar dagar mentioned this pull request May 27, 2022
Merged
@sfuhrer sfuhrer mentioned this pull request Jun 1, 2022
Merged
dagar referenced this pull request Jun 17, 2022
@potaito @dagar
New follow-me flight task
de1fa11 
rename follow_me_status to follow_target_status

enable follow_target_estimator on skynode

implement the responsiveness parameter:
The responsiveness parameter should behave similarly to the previous
follow-me implementation in navigator. The difference here is that
there are now two separate gains for position and velocity fusion.
The previous implemenation in navigator had no velocity fusion.

Allow follow-me to be flown without RC

SITL tests for follow-me flight task

This includes:
- Testing the setting for the follow-me angle
- Testing that streaming position only or position
  and velocity measurements both work
- Testing that RC override works

Most of these tests are done with a simulated model
of a point object that moves on a straight line. So
nothing too spectacular. But it makes the test checks
much easier.

Since the estimator for the target actually checks new
measurements and compares them to old ones, I also added
random gausian noise to the measurements with a fixed seed
for deterministic randomness. So repeated runs produce
exactly the same results over and over.

Half of the angles are still missing in MAVSDK. Need to create
an upstream PR to add center left/right and rear left/right options.
These and the corresponding SITL tests need to be implemented
later.

sitl: Increase position tolerance during follow-me

Astro seems to barely exceed the current tolerance (4.3 !< 4.0)
causing CI to fail. The point of the CI test is not to check
the accuracy of the flight behaviour, but only the fact that the
drone is doing the expected thing. So the exact value of this
tolerance is not really important.

follow-me: gimbal control in follow-me

follow-me: create sub-routines in flight task class

follow-me: use ground-dist for emergency ascent

dist_bottom is only defined when a ground facing distance sensor exist.
It's therefore better to use dist_ground instead, which has the distance
to the home altitude if no distance sensor is available.

As a consequence it will only be possible to use follow-me in a valley
when the drone has a distance sensor.

follow-me: point gimbal to the ground in 2D mode

follow-me: another fuzzy msg handling for the estimator

follow-me: bugfix in acceleration saturation limit

follow-me: parameter for filter delay compensation

mantis: dont use flow for terrain estimation

follow-me: default responsiveness 0.5 -> 0.1

0.5 is way too jerky in real and simulated tests.

flight_task: clarify comments for bottom distance

follow-me: minor comment improvement

follow-me: [debug] log emergency_ascent

follow-me: [debug] log gimbal pitch

follow-me: [debug] status values for follow-me estimator

follow-me: setting for gimbal tracking mode

follow-me: release gimbal control at destruction

mavsdk: cosmetics 💄
@dagar dagar changed the title [DO NOT MERGE] purge legacy mixers (exploratory) [WIP] purge legacy mixers (switch to control allocation) Jun 21, 2022
@dagar dagar requested review from sfuhrer, bkueng and MaEtUgR June 21, 2022 21:19
@dagar
Copy link
Member Author

dagar commented Sep 15, 2022

Done!

@dagar dagar closed this Sep 15, 2022
@dagar dagar deleted the pr-legacy_mixers_purge branch September 15, 2022 17:54
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfuhrer sfuhrer Awaiting requested review from sfuhrer

@bkueng bkueng Awaiting requested review from bkueng

@MaEtUgR MaEtUgR Awaiting requested review from MaEtUgR

Assignees
No one assigned
Labels
Admin: Cleanup (housekeeping) 🧹 Admin: Enhancement (improvement) 💡
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dagar @sfuhrer @taileron