[En-Nut-Discussion] AVR ADC sample fails

[Harald Kipp]

Hi all,

So far we at egnite never used the ADC routines, but regularly receive
requests about its API functions. Now we decided to provide a sample,
but miserably fail ourselves. :-(

Using a simple loop of ADC-reads plus a NutSleep(10000) lets the
NutSleep fail. It returns immediately. This look like a memory access
problem and I looked into arch/avr/dev/adc.c and started wondering about
a number of things, which I do not understand.

The allocation of ADC_buffer in ADCInit() is
 ADC_buffer = NutHeapAlloc(sizeof(uint16_t) * ADC_BUF_SIZE + 2);

Immediately after this, ADCBufInit() is called, which contains:

 buf[_adc_buf_head] = 0;
 buf[_adc_buf_tail] = 0;

The related defines are

 #define _adc_buf_head ADC_BUF_SIZE
 #define _adc_buf_tail ADC_BUF_SIZE+1

Actually

 buf[_adc_buf_tail] = 0;

is

 buf[ADC_BUF_SIZE+1] = 0;

and will corrupt heap memory. Did I overlook something? This code is
nearly 5 years old!

Please tell me, that I'm not getting too old for this stuff,

Harald




P.S.:

More things to wonder about. Like in

inline int ADCBufRead(uint16_t * buf, uint16_t * read)
{
    uint8_t tail, head;
    tail = buf[_adc_buf_tail];
    head = buf[_adc_buf_head];

Why the hell doesn't the compiler complain? buf[] is 16 bit, tail is 8
bit, so the upper byte is lost.

There are other things I still do not understand, but let's solve these
two first.

IMHO, the problem with the buffer overwrite is typical for _overusing_
preprocessor macros. To me

 buf[ADC_BUF_SIZE+1] = 0;

is much easier to read than

 buf[_adc_buf_tail] = 0;

Furthermore, the naming is most confusing. _abcd looks like a system
variable, while ABCD looks like a macro. Last not least, defining macros

 #define _adc_buf_tail ADC_BUF_SIZE+1

without brackets may be very dangerous, because _adc_buf_tail*2 will
give the wrong result.