FreqMeasureMulti for T4.x -- Bug Report/Fix

[joepasquariello]

I'm pretty sure I have found and fixed a bug in FreqMeasureMultiIMXRT.cpp. Please take a look.

In FreqMeasureMulti::isr(), when the capture interrupt occurs (as opposed to the overflow interrupts), the ISR is reading register VALx for the relevant timer, and it should be reading CVALx, which is the actual capture register. VALx is always 0 on the capture interrupt, so the values returned by read() are incorrect and are always exact multiples of 65536

For example, with defaults of FREQMEASUREMULTI_RAISING and 60 Hz, the sketch output is:

60.02(2:2490368:16.60 2:2490368:16.60 2:2490368:16.60 2:2490368:16.60 )
59.97(2:2490368:16.60 2:2490368:16.60 2:2555904:17.04 2:2490368:16.60 )
60.02(2:2490368:16.60 2:2490368:16.60 2:2490368:16.60 2:2490368:16.60 )
59.97(2:2490368:16.60 2:2555904:17.04 2:2490368:16.60 2:2490368:16.60 )

The count values are 2490368 (0x260000) and 2555904 (0x270000). What's happening is that read() is effectively returning a 32-bit value where the upper word is the number of overflows of the 16-bit capture register, without the lower 16 bits of the actual capture value.

With the ISR changed to read CVALx rather than VALx, the output looks as shown below, with the expected 60.00 Hz

60.00(2:2499968:16.67 2:2499968:16.67 2:2499968:16.67 2:2499968:16.67 )
60.00(2:2499968:16.67 2:2499968:16.67 2:2499968:16.67 2:2499968:16.67 )
60.00(2:2499968:16.67 2:2499968:16.67 2:2499968:16.67 2:2499968:16.67 )
60.00(2:2499968:16.67 2:2499968:16.67 2:2499968:16.67 2:2499968:16.67 )

The VALx registers are initialized in begin(), and I'm not sure if these should also be CVALx.

pflexpwm->SM[sub_module].VAL0 = 0;
pflexpwm->SM[sub_module].VAL1 = 65535;
pflexpwm->SM[sub_module].VAL2 = 0;
pflexpwm->SM[sub_module].VAL3 = 0;
pflexpwm->SM[sub_module].VAL4 = 0;
pflexpwm->SM[sub_module].VAL5 = 0;

I've only tested the single-PWM example sketch so far, and only on default pins 5/10, but I think the same bug exists for all channels.

 

[joepasquariello]

I tested the 3-PWM example sketch. Here is the output before the fix:

60.02, 100.03, 199.90
59.97, 100.04, 200.07
60.03, 99.95, 200.07
59.97, 100.04, 199.97
60.02, 99.95, 199.90
59.97, 100.04, 200.07

and here is the output after the fix

60.00, 100.00, 200.00
60.00, 100.00, 200.00
60.00, 100.00, 200.00
60.00, 100.00, 200.00
60.00, 100.00, 200.00
60.00, 100.00, 200.00

 

[MarkT]

Perhaps create a git pull-request for this?

 

[joepasquariello]

Perhaps create a git pull-request for this?

I'm rather new to this. Do I simply put this same info in the pull request?

 

[MarkT]

A pull request is a your GIT branch taken from the relevant repo, add your fix and submit it for consideration - yes
document it! I think you would take it from here: https://github.com/PaulStoffregen/FreqMeasure Paul may chime in here and give a more definitive anwser of course

 

[joepasquariello]

Will do. I'll wait a day and see if I get any other feedback here.

The initialization of VAL0-VAL5 in begin() seems to be unnecessary, as those registers are associated with PWM and not with the input capture. Those are now the only references to VAL0-VAL5 in the library, and CVAL0-CVAL5 are read-only.

 

[joepasquariello]

Just issued a Pull Request on Paul's GitHub repository for FreqMeasureMulti.

Please disregard my comment in previous message about initialization of INIT and VALx registers in function begin(). Those are necessary.

https://github.com/PaulStoffregen/FreqMeasureMulti/issues/12

 

[joepasquariello]

Pull Request actually just issued last night. That was my first pull request, and I had to figure out what I was doing. What I did on 3-23 was open an "Issue".

https://github.com/PaulStoffregen/FreqMeasureMulti/pull/13