LDSW, IBATT and NRF9160 examples #9

fredriknk · 2025-03-03T13:59:36Z

Hi, thanks for the awesome work on this driver, im implementing it in a project im working on but i needed some additions since i needed LDSW and ibat current measurements. I only have a nrf9160 board to test it on so i added examples for the nrf9160 soc and a .vscode settings.json folder to easily switch the different targets

The ldsw is pretty straight forward implementation, i dont have time to make the drivers for the LDO, but i thought it might be good to at least get ldsw implementation?

For the IBAT measurements im using the zephyr NPM1300charger.c as my reference since the nordic documentation has basically no info whatsoever. Its kind of a weird measurement because it scales depending on your config of the max charge and discharge currents, so you need to read those registers in order to calculate the full scale values.

For note, im in the early process of learning rust so my code is probably not kosher, so please dont blindly accept my additions. but it does seem to work well on my board at least.

…ples

fredriknk · 2025-03-03T14:34:50Z

I didnt see the issue you made untill now, but i do get some reasonable values out for the current consumption on both charge and discharge. Regarding the registers you mention, the zephyr drivers use the BCHARGERMODE and bchargericharge register as a 4 bit register with theese 4 statuses

/// define IBAT_STAT_DISCHARGE      0x04U
/// define IBAT_STAT_CHARGE_TRICKLE 0x0CU
/// define IBAT_STAT_CHARGE_COOL    0x0DU
/// define IBAT_STAT_CHARGE_NORMAL  0x0FU

And a status for no battery connected

IbatStatChargeError = 8, //0x08 undocumented in driver but occurs if you remove the battery while charging

badrbouslikhin

Thanks a lot for your contribution!
I've added a few comments, but nothing major.

Parts of the code are not formatted correctly (src/charger/mod.rs for eg), do you use rustfmt or rust-analyzer?
Can you also please make sure the docstrings follow the crate convention? Eg.

    /// Measure VBAT
    ///
    /// This function triggers a VBAT measurement and returns the result.
    ///
    /// # Returns
    ///
    /// * `Ok(f32)` - The measured VBAT voltage
    /// * `Err(NPM1300Error)` - An error occurred while reading the VBAT measurement result

badrbouslikhin · 2025-03-05T11:37:51Z

.vscode/settings.json

I am not familiar with VS Code, are these files mandatory?
Can you make them generic? I see a lot of MCU specific settings and this crate is MCU-agnostic.

badrbouslikhin · 2025-03-05T11:38:13Z

.vscode/.gitignore

I am not familiar with VS Code, how does this .gitignore differ from the main .gitignore?

badrbouslikhin · 2025-03-05T11:44:37Z

device.yaml

@@ -1708,7 +1708,7 @@ ADC:
          description: ADC VBAT (or VBUS) burst measurement result upper 8-bits
    ADCGP1RESULTLSBS:
      type: register
-      description: ADC result LSB's (VBAT burst 0, 1, 2 and 3)
+      description: ADC result LSB's (VBAT burst 0, 1, 2 and 3) or 2 IBAT/ 3 VBUS


Do you mean that VBAT2RESULTLSB contains IBAT LSB and VBAT3RESULTLSB VBUS LSB?

badrbouslikhin · 2025-03-05T11:53:13Z

README.md

@@ -22,7 +22,8 @@ This crate provides both low-level register access and a high-level API for mana
 | SYSREG — System regulator                 |        ✅         |         ✅         |
 | CHARGER — Battery charger                 |        ✅         |         ✅         |
 | BUCK — Buck regulators                    |        ✅         |         ✅         |
-| LOADSW/LDO — Load switches/LDO regulators |        ❌         |         ❌         |
+| LOADSW — Load switches                    |        ✅         |         ✅         |


It looks like many LOADSW configuration registers are not supported LDSW1GPISEL, LDSWCONFIG, LDSW1LDOSEL, LDSW1VOUTSEL, etc.
If that's really the case, I suggest making it explicit in the README and create an issue to add support for those registers.

badrbouslikhin · 2025-03-05T11:55:07Z

examples/nrf9160/Cargo.toml

