sensorium / Mozzi

sound synthesis library for Arduino
https://sensorium.github.io/Mozzi/
GNU Lesser General Public License v2.1
1.07k stars 186 forks source link

Fix overflow for mozziAnalogRead with STM32 #151

Closed tomcombriat closed 2 years ago

tomcombriat commented 2 years ago

Hello!

Similar to what happened with Teensy, there was an overflow in the array storing the reading for the ADC values for the STM32. An array of 16 is allocated (totally sufficient, at least for the BluePill) but pins PB0 and PB1 have channels 16 and 17 respectively, leading to an overflow.

I am not sure if the fact that I see it now is because the ADC library for STM32 have been recently upgraded, of if I had previously the chance not to hit an already allocated memory space.

The problem is solved in the same way than the teensy, defining a STM32PinMap function to remap these two pins in the range. Might be a bit overkill to make a file for that but this would be easier to expend for other STM chips is the same problem arise. I am open to suggestions as usual.

Best,

tfry-git commented 2 years ago

I suppose the bug will have existed independent of the ADC library. We probably simply never triggered it...

Perhaps the extra file really is overkill. The problem with additional headers always being that it's hard to know, whether they (and the functions within) can safely be removed or renamed, or if somebody is actually using them. So I'd suggest to use an inline function inside MozziGuts_impl_STM32.hpp, indeed. We can still add a separate file, if, and when the need arises.

Also, shouldn't that be if (pin > 15) return pin-16;?

tomcombriat commented 2 years ago

Hei, Thanks for the feedback. I'll change as you suggested very soon.

For the function, here is the mapping of the BluePill:

tfry-git commented 2 years ago

Ah, ok. I was looking at the wrong column of the pinout chart, and thought the analog pins were at 10-19.