Battery Monitoring System by rob-robinson-14 · Pull Request #1 · rob-robinson-14/sdk-zephyr

rob-robinson-14 · 2026-02-19T16:35:56Z

Initial review of health monitoring - Note WIP

Some comments in code to explain TODO / test features.

Implementation is in zephyr/soc/nordic/nrf54l/health_monitoring_nrfx

zephyr/soc/nordic/nrf54l/health_monitoring is a previous effort using zephyr rathen than nrfx but came into complications here.

To test / demo I have been using west build -b nrf54l15dk/nrf54l15/cpuapp ~/Documents/nrf7120/zephyr/samples/basic/blinky -DCONFIG_VBAT_MONITOR_ENABLE=y -p

Hi-Im-David · 2026-02-20T15:17:48Z

+/* TODO: If def around for measure battery and temp ADC
+ * Need to update to correct pins.
+ */
+	NRFX_SAADC_DEFAULT_CHANNEL_SE(NRFX_ANALOG_EXTERNAL_AIN6, 0),


Should the reference voltage, gain, divider etc. be configurable? E.g if the battery voltage is above the reference.

Yup some more configuration will be required. For now just looking at overall functionality of the feature. atm its just on a GPIO pin rather than internal battery (so we can test) but at a later stage will move to battery and ensure adc is configured correctly with everything required and get you to review. - I shall add a TODO to reflect this.

I wonder what's that best way to configure the ADC's considering you aren't using devicetree here

Above is what they show in NRFX examples but happy to consider other options.

have updated this to use a config_t - think it improves clarity and gives us flexibility if we need it

Hi-Im-David · 2026-02-20T15:38:37Z

+
+/** @brief SAADC channel to measure battery level */
+static const nrfx_saadc_channel_t m_multiple_channels[] = {
+/* TODO: If def around for measure battery and temp ADC


Does temp here mean temporary or temperature? Temperature uses a separate "temp" peripheral right?

I believe there are two temperatures to be measured, one is its own peripheral. The other I am not exactly sure - need to sync with Mike on this to see if its part of a WiFi core or an ADC use.

Hi-Im-David · 2026-02-20T16:23:11Z

+ *
+ * @return Pointer to atomic variable holding battery voltage
+ */
+atomic_t *get_battery_voltage_pointer(void)


Instead of exposing the pointer in the API, can we expose a read api? something like:

int32_t battery_monitor_get_vbat_mv(void) { return (int32_t)atomic_get(&battery_voltage); }

Request from WiFi is to have a struct of pointers that they can access for each of these metrics directly.

Given the values are 32bit I think all reads / writes will be atomic by default, so atomic_t is just belts and braces.

However the reads and writes are from different cores and so I am not sure if we still have an issue here and which point perhaps we need ICMsg of some form.

Agreed that we should have a get function as well (although unlikely to be used on app core).

to clarify, the Wi-Fi driver running on APP core will use these APIs and then inform the Wi-Fi firmware via IPC mechanism. So, we don't need multi-core access.

This is incorrect, Wi-Fi driver is only passing the address, and Wi-Fi core (LMAC) is reading periodically, so, @rob-robinson-14 you are right that we need race-free multi-core access. I also don't know how it works when APP is in low power state (if Wi-Fi is active we cannot fully suspend, so, may not be a problem).

dhsu-nordic · 2026-02-24T17:30:39Z

+	for (;;) {
+		k_sleep(K_SECONDS(2));
+		// uint32_t voltage = atomic_get(g_vbatt_value);
+		printk("Global ADC: %ld mV\n", *g_vbatt_value);


Since the values are defined as atomic variables. Would it be more safe to get the value via atomic_get? Same for g_die_temp_value.

Yes that would be more safe - I had similar comments from David. But in actual implementation we will be getting values from wifi core and so no Zephyr OS. Here I am just trying to prove that what I am doing is safe. Since the values are only 32bit the only impact from the race condition is that we get the old value - but for the context of wifi team, this is okay.

The only way to make this "safe" is to have some kind of ICMsg to request / notify updates but we are trying to reduce this overhead.

I see. Yeah, the RISC-V in Wi-Fi does not natively support atomic instruction anyway, and even then it is a different RTOS to Zephyr. Getting the old value from race condition does seem justified in that case.

github-actions · 2026-03-17T13:05:17Z

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_nordic	zephyrproject-rtos/hal_nordic@`d7dc801`	zephyrproject-rtos/hal_nordic@`3e5403c`	zephyrproject-rtos/hal_nordic@d7dc801a..3e5403c9
hostap	zephyrproject-rtos/hostap@`1a2fbb7`	zephyrproject-rtos/hostap@`f5708c5`	zephyrproject-rtos/hostap@1a2fbb79..f5708c52
nrf_wifi	zephyrproject-rtos/nrf_wifi@`29afb11`	zephyrproject-rtos/nrf_wifi@`1e37ff7`	zephyrproject-rtos/nrf_wifi@29afb11a..1e37ff7a
trusted-firmware-m	zephyrproject-rtos/trusted-firmware-m@`677e056`	zephyrproject-rtos/trusted-firmware-m@`3a128df`	zephyrproject-rtos/trusted-firmware-m@677e0565..3a128df2

✅ All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

I'm at a loss here. The feature this test case wants to see (on ARMv7M, not ARMv6M) is the ability to take a irq_lock() inside a system call and then see that future system calls from the same thread continue to hold the lock. That's not documented AFAICT. It's also just a terrible idea because either: 1. The obvious denial of service implications if user code is allowed to run in an unpreemptible mode, or: 2. The broken locking promise if this is implemented to release the lock and reacquire it in an attempt to avoid #1. (FWIW: my read of the code is that #1 is the current implementation. But hilariously the test isn't able to tell the difference!) And in any case it's not how any of our other platforms work (or can work, in some cases), making this a non-portable system call API/feature at best. Leave it in place for now out of conservatism, but disable with the new arch_switch() code, whose behavior matches that of other Zephyr userspaces. Signed-off-by: Andy Ross <andyross@google.com>

rob-robinson-14 force-pushed the health-monitoring-vbatt branch from 02a7a80 to 5da04bc Compare February 20, 2026 10:11

Hi-Im-David suggested changes Feb 20, 2026

View reviewed changes

rob-robinson-14 force-pushed the health-monitoring-vbatt branch 6 times, most recently from 67d41a9 to 98d4071 Compare February 23, 2026 17:52

dhsu-nordic reviewed Feb 24, 2026

View reviewed changes

rob-robinson-14 force-pushed the health-monitoring-vbatt branch 2 times, most recently from d699165 to 28e93be Compare February 25, 2026 12:22

rob-robinson-14 force-pushed the health-monitoring branch from 86a89bb to 0a18f07 Compare March 17, 2026 12:59

rob-robinson-14 added 2 commits March 17, 2026 13:02

Health Monitor - WIP

1f1971b

Add support to monitor die temperature

0093763

rob-robinson-14 force-pushed the health-monitoring-vbatt branch from 28e93be to 0093763 Compare March 17, 2026 13:03

github-actions Bot added manifest manifest-hal_nordic manifest-nrf_wifi manifest-hostap manifest-trusted-firmware-m labels Mar 17, 2026

rob-robinson-14 force-pushed the health-monitoring branch from 0a18f07 to 7033090 Compare March 17, 2026 13:07

Conversation

rob-robinson-14 commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rob-robinson-14 commented Feb 19, 2026 •

edited

Loading