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

STM32 HAL-only PWM driver #430

Draft
wants to merge 14 commits into
base: dev
Choose a base branch
from

Conversation

runger1101001
Copy link
Member

This is an initial attempt to port the PWM driver for STM32 to HAL only. Until now, our PWM driver has used the stm32duino HardwareTimer implementation. This PR replaces all references to the HardwareTimer API with direct calls to HAL or STM32 LL APIs.

Why?

  • HardwareTimer is not supported on the Arduino H7 boards (Portenta, Nicla, Giga) which use Mbed. Therefore our standard STM32 driver was not being used there. Instead we had the "portenta" driver specifically for the portenta, but it was not far in development. It's a shame that we were not supporting the most powerful Arduino boards properly. With this HAL-only driver it compiles on Arduino Portenta as well, and supports all the features of the STM32 driver.
  • HardwareTimer was restrictive in that it has an opinionated API which did not always suit our purposes. So we were using HAL and LL calls in several places anyway.
  • HardwareTimer is less performant - it performs lots of checks and maintains internal state we don't need.

Additionally, the HAL only driver implements a number of other improvements over the HardwareTimer version:

  • better multi-motor support
  • improved PWM period setting - faster PWM periods possible on fast hardware and maximum possible range used for desired frequency
  • better performance when setting PWM values: fewer floating point operations and comparisons
  • aligning timers is now per motor rather than globally
  • when using a single timer only in any PWM mode, the timer update is inhibited while the PWM values are being set. After all values are set, PWM updates are reactivated. This means PWM updates always occur in sync for all phases when a single timer is used.
  • when using software 6-PWM also on multiple timers, the timer updates are inhibited while the PWM values are being set. This guarantees glitch-free software 6-PWM.
  • aligned timers are configured in gated mode, meaning slaves are automatically started and stopped with the master timer. This guarantees timers will stay aligned

This PR is still in draft. Open issues to address:

  • test on Portenta, Giga and other H7 platforms
  • fix current sensing code for L4, F7, F1 and other series that I did not yet fix
  • test in combination with current sensing
  • see if some of the MBED only code can be replaced by code already included (discovering timer frequencies)
  • add attributions for the code adapted from Arduino HardwareTimer

@runger1101001 runger1101001 added the enhancement New feature or request label Aug 24, 2024
@runger1101001
Copy link
Member Author

This is the output from a Portenta H7:

Motor init ...
TIM1-CH1 TIM1-CH2 TIM3-CH1 score: 2
STM32-DRV: best: TIM1-CH1 TIM1-CH2 TIM3-CH1 score: 2
STM32-DRV: Initializing TIM1
STM32-DRV: Timer resolution set to: 4000
STM32-DRV: Configured TIM1_CH1
STM32-DRV: Configured TIM1_CH2
STM32-DRV: Initializing TIM3
STM32-DRV: Timer resolution set to: 4000
STM32-DRV: Configured TIM3_CH1
STM32-DRV: Syncronising timers! Timer no. 2
STM32-DRV: master timer: TIM1
STM32-DRV: slave timer: TIM3
PWM running ...
PWM running ...
PWM running ...

And this is a trace of the PWM:

image

I consider this a POC success. The STM32 driver is running on PortentaH7 under mbed.

  • PWM output is working
  • TImer linking is working
  • Driver code has been cleaned up
  • Driver code has several improvements

Still TODO:

  • Test it on M4 core of Portenta
  • Test it on Giga R1 board
  • Test with current sensing
  • Test with 6-PWM
  • Test improvement: update disable during PWM setting
  • Test improvement: fast and slow PWM speeds
  • Test performance (compare to previous version)

@runger1101001
Copy link
Member Author

With build_flags = -D SIMPLEFOC_PWM_ACTIVE_HIGH=false in 3-PWM:

image

The duty cycles are set to 0.1, 0.5 and 0.75, as in the first picture above, but with active-low polarity. This is on Portenta H7.

@runger1101001
Copy link
Member Author

Taking a look at the PWM frequency, we can now set it to 500kHz:

image

