search

p3p / pio-framework-arduino-lpc176x

10 stars 20 forks source link

HW PWM is not latched correctly when the value is changed #38

Closed mlehnhoff closed 3 years ago

mlehnhoff commented 4 years ago

This issue was discovered in MarlinFw where it causes a Servo (or BLTouch Sensor) not to operate correctly see (https://github.com/MarlinFirmware/Marlin/issues/16171).

mlehnhoff commented 4 years ago

I have just created a pull request for this issue. I analysed the code and found three things, that looked a littel odd to me. 1) there were some Register manipulations that were not necessary for a correct operating timerbased HarwarePWM. -> so i removed those 2) Register LPC_PWM1->LER was always set to 1 (bit_value(0)). This was not correct. It has to be LER |= 1. 3) The offset for the period (MR0) is not needed. If i want PWM from 0-100 % duty cycle and the apropriate timer value for the period is e.g. 1000. than i have to set MR0 to 1000. If i then want to have 0% duty cycle i have to set MR1 to 0. And for 100% duty cycle i have to set it to 1000! With this correction the two workarounds in set_match() could also be removed.

I have tested this quite a bit and it works perfectly for me in MarlinFW. Servo is working fine without any glitches. And also the fan is working fine with every duty cycle that is possible.

p3p commented 4 years ago

Thanks for looking into this, although you seem to just remove workarounda for issues I'd observed in the pwm signals, such at 0 not being off and still generating a 1 cycle pulse, and enable the shadow registers, It's been a while since I wrote this but I can't help but think I would have at least tested with the simplest form to see if it fixed the sporadically ignored match set, I'l get this tested out and see if it resolves that issue or if it is just not being triggered by your configuration.

mlehnhoff commented 4 years ago

Yes, i think you should test that. I am definitely not seeing an issue with 0 duty cycle here. But as you said, maybe this is only in my configuration. Anyways, it would be easy to re-add this workaround and i don't think that it would ruin the result. I might test this later.