+npm1300-rs = { path = "../../", features = ["defmt-03"] }
+
+[patch.crates-io]
+embassy-nrf = { git = "https://github.com/embassy-rs/embassy.git" }


I suggest pinning a specific embassy-nrf/embassy-executor/embassy-time revision to avoid a breaking changes (see the Cargo.toml of the nRF52840 example).

badrbouslikhin · 2025-03-05T11:59:50Z

src/charger/mod.rs

@@ -785,4 +809,163 @@ impl<I2c: embedded_hal_async::i2c::I2c, Delay: embedded_hal_async::delay::DelayN
            })
            .await
    }
+
+    /// Configure auto ibat measurement enabled. It seems you dont


It is indeed the case, according the datasheet, ADCIBATMEASEN: Enable Auto IBAT measurement after VBAT task

badrbouslikhin · 2025-03-05T12:10:51Z

src/charger/mod.rs

+    /// # Returns
+    /// 
+    /// a scaled signed integer in ma of the current charge or discharge current
+    pub async fn measure_ibat(&mut self) -> Result<f32, crate::NPM1300Error<I2c::Error>> {


It is looks like this method doesn't triggers IBAT measurements.
How can we make sure we aren't reading a VBAT measurement?

badrbouslikhin · 2025-03-05T12:33:05Z

src/charger/mod.rs

+        return Ok(result);
+    }
+    /// Measure IBAT current and convert it to a scaled value
+    /// This is more at home in the ADC function, but i couldnt


I agree with you, all adc methods should be in the ADC module. I've pushed that refactor.

badrbouslikhin · 2025-03-05T12:39:50Z

src/charger/mod.rs

+        let discharge_current_limit = self.get_discharge_current_limit().await?;
+        let ibat_scaled = match ibat_status {
+            IbatStatuscodes::IbatStatDischarge => {
+                let max_discharge_current = match discharge_current_limit{


It is better to implement this to the type struct directly, something like:

// Implement conversion to f32 impl From<DischargeCurrentLimit> for f32 { fn from(value: DischargeCurrentLimit) -> Self { match value { DischargeCurrentLimit::Low => 200.0, DischargeCurrentLimit::High => 1000.0, } } }

badrbouslikhin · 2025-03-05T12:45:22Z

src/charger/mod.rs

+            .bchgisetdischargelsb();
+
+        match (msb, lsb) {
+            (42, 0) => Ok(DischargeCurrentLimit::Low),  // 200mA case


Can you please add:

// NOTE: the following magic numbers come from the product specification doc.

fredriknk added 13 commits February 26, 2025 21:35

Added ldsw yml data

2096858

Added try_conversion: crate::common::Task

1c70694

feat: Added .vscode config for rust analyzer for multiple target exam…

bcf2986

…ples

fix: Changed target location to account for more targets

c9dfd46

Feat: Nrf9160 example

e85a71c

feat: Blinky example

0818200

feat: Mesure voltage and ntc temp

5fe03ee

feat: Added basic loadswitch functionality

6dd9460

fix: Removed unused libs

842f80a

feat: function to read charger current config

a423256

fix: Theese registers are allso used for ibat measurement, added info

158d016

feat: Added ibat current measurement and conversion

817e14e

fix: Removed a message i used for debug

7e19a89

fredriknk and others added 2 commits March 4, 2025 09:07

fix: Update README.md wit loadsw

278dcc9

refactor: move all adc related methods to adc module

f2ef647

badrbouslikhin requested changes Mar 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LDSW, IBATT and NRF9160 examples #9

LDSW, IBATT and NRF9160 examples #9

fredriknk commented Mar 3, 2025

fredriknk commented Mar 3, 2025 •

edited

Loading

badrbouslikhin left a comment

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

badrbouslikhin Mar 5, 2025

LDSW, IBATT and NRF9160 examples #9

Are you sure you want to change the base?

LDSW, IBATT and NRF9160 examples #9

Conversation

fredriknk commented Mar 3, 2025

fredriknk commented Mar 3, 2025 • edited Loading

badrbouslikhin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredriknk commented Mar 3, 2025 •

edited

Loading