search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.61k stars 6.5k forks source link

MAX17048 driver should return battery level in microvolts #75492

Closed Goyalrahul1516 closed 2 months ago

Goyalrahul1516 commented 3 months ago

Unknown unit of time to empty parameter

Interfaced MAX17048 sensor with nRF52832 dev kit, got four ouputs

  1. time to empty 128
  2. time to full 0
  3. charge 10%
  4. Voltage 3690 But i don't know the units of time to empty

<< You may also want to update the automatically generated issue title above. >>

Environment

github-actions[bot] commented 3 months ago

Hi @Goyalrahul1516! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

kartben commented 3 months ago

Please just refer to the API documentation. It's supposed to be expressed in minutes https://docs.zephyrproject.org/latest/hardware/peripherals/fuel_gauge.html#c.fuel_gauge_prop_type.FUEL_GAUGE_RUNTIME_TO_EMPTY

kartben commented 3 months ago

BTW, it would be most welcome if you're interested in submitting a pull request to update the sample so that the units are included in the console output.

Goyalrahul1516 commented 3 months ago

Please just refer to the API documentation. It's supposed to be expressed in minutes https://docs.zephyrproject.org/latest/hardware/peripherals/fuel_gauge.html#c.fuel_gauge_prop_type.FUEL_GAUGE_RUNTIME_TO_EMPTY

I just read the documentation, time to discharge unit is resolved but now the doc shows that FUEL_GAUGE_VOLTAGE is shown in micro Volts, the problem is the battery i connected has a rating of 3.6V but the output shows 3690 uV which means 3.69 mV. So am i getting something wrong here? :(

aaronemassey commented 3 months ago

Please just refer to the API documentation. It's supposed to be expressed in minutes https://docs.zephyrproject.org/latest/hardware/peripherals/fuel_gauge.html#c.fuel_gauge_prop_type.FUEL_GAUGE_RUNTIME_TO_EMPTY

I just read the documentation, time to discharge unit is resolved but now the doc shows that FUEL_GAUGE_VOLTAGE is shown in micro Volts, the problem is the battery i connected has a rating of 3.6V but the output shows 3690 uV which means 3.69 mV. So am i getting something wrong here? :(

If I understand this datasheet correctly, it looks like that falls well within error for measurement. Does the 0.09 mV make a significant difference in your use case?

Also please close this issue and open a new one. I think the documentation exists.

However, as a total aside, I'd like to making a major breaking API change to align with the charger API for having the units defined within the enum name.

By the way, my responses may be slow. I'm on leave taking care of some family right now.

Goyalrahul1516 commented 3 months ago

I think there has been a misunderstanding, i said my battery was 3.6V but the output shows 3.69mV according to the documentation. So i just wanted to clear that up.

Also no problem, Family comes before everything, take your time this can wait.

aaronemassey commented 2 months ago

My apologies for the misunderstanding.

Looking at 32b27384a68f250a1 it looks like you're completely correct that that MAX17048 was missed during a conversion to micro-volts. I'm honestly shocked I missed that conversion. Thank you for catching this.

That is a breaking change for current users, but it's something that should've occurred a while ago with a simple short fix. I need to setup my Chromebook I have with me for Zephyr development (should be pretty quick) but if you want this resolved faster you're more than welcome to push a PR to change here. Current was never implemented so that does not need to be adjusted.

Either way, a PR for this should be pushed this week. Let me know if you want to do it, otherwise I'll go ahead and do it. Afterwards you can cherry-pick it to your Zephyr for or update to top-of-tree.

Goyalrahul1516 commented 2 months ago

I am going to put a PR now, let me know if any further changes are required. I tested the changes and got 7 digit output across voltage parameter (output now in micro volts :) ). To convert to Voltage just need to divide by 1e6.

Goyalrahul1516 commented 2 months ago

I apologize for making the same PR twice, i am new to git and github and previous CI made me confused so i just made the changes and created a new PR.

aaronemassey commented 2 months ago

I apologize for making the same PR twice, i am new to git and github and previous CI made me confused so i just made the changes and created a new PR.

No worries! I appreciate you making the PR!