State of charge calculation for li-ion battery

So I am doing a battery monitoring system project for a 13v Li-ion battery used in a robot.
Currently, I am stuck in part to calculate the state of charge using coulomb counting.
This is my current code. AcsValueF is the current detected using a current sensor.

 currentMillis = millis();  
  if (currentMillis - startMillis >= period)  
  {
    totalCoulumbs = totalCoulumbs + AcsValueF;
   float TotalAh = totalCoulumbs/3600.0;
     SOC = (TotalAh / (maxAH))*100;
    startMillis = currentMillis;  
  }

Does this code work to calculate the state of charge?

Maybe. It depends what the rest of the code is doing.

What is your period value?

The period is 1000 milliseconds

The rest of code in the project senses temperature, humidity and voltage.

I was trying to prompt you to post your full code. That's the only way we can tell if the snippet you posted will work as you want.

oh, sorry didn't understand that
here's my full code

#include <EEPROM.h>
#include "WiFi.h"
#include <Wire.h>
#include "cactus_io_HIH6130.h"

const char* ssid = "wifi ssid";
const char* password =  " wifi password";

//define the address used by the HIH6130 sensor (default is 0x27)
byte address = 0x27;
HIH6130 hih6130(address);

const int voltageSensor = A0 ;
float vOUT = 0.0;
float vIN = 0.0;
float R1 = 30000.0;
float R2 = 7500.0;
int value = 0;

int smokesensor = A2;
int sensorThres = 400;

int buzzer = A3;
int led = A4;

float SOC;
float SOH;

unsigned long startMillis;  //some global variables available anywhere in the program
unsigned long currentMillis;
const int period = 1000;  //the value is a number of milliseconds
float totalCoulumbs = 0.0;

void setup() {
  
Serial.begin(115200);
WiFi.begin(ssid, password);
while (WiFi.status() != WL_CONNECTED) {
delay(500);
Serial.println("Connecting to WiFi..");
}
Serial.println("Connected to the WiFi network");

startMillis = millis();  //initial start time

Wire.begin();
//Serial.println("Honeywell HIH6130 Humidity - Temperature Sensor");
//Serial.println("RH\tTemp (C)\tHeat Index (C)");

pinMode(smokesensor, INPUT);

pinMode(buzzer, OUTPUT);
pinMode(led, OUTPUT);

SOH = EEPROM.read(0);


}

void loop() {
 value = analogRead(voltageSensor);
 vOUT = (value * 5.0) / 1024.0;
 vIN = vOUT / (R2/(R1+R2));
//Serial.print(vIN);

unsigned int x=0;
float AcsValue=0.0,Samples=0.0,AvgAcs=0.0,AcsValueF=0.0;
  for (int x = 0; x < 150; x++){ 
  AcsValue = analogRead(A1);       
  Samples = Samples + AcsValue;  
  delay (3); 
}
AvgAcs=Samples/150.0;//Taking Average of Samples
AcsValueF = (2.5 - (AvgAcs * (5.0 / 1024.0)) )/0.185;
//Serial.print(AcsValueF);

int analogSensor = analogRead(smokesensor);
//Serial.println(sensorValue, DEC);
//add if else for smoke more than smth
// if sensorValue more than thres,400, loop smth

hih6130.readSensor();
//Serial.print(hih6130.humidity); Serial.print("\t");
//Serial.print(hih6130.temperature_C); Serial.print("\t\t");
//Serial.print(hih6130.computeHeatIndex_C()); Serial.print("\t\t");

if(hih6130.temperature_C > 60 || hih6130.temperature_C < -20 || analogSensor > sensorThres || hih6130.humidity > 75) 
{
tone(buzzer, 1000);
digitalWrite(led, HIGH);
delay(1000);              
digitalWrite(led, LOW);   
delay(1000);             
} 
else
{
noTone(buzzer);
digitalWrite(led,LOW);  
} 

 currentMillis = millis();  //get the current "time" (actually the number of milliseconds since the program started)
  if (currentMillis - startMillis >= period)  //test whether the period has elapsed
  {
    totalCoulumbs = totalCoulumbs + AcsValueF;
   float TotalAh = totalCoulumbs/3600.0;
    // SOC = (TotalAh / (max AH of batt))*100;
    // SOH = (max Ah of battery / new batt max AH)*100; 
    startMillis = currentMillis;  //IMPORTANT to save the start time of the current state.
  }

}

the comments that have serial print are just for me to remember to display them to cloud

Please use Auto-Format in the IDE and re-post the code. Edit the post above for that if you like. When you understand the importance of correct indentation, you'll be one rung up the ladder from beginner!

Can you please explain what is happening in this line?

AcsValueF = (2.5 - (AvgAcs * (5.0 / 1024.0)) )/0.185;
#include <EEPROM.h>
#include "WiFi.h"
#include <Wire.h>
#include "cactus_io_HIH6130.h"

const char* ssid = "wifi ssid";
const char* password =  " wifi password";

//define the address used by the HIH6130 sensor (default is 0x27)
byte address = 0x27;
HIH6130 hih6130(address);

const int voltageSensor = A0 ;
float vOUT = 0.0;
float vIN = 0.0;
float R1 = 30000.0;
float R2 = 7500.0;
int value = 0;

int smokesensor = A2;
int sensorThres = 400;

int buzzer = A3;
int led = A4;

float SOC;
float SOH;

