-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add support for VDDHDIV5 voltage measurement #9204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
The reading is divided by 5 so I think you need to use different OPERATIVE_ADC_MULTIPLIER (5x) instead of the one configured for voltage divider. I assume with current setting your readings are like 30-50% lower than the actual battery state, unless you manually set ADC to 5.0 in device settings. |
Yes, of course, instead of #define ADC_MULTIPLIER to a value based on the specific voltage divider resistor values, when BATTERY_PIN is set to -1, ADC_MULTIPLIER should be set to 5 in variant.h or in device settings. |
|
I suspect it is fairly common to supply battery voltage to the VDDH pin when a single LiPo or LiIon cell is used in a design. I think I will I'll re-work this PR |
…instead of voltage divider resistors
|
I have re-worked this PR to allow, but not require, automatic run-time choice of voltage measurement via a BATTERY_PIN or via VDDHDIV5. It works like this:
This allows a variant to choose to use either an analog pin or the VDDH pin to sense the battery voltage and also allows a variant to use a single firmware with fallback to use of the VDDH pin if the BATTERY_PIN is connected to ground. In practice, I have found that, a floating BATTERY_PIN is also likely to result in a measurement of less than 1 volt, but obviously, the recommendation is to connect the BATTERY_PIN to ground on DIY builds when voltage divider resistors are not installed. |
|
Makes sense - I can test that later on promicro. Some variants use external 3.3V LDO regulator connected to VDD+VDDH in "low voltage mode" as Nordic coins that. I use similar VDD direct reading to protect device from undervoltage: #9211 BTW - I think it will be better to actually force using ADC_MULTIPLIER=5 when using VDDH reading instead of relying on any other compiled or configured value to be 5 to avoid problems or misconfigurations. Especially if we are in the territory of automatic fallbacks :) |
Yes, and the current PR already does exactly that by just hard coding 5. |
Allow a NRF52 variant, like the DIY Promicro NRF52 development boards which power the NRF52 directly via the VDDH pin to obtain battery voltage directly from this pin using VDDHDIV5. This can simplify DIY builds by avoiding the need to connect voltage divider resistors to an analog pin. This method also avoids the small current drain of the voltage divider. Note however, that a tradeoff in some cases is that this method may not allow monitoring battery voltage while charging via USB since some designs, including the Promicro boards, supply USB voltage to VDDH when USB is connected.
Note that there are a couple of NRF52 variants which are already defining BATTERY_PIN to -1; presumably to indicate the absence of a battery sense pin. It is not immediately obvious to me if these variants might happen to have battery voltage connected to VDDH and would thus benefit from this feature, but I don't expect these variants to suffer any issues do to this change.
If, however, it is preferred to have a new #DEFINE rather than overload BATTERY_PIN, please suggest a new symbol name, and I'll update this PR.
I would like to take advantage of this in the future to simplify my "Easy E22" build instructions: https://github.com/brad112358/easy_E22
See also #7579
🤝 Attestations
Tested on DIY NRF52 Promicro E22 board