Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

[Feature Request]: Report INA sensor voltage as device battery voltage & other Power module enhancements #3283

Closed
KeithMon opened this issue Feb 25, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@KeithMon
Copy link

Platform

NRF52, ESP32

Description

This request seeks to round out the Power module by tying up loose ends and by fully supporting various battery chemistries and configurations. This will primarily benefit solar nodes. More people are building solar nodes with battery chemistries and configurations that are not a 1S Lion.

Finish adding support for INA voltage to be reported as Device Battery Voltage
When power.device_battery_ina_address is set, report the voltage reading as the device battery voltage. This should be used for device battery voltage sent over the mesh (to other nodes) as well as battery voltage reported in the apps/clients. When set, this should entirely ignore voltage readings from the device itself (ADC, AXP, etc).

Support INA Sensors With Multiple Channels
When an INA3221 sensor is set in power.device_battery_ina_address we need the ability to select which channel should be used for the device battery voltage. Create a new user option that can be set in the apps/clients for "Device Battery INA Channel", power.device_battery_ina_channel. The options could be: 1, 2 or 3. Voltage readings from the selected channel will be used as the device battery voltage.

Select battery chemistry
Extend the functionality of the recent work by @Gabrielerusso in #3216 so battery chemistry is added as a user option in the Power module. It could be called Battery Type (power.device_battery_type). Battery Type would be the same options added with #3216. The firmware would use the selected battery type to choose the correct capacity-voltage curve (for example, how it defines CELL_TYPE_LIFEPO4 or CELL_TYPE_LEADACID). If we support device battery voltage coming from an INA sensor we can no longer expect battery voltage levels to be the same as a Lion. Nor can we expect a particular board variant to always use a specific battery type. This should be a user option and not tied to a particular variant.

Support battery configuration: number of series
To fully support #3216 and external voltages reported by an INA, we need another user options to define how many cells in series their battery is configured for: Number of Series (power.device_battery_num_series). The current variable used in #3216 is "NUM_CELLS". This name could confuse users who might think they should count cells they have in parallel (for example, the user might have 1S2P and mistakenly put 2 for this value. That's why we include "series" in the option name). There should be little risk exposing these options to users since it only impacts battery percentage reported and does not have an impact on actual charging voltage.

Support upcoming feature request: Battery Capacity
Create another user option in the Power module for Battery Capacity (power.device_battery_capacity). This value is the mAh capacity of the connected battery bank. Users can enter the mAh capacity of their battery bank. Battery capacity is important to know for a future feature request. It will be used to determine the remaining energy in the battery at a particular voltage. (Knowing the battery capacity has other benefits and could support other enhancements like expected runtime at current usage.)

@KeithMon KeithMon added the enhancement New feature or request label Feb 25, 2024
@tropho23
Copy link
Contributor

I am really excited to potentially see the last item, 'Support upcoming feature request: Battery Capacity'. This would make battery capacity calculation not only easy, but very accurate.

@Gabrielerusso
Copy link
Contributor

I am really excited to potentially see the last item, 'Support upcoming feature request: Battery Capacity'. This would make battery capacity calculation not only easy, but very accurate.

Voltage curve now are pretty accurate, including a Coulomb meter seems a bit overkill for a project like this, but it's easy and would take a couple of minute to implement it, I Just don't have time right now.

For the other points there are things suggested that I don't like, like exposing some too advanced thing to the final user.

@garthvh
Copy link
Member

garthvh commented Feb 27, 2024

Yeah battery chemistry seems like a compile time option, not a user managed one.

@KeithMon
Copy link
Author

Would you explain why you think adding these options as a user-setting is a bad idea?

If we enable device battery voltage to be read from an INA sensor then we cannot assume the voltage being read will be li-ion. As of 2.2.23 the firmware supports reading battery voltage for different chemistries (at least at some level)). That tells me there is community support for the concept of using different battery types. Why wouldn't we make it easier for people to configure their solar node? Otherwise, you're talking about creating a firmware variant for each battery chemistry and configuration.

I've seen members in the community build solar nodes with:

  • Li-ion
  • LifePo4
  • LTO
  • Lead acid