unsigned long startMillis;  //some global variables available anywhere in the program
unsigned long currentMillis;
const int period = 1000;  //the value is a number of milliseconds
float totalCoulumbs = 0.0;

void setup() {

  Serial.begin(115200);
  WiFi.begin(ssid, password);
  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.println("Connecting to WiFi..");
  }
  Serial.println("Connected to the WiFi network");

  startMillis = millis();  //initial start time

  Wire.begin();
  //Serial.println("Honeywell HIH6130 Humidity - Temperature Sensor");
  //Serial.println("RH\tTemp (C)\tHeat Index (C)");

  pinMode(smokesensor, INPUT);

  pinMode(buzzer, OUTPUT);
  pinMode(led, OUTPUT);

  SOH = EEPROM.read(0);


}

void loop() {
  value = analogRead(voltageSensor);
  vOUT = (value * 5.0) / 1024.0;
  vIN = vOUT / (R2 / (R1 + R2));
  //Serial.print(vIN);

  unsigned int x = 0;
  float AcsValue = 0.0, Samples = 0.0, AvgAcs = 0.0, AcsValueF = 0.0;
  for (int x = 0; x < 150; x++) {
    AcsValue = analogRead(A1);
    Samples = Samples + AcsValue;
    delay (3);
  }
  AvgAcs = Samples / 150.0; //Taking Average of Samples
  AcsValueF = (2.5 - (AvgAcs * (5.0 / 1024.0)) ) / 0.185;
  //Serial.print(AcsValueF);

  int analogSensor = analogRead(smokesensor);
  //Serial.println(sensorValue, DEC);
  //add if else for smoke more than smth
  // if sensorValue more than thres,400, loop smth

  hih6130.readSensor();
  //Serial.print(hih6130.humidity); Serial.print("\t");
  //Serial.print(hih6130.temperature_C); Serial.print("\t\t");
  //Serial.print(hih6130.computeHeatIndex_C()); Serial.print("\t\t");

  if (hih6130.temperature_C > 60 || hih6130.temperature_C < -20 || analogSensor > sensorThres || hih6130.humidity > 75)
  {
    tone(buzzer, 1000);
    digitalWrite(led, HIGH);
    delay(1000);
    digitalWrite(led, LOW);
    delay(1000);
  }
  else
  {
    noTone(buzzer);
    digitalWrite(led, LOW);
  }

  currentMillis = millis();  //get the current "time" (actually the number of milliseconds since the program started)
  if (currentMillis - startMillis >= period)  //test whether the period has elapsed
  {
    totalCoulumbs = totalCoulumbs + AcsValueF;
    float TotalAh = totalCoulumbs / 3600.0;
    // SOC = (TotalAh / (max AH of batt))*100;
    // SOH = (max Ah of battery / new batt max AH)*100;
    startMillis = currentMillis;  //IMPORTANT to save the start time of the current state.
  }

}

this is to get the current that is being discharged from the battery after getting samples and taking the average of it

Thanks for formatting your code, easier to read now.

Then the result of calculation you asked about in your first post is correct, but only by accident. It is supposed to be totalling the charge in coulombs but in fact it is totalling a number of current measurements, which makes no sense. To convert amps to coulombs, you must multiply the current in amps by the time for which it flows in seconds. Because 'period' is set to 1000ms = 1s, you should multiply the current by 1 second, which of course makes no difference to the result, which is why it is, by chance, correct. But if you changed period to 500ms or 2000ms for example, the result would be incorrect.

It would be more correct to change the line like this

totalCoulumbs = totalCoulumbs + AcsValueF * period / 1000.0;

then it would be correct for other values of 'period'.

But even with that change, your code could be improved. Right now, it takes many measurements of the current per second, but only one of those is used to update the total coulombs. The other current measurements are discarded. If one current measurement per second is accurate enough, then you could make your code more efficient by taking only one current measurement per second. This would free up the Arduino's processing time to perform other tasks or to go into a low power sleep mode to extend battery life, if your sensor is battery powered. If there is no need to be more efficient, then your code could measure the coulombs more accurately by averaging the current over each second, rather than using one snapshot from each second.

Oh I'll try it
But the code for the current sensor is so that the sampling could be more accurate.
So I can remove It safely right ?

Sorry, I don't understand what you are saying about the code, or what you are asking can be removed.

Sorry about that. The snippet you talked about is for the acs712 current sensor. This takes a lot of samples and gets the average of it to get the final current reading

Yes, it's taking average of 150 readings to get a more accurate answer. I don't know how long that process takes, but let us guess it takes 10ms. It does this each time loop() executes, which would be a hundred times per second @ 10ms per loop. So 100 measurements are taken but 99 of these results are discarded. Only one result from each hundred is used to update the total Ah. The Arduino is wasting 99% of its time.

Does that help you understand what I was saying in post #11?

EDIT: sorry, I just noticed there is a delay(3) in the loop, so taking the average of 150 readings will take at least 450ms. So the Arduino is probably wasting 50% of its time.

I think I would remove the for-loop and simply take one reading each time loop() executes and count the number of readings that have been taken. Then, when 'period' has passed, calculate the average reading for use updating the Ah total.

Ohhhhhhh
I understand it now
Ok I will change it thank you very much
So after this the soc calculation would be more accurate right?

Yes, in theory. But I'm not sure if it would be noticeable in practice. Certainly it would seem a more sensible approach to me.

Oh ok I would need to try it when the products arrive to me. I'm using an esp32 for this