Using I2C for SMBus device communications


#104

Yea, I thought about that awhile back.

I figured I could just write a separate block read function for each variable I need to read and then as you mention just save the data directly into the correct variable.

The only downside to that is how many functions this fuel gauge has that require a block read.

For example here are all the registers on the fuel gauge that require a Block Read and each of them return different amounts of data. I’ll have to try to figure out a way to efficiently save the returned data from all these block reads directly into the properly variables :smiley:

int BQ20Z45::GaugingStatus(char *result)
{

 return readStringB(BQ20Z45_GaugingStatus, result);
}

 int BQ20Z45::SafetyAlert(char *result)
{

    return readStringB(BQ20Z45_SafetyAlert, result);
}

int BQ20Z45::SafetyStatus(char *result)
{

    return readStringB(BQ20Z45_SafetyStatus, result);
}

int BQ20Z45::PFAlert(char *result)
{

    return readStringB(BQ20Z45_PFAlert, result);
}

int BQ20Z45::PFStatus(char *result)
{

    return readStringB(BQ20Z45_PFStatus, result);
}

int BQ20Z45::OperationStatus(char *result)
{

return readStringB(BQ20Z45_OperationStatus, result);
}

int BQ20Z45::ChargingStatus(char *result)
{

    return readStringB(BQ20Z45_ChargingStatus, result);
}

int BQ20Z45::ManufacturingStatus(char *result)
{

return readStringB(BQ20Z45_ManufacturingStatus, result);
}

int BQ20Z45::GetLifetimeDataBlock1(char *result)
{

    return readStringB(BQ20Z45_LifetimeDataBlock1, result);
}

int BQ20Z45::GetLifetimeDataBlock2(char *result)  // Last 2 returns on BQ Studio are different, PEC maybe?
{

    return readStringB(BQ20Z45_LifetimeDataBlock2, result);
}

int BQ20Z45::GetLifetimeDataBlock3(char *result)
{

    return readStringB(BQ20Z45_LifetimeDataBlock3, result);
}

int BQ20Z45::GetLifetimeDataBlock4(char *result)
{

    return readStringB(BQ20Z45_LifetimeDataBlock4, result);
}

int BQ20Z45::GetLifetimeDataBlock5(char *result)
{

    return readStringB(BQ20Z45_LifetimeDataBlock5, result);
}

int BQ20Z45::GetLifetimeDataBlock6(char *result)
{

    return readStringB(BQ20Z45_LifetimeDataBlock6, result);
}


#105

I just repeat: You are passing in a parameter char* result to all of them, just use that parameter properly :wink:


#106

Yes, that makes perfect sense and I understand how that works conceptually but right now I’m not 100% sure exactly which of the 4 separate clumps of code need changed to send the result to their corresponding variables.

Some registers will return 32 bytes of status data so I could just instead of sending the data to a Char I can just send it to a uint32_t variable instead. Right?

I’ve tried making some changes to the .H & .CPP code for the bms.GaugingStatus(strBuffer); in an attempt to get it setup to pass the data into a new variable but so far with no luck :smirk:

If you could provide some sort working example it would save me tons of time and I will then be able to change the other 15+ functions over to this new method also which further burn the correct way to do this into my mind.

If you want me to figure this out the hard & long way then I understand :smile:

Below is my current code structure.

Here is the .ino file:

#include "BQ20Z45.h"            //Include BQ78350 Header File

// Store an instance of the BQ20Z45 Sensor
BQ20Z45 bms;

char strBuffer[33];    // This is a Buffer to store 32 bit data strings read from the BQ78350 Fuel Gauge. Not Sure if this is the best place for this code.


void setup(void)
{
  // We start the serial library to output our messages.
  Serial.begin(115200);

  // Start i2c communication.
  Wire.begin();
}


void loop(void)
{

//Serial.println("Gauging Status");
bms.GaugingStatus(strBuffer);


delay(4000);

}

Here is the .H file:

#ifndef BQ20Z45_h
#define BQ20Z45_h


#define BQ20Z45_Address           0x0B
#define BQ20Z45_ManBlockAccess    0x44
#define BQ20Z45_ManAccess         0x00

#define BQ20Z45_GaugingStatus     0x56

class BQ20Z45
{
    public:


    int GaugingStatus(char *result);


    protected:

    private:


    int readStringB(uint8_t address, char* result);

};
#endif

Here is the cpp file:

#include "application.h"
#include "BQ20Z45.h"
//#include "Arduino.h"
#include "application.h"

