add checkBatteryLow() function

[joshgoebel]

trafficstars

[joshgoebel]

Will add it to the frame loop after https://github.com/MLXXXp/Arduboy2/pull/2 is merged.

[joshgoebel]

We could also suggest this and make it optional (for sketches to call), but then I think it starts to lose value. Thoughts?

[MLXXXp]

Based on the 32u4 datasheet, I'm skeptical that the bandgap voltage will be precise enough from unit to unit to provide a meaningful low battery indication, without needing some calibration calculations using values set in EEPROM, but I'd love to be convinced otherwise. We need some way of testing a number of units, such as comparing the actual battery voltage measured with a meter to the value reported by the code.

Will add it to the frame loop

I'd like to leave nextFrame() as it is and add a nextFrameWithBatteryCheck() (actual name negotiable).

In general, the problem with adding a battery check so late in the game is that some sketches will have it and some won't. How is a user who is just loading games to play (especially from .arduboy files) to know whether they will get a low battery indication or not?

[MLXXXp]

If we're going to be checking the battery constantly in a loop, such as from nextFrameWhatever(), it would be better not to always wait on the conversion to complete, which would eat up render time.

// called fairly frequently, such as in _nextFrameWhatever()_
pollBatteryLow()
{
  if (bit_is_clear(ADCSRA, ADSC)) { // if conversion complete
    voltage = ADC;

    // code to calculate battery level and set TXLED on or off goes here

    ADCSRA |= _BV(ADSC); // start next conversion
  }
}

This wouldn't waste time spinning on a conversion but the drawback is the ADC would have to remain always powered up.

To allow powering down the ADC, we could probably use the ADC interrupt.

// called fairly frequently, such as in _nextFrameWhatever()_
pollBatteryLow()
{
  if (bit_is_clear(ADCSRA, ADSC)) { // if conversion complete
    power_adc_enable(); // ADC on
    ADCSRA |= (_BV(ADSC) | _BV(ADIE); // start next conversion and enable the interrupt
  }
}

The ADC ISR would calculate the battery level and set TXLED on or off, then do power_adc_disable(). If we don't want the ISR to do so much work, it could just save the ADC value to RAM and the poll function above could use it to do the calculations and set TXLED, before powering up and starting the next ADC conversion.

However if the ADC interrupt is used, I think all this should go in a separate library, to avoid locking out the interrupt from being used by sketches for other purposes (the same reason that tunes was removed from Arduboy2).

---

All of the above would mean the ADC couldn't be used for anything else. RAM semaphore flags could be used to coordinate sharing the ADC.

[joshgoebel]

In general, the problem with adding a battery check so late in the game is that some sketches will have it and some won't. How is a user who is just loading games to play (especially from .arduboy files) to know whether they will get a low battery indication or not?

Better to start forcing it and then eventually all games will have it. Then x months from now this will no longer be a discussion point. And it's also easy enough to do something at bootup to make it clear that the ROM supports the battery check or not. You added the name to boot screen, why not a tiny battery icon also.

If we're going to be checking the battery constantly in a loop

We're not. You only need to check it every minute or so, not constantly. But I can also consider your suggestion.

However if the ADC interrupt is used,

I don't think an interrupt is needed at all. A single global byte can hold battery status... when we poll we set it to a "invalid valid" (255, 01, etc) and the next few frame loops check for that invalid valid and see if the ADC is done... when it's done we see if the value is out of range or not.

, I'm skeptical that the bandgap voltage will be precise enough from unit to unit to provide a meaningful low battery indication

In my testing there was a long runway. It doesn't have to turn in 5 minutes before the power goes out.. if what it means is "you're down to 1/3 battery" then I think it's useful to have.

[joshgoebel]

I'd like to leave nextFrame() as it is and add a nextFrameWithBatteryCheck() (actual name negotiable).

To me the whole point here is if it's BUILT in by default. I'm fine with adding an opt-out (if you insist) but I don't think this is something people should opt out of.