The Portenta H7 has 200MHz timer clock, so this still gives us a resolution of 200. But as we see the output warns about this:

Motor init ...
TIM1-CH1 TIM1-CH2 TIM3-CH1 score: 2
STM32-DRV: best: TIM1-CH1 TIM1-CH2 TIM3-CH1 score: 2
STM32-DRV: Initializing TIM1
STM32-DRV: WARN timer resolution too low (<8bit): 200
STM32-DRV: Configured TIM1_CH1
STM32-DRV: Configured TIM1_CH2
STM32-DRV: Initializing TIM3
STM32-DRV: WARN timer resolution too low (<8bit): 200
STM32-DRV: Configured TIM3_CH1
STM32-DRV: Syncronising timers! Timer no. 2
STM32-DRV: master timer: TIM1
STM32-DRV: slave timer: TIM3
PWM running ...

@runger1101001
Copy link
Member Author

Also very slow PWM speeds can be set. Here is 500Hz:

image

Note the high range we get on the PWM resolution when we can use the pre-scaler due to the low frequency:

Motor init ...
TIM1-CH1 TIM1-CH2 TIM3-CH1 score: 2
STM32-DRV: best: TIM1-CH1 TIM1-CH2 TIM3-CH1 score: 2
STM32-DRV: Initializing TIM1
STM32-DRV: Timer resolution set to: 50000
STM32-DRV: Configured TIM1_CH1
STM32-DRV: Configured TIM1_CH2
STM32-DRV: Initializing TIM3
STM32-DRV: Timer resolution set to: 50000
STM32-DRV: Configured TIM3_CH1
STM32-DRV: Syncronising timers! Timer no. 2
STM32-DRV: master timer: TIM1
STM32-DRV: slave timer: TIM3
PWM running ...

@runger1101001
Copy link
Member Author