/////////////////////////////////////////////////////////////////////////////
// Functions Below


// pass a pointer to a char[] that can take up to 33 chars
// will return the length of the string received
int BQ20Z45::readStringB(uint8_t address, char* result)
{
    int pos = 0;
    int len;

        // Read the length of the string
    Wire.beginTransmission(BQ20Z45_Address);      //Setup Write to Gauge
    Wire.write(0x44);                             //Manufacturer Block Read
    Wire.write(0x02);                             //How Many Bytes to expect Next
    Wire.write(address);                          //Manufacturer Access Command - Example 0x01 Device Type
    Wire.write(0x00);                             //First Byte of Command
    Wire.endTransmission(true);                   //Repeated Start = True

    Wire.beginTransmission(BQ20Z45_Address);      //Setup Write to Gauge
    Wire.write(0x44);                             //Manufacturer Block Read
    Wire.endTransmission(true);                   //Repeated Start

    Wire.requestFrom(BQ20Z45_Address, 1, true);   //Setup Reading of Returned Data
    len = Wire.read();    // length of the string thats about to return
        len++;            // plus one to allow for the length byte on the reread
                          // if len > 32 then the it will be truncated to 32 by requestFrom


    len = Wire.requestFrom(BQ20Z45_Address, len, true);    // readRequest returns # bytes actually read

        len--;           // we won't move the first 3 bytes as its not part of the string
        len--;
        len--;
    if (len > 0)
    {
                Wire.read();
                Wire.read();
                Wire.read();
        for (pos = 0; pos < len; pos++)
            result[pos] = Wire.read();
    }
    result[pos] = '\0';  // append the zero terminator

    return len;

}

/////////////////////////////////////////////////////////////////////////////
// Class Methods Below

int BQ20Z45::GaugingStatus(char *result)
{

 return readStringB(BQ20Z45_GaugingStatus, result);
}

#107

No need to change anything in the “library” just the call in .ino itself would be sufficient.
Although having a “typesafe” function that will only read the allowed number of bytes and flush the rest would be good practice.


little hint

// don't need that any more --> char strBuffer[33];    <--
// but this
uint16_t gaugeStatus = 0;

void loop(void)
{
  // this used to be
  //bms.GaugingStatus(strBuffer);
  // but what now?
  ...
}

#108

Great.

I think what you mean by that is to change this:
bms.GaugingStatus(strBuffer);

to this:

bms.GaugingStatus(GaugingStatus);

When I make that change:

and then hit compile I get this error message.

Sounds like that’s not supposed to happen.


#109

Based on what you are telling me I’m pretty sure I have the code right, see below:

#include "application.h"
#include "BQ20Z45.h"            //Include BQ78350 Header File

// Store an instance of the BQ20Z45 Sensor
BQ20Z45 bms;

//char strBuffer[33];    // This is a Buffer to store 32 bit data strings read from the BQ78350 Fuel Gauge. Not Sure if this is the best place for this code.

uint16_t GaugingStatus = 0;

void setup(void)
{
  // We start the serial library to output our messages.
  Serial.begin(115200);

  // Start i2c communication.
  Wire.begin();
}


void loop(void)
{


bms.GaugingStatus(GaugingStatus);


Serial.println("");
Serial.println("------------------------------");
Serial.println("");

delay(4000);

}

But that throws this compile error. Of course it couldn’t be as simple as just changing strBuffer to GaugingStatus :smile:

I went into the .H and .CPP files and tried changing char to uint16_t step by step but I only created more compile errors.

You know more than I do about all this and you said I could leave the library alone and just change the pointer from strBuffer to another variable location but that’s not working.

Before I go digging further into trying to copy my other read functions that return uint16_t data I have to ask if you think the compile error may be caused by a problem with Particle DEV or if it’s a mismatch of data types in the functions in the .H & .CPP files?


#110

The magic words are type cast and address of operator (&)


Update:

With

// this
char strBuffer[33];
bms.GaugingStatus(strBuffer);
// change to this
uint16_t GaugingStatus;
bms.GaugingStatus(GaugingStatus);

you are not changing one pointer for another, you are changing a pointer for a variable. You need to get the pointer to that variable and then “disguise” the uint16_t* pointer as a char* pointer via type casting.

Hence this should build without error

bms.GaugingStatus((char*)&GaugingStatus);

