ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
10.18k stars 16.75k forks source link

Solo Slew Rate software fix for ESC signal issues #5988

Closed proficnc closed 5 years ago

proficnc commented 7 years ago

Issue details

Please describe the problem, or desired feature Solo ESC's have a serious flaw that can cause them to stop for a second or two if they fail to receive pulses above 2.4v the overall design of the Solo electrical system causes a ground lift of over 1v in some situations. So at 3.3v signal level coming out of the flight control is only seen as 2.3v at the esc in this situation, causing the ESC to register no pulses, and shut down

there are some seriously negative side effects of trying to fix this serious hardware problem in software, however, upgrading the autopilot in the solo is a cost, and some people may be happy with the reduced performance of adding the slew rate limiter.

It is also worth noting that even with this limit, a heavily loaded vehicle could still have this symptom regardless of the slew rate setting.

Version

What version was the issue encountered with Any version that may be used on a SOLO or any other airframe with this issue.

Platform

[ ] All [ ] AntennaTracker [ X ] Copter [ ] Plane [ ] Rover

Airframe type

What type of airframe (flying wing, glider, hex, Y6, octa etc) SOLO

Hardware type

What autopilot hardware was used? (pixhawk, pixracer, PX4FMU etc) Pixhawk 2.0 Pixhawk 2.1 (no mod) Doesnt affect the Solo Green cube modified version of the cube. This version outputs 5V on the PWM lines which gives a 2.6v head on the ESC required voltage level.

Pedals2Paddles commented 7 years ago

Here is a log file showing this happening on my solo. They show desired vs actual roll parting ways as actually two ESCs shut down at once. Then come back on and recover the tumble. https://drive.google.com/open?id=0BwxNJyHTG9XJQW9BZzltTWRVNXM

Here is a video of the incident. https://youtu.be/MdLdnYLlMw4

At the risk of adding complications to master... But also broadening the user base... Could a parameter be added that will enable/disable the slewing that handicaps the throttle output like it does in the solo version? That would let the stock solo hardware run master at least as safely as it runs the solo version, with the software handicapping. But if someone has upgraded to a 2.1 or green cube with 5v signalling, they could disable it and have the full non handicapped output.

I'm thinking a parameter like MOT_SLEW_SOLO. Value of zero for default with no slewing. Value of 1 for solo slewing.

This would allow more users to upgrade to master without falling out of the sky. But still allow those of us who upgrade to a PH2.1 to not be handicapped.

jmachuca77 commented 7 years ago

I think this is definitely something we want in. There are thousands of Solo's out there, and as Philip said not all of them will have the means to switch the Pixhawk, and it is likely once apps like Mission Planner and Solex start letting you upgrade to master that they will upgrade. Using a parameter or some other means of enabling it would be a great option, also if this could be done automatically without the user having to enable it, that would be even better.

Pedals2Paddles commented 7 years ago

Exactly. We can say "there is a risk to doing this without a hardware upgrade" all day long. But 1000 people will do it anyway on the assumption "it will probably be ok". We'll end up running a Solo boneyard as a side business. So while the hardware upgrade is the best and most complete solution, we can't ignore the software solution too. By having a MOT_SLEW_SOLO parameter default of zero with new slewing, that will prevent it from handicapping everyone else using Arducopter as well. This method virtually eliminates the need for a solo specific compile in my opinion.

Pedals2Paddles commented 7 years ago

The relevant commit in the Solo Arducopter branch is here: https://github.com/3drobotics/ardupilot-solo/commit/dad46b1ecc26494b7dd4f4817d59a7c9559da338

It appears the master version of AP_MotorsMatrix.cpp and AP_MotorsMatrix.h is vastly different than the Solo branch version. So I can't determine where exactly in each file this code needs to go, or how to link it to a new Parameter. All way over my head.

hugheaves commented 7 years ago

Just thought I'd add pointers to the commits that implement the ESC failure detection / motor recovery as well. As the detection / recovery code was disabled in the solo-1.2.3 firmware when the slew rate fix was introduced, and then re-enabled later in the solo 1.2.22 firmware, I'm guessing it still provides a useful backup when the slew rate fix isn't sufficient.

Relevant commits:

May 23, 2015: Copter: add ESC failure workaround https://github.com/3drobotics/ardupilot-solo/commit/dbb1c1e362edf0363f1c407f9bc5038cbe500699