On the Giga R1, from ArduinoIDE (since PlatformIO doesn't have board files for it):

image

Motor init ...
TIM4-CH3 TIM4-CH2 TIM4-CH4 score: 1
STM32-DRV: best: TIM4-CH3 TIM4-CH2 TIM4-CH4 score: 1
STM32-DRV: Initializing TIM4
STM32-DRV: Timer resolution set to: 4800
STM32-DRV: Configured TIM4_CH3
STM32-DRV: Configured TIM4_CH2
STM32-DRV: Configured TIM4_CH4
STM32-DRV: Syncronising timers! Timer no. 1
PWM running ...

Note that we get even more PWM resolution for 25kHz than on the Portenta H7 - the Giga must be running with a higher clock frequency.

@runger1101001
Copy link
Member Author

Testing 6-PWM on Giga R1 hits a snag: it's very tricky to find all the pins from TIM1 or TIM8 on the headers - but there is a combination for TIM8 that should work. But it doesn't because the PinMap_PWM seems to contain only one timer entry for each pin. Fixing this would require patching the PinMap used by mbed - its pre-compiled :-(

Testing software-6-PWM is possible, for example with the pin combination:

BLDCDriver6PWM driver = BLDCDriver6PWM(10,12, 7,15, 8,9);

Motor init ...
TIM1-CH1 TIM1-CH2 TIM3-CH1 TIM3-CH2 TIM4-CH3 TIM4-CH4 score: 13
STM32-DRV: best: TIM1-CH1 TIM1-CH2 TIM3-CH1 TIM3-CH2 TIM4-CH3 TIM4-CH4 score: 13
STM32-DRV: Initializing TIM1
STM32-DRV: Timer resolution set to: 4800
STM32-DRV: Configured TIM1_CH1
STM32-DRV: Configured TIM1_CH2
STM32-DRV: Initializing TIM3
STM32-DRV: Timer resolution set to: 4800
STM32-DRV: Configured TIM3_CH1
STM32-DRV: Configured TIM3_CH2
STM32-DRV: Initializing TIM4
STM32-DRV: Timer resolution set to: 4800
STM32-DRV: Configured TIM4_CH3
STM32-DRV: Configured TIM4_CH4
STM32-DRV: Syncronising timers! Timer no. 3
STM32-DRV: master timer: TIM1
STM32-DRV: slave timer: TIM3
STM32-DRV: slave timer: TIM4
PWM running ...

Which gives:

image
image

(note the 9% vs 11% duty cycle, shown are phases Ah, Al, Bh)

@runger1101001
Copy link
Member Author

Still TODO:

Test it on M4 core of H747 MCUs
Test with current sensing
Test with hardware 6-PWM
Test improvement: update disable during PWM setting
Test performance (compare to previous version)
Issue found: seems like software 6-PWM needs explicit enable() while 3-PWM is enabled directly after init() - investigate and correct (all modes should init disabled, and enable after the call to enable() )
Issue found: looks like the first PWM cycle isn't centre aligned
Todo: enabling linked timers - correct method and sequence for enabling the slave timers so they start concurrently with the master timer

Edge case: shared timers between different motors, and enabling/disabling individual motors

@runger1101001
Copy link
Member Author

6-PWM is working too, tested on Nucleo-64 G431RB:

image

@runger1101001
Copy link
Member Author

I've been spending quite some time investigating the software 6-PWM.

I have found something that irritates me, and I think it is probably an error of sorts:

image

Looking at the trace above, what is happening is that I am updating the duty cycles very often, many times per PWM cycle. So the shadow registers of the CCxR registers are being constantly written to.
The 6 channels represent the high and low signals for the three phases, in the order AH, AL, BH, BL, CH, CL. Phase A is being switched between 10% and 90% duty cycle. Phase B is constant at 40% duty cycle. And Phase C is being switched between 25% and 75% duty cycle.

Examining phase A, I would expect to see a pattern like this:
image
... everywhere either 10% or 90% duty cycle, maybe not strictly alternating, but only these two duty cycles should be present, right?

But actually we see many sections like this:
image
What's going on here?
--> so the update event for the timer occurs on both overflow and underflow. But we use centre-aligned PWM, so each cycle counts from 0 to ARR and back down, first up counting. then down counting. So what we're seeing here is the switch between 10% and 90% also occurring on the overflow update events... so basically half way through the PWM cycle, during the high side on-time, we switch between duty cycle values.
This results in the "middle width" duty cycles we see in the pic above - it's half of 90% + half of 10%. It also means the on-time is not well centred in the PWM cycle.

Observations:

  1. I will cross-check, but I wonder if this has to do with my new driver code or if the old code was also doing it. So far I can't find a way to disable the update on the CCxR registers on overflow.
  2. Probably this problem occurs just as much in 3-PWM or hardware 6-PWM mode, it doesn't seem related to the software 6-PWM specifically. I will cross-check it.

I'm guessing this doesn't normally interfere with operations - we normally don't switch 10%-90% - so in practice the differences between duty cycles set will be smaller, and therefore the amount of "off-centreness" will be smaller in practice.

Other observations on software 6-PWM:

  • Phase A is TIM3 while the other two are on TIM1. Only TIM1 phases are properly aligned. The TIM3 phase is not. So there is still a bug in the timer alignment
  • Using the Logic Analyser software's search function, even on long samples of several seconds, I cannot find any instances of shoot-through. The different timer channels for H and L are always updated synchronously (even if sometimes during overflow event, they still update together). This is expected since we are locking the updates on the timer while we set the shadow registers. We were not doing that in the previous stm32 driver, so I will cross check this to see if I can produce the error in the old driver.

@runger1101001
Copy link
Member Author

Cross-Checking the same software 6-PWM config using 2.3.4 version of the library:

image
image

Ok, that's pretty catastrophic.... :-(

So results, as far as I can see are:

  1. cross check that update on overflow problem exists in old code is confirmed - it certainly does
  2. cross check that not locking update events while updating the CCxR registers is a problem that leads to shoot-through: confirmed, shoot-through is clearly visible in the traces. From my point of view this problem would exist also if updates occurred on down-counting only, but with two updates per PWM cycle it becomes very visible...
  3. Timer Alignment is incorrect also in this version of the library - it looks in these traces like the TIM3 and TIM1 are in fact out-of-phase, but this may be a coincidence.

@Candas1
Copy link
Collaborator

Candas1 commented Sep 19, 2024

Hi Richard,

I am not sure if I understood your point well.
Regarding this:
image

As you are alternating 10% and 90% many times within the PWM period, it could the be last pwm set for 2 consecutive periods was 90% ?
That's why it's usually good to run the foc code in an interrupt, so that you just set the pwm once before the update event when downcounting.

Regarding the PWM signals being out of phase, it should not happen with Antun's new Timer alignment code(master/slave) if timers could be aligned.

I believe this alignment only happens when enabling the timers and not at the update, but this shouldn't drift.

@runger1101001
Copy link
Member Author

@Candas1

As you are alternating 10% and 90% many times within the PWM period, it could the be last pwm set for 2 consecutive periods was 90% ?

The aim of my test code is to set the CCRx registers very quickly, so to force the situation where the updates happen "as the new period starts". So I am trying to maximally provoke the error we expect to see without LL_TIM_DisableUpdateEvent, so to speak.

So the CCRx are changing all the time, but even so we only want the shadow register to update "in sync" for the different channels. This is actually working, if we compare the old version 2.3.4 without LL_TIM_DisableUpdateEvent and my new version that includes it.

That's why it's usually good to run the foc code in an interrupt, so that so just set the pwm once before the update event when downcounting.

yes, absolutely, but the normal SimpleFOC code doesn't use interrupts for this, so the CCRx writes happen at a "random" time compared to the PWM period. It leads to the very unfortunate waveforms shown, where the on-time is determined half from the previous PWM value and half from the new PWM value.
For me it's a totally unexpected result. When I switch centre-aligned PWM between 10% and 90% I expect to see only 10% or 90% duty cycles, always well centred. Instead we see sometimes 10%, sometimes 90% and sometimes 50%, and the 50% is not properly centred.

Regarding the PWM signals being out of phase, it should not happen with Antun's new Timer alignment code(master/slave) if timers could be aligned.

Unfortunately it does not work as expected. I am using TIM3 for phase A, and TIM1 for phase B & C, and you can see that both for v2.3.4 and my new code, the TIM3 is exactly out of phase compared to TIM1. (of course, phases B & C are coming from the same timer so they're always aligned).
In this case the alignment is succeeding, supposedly. Both old version and new version report successful alignment of the timers.
The result is the same if I switch the order and put TIM1 first. In this case it does not matter because on the STM32G431RB TIM1 and TIM3 can be master/slave of each other.

It's totally confusing to me, because from my point of view it should be working. Both TIM1 and TIM3 are initialised in the same way: CENTRE_ALIGNED3 mode, PWM2_MODE on the high side and PWM1_MODE on the low side. In all cases they use the same polarity (TIM_OCPOLARITY_HIGH).

The only real difference is that TIM1 is an advanced control timer, while TIM3 is a general purpose timer, and therefore TIM1 gets an extra MOE_ENABLE when initialising it.

Here's a trace of the startup output:

Motor init ...
Core Clock: 170000000
TIM3-CH2 TIM3-CH1 TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: 12
TIM3-CH2 TIM3-CH1 TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -7
TIM3-CH2 TIM3-CH1 TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -7
TIM3-CH2 TIM3-CH1 TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -7
TIM3-CH2 TIM8-CH2N TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: -6
TIM3-CH2 TIM8-CH2N TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -6
TIM3-CH2 TIM8-CH2N TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -6
TIM3-CH2 TIM8-CH2N TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -6
TIM3-CH2 TIM16-CH1 TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: -7
TIM3-CH2 TIM16-CH1 TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -7
TIM3-CH2 TIM16-CH1 TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -7
TIM3-CH2 TIM16-CH1 TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -7
TIM8-CH3N TIM3-CH1 TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: -5
TIM8-CH3N TIM3-CH1 TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -5
TIM8-CH3N TIM3-CH1 TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -5
TIM8-CH3N TIM3-CH1 TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -5
TIM8-CH3N TIM8-CH2N TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: -5
TIM8-CH3N TIM8-CH2N TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -5
TIM8-CH3N TIM8-CH2N TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -5
TIM8-CH3N TIM8-CH2N TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -5
TIM8-CH3N TIM16-CH1 TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: -5
TIM8-CH3N TIM16-CH1 TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -5
TIM8-CH3N TIM16-CH1 TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -5
TIM8-CH3N TIM16-CH1 TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -5
TIM17-CH1 TIM3-CH1 TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: -7
TIM17-CH1 TIM3-CH1 TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -7
TIM17-CH1 TIM3-CH1 TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -7
TIM17-CH1 TIM3-CH1 TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -7
TIM17-CH1 TIM8-CH2N TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: -6
TIM17-CH1 TIM8-CH2N TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -6
TIM17-CH1 TIM8-CH2N TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -6
TIM17-CH1 TIM8-CH2N TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -6
TIM17-CH1 TIM16-CH1 TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: -7
TIM17-CH1 TIM16-CH1 TIM1-CH1 TIM1-CH2 TIM2-CH4 TIM1-CH4 score: -7
TIM17-CH1 TIM16-CH1 TIM1-CH1 TIM2-CH3 TIM1-CH3 TIM1-CH4 score: -7
TIM17-CH1 TIM16-CH1 TIM1-CH1 TIM2-CH3 TIM2-CH4 TIM1-CH4 score: -7
STM32-DRV: best: TIM3-CH2 TIM3-CH1 TIM1-CH1 TIM1-CH2 TIM1-CH3 TIM1-CH4 score: 12
STM32-DRV: Initializing TIM3
STM32-DRV: Timer resolution set to: 3400
STM32-DRV: Configured TIM3_CH2
STM32-DRV: Configured TIM3_CH1
STM32-DRV: Initializing TIM1
STM32-DRV: Timer resolution set to: 3400
STM32-DRV: Configured TIM1_CH1
STM32-DRV: Configured TIM1_CH2
STM32-DRV: Configured TIM1_CH3
STM32-DRV: Configured TIM1_CH4
STM32-DRV: Syncronising timers! Timer no. 2
STM32-DRV: master timer: TIM3
STM32-DRV: slave timer: TIM1
PWM running ...

@Candas1
Copy link
Collaborator

Candas1 commented Sep 19, 2024

Is the offset stable between the timers ?
I think copperz had a problem with timer arr being off by one, so one phase/timer was constantly moving relative to the other one.

@runger1101001
Copy link
Member Author

Yes, it's not a frequency issue. the ARR is set to the same value for both timers, and they have the same clock...

It's some other kind of problem, I really don't get it yet. Its almost like the PWM mode and the polarity are "reversed" for the general purpose timer...

@runger1101001
Copy link
Member Author

runger1101001 commented Sep 19, 2024

Ok, so for this PR we can conclude:

  • Software 6-PWM is working in principle
  • The LL_TIM_DisableUpdateEvent is a big improvement - software 6-PWM is now free of shoot-through on STM32
  • The "update on overflow" problem is a newly discovered problem but also existed in the old driver version in 2.3.4 and before. So the new driver is not worse than the old one in this regard. The expectation is that the real-world impact is not so big.
  • The timer alignment issue is baffling, but also exists in 2.3.4. If possible we should find the cause and fix it, since as it is now low side sensing cannot work well.

@runger1101001
Copy link
Member Author

Still TODO:

  • Issue found: timers are not aligned (software 6-PWM mode with TIM3 and TIM1 on STM32G431RB)
  • Test with current sensing
  • Test performance (compare to previous version)
  • Issue found: seems like software 6-PWM needs explicit enable() while 3-PWM is enabled directly after init() - investigate and correct (all modes should init disabled, and enable after the call to enable() )
  • Issue found: looks like the first PWM cycle isn't centre aligned
  • Test it on M4 core of H747 MCUs

Edge case: shared timers between different motors, and enabling/disabling individual motors

@runger1101001
Copy link
Member Author

Performance comparison:

Note: comparing setup time performance is not really interesting

Note: this is comparing software 6-PWM, which is the most computationally intensive setPwm() case

New driver version calls to setPwm() per second:
Updates/s: 124664.00

2.3.4 driver version calls to setPwm() per second:
Updates/s: 90090.00

So the new version is considerably faster, despite introducing the calls to LL_TIM_DisableUpdateEvent.

@runger1101001
Copy link
Member Author

So, more debugging on the timer alignment issue:

In this experiment, the duty cycles are no longer being set rapidly. They remain fixed at 10%, 40%, 75%.
Instead the master timer is disabled for 100us every 1000us.

We can see that the timers remain perfectly in sync, and the gated mode means both master and slave timers switch concurrently:

image

We also see the duty cycle of TIM3, which should be 10%, is actually 90%. So it looks very much like the timers are well aligned, as they should be, and somehow the setup of phase A (TIM3) is not correct. Probably I have reversed the high and low side pins by mistake.

Printing the TIM1 and TIM3 counter values during the pauses also shows that the timers are perfectly in sync:

PWM running ...
TIM1: 1345
TIM3: 1345
PWM running ...
TIM1: 1064
TIM3: 1064
PWM running ...
TIM1: 780
TIM3: 780
PWM running ...
TIM1: 415
TIM3: 415
PWM running ...
TIM1: 43
TIM3: 43
PWM running ...
TIM1: 3
TIM3: 3
PWM running ...
TIM1: 495
TIM3: 495

@runger1101001
Copy link
Member Author

Ok, much better, with the pins for phase A not "reversed" on the logic analyser 🤣 :
image

Still TODO:

  • Test with current sensing
  • Issue found: seems like software 6-PWM needs explicit enable() while 3-PWM is enabled directly after init() - investigate and correct (all modes should init disabled, and enable after the call to enable() )
  • Issue found: looks like the first PWM cycle isn't centre aligned
  • Test disabling/enabling the motor. As we see from the picture above, pausing the timer is not the right approach as the outputs will remain at whatever their current level is.

Edge case: shared timers between different motors, and enabling/disabling individual motors --> I tend to think at the moment that we should simply not allow shared timers between different motors.

@runger1101001
Copy link
Member Author

runger1101001 commented Sep 23, 2024

During the startup we see some small glitches as the timers are initialized:

image

And the first duty cycle is odd. We see that the master timer (phase A) starts with a full duty cycle, while the slave timer (B & C) gets a "half duty cycle" in the first period:

image

TBH, I think this is something we can live with. I assume it is the same underlying issue as the non-centred PWM when switching duty cycles: the update event occurs on both overflow and underflow. The CCRx registers are buffered.

@runger1101001
Copy link
Member Author

Regarding enable/disable differences for modes:

3-PWM does not use phase state - nor any other modes except 6-PWM. In 6-PWM it is used for both HW and SW mode.

So in 6-PWM modes the call to enable() is required to switch the phases on. Otherwise there is no output even if you call setPwm().
In the other modes, calling enable() is not needed to switch the phases on unless enable pins are being used. If you call setPwm() the phases come live.

The motor enable/disable is stateful, and so calling motor.disable() will prevent the motor updating until you enable it again. But the driver enable/disable is not stateful, so we can't really check for disabled state in setPwm() at the moment.

TBH none of this code is changed in the new HAL only version, so this is the same as it was in release 2.3.4. So I think if we decide to improve this, we'll do it in a seperate PR.

Regarding enable/disable functionality in general:

the implementation of driver.enable/disable sets the PWM to 0,0,0 for the phases, and operates on the defined enable pins and the phase state (used in 6-PWM). It does not operate on the timers directly by pausing them or reconfiguring their output parameters directly (though the hardware specific driver may do this internally depending on the phase state).
This is unchanged between the HAL only version and the 2.3.4 release, so I don't think we need to investigate this further as part of this PR.

Still TODO:

  • Test with current sensing
  • Edge case: shared timers between different motors, and enabling/disabling individual motors --> I tend to think at the moment that we should simply not allow shared timers between different motors.

@runger1101001 runger1101001 self-assigned this Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants