-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Notch filter df1 #14755
Notch filter df1 #14755
Changes from 6 commits
c9b62d5
9bb4e14
64079b5
18ba48a
aa46f20
54ab5a1
2012c00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,7 @@ | |
* @brief Implementation of a Notch filter. | ||
* | ||
* @author Mathieu Bresciani <brescianimathieu@gmail.com> | ||
* @author Samuel Garcin <samuel.garcin@wecorpindustries.com> | ||
*/ | ||
|
||
#pragma once | ||
|
@@ -69,7 +70,7 @@ class NotchFilter | |
void setParameters(float sample_freq, float notch_freq, float bandwidth); | ||
|
||
/** | ||
* Add a new raw value to the filter | ||
* Add a new raw value to the filter using the Direct form II | ||
* | ||
* @return retrieve the filtered result | ||
*/ | ||
|
@@ -85,6 +86,28 @@ class NotchFilter | |
return output; | ||
} | ||
|
||
/** | ||
* Add a new raw value to the filter using the Direct Form I | ||
* | ||
* @return retrieve the filtered result | ||
*/ | ||
inline T applyDF1(const T &sample) | ||
{ | ||
// Direct Form I implementation | ||
const T output = _b0 * sample + _b1 * _delay_element_1 + _b2 * _delay_element_2 - _a1 * _delay_element_output_1 - _a2 * | ||
_delay_element_output_2; | ||
|
||
// shift inputs | ||
_delay_element_2 = _delay_element_1; | ||
_delay_element_1 = sample; | ||
|
||
// shift outputs | ||
_delay_element_output_2 = _delay_element_output_1; | ||
_delay_element_output_1 = output; | ||
|
||
return output; | ||
} | ||
|
||
float getNotchFreq() const { return _notch_freq; } | ||
float getBandwidth() const { return _bandwidth; } | ||
|
||
|
@@ -99,8 +122,26 @@ class NotchFilter | |
b[2] = _b2; | ||
} | ||
|
||
/** | ||
* Bypasses the filter update to directly set different filter coefficients. | ||
* Note: the filtered frequency and quality factor saved on the filter lose their | ||
* physical meaning if you use this method to change the filter's coefficients. | ||
* Used for creating clones of a specific filter. | ||
*/ | ||
void setCoefficients(float a[2], float b[3]) | ||
{ | ||
_a1 = a[0]; | ||
_a2 = a[1]; | ||
_b0 = b[0]; | ||
_b1 = b[1]; | ||
_b2 = b[2]; | ||
} | ||
|
||
|
||
T reset(const T &sample); | ||
|
||
void update(float sample_freq, float notch_freq, float bandwidth); | ||
|
||
protected: | ||
float _notch_freq{}; | ||
float _bandwidth{}; | ||
|
@@ -115,6 +156,8 @@ class NotchFilter | |
|
||
T _delay_element_1; | ||
T _delay_element_2; | ||
T _delay_element_output_1; | ||
T _delay_element_output_2; | ||
}; | ||
|
||
template<typename T> | ||
|
@@ -125,6 +168,8 @@ void NotchFilter<T>::setParameters(float sample_freq, float notch_freq, float ba | |
|
||
_delay_element_1 = {}; | ||
_delay_element_2 = {}; | ||
_delay_element_output_1 = {}; | ||
_delay_element_output_2 = {}; | ||
|
||
if (notch_freq <= 0.f) { | ||
// no filtering | ||
|
@@ -165,4 +210,27 @@ T NotchFilter<T>::reset(const T &sample) | |
return apply(sample); | ||
} | ||
|
||
/** | ||
* Allows to dynamically update the filtered frequency, refresh rate and quality factor. | ||
* Changes the filter's coefficients while conserving the filter's history | ||
* Note: This method will only work if using the applyDF1 function (Direct Form I). | ||
*/ | ||
template<typename T> | ||
void NotchFilter<T>::update(float sample_freq, float notch_freq, float bandwidth) | ||
{ | ||
// backup state | ||
T delay_element_1_bkup = _delay_element_1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we could gain a bit of efficiency if we move the zeroing of the delay elements out of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds good, then the update() method is redundant and can be deleted. We would be using the setParameters() method directly to update the filters when needed. In that case, I guess we can just initialise the delay elements directly in the class definition and it should do the job just fine. Is that what you meant? Concerning the reset() method, I guess I should update it with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @bresch , I have made changes according to your review, following the approach outlined my comment above. You can refer to a more detailed description within the commit message. Not all of the CI tests are passing, with the Offboard position control test failing. However I believe it is a problem with the test itself as I haven't modified code relevant to that test. Let me know if there are any further changes required. Thanks! |
||
T delay_element_2_bkup = _delay_element_2; | ||
T delay_element_output_1_bkup = _delay_element_output_1; | ||
T delay_element_output_2_bkup = _delay_element_output_2; | ||
|
||
setParameters(sample_freq, notch_freq, bandwidth); | ||
|
||
// restore state | ||
_delay_element_1 = delay_element_1_bkup; | ||
_delay_element_2 = delay_element_2_bkup; | ||
_delay_element_output_1 = delay_element_output_1_bkup; | ||
_delay_element_output_2 = delay_element_output_2_bkup; | ||
} | ||
|
||
} // namespace math |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@francelico I just noticed the actual update() code is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the
update
isn't required as we can update the coefficients usingsetParameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, yes you are correct @bresch this method is not used anywhere else, it can be deleted. @dagar how should I proceed for the fix, should I make a new PR (and mention this one in the description)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll delete it quickly if there's no intention of using it. Thanks for the followup. #16061