Problems in AVR C combining ADC readings to generate PWM output

Question

I'm writing a program for an ATMega328P that will take readings from several ADC channels, combine them into a single signal and output this signal through PWM.

I've successfully backed off my ADC polling to 50Hz per channel using Single Conversion mode. I'm using Timer/Counter2 for PWM generation, and Timer/Counter1 for doing the calculations I need to do to set compare values for Timer/Counter2. This is the ISR for Timer/Counter1:

// Interrupt service routine called to generate PWM compare values
ISR(TIMER1_COMPA_vect)
{
    // Grab most recent ADC reading for ADC0
    uint32_t sensor_value_0 = adc_readings[0];

    // Get current value for base waveform from wavetable stored in sinewave_data
    uint32_t sample_value_0 = pgm_read_byte(&sinewave_data[sample_0]);

    // Multiply these two values together
    // In other words, use the ADC reading to modulate the amplitude of base wave
    uint32_t sine_0 = (sample_value_0 * sensor_value_0) >> 10;

    // Do the same thing for ADC2    
    uint32_t sensor_value_1 = adc_readings[1];
    uint32_t sample_value_1 = pgm_read_byte(&sinewave_data[sample_1]);
    uint32_t sine_1 = (sample_value_1 * sensor_value_1) >> 10;

    // Add channels together, divide by two, set compare register for PWM
    OCR2A = (sine_0 + sine_1) >> 1;

    // Move successive ADC base waves through wavetable at integral increments
    // i.e., ADC0 is carried by a 200Hz sine wave, ADC1 at 300Hz, etc.
    sample_0 += 2;
    sample_1 += 3;

    // Wrap back to front of wavetable, if necessary
    if (sample_0 >= sinewave_length) {
        sample_0 = 0;
    }

    if (sample_1 >= sinewave_length) {
        sample_1 = 0;
    }
} // END - Interrupt service routine called to generate PWM compare values

My problem is that that I get no PWM output. If I set either sensor_value_0 or sensor_value_1 to 1024 and leave the other sensor_value_ set to read from the ADC, I do get one full-amplitude component wave, and an amplitude-modulated component wave. If however, I choose a different value for the hardcoded, mock amplitude, I am not so lucky (1023, for instance). Any other values give me no PWM output. If I set both sensor_value_s to look at the same ADC channel, I would expect two component waves whose amplitudes are modulated identically. Instead, I get no PWM output. What is most confusing of all to me is that if I choose a value for the hardcoded amplitude that is an exact power of two, all is well.

The whole power-of-two part makes this seem to me to be a bit-twiddling issue that I'm not seeing. Can you see what I must have clearly missed? I'd appreciate any tips at all!

(I've posted my entire source here to keep things as neat as possible on SO.)

Answer 1

@Devrin, I appreciate the response, but just manipulating types didn't do it for me. Here's what I ended up doing:

uint8_t sine_0 = (pgm_read_byte(&sinewave_data[sample_0]) >> 5) * (adc_readings[1] >> 5);
uint8_t sine_1 = (pgm_read_byte(&sinewave_data[sample_1]) >> 5) * (adc_readings[2] >> 5);
OCR2A = (sine_0 >> 1) + (sine_1 >> 1);

Essentially, I've done all my shifting immediately, instead of waiting until the last minute. Unfortunately, I lose a lot of precision, but at least the code works as expected. Now, I wil begin cranking things back up to find the initial cause of my issues.

Answer 2

Your issue may be caused by the architecture of the AVR that you're developing on. The ATMega328p has 8 bit registers, similar to most other AVR chips. This means that the 32b values that you're working with must be stored in memory by the compiler and broken up into four separate registers every time you perform arithmetic on them. In fact, there are no arithmetic instructions that perform on more than one register at once, so I'm really not sure what the compiler is doing!

I'd be interested to know what the disassembly of your code is, but my guess is that gcc is using the MUL instruction to execute the sample_value_0 * sensor_value_0 code. This instruction operates on two 8b values and produces a 16b value, so I wouldn't be surprised if the reason you're seeing an odd dependence on multiples of two produce results.

I'd say try reworking this block of code by changing the data types of the variables. Use uint8_t for sensor_value_* and sample_value_*, and uint16_t for sine_*. Then, to make sure everything fits in the 8b OCR2A register, change the assignment to something like:

OCR2A = (sine_0 + sine_1) & 0xFF;