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

WIP - Live mixer tuning through mavlink including px4io #5726

Closed
wants to merge 18 commits into from

Conversation

crashmatt
Copy link

@crashmatt crashmatt commented Feb 14, 2017

WORK IN PROGRESS - FOR REVIEW ONLY

This is the ardupilot version of this px4 pull request to enable fast tuning of mixer parameters.
PX4/PX4-Autopilot#6357

REVIEW ACTIONS

Mixer solution
Fix a consistent mixer solution for all platforms

px4io
PX4io resource and I2C loading.
Restriction on parameter writes according to operating/safety mode. The mixer has to be live for this to work.

Build configuration
The mixer tuning feature is completely optional. MIXER_CONFIGURATION must be defined for the build. This is currently hard coded into px4_targets. The original intent of this option is to reduce risk and reduce code build size on px4.

CHANGES
Changed from mavlink messages to mavlink commands.

TODO

  • Fix submodules.. again
  • Mixer save and recall.
  • Fix the standard builds to pass the build tests
  • Possibly change MIXER_CONFIGURATION to MIXER_TUNING. This will enable space for a true mixer configuration feature.
  • Tidy commits into one single item.
  • Tidy and comment code

NOTE: Mixer parameters are not the same as mavlink parameters. There is good reason for this. A description document is being maintained here:

https://docs.google.com/document/d/1LeQC2qtxqdesNuJ3RAWH_MJ_30xjf-PMN3bd5wtUAUo/edit?usp=sharing

@crashmatt
Copy link
Author

@magicrub Just so no-one feels left out, I did the same mixer stuff for ardupilot. That and you talked me into thinking it is the best choice.

@magicrub
Copy link
Contributor

@crashmatt Hi Matt! Thanks for the PR, we'll take a look.

and you talked me into thinking it is the best choice

I'm not sure when I did that but I appreciate you checking out ArduPilot! Never hurts to see how other systems work, they all have their pros/cons and do things a little different.

@crashmatt
Copy link
Author

@magicrub Your not wrong about ArduPilot and px4 having different styles. I can't work out which will get me to my target quicker. px4 doesn't look strong on fixed wing support. ArduPilot has TECS and nice estimator features.

It looks like ArduPlane doesn't run the px4 mixers when running SITL or HITL. This makes progress difficult and testing more so. A fix is to add the px4-fmu mixer devices to ArduPlane including SITL.

@magicrub
Copy link
Contributor

@crashmatt this PR may be of interest to you: autonomous thermaling #5737

@crashmatt
Copy link
Author

@magicrub The checks are failing on git submodule update --init --recursive
I had to fork and branch the PX4Firmware and mavlink submodules.
I downloaded a fresh repo, checked out the mixer branch and ran submodule update. Everything works.
Any idea what might be going wrong?
Thanks for the auto thermalling PR. Now I just need a way to get control of the aircraft and find a way to get an autopilot inside. Simple...

@@ -1,6 +1,6 @@
[submodule "modules/PX4Firmware"]
path = modules/PX4Firmware
url = git://github.com/ArduPilot/PX4Firmware.git
url = https://github.com/crashmatt/Firmware.git
Copy link
Contributor

Choose a reason for hiding this comment

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

This is why Travis is failing. Changes like this are not allowed. You need to do separate PRs for submodules. Travis will only pull in branches from origin.

Copy link
Author

Choose a reason for hiding this comment

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

The PX4 project appears to pull submodules from fork/branch. I might have been fooling myself.

Next step is to get those mavlnk messages fixed.

Considering the volume of real world fixed wing users I am tending to think that ArduPlane is best bet.

Copy link
Member

Choose a reason for hiding this comment

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

Travis will pull whatever is in the submodules file, Semaphore might not. In any case, this would always have to be removed before merging. The error was there because the commit didn't exist in GitHub at the time - apparently it's there now so I've restarted the build and it worked; there are still errors but not related to this.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks Francisco. Travis is giving an error I understand. Semaphore is stuck with the module change as you suggest.

@WickedShell
Copy link
Contributor

Okay I think I'm missing something but what are you looking to tune with the mixer?

There are a couple of caveats that need to be thrown out there.

First is that ArduPlane only uses the PX4IO mixer if you have activated overrides, otherwise ArduPilot simply submits PWM values that the IO directly outputs.

