simavr icon indicating copy to clipboard operation
simavr copied to clipboard

reverse patch from d2aaebd

Open gin66 opened this issue 1 year ago • 5 comments

This patch reverses part of d2aaebd. If this reverse patch is not applied, then the library FastAccelStepper stops working on simavr. As FastAccelStepper works on the real device, simavr's behavior seems to be not correct and maybe that reverse patch is a fix.

gin66 avatar May 21 '23 11:05 gin66

This could be supported by an explanation of why it works. My guess is that the problem is that PWM output changes are not reflected in the corresponding PIN register. That can be reproduced by a simple test program (attached atmega324a_ioport.txt (C code but there is a stupid restriction on ".c" filename extensions.))

It sets up PWM output and prints the contents of the corresponding PIN register: after initialisation; after the first match event; after output has been enabled immediately following the match; and after the pin again changes state. The results should be 0, 0, 0x08, 0. With current git code they are 0, 0, 0, 0, so some changes in output are not reflected in the PIN register. With this PR: 0, 0x08, 0, 0. So this PR may fix one problem, but it will make spurious changes to PIN. With an alternative fix (patch.txt) that checks the state of DDR before copying PWM output to PIN: 0, 0, 0x08, 0x08. Still not right, but the last value is wrong because of a bug in the timer code. In my fork that is fixed and the correct result is shown.

gatk555 avatar May 30 '23 19:05 gatk555

Your patch works with my simavr based test suite. Later I will try your fork, which apparently is more active maintained.

gin66 avatar May 30 '23 21:05 gin66

@gatk555 do you have a 'proper' PR or a commit for that? ie, c8bcdc7?

buserror avatar Aug 11 '23 18:08 buserror

That is the change. I can not now recall how much testing I did against upstream code, but probably not enough. Does "future PR" sound OK?

gatk555 avatar Aug 13 '23 05:08 gatk555

Ahah fine :-) As you've noticed I've reactivated my tree (mostly because I got an active AVR project!) so it's a good time to get stuff merged, if you hadn't given up ;-)

buserror avatar Aug 13 '23 05:08 buserror