But for better coding style and integrity of memory you may well want to change something in the library.
I’d start with changing the name of readStringB() to something like readBytes() and put back that int len paramter you had in post #70 - you will need that for data integrity.
So make it readBytes(uint8_t address, void* buffer, int len) since you are not only getting strings and will not only get char back and depending on the address you should know how many bytes to read. And only read that many bytes into the target.
If you get a different length than you expected this is an error condition and should be treated and signalled as such (read and drop extra bytes to keep the bus tidy - mark too short or too long with return -receivedLength).
Don’t add any zero-terminator in that function but only in a higher level function that knows the requested address will in fact return a string which requires termination.
Change any of your existing higher level functions like this

//int BQ20Z45::GaugingStatus(char *result)
//{
//  return readStringB(BQ20Z45_GaugingStatus, result);
//}

int BQ20Z45::GaugingStatus(uint16_t* result)
{
  return readBytes(BQ20Z45_GaugingStatus, (void*)result, 2);
  // for platforms with incompatible endianness you may want to 
  // add some code to correct the byte order
}

#111

@ScruffR Good advice :thumbsup:

I went over this for hours and I think I have modified the function as you recommended to make it more efficient.

Below is the new function, take a look and let me know if it’s what you had in mind :wink:

I also question if in the first line where it says: void* buffer, should it be void* result instead?