Second you don't ever want to touch the PX4IO mixer while in flight. It can fail to accept a mixer, which would then leave you with no valid mixer. I typically see 4 attempts on average to program a mixer, and I've had the same mixer fail 1000's of times (It hadn't succeeded in programming it after 10 minutes of retrying). The current mixer protocol is just to weak to do this. The other note is that there is a very noticeable hitch in being able to control servos with overrides while the mixer is programmed (typically manifests as a freeze for about a second).

Now it's possible I've missed the intent of this entirely (I haven't glanced at code yet), but @tridge has been talking about creating a proper RC Tx like mixing system on the ArduPilot side for a long time, so if that is what this is meant for that could be interesting.

@crashmatt
Copy link
Author

crashmatt commented Feb 16, 2017 via email

@WickedShell
Copy link
Contributor

@crashmatt Are you trying to tune manual flight mode? As I stated the mixers are unused for any autopilot controlled flight stages, so tuning them would not have an effect. And under manual control your mixing can be done on the RC Tx side.

@crashmatt
Copy link
Author

@WickedShell The answer is both. The reason is a demand of aerodynamics. Consider aileron differential. Aileron differential depends on flap and airbrake setting and perhaps pitch. Flap position depends on airspeed and pitch. The transmitter has to send a roll command that the transmitter understands. How does it do this while also sending the mixed servo output? The mixer must be on the autopilot and be always in the loop.

I am concerned that ardupilot overrides the mixing in autopilot mode. If that is the case I would not be surprised if the community has not moved much beyond flying wings and vtail.

@crashmatt
Copy link
Author

@WickedShell and here is the offending article:
https://github.com/ArduPilot/ardupilot/blob/master/ArduPlane/servos.cpp#L663

To make it worse this whole thing is lumped into the Plane class. Putting an alternative mixer system in means significant modifications. It is possible to build a new system in parallel with conditional build. That keeps the legacy code untouched for lowest risk. I have done all of this once before on UDB/MatrixPilot and that was back in 2013.
http://diydrones.com/profiles/blogs/anything-you-want-mixing

And the next question is what mode that X-Plane override is running on. There is an override before and after the mixers. MatrixPilot used a plugin where the servos outputs could go to the flight surfaces rather than the pilot controls. If we can't get that level of control then there is no good way forward.

@WickedShell
Copy link
Contributor

@crashmatt The mixers need to be on the ArduPilot side rather then the PX4IO layer. The problem with the PX4IO layer is that it doesn't apply to all the different hardware platforms. We have far more control if we do it on the ArduPilot side. The only role the PX4IO is meant to fulfill is to actually generate the requested PWM signal, and takeover if the main FMU fails.

I'm not opposed to a new mixer setup, but I don't think chasing the PX4IO side is a productive and safe solution. Especially as this is a feature that doesn't apply to all the supported board types, and a new mixer system that works on all platforms has been on the list.

@crashmatt
Copy link
Author

crashmatt commented Feb 17, 2017

@WickedShell I agree that you need control of functionality on the ArduPilot side. The upstream px4io firmware has changed significantly. It has gained more multirotor hacks which have made the whole thing more cluttered. Having had a good look at both codebases, it is my opinion that Ardupilot px4io firmware has already forked permanently away from upstream. Best to make a clean break and do whatever is required for ArduPilot.

The existing px4io mixer structure will not support my needs so I must re-write it anyway. It makes complete sense to do something cross platform compatible. Mixers run SITL for the px4 platform and that is the only way to succeed.
That and I have a desire to remove the px4io mixer setup by sending it human readable text. Honestly!

The px4 mixers are compiled for both fmu and px4io. I don't see an immediate reason that they won't compile for ardupilot. My suggestion is to start this way, modify the px4io mixers to our liking and then make it a submodule. That way all platforms can use it. Does this make sense?

For now I am finished prototyping the mavlink mixer tuning. It's main objective was to test if px4io mixers could be modified via mavlink. Given our conversation the details will probably change before commit. I will change focus to this new issue.

UPDATE: I just compiled px4io MixerGroup direct into the Plane class and it works :-)

@tridge
Copy link
Contributor

tridge commented Feb 18, 2017

Hi Matthew, adding a generic mixer is certainly a goal for ArduPilot. It was one of the motivations of the recent addition of the libraries/SRV_Channel work.
I don't want to use the px4 mixer though. As I think you've noticed already, the px4 mixer doesn't currently do anything in ArduPilot unless either the FMU is in failsafe in fixed wing, or you setup an override channel to use the mixer in manual mode.
We want a mixer that works in:

  • all vehicle types (copter, plane, rover, sub)
  • all board types (SITL, Linux, px4 etc)
  • allows access to many internal state variables
  • uses a binary internal representation
  • preferably supports arbitrary arithmetic operations

I'm thinking of doing it by defining new k_* variables in SRV_Channel, called k_custom1 to k_custom32. Then in a mixer file you could have:

C1 = sqrt(Roll2 + Pitch3) * Rudder*0.1

that isn't a useful mixer of course, but it gives you the idea of the type of syntax I'd like.
You could then set:

SERVO7_FUNCTION=101
where 101 would be k_custom1. That would get activate that expression on servo7.

@crashmatt
Copy link
Author

@tridge Thanks for the list of requirements.

I agree that the px4 mixer is not optimal. I think I am stuck with the px4io failsafe so I have to make it work nicely for manual flight.

How deep do you wish to have access to internal state variables? Is it good enough to copy them to the mixer inputs or do you wish to have direct access?

The existing .mix files are difficult to use since there is no context of what each value in the file does. Your syntax suggestion is much better.
I need to think of the possibility for mavlink tuning with this script concept. I have ~150 parameters in my aircraft mixer so it would help if this was easy to do.

@tridge
Copy link
Contributor

tridge commented Feb 19, 2017

Hi Matthew,
The plan was to implement the mixer so that it is linked into both the FMU and px4io firmware. As you point out, the access to state variables will be the trick. For example, if you make the mixer depend on airspeed then if FMU is dead the px4io won't have an airspeed estimate to use.
Note that by default when flying manual in fixed wing now with ArduPilot the mixer is not used. It is only used if you setup the OVERRIDE_CHAN parameter to specifically force operation of mixing with px4io. The only other time the mixer is used if if the FMU stops communication with px4io, such as a fatal software error in the main flight stack. That is the primary motivation for setting up the existing mixer.
I'd suggest we continue to operate in this way with the mixer primarily running on FMU, with the mixer only activating on px4io if the user commands it or we have a fatal error. That allows a richer mixer to be used, even in manual modes.
For the case where we are in px4io override mode (where mixer is active on px4io), we have a few choices:

  • we could define fixed values for variables that are not available. For example, if we allowed Airspeed in the mixer then we could have a "Airspeed = 15" default in the mixer for when airspeed is not available.
  • we could require that the mixer not use variables not available to px4io when defining mixes for manual mode

I know this is all a bit vague at the moment. I'm still thinking through how all of this will work. Suggestions welcome!
Cheers, Tridge

@crashmatt
Copy link
Author

@tridge I agree that px4io/uavcan should not limit the ardupilot mixers. If failsafe mode has to operate with a subset of full functionality then that is ok. I just have the need to get my aircraft safely back to the ground, preferably in one piece.

Have been thinking on your script proposal and the more I look the more I like it. It almost looks like it would run directly as python script which would make an interesting mavproxy module. As a use case test I started to re-write my existing mixer in python to see what it looks like. I will share this once I have it properly commented.

Doing mavlink tuning is still a bit of a mystery but I am starting to get a grip on it.

Very slightly off topic:
One concern about SRV_Channel which might be misguided. The community still has a strong desire to keep using mixers at the transmitter. This is not surprising since there isn't anything good enough to replace it. To make progress I suggest we do all we can to get away from that thinking. As such I recommend to clearly separate code for transmitter commands (roll, pitch, yaw) and aircraft controls (aileron, elevator, rudder). Does this make sense?

@crashmatt
Copy link
Author

@tridge One of the problems with this script is generating it from the autopilot. If changes are made to the mixer through mavlink then a new script need generating. It might be that we can have variables declared and those are the only changes allowed to the script.

I can see a way to get all of the mixer descriptions through mavlink. I can also see a way that you might directly use system variables and pass the names of those variables to mavlink.

Things are moving quickly now. Moving back to development on the px4 side. This is much quicker since the px4 mixers run under SITL.

I hope to deliver a mavlink mixer support architecture that will support any mixer topology. This is the short term goal (1-3wks). After that I can come back to the mixer itself.

@WickedShell
Copy link
Contributor

Closing this for now as the conclusion is to approach this differently, as per discuss.ardupilot.org/t/better-mixers-for-arduplane-px4-mavlink-etc/9390

@crashmatt crashmatt deleted the mixer branch April 30, 2017 21:40
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

6 participants