If you just consider 1S and 2S that's 8 unique variants just for a RAK. That seems less practical than allowing the user to choose their chemistry and configuration in the app.

@Gabrielerusso
Copy link
Contributor

Gabrielerusso commented Feb 27, 2024

Would you explain why you think adding these options as a user-setting is a bad idea?

Most user have no idea what they are using and what changing parameters do, yourself said that "This name could confuse users who might think they should count ..." . Generally speaking the battery configuration, including series, parallel and battery chemistry are advanced options that are not meant to be exposed to the average person. Battery capacity could be inserted and exposed, as it could be used for estimate the remaining battery life time.

As of 2.2.23 the firmware supports reading battery voltage for different chemistries (at least at some level)). That tells me there is community support for the concept of using different battery types.

Yes, I implemented multiple curves so it can be adapted to different boards and someone that will make it's varian can in a couple of minute just put it's configuration and compile it's firmware variant.

Why wouldn't we make it easier for people to configure their solar node? Otherwise, you're talking about creating a firmware variant for each battery chemistry and configuration.

Code space is limited doing everything at runtime would use more space and slow things, this is not a BMS and to me it doesn't need to act like one.

I've seen members in the community build solar nodes with:

  • Li-ion
  • LifePo4
  • LTO
  • Lead acid

If you just consider 1S and 2S that's 8 unique variants just for a RAK. That seems less practical than allowing the user to choose their chemistry and configuration in the app.

You can just compile and install the version you need, there is no sense in pre-compiling each variant for each possible combination of all the parameters ...

I think there is a need to do a cost/benefit comparison, if out of 1000 people just 3 people need that specific configuration, is really worth it to add those at runtime or the 3 person could just wait 2 minute to compile the version for their specific use case?

Just keep in mind that the battery informations from the boards are just for display purpose, any safety manner, like over discharging, over charging, unbalancing, ecc... should be done from a BMS with an hardware protection, nrf52/esp32 and so on are not battery monitoring and protection ICs .

@KeithMon
Copy link
Author

KeithMon commented Feb 27, 2024

I would clarify that none of these features were requested so they could be used in place of a BMS. The purpose of this request is so users can see the correct battery voltage and percentage in the app. That's it.

If nothing else is implemented, getting the INA voltage reading to display as device battery voltage is the primary request. Some work has already been done to support this.

The other requests are related to displaying the correct battery percentage. An LTO battery (which could range from 1.5v to 2.8v) will always display 0% when a li-ion voltage profile is applied. That's what we're trying to fix by understanding the battery type and configuration. When we know battery type and configuration (1S, 2S) then we can apply the correct capacity-voltage curve to get the battery percentage. That's the only thing these features would impact.

That's why I don't understand how these would cause problems if exposed as user settings. If the user messes up these values it only impacts what they see as their battery percentage.

@Gabrielerusso
Copy link
Contributor

Gabrielerusso commented Feb 27, 2024

The other requests are related to displaying the correct battery percentage.

But you can already do that, just select the battery configuration you want to use, it's not something you change every day, it's one time only.

That's why I don't understand how these would cause problems if exposed as user settings. If the user messes up these values it only impacts what they see as their battery percentage.

I forgot on key-point, most board have a battery port but don't have a bms or any battery protection IC, so the only protection done by the firmware (as most don't use a BMS and have no idea what it is) is to "shutdown" the board in case an undervoltage is detected.
If someone puts "Lead Acid" insted of "LiIon" that limit will fail and the battery will be over discharged, and then if the charging circuit fails to prevent charge, or charges with a too high current it could lead to a fire.

@KeithMon
Copy link
Author

What do you mean by "select the battery configuration you want to use"? Are you talking about selecting the firmware variant to flash?

It sounds like you don't have any objection to reporting INA voltage as device battery voltage. Is that correct? Because that's the main thing. It would be a huge win for a lot of people if we just had that.

@meshtastic meshtastic locked and limited conversation to collaborators May 7, 2024
@thebentern thebentern converted this issue into discussion #3825 May 7, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants