-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Adjusted PWM switching frequency to match the setting #397
Adjusted PWM switching frequency to match the setting #397
Conversation
Personally I lean more towards keeping it the way it is as there is more power in the zero-vector frequency than in the repetition-frequency of the timer, so if you try to answer the question of how much ripple current you are going to see in a motor with a given inductance the zero-vector frequency is going to give the most correct answer (unless I'm mistaken, was a while ago that I looked into it). However, if "people in the industry" have a strong preference for this definition I can go with that too as it also makes sense. There are a few things to consider with this change though:
My biggest problem with this change is that I don't like breaking backwards-compatibility unless it is for a good reason. Avoiding major confusion would be a good reason, but there are other ways to achieve that. Here are a few examples:
I'm leaning mostly towards 1), but 3) is fine with me too if people really have a strong preference. What do you think? |
TLDR; I am strongly in favor of the change, but not the way it's proposed in this PR. I would go the route @vedderb proposes in steps 2 and 3.
I think this is an under-appreciated point. It might seem churlish for someone to abandon a project because of something so trivial, but it's precisely because it's so trivial that it's a big red flag. It leaves the user wondering what else is wrong. PWM switching frequency has a long-standing definition which is used by the power electronics industry world-wide. A simple google search for "switching frequency" yields vast amounts of industry and academic references which use this term. It's great to go against the grain, but IMHO this isn't the battle to pick with the industry.
I agree as well. I don't like that this PR allows silent breakage for third-party builds[*], and it also silently breaks documentation.
Long-term, documentation of this non-standard usage is a kludge. If someone forks the project, or if the project server dies, then the forum thread is lost. Likewise, a tool-tip is non-obvious in a world were interfaces are intuitive and the VESC-Tool interface uses the familiar-- and thus intuitive-- term "switching frequency".
Love it!
I would do this in tandem with No. 2. It makes the code correct internally, and any third-party[*] developers will be forcibly aware of the change. I also like it because it makes it easy to decouple the PWM frequency from the control loop frequency. (Who knows, maybe some future chip or approach will allow for the PWM frequency to be some other multiple than 2?) [*] "It will break some out-of-tree builds (is this the correct term?)" |
To avoid confusion to users and keep the code as is the everything is kept the same, except the name. Now it is called Zero Vector Frequency, which will always be twice the switching frequency of the mosfets. |
Industry standard for the switching frequency is to have it define the switching frequency of the igbts / mosfets.
Yes, center aligned PWM has less first harmonic content than edge aligned, but still there is a large first harmonic component at higher duty cycles.
For reference:
![afbeelding](https://user-images.githubusercontent.com/79989749/148283062-e70d69b8-0712-4b22-8557-c6fb57ebbec1.png)
You can also easily confirm this using rotor lock and an audio spectrum analyzer (e.g. Spectroid). At higher duty cycles there is a prominent first harmonic, unless you excite in the (0 .. 60. .. 120 ... deg) angle direction.
20 kHz VESC setting without the fix, exciting in 90 deg rotor lock:
![Screenshot_20220105-212845_Spectroid](https://user-images.githubusercontent.com/79989749/148285204-130b0870-2f69-48b7-902b-d2eb0250fd93.jpg)
Clear 10 kHz visible.
20 kHz VESC setting without the fix, exciting in 0 deg rotor lock:
![Screenshot_20220105-212946_Spectroid](https://user-images.githubusercontent.com/79989749/148285252-9e118d08-5afa-4398-ba7c-3fd014315ba7.jpg)
No 10 kHz visible.
When the motor is rotating of course all angles are being excited so you will hear the 10 kHz. Especially of higher duty cycles the first harmonic becomes large.
Most important reason why I would like to get this changed/fixed is that this might actually scare away people from industry that might actually be able to bring a lot to the VESC project. I almost stopped looking into VESC due to this, and only continues because a college lent me a VESC of his. Would be sad if we lose out on knowledgeable people due to a simple semantics thing.
Interested to hear your thoughts on this!
PS. everything runs fine with this PR on my motors. Adjusted all locations I could find that use the parameter, to not cause any change in behavior.