Oct 16, 2015: AP_Motors: add slew rate limiter https://github.com/3drobotics/ardupilot-solo/commit/dad46b1ecc26494b7dd4f4817d59a7c9559da338

Oct 16, 2015: Copter: don't run motor recovery https://github.com/3drobotics/ardupilot-solo/commit/0336bf80a29862cfd8b0f62a259e306b4db768e3

Feb 4, 2016: Copter: re-enable motor recovery https://github.com/3drobotics/ardupilot-solo/commit/2efbdf3132371da80bb35893e6418b237ff39c81

Pedals2Paddles commented 7 years ago

I had no idea that stuff existed! Those should definitely go in as well. TBH the slew rate is at the bottom of the to-do list for master at the moment. I think it does eventually need to get in there though.

hugheaves commented 7 years ago

I don't know if it's any help, but I took a whack at updating and merging the ardupilot-solo ESC fixes with Copter 3.5. They're currently sitting in a branch in my ardupilot fork:

https://github.com/hugheaves/ardupilot/commits/Copter-3.5-solo

This is obviously going to need some testing. :)

Anyway, now that I've poked around in the code, it seems there are some improvements and simplifications that could be made to this fix. In no particular order:

  1. The ESC failure recovery and slew rate limiting code do essentially the same thing (limit rate of change of motor input), but in completely different ways. Should they be merged into a single slew rate limiting code path. (or into the already existing "soft start" code)
  2. The ESC fix is sprinkled throughout AP_MotorsMatrix, AP_MotorsMulticopter, Copter, and a new "recovery.cpp" containing some new Copter methods. This structure seems to be due to the need to access some protected members in the motors class hierarchy. It mighbe better to place of the ESC fix code into a subclass of AP_MotorsMatrix.
  3. The ESC / motor failure is detected fairly indirectly - by monitoring PWM outputs for an imbalanced condition. I guess this has worked for a while now, but I wondered if there might be a way to augment this with battery current sensing. (sample / response rate too low, maybe?)
hugheaves commented 7 years ago

I went ahead and re-wrote and moved the code into a subclass of AP_MotorsMatrix. It's definitely a lot less clutter in the mainline code that way - just a matter of instantiating a different motors class.

Pedals2Paddles, I just noticed you've worked on this too. Let me know if you want to sync up.

Would any of you experienced developers have a second to chime in on why the current approach limits the prop deceleration rate as well as the acceleration rate? Does the symmetry make the flight control smoother / more stable?

Would it be worth experimenting with asymmetrical acceleration / deceleration rates, possibly also varying over the output range, with faster slew rates at lower outputs? I thought it might make things a little more responsive in a less heavily laden craft.

I realize of course, that the PixHawk 2.1 is the real fix, but it's fun to see what can be squeezed out of broken hardware. :)

Pedals2Paddles commented 7 years ago

@hugheaves I must have missed this last week. Nice work! I know some people that are ready and willing to test it. I can also pull one of my fleet to test it. Do you mind if I pull your commits into a branch of my beta and distribute for testing?

This is all way over my head when it comes to C++. So my work on it was sloppy mess that didn't work. I know it didn't work since I had to fish that Solo out of the lake....

hugheaves commented 7 years ago

@Pedals2Paddles, as you just got your drone dried out, let me make sure my rewrite doesn't result in similar ArduSub behavior. I want to get some good bench testing in, and at some basic flight testing before telling you give it a try.

I'll let you know how its going in a few days.

Pedals2Paddles commented 7 years ago

lol sounds good!

hugheaves commented 7 years ago

@Pedals2Paddles, bench testing looks good so far. I can adjust the "MOT_SLEW_PER_SEC" parameter, and clearly see the change in slew rate / responsiveness. However, with my copter strapped to the bench, I haven't been able to induce an ESC comm failure, so I haven't been able to do any full-stack verification on the motor recovery code.

Still trying to figure that out before I do any flight testing, but thought I'd let you know that progress is being made. :)

Pedals2Paddles commented 7 years ago

Nice! I was never able to see a difference when I was doing it so clearly you did it right this time!

proficnc commented 7 years ago

Be aware, it's highly unlikely that you will ever replicate the failure on the bench.

Do not underestimate this hardware bug.

Also, ensure Jon Challinger and Leonard Hall both go over this in detail before even thinking about test flying it

hugheaves commented 7 years ago

@proficnc, understood, and thanks for the feedback. Despite my rather dry humor, i can assure you that I'm actually a worst case scenario type of guy. I like to keep in mind that a 2kg copter at 100 meters has more potential energy than a rifle bullet, and proceed accordingly with testing.

hugheaves commented 7 years ago

Just a quick update. I was able to perform some good bench tests on the failure detection / recovery. I created a "test jig" to tilt the copter (with me well out of prop range :) , which was sufficient to trigger the failure detector and start the recovery. I was surprised to see how much difference the slew rate limit makes to peak current draw. I guess that's why it works. :)

Anyway everything seems to work well, so I'm going to clean up the code a little, and submit a PR in the next day or two for code review.

Pedals2Paddles commented 7 years ago

What does the failure detection do? Is it looking for the copter attitude to be out of whack, assume an ESC is offline, and recycle the signal output?

hugheaves commented 7 years ago

@Pedals2Paddles , nope, it just looks at the motor output levels. If one motor output is a lot higher than the others for more than 200ms, then it assumes that the FC is trying to compensate for a failed motor and initiates a power down and soft restart of all the motors.

meee1 commented 7 years ago

@hugheaves is there some kind of debounce on that filter? to prevent multiple back to back commanded motor cuts?

hugheaves commented 7 years ago

@meee1, yes. It uses a low pass filter on the input side to filter out transients, and on the output side, it has a 750ms "post recovery" period where it will not initiate another recovery. Of course, the loss of thrust during restart means that the motors are going to have to work harder after the restart, which probably increases the chance of another comm failure, etc.

lthall commented 6 years ago

I will add the slew limit into the code but I won't add the solo specific recovery code as it only protects the aircraft that don't loose enough thrust to flip. I suspect that adding the slew limit will be enough for 95% of the aircraft and the recovery code will only catch an additional 2.5%. (numbers not based on measured data)

proficnc commented 6 years ago

I support Leonard’s proposal

hugheaves commented 6 years ago

Leonard, I'm guessing you might just want to roll your own solution here, but if you want me to update my pull request with my latest code, let me know. I fixed a few issues in the original PR, and it's had quite a bit of flight testing since then.

lthall commented 6 years ago

@hugheaves If you are happy to pull the reset code out we can start from there. The only other thing I think we should do is make the slew rate based on a normalised rate. So 1 is 100% per second.

I would be more than happy to use your code!

hugheaves commented 6 years ago

Will do. I'll remove the reset portion, and adjust the basis of the rate.

Pedals2Paddles commented 5 years ago

Reviving this for dev call discussion on Monday. From prior discussion on this matter, I think Leonard was planning to make this happen at some point. It would spectacular if this could make it into the 3.6 release candidates if there is still time. That would make AC 3.6.0 fully compatible with Solo without any replacement hardware required.

proficnc commented 5 years ago

136 requests from solo users in less than one day...

This is a well wanted feature... maybe Chibios only?

Pedals2Paddles commented 5 years ago

@lthall

hsteinhaus commented 5 years ago

Just discovered this issue and got reminded about a few weeks that I wasted some years ago. At that time, I tried to "cure" sync loss problems by limiting the slew rate of motors to prevent short-term over current situations, that caused the sync losses. I tried both, implementing it in ESC and in FC software and I failed. Here is why:

As soon as the copter is maneuvering dynamically at higher air speeds or is flying in gusty wind, the load of a motor can change suddenly even at constant throttle, mostly due to sharp angle of attack changes on the prop blades. This kind of load change can only be detected by measuring (phase) current in the ESC itself, it cannot be prevented by any kind of open loop control.

TLDR: you will still crash as soon as you do more than gentle hovering in still air

Pedals2Paddles commented 5 years ago

TLDR: you will still crash as soon as you do more than gentle hovering in still air

That is simply not the case. Tens of thousands of solos fly with this slew rate limiter, and it does work in all phases of flight, not just a gentle hover There aren't Solo's falling from the sky like rain. Perhaps it was useless in the vehicle you tried it in. But the design characteristics of the Solo are such that it does work.

Of course it isn't perfect and there will be edge cases that exceed it's ability to protect the vehicle. And it has performance impacts too. But the notion that it only works in a gentle hover on the Solo is simply incorrect.

hdtechk commented 5 years ago

I believe this can be closed now?

Pedals2Paddles commented 5 years ago

Happily yes! Closed by https://github.com/ArduPilot/ardupilot/pull/9929