Adjusted PWM switching frequency to match the setting

[ElwinBoots]

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:

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:

Clear 10 kHz visible.

20 kHz VESC setting without the fix, exciting in 0 deg rotor lock:

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.

[vedderb]

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:

1. It will not change anything other than a definition, and things will still work the same without any improvements.
2. It will break all old XML motors configs.
3. It will break some out-of-tree builds (is this the correct term?) for custom hardware that I have and probably other people too.

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:

1. Keep things exactly the way they are, but add this information with some links to the forum threads and this PR in the help text in VESC Tool.
2. Rename switching frequency to e.g. zero vector frequency or some better name.
3. Implement this change, but change foc_f_sw to e.g. foc_f_tim so that loading old configs won't apply the switching frequency and use the default one as that is likely to cause less damage. This will also cause compilation errors in 3) above, which makes people aware of the problem and points them to investigate the new definition.

I'm leaning mostly towards 1), but 3) is fine with me too if people really have a strong preference. What do you think?

[kubark42]

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.

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.

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.

---

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.

I agree as well. I don't like that this PR allows silent breakage for third-party builds[*], and it also silently breaks documentation.

1. Keep things exactly the way they are, but add this information with some links to the forum threads and this PR in the help text in VESC Tool.

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".

2. Rename switching frequency to e.g. zero vector frequency or some better name.

Love it!

3. Implement this change, but change foc_f_sw to e.g. foc_f_tim so that loading old configs won't apply the switching frequency and use the default one as that is likely to cause less damage. This will also cause compilation errors in 3) above, which makes people aware of the problem and points them to investigate the new definition.

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?)" out-of-treemeans when you build from outside the source tree. E.g. mkdir build && make -f ../build/bldc/Makefile

[ElwinBoots]

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.

[kubark42]

Zero Vector Frequency renaming occurs in these commits:

https://github.com/vedderb/bldc/commit/af55c79a2a8bfded65b6d4f3109fd75c6ff14239 https://github.com/vedderb/bldc/commit/a092ea99d2d0a64a968a80b895f10e4d792eabbd https://github.com/vedderb/bldc/commit/648b8a33d52500212c254b4f202124f57969a03a https://github.com/vedderb/bldc/commit/768a9a633a5d511d4cc6d0bb923db64de322d2c6