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

Filter: added test of phase lag vs attenuation #27164

Merged
merged 2 commits into from
Jun 1, 2024

Conversation

tridge
Copy link
Contributor

@tridge tridge commented May 26, 2024

image

this was for a single notch set at 60Hz with bandwidth of 60Hz. Source frequency was 50Hz, and the test frequency varies for the 5 graphs from 10Hz to 50Hz.

@IamPete1
Copy link
Member

I guess if you were to interpolate the crossing point it would fix the stepping.

@tridge tridge force-pushed the pr-phase-lag-vs-attenuation branch from 2845d71 to c876c59 Compare May 28, 2024 01:06
@IamPete1 IamPete1 self-assigned this May 28, 2024
@IamPete1
Copy link
Member

IamPete1 commented May 28, 2024

Fix here: IamPete1@25ccdb3

image

We do pick up some funny stuff at 0 attenuation because we don't deal with the phase wrap correctly, moving to floating point math means we get a phase of about 360 and about 0 so the averaging means we get some value in between (eg.180 not 0).

To elaborate slight more on the issue, because the noise frequency of 50Hz is a exact multiple of the 2000Hz sample rate we get the samples happening at the same point in the sine every time. This means the error in the zero crossing caculation does not average out.

@IamPete1 IamPete1 removed their assignment May 28, 2024
@IamPete1 IamPete1 force-pushed the pr-phase-lag-vs-attenuation branch from c876c59 to 68e35cd Compare May 29, 2024 10:56
@tridge tridge force-pushed the pr-phase-lag-vs-attenuation branch from 68e35cd to 169bac6 Compare June 1, 2024 04:08
@tridge tridge merged commit 51c77fe into ArduPilot:master Jun 1, 2024
91 checks passed
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.

3 participants