Secret voltmeter question

Hi everyone,
I have been experimenting with the secret voltmeter code for some time now, though I don't really understand how to work with registers a whole lot.
My question is: is the way I have rewritten the secret voltmeter function correct? I have taken it from a blocking function:

 long readVcc() {
  // Read 1.1V reference against AVcc
  // set the reference to Vcc and the measurement to the internal 1.1V reference
  #if defined(__AVR_ATmega32U4__) || defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__)
  ADMUX = _BV(REFS0) | _BV(MUX4) | _BV(MUX3) | _BV(MUX2) | _BV(MUX1);
  #elif defined (__AVR_ATtiny24__) || defined(__AVR_ATtiny44__) || defined(__AVR_ATtiny84__)
  ADMUX = _BV(MUX5) | _BV(MUX0) ;
  #else
  ADMUX = _BV(REFS0) | _BV(MUX3) | _BV(MUX2) | _BV(MUX1);
  #endif

  delay(2); // Wait for Vref to settle
  ADCSRA |= _BV(ADSC); // Start conversion
  while (bit_is_set(ADCSRA, ADSC)); // measuring

  uint8_t low  = ADCL; // must read ADCL first - it then locks ADCH
  uint8_t high = ADCH; // unlocks both

  long result = (high << 8) | low;

  result = 1125300L / result; // Calculate Vcc (in mV); 1125300 = 1.1*1023*1000
  return result; // Vcc in millivolts
  }

to a non blocking one:

unsigned long readingTakenTime;
bool takeReading = true;
long result = 5000;

long readVcc() {

  // Read 1.1V reference against AVcc
  // set the reference to Vcc and the measurement to the internal 1.1V reference
#if defined(__AVR_ATmega32U4__) || defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__)
  ADMUX = _BV(REFS0) | _BV(MUX4) | _BV(MUX3) | _BV(MUX2) | _BV(MUX1);
#elif defined (__AVR_ATtiny24__) || defined(__AVR_ATtiny44__) || defined(__AVR_ATtiny84__)
  ADMUX = _BV(MUX5) | _BV(MUX0) ;
#else
  ADMUX = _BV(REFS0) | _BV(MUX3) | _BV(MUX2) | _BV(MUX1);
#endif


  if (millis() - readingTakenTime >= 2) {
    readingTakenTime = millis();
    ADCSRA |= _BV(ADSC); // Start conversion
    while (bit_is_set(ADCSRA, ADSC)); // measuring

    uint8_t low  = ADCL; // must read ADCL first - it then locks ADCH
    uint8_t high = ADCH; // unlocks both

    result = (high << 8) | low;

    result = 1125300L / result; // Calculate Vcc (in mV); 1125300 = 1.1*1023*1000

  }
  return result; // Vcc in millivolts
}

Is this one of the many correct ways to do it? How would you do this? Many thanks in advance

What is secret about the code, or aren't you able to say ?

I really don't know :joy:
I've just seen they call it this way on the web
Anyways, what would you say about my implementation?

Note that the chances of the INTERNAL voltage reference being exactly 1.1V is small. It is stable, but is specified as somewhere between 1.0V and 1.2V. You should write a sketch to set the analog reference to INTERNAL, do an analogRead() (doesn't matter which pin) and then read the voltage on the "Aref" pin.

I have no idea how to do that. Could you please post an example?

And what about my current implementation? I am not super interested in absolute precision, but I am dubious about setting result = 5000 and the way the if(millis()-.....) includes only part of the code. Should I also put the if statements in there as well?

that seems to be the only "blocking" part of the process - so why not just delete it?
of course the while .. is also to an extent "blocking" so you would need to look at that also.

The purpose of the "secret voltmeter" code is to measure Vcc - but the internal reference voltage on arduinos is not precise. You can do better by measuring a precise external reference on one of the analog pins, using Vcc as a reference. Which avoids "blocking"

Thanks for your explanation John.
I was scared of deleting the delay as it says it allows Vref to settle, isn't it risky to use the calculation without letting Vref settle?

Part of the non-blocking strategy is embodied in how the function is utilized in the main sketch. So please post the entire sketch.

I think there is a conceptual problem in your non-blocking conversion. You had:

define read
{
  event 1
  delay
  event 2
  return value = result
}

Now you have

define read
{
  event 1
  if interval elapsed
    {
    reset interval timer
    event 2
    }
  return value = result
}

The problem is that the non-blocking version can return a value regardless of whether event 2 has occurred. That could happen on the first call, or on a subsequent call that happens to follow a previous one by less than the delay interval. In the first case, event 2 is guaranteed to happen.

event 1 is the mux setup
event 2 is the read and calculation

Rather than elaborate, I'll leave it like this for your thoughts.

Hint: only rarely can you use non-blocking without flags and/or state variables.

I always prefer to let the compiler do the math. It's more foolproof.

 // put near the top of your code so you can enter the calibrated value
const long VREF_INTERNAL_MV = 1100;
const long NUM_ADC_STEPS = 1024;
...
    // Calculate Vcc (in mV) = 1.1*1024*1000
    result = VREF_INTERNAL_MV * NUM_ADC_STEPS / result; 

Not that it matters because of the above mentioned limits on the accuracy of the 1.1V reference, but the correct scale factor is the number of ADC steps, 1024. Not 1023.

Compiler optimization means there is no object code overhead to such calculations.

It's the fault of the creator(s) of this software that you are asking this question, if they didn't document it.

Edit - consider another aspect. If you must wait, then you must wait sometime that is controlled somewhere in your code. Otherwise the "result" can't be valid. When and where do you want to do it?

Vref is REASONABLY stable - the problem is you dont know what the value is, unless you calibrate it.
There are many on this forum wiser than I, that may see a benefit in "allowing it to settle" - I cant see any unless its to allow time for the chip temperature to stabilise - and 2ms would not do that.

However there may be a benefit in allowing a short time for the ADC input to settle, but a "dummy read" is the usual solution.

Why are you concerned with blocking - do you need to monitor the supply continuously? Its normally enough (maybe even overkill) to check it once in setup().

I kep pressing this too - its not critically "important" in that the error introduced is less than the adc errors. However it DOES reflect a misunderstanding of what the ADC raw value represents.

Also (like yourself ) I HATE to see "magic numbers" in a program where the source of those numbers can be explicit.
And (another personal "fad") I worry when users unnecessarily convert an integer value (especially from an ADC ) to a float, as it leads to an unrealistic treatment of the precision of the reading.

Rant over (for now)

Also, not a deal breaker or deal maker, but using the power of two ADC number of steps allows the compiler (or your code, as you see fit) to use it to scale values using bit shift arithmetic for multiplication and division, which is a real execution time saver.

Thanks to everyone for your precious contribution
In order to see whether something would change between different implementations I decided to write 3 versions of the software: one with delay, one with delay removed, one with millis().
It turns out that all three versions give out the same result after the first reading, in which obviously the version I posted above would return 5000 for the first two milliseconds

because it lacks a completion flag (semaphore) that the calling program can check

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.