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

Servo_LPF_Hz default value - update to 150hz #10414

Closed

Conversation

trailx
Copy link
Contributor

@trailx trailx commented Oct 15, 2024

In 2017, #1267 set the servo_lpf_hz value to 20hz. This LPF is running a biquad filter, and per the calculations I've found should be producing around 16mS of control delay for fixed-wing. This could be a huge factor in control response when most of the other LPF filters in the stabilization control loop are delivering more like 1-3mS of latency.

I have tested this with the filter turned up to 400mS and (by feel) it felt smoother with less bounciness evident especially in roll. Discussion on discord brought up that it may make sense to maintain some sensible LPF, which is why I am proposing setting it to 150hz. This should only produce a delay of around 2.1mS according to estimates I've found, while still providing aliasing protection for a typical 330hz digital servo.

Added notes to the settings page highlighting the possibility of jitteriness (as brought up in #1267 ) and the potential remedy of turning this down to 20hz.

This change is easy to test by changing servo_lpf_hz to 150 in the CLI.

Servo LPF was 20 hz, changing to 150 hz.  Looking back at the reasons before don't make much sense, plus with digital servos nowadays, they are updating at 330hz typically.  The 20 hz number is outdated and is introducing excessive control delay.
@sensei-hacker
Copy link
Collaborator

sensei-hacker commented Oct 15, 2024

Something discussed on Discord is the purpose of setting it to 20 Hz was to avoid aliasing by having it lower than half the servo refresh rate of 50 Hz. Aliasing will occur if the input has a frequency component higher than half the output rate.

This PR was tested with a servo output rate of 330 Hz or so. An LPF of 150 absolutely makes sense for an output rate of 330 Hz.

The default servo output rate is still 50 Hz. Thus with default settings the lpf needs to be lower than 25 to avoid aliasing. Because the required servo lpf cutoff is entirely dependent on the servo output rate, it may make sense to by default set the LPF "auto", with auto being the servo rate X 0.4.

Unfortunately, setting the LPF to three times higher than the output rate would be expected to cause aliasing. But having the setting default to auto, with auto being just under half the output rate, might work well?

@trailx
Copy link
Contributor Author

trailx commented Oct 15, 2024

Considering that the gyro is filtered fairly well before getting to the PID controller, I'd be surprised if you'd get much oscillatory signal passthrough above 50hz. Is the potential aliasing of higher frequency servo signals even an issue here?

(hopefully I'm applying the term 'aliasing' correctly here)

I like the idea of setting this value to an automatic half or 4 tenths of the servo rate, but the programming behind that is beyond my comfort level. I can change a value and write text, but I'm a little limited in my code execution abilities.

I'd assume you'd want to keep the CLI variable for the servo LPF still a 0-400, but you'd add an if statement for if servo LPF is set to 0, then it turns to an automatic value?

Or, would a better execution be to change this term to set the ratio? So servo LPF would be like a 0-100, and it would apply that percentage rather than make it fixed? Default would be 40-50 in that case.

At this point I'm just spitballing solutions that I'm not sure I can code properly.

@sensei-hacker
Copy link
Collaborator

I'd assume you'd want to keep the CLI variable for the servo LPF still a 0-400, but you'd add an if statement for if servo LPF is set to 0, then it turns to an automatic value?

Yeah that's about what I'm thinking. With the auto value being about 40% of the output rate. For me, coding that would be easy. But I would like to get an opinion from someone else who knows more about filters etc.

@0crap
Copy link
Contributor

0crap commented Oct 16, 2024

Any reason to leave this servo filter type as a BIQUAD?

BF as well as INAV v8 use a PT1 for the gyro anti alias filter.
If it's good enough for the gyro, why not for the servo?

@0crap
Copy link
Contributor

0crap commented Oct 16, 2024

I bench tested two planes for jitteriness on the bench.
Zero difference with servo_lpf_hz = 400 vs default.

One cheap set of analog servos set at 50Hz PWM.
One nice set of digital servos set at 160Hz PWM.

@trailx
Copy link
Contributor Author

trailx commented Oct 16, 2024

LPF Delays
I agree, I don't see a need for this to be a biquad, especially considering the latency, and the fact that the input signal is already fairly smooth. Converting this to a PT1 should be adequate and would really cut down the latency more than just shifting the biquad to a higher fraction of the servo frequency.

@0crap
Copy link
Contributor

0crap commented Oct 25, 2024

I tested this today on my AR.Wing 900 in flight.
It uses cheap analog servos running at 50 Hz.

Tested doing a full stick roll in acro mode.
Used the BBL log to measure the time difference between PID_sum[roll] and servo[x]
servo_lpf_hz on 400Hz value gives 8mS.
servo_lpf_hz on stock value gives 14mS.

I don't see any issues running lpf on 400Hz in flight.
Also on the bench, servos are silent and behaving/sounding the same as with the stock lpf value.

@0crap
Copy link
Contributor

0crap commented Oct 25, 2024

I'd assume you'd want to keep the CLI variable for the servo LPF still a 0-400, but you'd add an if statement for if servo LPF is set to 0, then it turns to an automatic value?

Please don't use this method.
Setting the value to zero should indicate the filter is OFF.
Just like we do with the other (e.g. gyro) filters.

@0crap
Copy link
Contributor

0crap commented Oct 26, 2024

And tested with servo_lpf_hz = 0 (effectively switching the filter OFF)
No problems in flight. (Also not on the bench.)
Still clean servo traces in BBL.
Time difference between PID_sum[roll] and servo[x] is now 6mS.
Of course I set the markers when zoomed in to the maximum for best accuracy.
Added screenshot zoomed out a bit.

servo_lpf_hz_off

@sensei-hacker @trailx my recommendation would be to set this filter default to 400Hz.
(On my planes without a FC I have no servo filters at all and it also works fine.)
If no issues are reported, with the 400Hz setting, set it to 0 Hz on INAV 9 release.

@trailx
Copy link
Contributor Author

trailx commented Oct 26, 2024

Wow thank you for the test results. I generally feel similarly that this feature really only needs to be used by a rare few, maybe larger crafts only. The aliasing concern should be taken care of upstream by the series of gyro filters.

I am surprised at the remaining latency when set to 0 (off). Off should theoretically show no delay, so I'm not sure where the additional 6ms is coming from. Something else to look into.

I don't know why we wouldn't make this change in 8.0 or at latest 8.1.

@rts18
Copy link

rts18 commented Nov 14, 2024

I don't know why we wouldn't make this change in 8.0 or at latest 8.1.

Because its pushing a setting change that will wear-out servo's faster. I tested it on some coreless servo's. They buzzed like crazy!

Its a setting. Why not add a document that describes the benefits and downsides, so users can make their own decision on whether to alter it.
Forcing this change on unsuspecting new users isn't a good idea. Most don't read the release notes.
It's like forcing them to except extra damage to their hardware, with moderate benefits.

@MrD-RC
Copy link
Collaborator

MrD-RC commented Nov 14, 2024

I agree with @rts18 on this. There has also been a lot of discussion about this on Discord recently with @Jetrell also providing information about servos getting prematurely killed if this setting is increased.

This setting should stay at 20Hz. If a pilot wants to increase it. It's easy to do in CLI. We should not wear out equipment for minimal if any real benefit by default.

@MrD-RC
Copy link
Collaborator

MrD-RC commented Nov 14, 2024

Also, the PR itself adds in the description that it may be too much for some 50Hz servos. In which case, it can't be the default. The default should be safe with all servos.

@b14ckyy
Copy link
Collaborator

b14ckyy commented Nov 14, 2024

Additionally to what Darren said, we had a very very long discussion on Discord yesterday that showed, that a complete change for minimal filtering brings just a very small benefit for small, super agile builds with extremely fast servos. for 90% of planes out there it makes no difference in the overall control loop (from RC in to physical plane movement) while these 90% would risk damaged or molten servos. Too much risk for little to no reward.

image

So no this default should not change.

@MrD-RC MrD-RC closed this Nov 15, 2024
@trailx
Copy link
Contributor Author

trailx commented Nov 15, 2024

One of the biggest issues I have is that there is little visibility of this setting. Only when digging into the code base can you find this, which is contributing a decent delay (16ms), which is significant for most planes. My planes show a 25ms response time, so 16ms would be half that. It would be nice to at least include this on the outputs tab next to the servo update rate so people can be aware of it. 20hz is quite simply too low, in my opinion too low even for a 50hz servo. You might as well stick a 20hz PT2 on your main gyro filter and see how well the plane stays stable.

I'm still not convinced that this will actually burn out servos appreciably faster. If you truly have 100ms control loop delay, the servo travel has to be more severe to counter gyro motion because its playing catchup and the plane has moved further by that point than if the control loop was faster. How do we get actual reliability data on this? I will continue to fly this at 150 and report back any servo issues.

@MrD-RC
Copy link
Collaborator

MrD-RC commented Nov 15, 2024

It's in the CLI, so not hidden at all. The discussions on Discord went to show a minimal, if even noticeable effect, on most aircraft. The parameter is there for people to tune if they wish. But it has to be at their own risk. There can't be default settings that can burn out or shorten the life of components. Slower updates of servo positions may have bigger movements. But that is much less of an issue than more frequent shorter movements.

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.

6 participants