ADC icon indicating copy to clipboard operation
ADC copied to clipboard

startSynchronizedSingleDifferential() + readSynchronizedSingle() != analogSynchronizedReadDifferential()

Open gwgill opened this issue 5 years ago • 1 comments

In attempting to convert my code from a blocking to non-blocking code sequence, it appears that the code

... = analogSynchronizedReadDifferential(...)

returns a different result to

startSynchronizedSingleDifferential(...)
... do some other work and/or wait for an interrupt ...
... = readSynchronizedSingle()

The result had both glitching and a different voltage level. Eyeballing the code it seems that the code from analogSynchronizedReadDifferential is not fully implemented in the asynchronous equivalent, and I had to modify readSynchronizedSingle() to get it working:

// Reads the analog value of a single conversion.
/*
*   \return the converted value.
*/
ADC::Sync_result ADC::readSynchronizedSingle() {
    Sync_result res = {ADC_ERROR_VALUE, ADC_ERROR_VALUE};;

    uint8_t resolution0 = adc0->getResolution();
    uint8_t resolution1 = adc1->getResolution();

    // ensure both ADCs have finished
    while( (adc0->isConverting()) || (adc1->isConverting()) ) {
        yield();
        //digitalWriteFast(LED_BUILTIN, !digitalReadFast(LED_BUILTIN) );
    }
    __disable_irq(); // make sure nothing interrupts this part
    if (adc0->isComplete()) { // conversion succeded
        res.result_adc0 = adc0->readSingle();
        if(resolution0==16) { // 16 bit differential is actually 15 bit + 1 bit sign
            res.result_adc0 *= 2; // multiply by 2 as if it were really 16 bits, so that getMaxValue gives a correct value.
        }
    } else { // comparison was false
        adc0->fail_flag |= ADC_ERROR_COMPARISON;
    }
    if (adc1->isComplete()) { // conversion succeded
        res.result_adc1 = adc1->readSingle();
        if(resolution1==16) { // 16 bit differential is actually 15 bit + 1 bit sign
            res.result_adc1 *= 2; // multiply by 2 as if it were really 16 bits, so that getMaxValue gives a correct value.
        }
    } else { // comparison was false
        adc1->fail_flag |= ADC_ERROR_COMPARISON;
    }
    __enable_irq();

    // if we interrupted a conversion, set it again
    if (adc0->adcWasInUse) {
        //digitalWriteFast(LED_BUILTIN, !digitalReadFast(LED_BUILTIN) );
        adc0->loadConfig(&adc0->adc_config);
    }
    if (adc1->adcWasInUse) {
        //digitalWriteFast(LED_BUILTIN, !digitalReadFast(LED_BUILTIN) );
        adc1->loadConfig(&adc1->adc_config);
    }

    //digitalWriteFast(LED_BUILTIN, !digitalReadFast(LED_BUILTIN) );

    return res;
}

I haven't looked at other read* functions to see if they suffer from similar problems.

It would be good to see this fixed in the library, or (if the intention of readSynchronizedSingle() is not the above) to better document what readSynchronizedSingle() is intended for, and to provide a function to complete a non-blocking conversion in a way that is equivalent to the blocking version.

gwgill avatar Jul 01 '20 01:07 gwgill

readSynchronizedSingle is supposed to just read the values, like it does now. The user is supposed to check that the conversion has finished. I guess that maybe there should be a function called readSynchronizedDifferential that corrects the 16 bit issue for the user.

pedvide avatar Jan 15 '21 19:01 pedvide