int BQ20Z45::readBytes(uint8_t address, void* buffer, int len)
{
	int pos = 0;
	//int len;

  // Read the length of the string
	Wire.beginTransmission(BQ20Z45_Address);      //Setup Write to Gauge
	Wire.write(0x44);                             //Manufacturer Block Read
	Wire.write(0x02);                             //How Many Bytes to expect Next
	Wire.write(address);                          //Manufacturer Access Command - Example 0x01 Device Type
	Wire.write(0x00);                             //First Byte of Command
	Wire.endTransmission(true);                   //Repeated Start = True

	Wire.beginTransmission(BQ20Z45_Address);      //Setup Write to Gauge
	Wire.write(0x44);                             //Manufacturer Block Read
	Wire.endTransmission(true);                   //Repeated Start

	Wire.requestFrom(BQ20Z45_Address, len, true);   //Setup Reading of Returned Data

        Wire.read(); //We will call 3 Wire.Reads and nto use them since they contain
	Wire.read(); //the returned data length, and address that is returning data
	Wire.read(); //which we do not want included in the returned data string.

        for (pos = 0; pos < len; pos++)
		result[pos] = Wire.read();

	//result[pos] = '\0';  // append the zero terminator

	return len;

And then as you already modified this will call that function:


int BQ20Z45::GaugingStatus(uint16_t* result)
{
  return readBytes(BQ20Z45_GaugingStatus, (void*)result, 2);
  // for platforms with incompatible endianness you may want to 
  // add some code to correct the byte order
}

Now I know I need to add this function to the Class line up in the .H file as shown below but I’m not really sure what to replace the char* result with that’s currently being used in

int readStringB(uint8_t address, char* result);

When I try to compile the new code in the .cpp file I get this compile error because the function is not properly added to the Private section of the .H file.

I tried adding int readBytes(uint8_t address, void* result, int), but that didn’t help :sleepy:

Thanks for your help with this! I’d be lost without your help for sure :thumbsup:


#112

This would be what I had in mind (during implementation I felt the need for an extra flag)

// .h 
int readBytes(uint8_t address, void* buffer, size_t len, boolean exactLen = true);

// .cpp
int BQ20Z45::readBytes(uint8_t address, void* buffer, size_t len, boolean exactLen)
{
  int pos;
  int cnt;                                            // to hold the actual length the address would hold
 
  // Read the length of the string
  Wire.beginTransmission(BQ20Z45_Address);            //Setup Write to Gauge
  Wire.write(0x44);                                   //Manufacturer Block Read
  Wire.write(0x02);                                   //How Many Bytes to expect Next
  Wire.write(address);                                //Manufacturer Access Command - Example 0x01 Device Type
  Wire.write(0x00);                                   //First Byte of Command  
  Wire.endTransmission(true);                         //Repeated Start = True

  Wire.beginTransmission(BQ20Z45_Address);            //Setup Write to Gauge
  Wire.write(0x44);                                   //Manufacturer Block Read
  Wire.endTransmission(true);                         //Repeated Start

  Wire.requestFrom(BQ20Z45_Address, 1, true);         //Setup Reading of Returned Data
  cnt = Wire.read();                                  // length of the string thats about to return
  cnt++;                                              // plus one to allow for the length byte on the reread

  cnt = Wire.requestFrom(BQ20Z45_Address, cnt, true); // readRequest returns # bytes actually read

  if (cnt == len+3 || !exactLen && cnt > 3)
  {                                                   // we got the expected read count
    cnt -= 3;                                         // drop the first three bytes   
    Wire.read();                                      //   length received lsb
    Wire.read();                                      //   length received msb (always 0) ???
    Wire.read();                                      //   address from which we just read
    memset(buffer, 0, len);                           // clear the buffer for safe measure
    for (pos = 0; pos < cnt; pos++) 
    {
      if (pos < len) buffer[pos] = Wire.read();       // as long we have room in the buffer add to it
      else Wire.read();                               // otherwise just drop it
    }
    return cnt; 
  }
  else
  {                                                   // drop them all
    for(pos = 0; pos < cnt; cnt++)
      Wire.read();
  }

  return -cnt;                                        // includes the extra three bytes
}

Is this comma , at the end there a typo? Shouldn’t that be a semi-colon ;?

And for ease of coding you can modify the high level functions this way to keep the len parameter consistent with the datatype

int BQ20Z45::GaugingStatus(uint16_t* result)
{
  return readBytes(BQ20Z45_GaugingStatus, (void*)result, sizeof(*result));
  // for platforms with incompatible endianness you may want to 
  // add some code to correct the byte order
}

#113

Thanks for that :smiley: I added your modified code to the library and all is good except for one minor thing that I can’t seem to figure out yet and I kinda feel like an idiot because of it.

I’ve tried about 10 different combinations of different codes but none of them compile which leads me to believe that I must now understand what is going on although I feel I do.

I need to change the class variable here:

It’s this line that is causing the complie errow below:

I’ll spare you all the code that I tried that didn’t work but some guidance is appreciated as usual.

Thanks!


#114

Isn’t this the function signature int BQ20Z45::GaugingStatus(uint16_t* result) and hence the header should have int GaugingStatus(uint16_t* result);?

The error message is quite explicit.
It complains about a non-exixtent definition for that function that takes a uint16_t* and that’s true, since you only have one that takes a ***char****.


#115

I made the changes but still get compile errors.

In the Particle DEV I get compile errors but no details on the reason for the compile failure which you can see below:

So I tried to compile it using the Particle Build online using the same code and it does provide a reason for the errors which you can see below:

The exact code I used is below:

The .ino :

// This #include statement was automatically added by the Particle IDE.
#include "BQ20Z45.h"

#include "application.h"
#include "BQ20Z45.h"            //Include BQ78350 Header File

// Store an instance of the BQ20Z45 Sensor
BQ20Z45 bms;

//char strBuffer[33];    // This is a Buffer to store 32 bit data strings read from the BQ78350 Fuel Gauge. Not Sure if this is the best place for this code.

uint16_t GaugingStatus = 0;

void setup(void)
{
  // We start the serial library to output our messages.
  Serial.begin(115200);

  // Start i2c communication.
  Wire.begin();
}


void loop(void)
{

//Serial.print("Device Name: ");
bms.GaugingStatus(result);


delay(4000);

}

The .CPP :

#include "application.h"
#include "BQ20Z45.h"
//#include "Arduino.h"
#include "application.h"

/////////////////////////////////////////////////////////////////////////////
// Functions Below


// Fuel gague function to read Manufacturer Access Data registers when in sealed mode.
int BQ20Z45::readBytes(uint8_t address, void* buffer, size_t len, boolean exactLen)
{
  int pos;
  int cnt;                                            // to hold the actual length the address would hold

  // Read the length of the string
  Wire.beginTransmission(BQ20Z45_Address);            //Setup Write to Gauge
  Wire.write(0x44);                                   //Manufacturer Block Read
  Wire.write(0x02);                                   //How Many Bytes to expect Next
  Wire.write(address);                                //Manufacturer Access Command - Example 0x01 Device Type
  Wire.write(0x00);                                   //First Byte of Command
  Wire.endTransmission(true);                         //Repeated Start = True

  Wire.beginTransmission(BQ20Z45_Address);            //Setup Write to Gauge
  Wire.write(0x44);                                   //Manufacturer Block Read
  Wire.endTransmission(true);                         //Repeated Start

  Wire.requestFrom(BQ20Z45_Address, 1, true);         //Setup Reading of Returned Data
  cnt = Wire.read();                                  // length of the string thats about to return
  cnt++;                                              // plus one to allow for the length byte on the reread

  cnt = Wire.requestFrom(BQ20Z45_Address, cnt, true); // readRequest returns # bytes actually read

  if (cnt == len+3 || !exactLen && cnt > 3)
  {                                                   // we got the expected read count
    cnt -= 3;                                         // drop the first three bytes
    Wire.read();                                      //   length received lsb
    Wire.read();                                      //   length received msb (always 0) ???
    Wire.read();                                      //   address from which we just read
    memset(buffer, 0, len);                           // clear the buffer for safe measure
    for (pos = 0; pos < cnt; pos++)
    {
      if (pos < len) buffer[pos] = Wire.read();       // as long we have room in the buffer add to it
      else Wire.read();                               // otherwise just drop it
    }
    return cnt;
  }
  else
  {                                                   // drop them all
    for(pos = 0; pos < cnt; cnt++)
      Wire.read();
  }

  return -cnt;                                        // includes the extra three bytes
}


/////////////////////////////////////////////////////////////////////////////
// Class Methods Below



int BQ20Z45::GaugingStatus(uint16_t* result) //int BQ20Z45::GaugingStatus(char *result)
{

  return readBytes(BQ20Z45_GaugingStatus, (void*)result, sizeof(*result));
   // for platforms with incompatible endianness you may want to
   // add some code to correct the byte order
}

The .H :

#ifndef BQ20Z45_h
#define BQ20Z45_h


#define BQ20Z45_Address           0x0B
#define BQ20Z45_ManBlockAccess    0x44
#define BQ20Z45_ManAccess         0x00

#define BQ20Z45_GaugingStatus     0x56



class BQ20Z45
{
    public:

    int GaugingStatus(uint16_t *result);
    
    protected:

    private:

  int readBytes(uint8_t address, void* buffer, size_t len, boolean exactLen = true);
  
};
#endif

The mismatch errors I could probably figure out but the others are beyond my current understanding.

Sucks that Particle DEV does not provide the same detailed error list and that the online build does. Any idea on why that happens?


#116

That was my bad :blush:
This should have been

if (pos < len) ((uint8_t*)buffer)[pos] = Wire.read(); 

Because Dev (as it currently is) sucks all together. I rather use CLI.

To get rid of the warnings (which don’t harm) you could slightly alter some lines

  if ( (cnt == (int)(len+3)) || (!exactLen && cnt > 3) )
 ...
  if (pos < (int)len) ((uint8_t*)buffer)[pos] = Wire.read(); 

#117

You also need to change the above line in loop() to,

bms.GaugingStatus(&GaugingStatus);

To be more consistent with naming conventions though, I would name that variable gaugingStatus (with a lowercase g).


#118

@ScruffR Success!

That was a long road to travel, but it’s working now :smile: Feels good :wink:

THANK YOU SO MUCH FOR HELPING ME WITH THIS!

@Ric Thanks for that tip also! One less than I had to ask ScruffR for help with, which is nice :smile:

I would have figured out to add the GaugingStatus but adding the & sign in front of it is something I have not fully grasped yet, although @ScruffR did mention this to me in a post above:


#119

And the more important point IMHO is not the success but the insight you gained on the way :wink:
The success is one benefit, but the lessons learnt will benefit you over and over when you run into similar questions.


Give a starving man a fish or teach him how to fish.


#120

@ScruffR I agree 100% :smile:

I have come a long way over the past few years thanks to all the wonderful and helpful people on the forums and internet.

Are you located in Germany?


#121

Salzburg in Austria acutally, but just 5 minutes from the border to Germany.


#122

Sweet! I’ve never visited Germany but hope to in the future. Crazy how the internet can bring people together from all over the world.

I’m located in Indiana, USA.


#123

@ScruffR Another question :smiley:

I have one command that is returning 34 bytes of data and I’m saving that in a StrBuffer that is sized to 34 bytes in size so it can actually hold that much data. Even though I increased the buffer size I now see that the Wire.RequestFrom function is coded to max out to only return 32 bytes max.

I found this on the Arduino forums:


So it sounds like I have to manually change the count BUFFER_Legnth line in the Wire.H file?

Does this apply the same to the Particle product line?

Here is the 34 byes the chip is sending back to the PC evaluation software:

The Photon’s Wire.RequestFrom function is cutting off the data at 32 bytes which is causing me to miss the reading for 3 variables that are important:

What do you think?