h2zero / n-able-Arduino

An arduino core for ARM based BLE devices supported by the NimBLE stack.
GNU Lesser General Public License v2.1
34 stars 12 forks source link

analogRead doesn't work because value is set to 0 #21

Open SKnight-s opened 1 year ago

SKnight-s commented 1 year ago

Hello, I'm using BBC microbit v2 and I noticed that the analogRead function always gave us 0 (trying to read from a microphone). In the file wiring_analog_nRF52.c, in the function analogRead , the variable is set to 0 when initialized.

The solution is to replace the line 109 "int16_t value = 0;" by "int16_t value;" Then, it solves this issue.

Maybe I should have proposed a PR, but I'm not yet comfortable with that.

Thanks anyway for this library, because sandeepmistry/arduino-nRF5 doesn't support BLE for BBC microbit v2 and you do with NimBLE.

h2zero commented 1 year ago

Thanks, I will have a look at this and update it 👍

krzysiekkajot commented 8 months ago

Hi int16_t value = 0; is for sure ok, at it should be - it just initialize the value to 0, without it value will get some random value (not strict random but depend on previous values at memory where value is located). Fix should be like below: volatile int16_t value = 0; Because value pointer is given to SAADC and its modify memory under address coresponded to value, without volatile compiler optimise code to do not allways load data from memory. SK.night suggestion works (orginal sandeepmistry / arduino-nRF5 code is like suggested) but it is wrong. Its probably works because there is no strict value in code so compiler see only reading, without any set so it really do memory read when value is returned, but I'm not sure it is defined like that in standard so it can change. For this kind of operations (external variable modification - like there by soem kind of DMA) volatile was designed.

h2zero commented 8 months ago

Thank you for pointing this out and @krzysiekkajot for the potential fix. I will test the PR and make a new release if resolved.