Using I2C for SMBus device communications

The repeated start is required for all read operations with this device. First you write the register address/command to the device and then you read the data. A repeated start is generated by sending a new I2C start command (device address + R/W bit) again without sending an intervening stop condition.

Unfortunately the endTransmission methods you use do not contain the parameter to send/not send the stop condition. The spark I2C implementation defaults then to always send the stop. This will not produce the desired results and may explain why EC3-EC0 indicate an error.

After you perform the Wire.write(address); change the next line to Wire.endTransmission(false); - this will then not send the stop condition after the register address has been sent. The next Wire.endTransmission() just before the return statement will, as it has a void parameter and that is correct. This endTransmission sends a new start ()in this case now a restart as no stop was sent for the previous start), and sends a stop after the data has transferred. You might want to change this to Wire.endTransmission(true); for readability and clarity.

3 Likes

@pra, thanks for the clarification, but if Iā€™m not mistaken Wire.requestFrom() does implicitly send an I2C start command and does the reading into the buffer internally.
The Wire.read() function only retrieves the individual bytes from the already filled buffer.

yes but the device has no idea where to read from as the previous start that set the address was terminated via a stop condition. So the requestFrom is a new start not a repeated start.

Edit: removed uninformed contents :blush:

@pra is correct

Wire,write does not initiate an I2C transaction, just stores the data in the buffer. endTransmission is actually what transmits the buffer, which is commented out. So in your example, the address is stored but never sent.

@pra, @ScruffR this is directly from the master firmware branch in spark_wiring_i2c.cpp:

// Originally, 'endTransmission' was an f(void) function.
// It has been modified to take one parameter indicating
// whether or not a STOP should be performed on the bus.
// Calling endTransmission(false) allows a sketch to
// perform a repeated start.
//
// WARNING: Nothing in the library keeps track of whether
// the bus tenure has been properly ended with a STOP. It
// is very possible to leave the bus in a hung state if
// no call to endTransmission(true) is made. Some I2C
// devices will behave oddly if they do not see a STOP.

:smile:

@peekay123 - Thanks. That was actually the code element on github I looked at the figure out how Spark did an I2C restart.

Thanks @peekay123, and also @pra.
Now things are coming together.
For one thing I was looking at my local copy of the Core firmware - which is obviously outdated and I did only check the request/read side and not the write why I was missing the need to endTransmission

Once again, I repent in sackcloth and ashes :wink:

So @RWB, now may take part on your own thread again - sorry for leading it off track :blush:

@ScruffR
No problem :slight_smile: Continue your good work in helping @RWB getting it to work!

1 Like

@RWB, I have seen you withdrew your last posts, but have you tried this already?

Did it change anything?

@ScruffR @pra @peekay123

OK guys after going over @pra 's advice of adding the (false) to the end transmission code & @ScruffR adding the +1 to the byte read line the ā€œBattery Statusā€ register is returning the correct info so now all the bits are exactly the same as the PC software. :smiley: So we have made progress on this front!!!

Now lets take a look at what the difference is with the i2c communication between using the Repeated Start + adding the +1 to the Byte read total. Below is the new code format.

uint16_t BQ20Z45::read16u(uint8_t address)
{
	uint16_t registerValue;
      Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address,2+1,true);
	registerValue = Wire.read();
     registerValue |= (Wire.read()<<8);
	Wire.endTransmission(true);
        return registerValue;
} 

And here is what the communication looks like on the $100 Saleae Logic 4 Analyzer.

Now lets look at the code from before without the repeated start or adding the +1 to the read byte length.

uint16_t BQ20Z45::read16u(uint8_t address)
{
	uint16_t registerValue;
      Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission();
	Wire.requestFrom(BQ20Z45_Address,2,true);
	registerValue = Wire.read();
     registerValue |= (Wire.read()<<8);
	Wire.endTransmission();
        return registerValue;
}

Here is the actual communication that returns the data with incorrect data that shows the last 4 bits wrong.

Can you see the difference?

I need to test the 32bit read function now to see if thats working correctly also. Be right back with test.

Thanks guys!

1 Like

@pra @peekay123 @ScruffR

OK, I tried this modified code to try to pull the 4+1 Byte Hex Registers and its not working. Below is the modified function code:

uint32_t BQ20Z45::read32u(uint8_t address)
{
	uint32_t registerValue;
      Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address,4,true);
	registerValue = Wire.read();
        registerValue |= (Wire.read()<<8);
        registerValue |= (Wire.read() << 16);
        registerValue |= (Wire.read() << 24);
	Wire.endTransmission(true);
        return registerValue;
}

The returned data is not correct compared to the PC Software. See the communication log:

So next I logged the communication between the PC software and the fuel gauge while I manually did a block read from the PC software which does return the correct info so we know its request format is correct.

Here is what the PC software looks like:

Here is what the communication looks like. If you look at the bottom right you can see how it handles the read and write commands.

OK, so to recap here is the PC communication that returns the correct info

vs what the new code shown above that does not return the correct info:

Any ideas?

I have to go to bed now, stayed up till 6 AM working on this again :smile:

Thanks for helping me with this everybody.

@RWB, one idea would be to add the +1 here as well

    ...
    Wire.requestFrom(BQ20Z45_Address, 4+1, true);
    ....

and maybe you could also try dropping the last Wire.endTransmission(true); since you are already stopping the transmission, be passing true to Wire.requestFrom()

BTW: Why did you have a 2+1 for the 16bit read, while the datasheet only states 2?

@RWB
Looking at your LA graphics, my guess is that working on this to 6 AM may have been counter-productive. The Block Read is initiated OK, but terminated by the Spark sending a NAK after receiving the 3rd byte. Looking at the Spark code, this can only happen when the expected bytes to receive counter is about to decrement from 1 ton 0. Therefore, it was expecting to receive 3 bytes, not 4+1. I suspect you are still calling the read16u method with its 2+1 requestFrom, not your read32u method with a requesfFrom of 4 bytes (which should be 5 by they way).

As @ScruffR pointed out, the last Wire,endTransmission in these methods should be removed. It causes another Start to be sent to the device when there shouldnā€™t be. Also, the +1 is only required for the block reads to get the byte count and read16u is a read word method, so you should remove it here. In your read32u method, the first byte transferred is the byte count, so you need to throw that one away.
Wire,read();
registerValue = Wire,read();
registerValue |= (Wire.read() << 8);
etcā€¦

The PC is reading 6 bytes, the last being the PEC, which the TI program obviously knows how to handle. The spec says the host should NAK after the last data byte if it doesnā€™t handle the PEC, so 4+1 is good for you. The PC is also doing a lot of clock stretching between the last bit of a byte and the ACK/NAK clock which causes a different timing visual between the graphics.

BTW - Exactly which TI fuel gauge are you using. Its not the BQ20Z45 as its TRM says 0x54 is a word read not a block read.

Let us know what you find :smile:

1 Like

Hey Guys! @pra @ScruffR

I was trying all kinds of stuff last night so some of it may not make sense or look right. I was basically testing out what was returned when I used 2 vs 2+1.

It looks like your latest code changes might have solved the issues. Lets take a look.

Here is the 32 byte Block Read code with your latest suggested changes.

uint32_t BQ20Z45::read32u(uint8_t address)
{
	uint32_t registerValue;
      Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address,5+1,true);
	    Wire.read();
        registerValue = Wire.read();
        registerValue |= (Wire.read()<<8);
        registerValue |= (Wire.read() << 16);
        registerValue |= (Wire.read() << 24);
		
        return registerValue;
}

Here is the scan of the communication between the Micro Controller <> Fuel Gauge. Iā€™m reading Register 0x50 which returns 4+1 Bytes Hex.

Here is the Serial return:----

So all bits are set to 0-OFF. Lets see what the PC Software Says now.

:smiley: Yea!!! Its correct!!!

Lets read from the same register 0x50 via the PC Software now and compare the communication.

On the Left is the communication via the uController and on the right is the PC Software communication.

Bingo! Identical! :fireworks:

Lets try that again!

Iā€™m going to read register 0x51 SafetyStatus which returns 4+1 Bytes Hex just like the last read.

Here is the Serial Return:-----

Lets see if that lines up with the bit map in the PC software real quick:

Perfect :ok_hand:

The communication is exactly the same:

///////////////////////////////////////////////////////////////////////////////

Now I changed the function Code to read a register that returns 2 Bytes +1 instead of 4 Bytes +1. After a few unsuccessful code changes and looking at the data communication logs I got it right.

   uint16_t BQ20Z45::read16u2(uint8_t address)
{
	uint16_t registerValue;
      Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address,3+1,true);
	Wire.read();
        registerValue = Wire.read();
        registerValue |= (Wire.read()<<8);;
	
        return registerValue;
}

So now I can pull data from all the HEX registers also!!! :collision:

I couldnā€™t have done it without your help guys! Thank you very much.

I also now see how valuable the Saleae Logic 4 analyzer is when it comes to troubleshooting this type of communication. It really helps you understand whats going on in a way you can visualize.

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

Now the next type of register type I need to read returns a string anywhere from 10 to 32 byes in size +1.

@pra Can you look over the code below that is a stripped down version of what @ScruffR threw together to try to help out awhile back for reading from the registers that return strings.

Most of it makes sense to me. Iā€™m just having trouble understanding how to tie this function together with with the code that calls this function in the .ino & .h files.

Here is the code Iā€™m working with to try to return string data from registers that return anywhere from 4+1 up to 32+1.

 // pass a pointer to a char[] that can take up to 33 chars
// will return the length of the string received
int readString(uint8_t address, char* result)
{
	int pos = 0;
	int len;
    
	Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address, 1, true);
	
	len = Wire.read();
	if (len > 0)
	{
		// then request the rest
		Wire.requestFrom(BQ20Z45_Address, len, true);
		
		for (pos = 0; pos < len; pos++)
			result[pos] = Wire.read();
	}
	result[pos] = '\0';  // append the zero terminator

Here is the communication recording between the PC Software <> Fuel Gauge when reading register 0x20 ManufacturerName which returns 16+1 Bytes of ASCII data because the data in that register is: Texas Instruments . I guess this register could be holding up to 32 bytes + 1 which is why @ScruffR is telling me to "pass a pointer to a Char that can hold up to 33 bytes.

I need help understanding how to pass a pointer. I think I know what you want me to do but Iā€™m not sure so Iā€™ll ask those that know just to be sure.

Here is the string read

So I can see how the wire reads the bytes one by one unit it has read the amount of bytes the slave has told us it was holding and then it sends a NAK to end it all.

The code is really close to the other uint16_t , uint32_t block read functions except its getting the ā€œlenā€ aka byte length variable filled by the data returned when the 1st byte is read. That all makes sense.

Iā€™m not 100% sure how I should write the rest of the code that calls this function into action from the .ino & .h files. Iā€™ll post code for a block read function to show you what that looks like which hopefully will provide the info you need to help me out. Iā€™m sure itā€™s really simple but over my head at the moment.

Here is how the library code is written now for the 4+1 Byte Hex read of the SafetyAlert Register 0x50:

.ino file code

// Include the Wire library for I2C access.
#include <Wire.h>

// BQ20Z45 library.
#include <BQ20Z45.h>

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

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

void loop()
{
    
    Serial.print("Safety Alerts: ");
    Serial.println(bms.SafetyAlert(),HEX);

    Serial.println();
    delay(3000); // Show new results every second.
  
}

.h file code for this function

#ifndef BQ20Z45_h
#define BQ20Z45_h

#include <Wire.h>

#define BQ20Z45_Address           0x0B

#define BQ20Z45_SafetyAlert       0x50

class BQ20Z45
{
	public:

  uint32_t SafetyAlert(void);

protected:
	  
	private:
	  
	void write(uint8_t,uint8_t);
	uint8_t read(uint8_t);	
    uint16_t read16u(uint8_t);
    int16_t read16(uint8_t);
	uint32_t read32u(uint8_t);
	uint16_t read16u2(uint8_t);
	
};
#endif

Here is the .cpp file code for this function:

#include "BQ20Z45.h"
#include "Arduino.h"

uint32_t BQ20Z45::read32u(uint8_t address)
{
	uint32_t registerValue;
      Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address,5+1,true);
	    Wire.read();
        registerValue = Wire.read();
        registerValue |= (Wire.read()<<8);
        registerValue |= (Wire.read() << 16);
        registerValue |= (Wire.read() << 24);
		
        return registerValue;
}



uint32_t BQ20Z45::SafetyAlert()
{
 
	return read32u(BQ20Z45_SafetyAlert);
}

Iā€™m not sure what or how to properly change the variable to Char so the string function properly returns all the received data and no junk data.

Its 5:21 AM now, gotta get some sleep :sleeping:

Happy New Year Everybody!! :fireworks:

Glad to hear, you got some improvement :+1:

Have you tried 4+1 instead of 5+1 as well? Since this is what the datasheet says you might try to stick to it - otherwise you should comment the reason for this 5 while datasheet states 4 really well.

Furthermore, in my code I suggested this

	...
	Wire.requestFrom(BQ20Z45_Address, 1, false);
	...
	len = Wire.read();
	if (len > 0)
	{
		Wire.requestFrom(BQ20Z45_Address, len, true);
		...
	}
	else
		Wire.endTransmission();

to keep the line open, rather than giving it free after one byte by passing true for the stop parameter.

To call the function, youā€™d do something like this

char strBuffer[33];  // must be long enough to take the longest possible string
  ...
  bms.readString(BQ20Z45_Whatever, strBuffer);
  Serial.println(strBuffer);
  ...

@RWB,
Congratulations, Ryan, looks like you almost have this beast under control :smile:

As @ScruffR pointed out, you should use +1 only for Block Read functions to handle the length byte that is returned as the first byte. Using +1 is for receiving the PEC (8 bit CRC) byte that most SMBus devices support. This is totally optional, and of little value, and I haven;t seen you attempt to use the value anywhere. So, why request it? Use 2 for U16bit reads, and 5 (4 + length byte) for U32bit reads. The way it works is the Spark I2C driver will NAK the last expected byte, which tells the device to stop sending data. Otherwise if the device gets an ACK it sends the PEC. This is how the PEC is made optional. SO 2 + 1, 5 + 1, you are requesting the PEC; 2, 5, no PEC (what you want).

String reads - I think if you hook your logic analyzer up and run the code you have written for this function:

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

	Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address, 1, true);

	len = Wire.read();
	if (len > 0)
	{
		// then request the rest
		Wire.requestFrom(BQ20Z45_Address, len, true);

		for (pos = 0; pos < len; pos++)
			result[pos] = Wire.read();
	}
	result[pos] = '\0';  // append the zero terminator

You would see a Start Write, One Data byte, ReStart Read, One Data byte, NAK, Stop, Start Readā€¦

This isnā€™t correct and not what you see from the PC when it does it. @ScruffR alternative, although logical for most I2C devices wonā€™t work either, as it will only remove the Stop and change the next Start into a Restart.

The sequence you want is Start Write, One Data Byte, Restart Read, data byte, ACK, data byte, ACK, ā€¦, data byte, NAK, Stop.

I suggest the following:

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

        // Read the length of the string
	Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address, 1, true);
	len = Wire.read();                                // length of the string
        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

        // Now that we know the length, repeat the read to get all the string data. 
        // we need to write the address again and do a restart so its a valid SMBus transaction
	Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	len = Wire.requestFrom(BQ20Z45_Address, len, true);            // readRequest returns # bytes actually read

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

See how that works, and let us know the result

Peter

1 Like

:boom: Happy New Year!! :boom: guys!

@Pra @ScruffR

Yea after reading @Pra 's latest advice I can now see that adding the +1 just includes the PEC (Packet Error Checking) byte which I am not using due to the simple fact that I'm trying to keep things as simple as possible.

The data recording of the 5+1 read & the 5 read.

You can see the +1 Returns the PEC byte just as Peter has pointed out.

So while the data sheet says this Register returns 4+1 Bytes, we just need to ignore adding the last byte and remember to add 1 extra read byte to account for the first byte returned which contains the byte read quantity.

If I want to include the PEC byte I have to add a +1 to the five as shown above. Peter says it even more efficiently:

I can see and understand whats going here like its written in plan English vs Chinese! I must have learnt a few things along the way here :smile: Actually I have learnt a ton of new stuff while trying to get this working :wink:

But now I'm stuck again, See below.

////////////////////////////////////////////////////////////////////////////////////////

I'm stuck with how to properly setting up the rest of the code that triggers the readArray function that is stored in the .cpp file.

Guys I spent the last 3 hours watching videos, searching forums, and looking over the Adafruit GPS library cause I knew it's using Char return functions but I can' t figure out how to properly tie into the function code that @pra provided here:

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

        // Read the length of the string
	Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	Wire.requestFrom(BQ20Z45_Address, 1, true);
	len = Wire.read();                                // length of the string
        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

        // Now that we know the length, repeat the read to get all the string data. 
        // we need to write the address again and do a restart so its a valid SMBus transaction
	Wire.beginTransmission(BQ20Z45_Address);
	Wire.write(address);
	Wire.endTransmission(false);
	len = Wire.requestFrom(BQ20Z45_Address, len, true);            // readRequest returns # bytes actually read

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

Now I have no idea what it means to:

// pass a pointer to a char[] that can take up to 33 chars
// will return the length of the string received

I did look over people trying to explain this in C++ forums but it didn't really help me out any.

I'm gonna need help here also it looks like. :blush:

I tried @ScruffR 's recommendations but they were not clear enough or I simply didn't understand them enough to get the code to compile no matter what I tried.

Here is @ScruffR 's recommended example code.:

char strBuffer[33];  // must be long enough to take the longest possible string
  ...
  bms.readString(BQ20Z45_Whatever, strBuffer);
  Serial.println(strBuffer);
  ...

I couldn't figure out how to integrate this to the current code to get it to work right.

So I'm going to provide the stripped down versions of the .ino &.h & .cpp files for my sketch to keep it simple so you can take a peek and see what I need to change to get this working.

First I'll start out with the .cpp file which is where the Wire read functions are located and where the new readString function has been placed and this should be fine as is.

I need help with the constructor down at the bottom that I do not think is correct.

char BQ20Z45::ManufactureName()
{
 
	return readString(BQ20Z45_ManufactureName);
}

Here is the whole .cpp file, the constructor above this line of text is at the bottom of this code.

    #include "BQ20Z45.h"
    #include "Arduino.h"
    
    
    /////////////////////////////////////////////////////////////////////////////
    // Functions Below
    ////////////////////////////////////////////////////////////////////////////

    void BQ20Z45::write(uint8_t address, uint8_t data)
    {
      Wire.beginTransmission(BQ20Z45_Address);
      Wire.write(address);
      Wire.write(data);
      Wire.endTransmission();
    }
    
    uint8_t BQ20Z45::read(uint8_t address)
    {
    	uint8_t registerValue;
          Wire.beginTransmission(BQ20Z45_Address);
    	Wire.write(address);
    	Wire.endTransmission();
    	Wire.requestFrom(BQ20Z45_Address,1,true);
    	registerValue = Wire.read();
    	Wire.endTransmission();
            return registerValue;
    }
    uint16_t BQ20Z45::read16u(uint8_t address)
    {
    	uint16_t registerValue;
          Wire.beginTransmission(BQ20Z45_Address);
    	Wire.write(address);
    	Wire.endTransmission(false);
    	Wire.requestFrom(BQ20Z45_Address,2,true);
    	registerValue = Wire.read();
         registerValue |= (Wire.read()<<8);
    	
            return registerValue;
    }
    
    uint16_t BQ20Z45::read16u2(uint8_t address)
    {
    	uint16_t registerValue;
          Wire.beginTransmission(BQ20Z45_Address);
    	Wire.write(address);
    	Wire.endTransmission(false);
    	Wire.requestFrom(BQ20Z45_Address,4,true);
    	Wire.read();
            registerValue = Wire.read();
            registerValue |= (Wire.read()<<8);;
    	
            return registerValue;
    }
    
    int16_t BQ20Z45::read16(uint8_t address)
    {
    	int16_t registerValue;
            Wire.beginTransmission(BQ20Z45_Address);
    	Wire.write(address);
    	Wire.endTransmission(false);
    	Wire.requestFrom(BQ20Z45_Address,2,true);
    	registerValue = Wire.read();
            registerValue += (Wire.read()*256);
    
            return registerValue;
    }
    
    uint32_t BQ20Z45::read32u(uint8_t address)
    {
    	uint32_t registerValue;
          Wire.beginTransmission(BQ20Z45_Address);
    	Wire.write(address);
    	Wire.endTransmission(false);
    	Wire.requestFrom(BQ20Z45_Address,5,true);
    	    Wire.read();
            registerValue = Wire.read();
            registerValue |= (Wire.read()<<8);
            registerValue |= (Wire.read() << 16);
            registerValue |= (Wire.read() << 24);
    		
            return registerValue;
    }
    
    
    // pass a pointer to a char[] that can take up to 33 chars
    // will return the length of the string received
    int readString(uint8_t address, char* result)
    {
    	int pos = 0;
    	int len;
    
            // Read the length of the string
    	Wire.beginTransmission(BQ20Z45_Address);
    	Wire.write(address);
    	Wire.endTransmission(false);
    	Wire.requestFrom(BQ20Z45_Address, 1, true);
    	len = Wire.read();    // length of the string
            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
    
            // Now that we know the length, repeat the read to get all the string data. 
            // we need to write the address again and do a restart so its a valid SMBus transaction
    	Wire.beginTransmission(BQ20Z45_Address);
    	Wire.write(address);
    	Wire.endTransmission(false);
    	len = Wire.requestFrom(BQ20Z45_Address, len, true);    // readRequest returns # bytes actually read
    
            len--;                                             // we won't move the first byte as its not part of the string
    	if (len > 0)
    	{
                    Wire.read();
    		for (pos = 0; pos < len; pos++)
    			result[pos] = Wire.read();
    	}
    	result[pos] = '\0';  // append the zero terminator
    }
    



    /////////////////////////////////////////////////////////////////////////////
    // Constructors Below
    ////////////////////////////////////////////////////////////////////////////
   

 
    float BQ20Z45::GetTemp()
    {
     
    	return (float)read16u(BQ20Z45_Temp)/10;
    }
    
    float BQ20Z45::GetVoltage()
    {
     
    	return (float)read16u(BQ20Z45_Volt)/1000;
    }
    
    float BQ20Z45::GetCurrent()
    {
     
    	return (float)read16(BQ20Z45_Current)/1000;
    }
    float BQ20Z45::AverageCurrent()
    {
     
    	return (float)read16(BQ20Z45_AveCurrent)/1000;
    }
    uint8_t BQ20Z45::RelativeSOC()
    {
     
    	return (float)read(BQ20Z45_RelativeSOC);
    
    }
    uint8_t BQ20Z45::AbsoluteSOC()
    {
     
    	return (float)read(BQ20Z45_AbsoluteSOC);
    
    }
    float BQ20Z45::RemainingCapAlarm()
    {
     
    	return (float)read16u(BQ20Z45_RemainCapAlarm);
    }
    float BQ20Z45::RemainingTimeAlarm()
    {
     
    	return (float)read16u(BQ20Z45_RemainTimeAlarm);
    }
    float BQ20Z45::AtRate()
    {
    
    	return (float)read16(BQ20Z45_AtRate);
    }
    uint8_t BQ20Z45::MaxError()
    {
     
    	return (float)read(BQ20Z45_MaxError);
    }
    float BQ20Z45::AtRateTimeToFull()
    {
     
    	return (float)read16u(BQ20Z45_AtRateTimeToFull);
    }
    float BQ20Z45::AtRateTimeToEmpty()
    {
     
    	return (float)read16u(BQ20Z45_AtRateTimeToEmpty);
    }
    float BQ20Z45::AtRateOK()
    {
     
    	return (float)read16u(BQ20Z45_AtRateOK);
    }
    float BQ20Z45::RemainingBatteryCapacity()
    {
     
    	return (float)read16u(BQ20Z45_RemCap );
    }
    float BQ20Z45::FullBatteryCapacity()
    {
     
    	return (float)read16u(BQ20Z45_FullChargCap );
    }
    float BQ20Z45::RunTimeTillEmpty()
    {
     
    	return (float)read16u(BQ20Z45_RunTime2Empty );
    }
    float BQ20Z45::AverageTimeTillEmpty()
    {
     
    	return (float)read16u(BQ20Z45_AveTime2Empty );	
    }
    float BQ20Z45::AverageTimeTillFull()
    {
     
    	return (float)read16u(BQ20Z45_AveTime2Full );	
    }
    float BQ20Z45::ChargingCurrent()
    {
     
    	return (float)read16u(BQ20Z45_ChargCurrent );	
    }
    float BQ20Z45::ChargingVoltage()
    {
     
    	return (float)read16u(BQ20Z45_ChargVolt );	
    }
    float BQ20Z45::CycleCount()
    {
     
    	return (float)read16u(BQ20Z45_CycleCount );	
    }
    float BQ20Z45::DesignCapacity()
    {
     
    	return (float)read16u(BQ20Z45_DesignCapacity );	
    }
    float BQ20Z45::DesignVoltage()
    {
     
    	return (float)read16u(BQ20Z45_DesignVoltage );	
    }
    float BQ20Z45::CellVoltage1()
    {
     
    	return (float)read16u(BQ20Z45_CellVolt1 );	
    }
    float BQ20Z45::CellVoltage2()
    {
     
    	return (float)read16u(BQ20Z45_CellVolt2 );	
    }
    float BQ20Z45::CellVoltage3()
    {
     
    	return (float)read16u(BQ20Z45_CellVolt3 );	
    }
    float BQ20Z45::CellVoltage4()
    {
     
    	return (float)read16u(BQ20Z45_CellVolt4 );	
    }
    float BQ20Z45::PendingEVD()
    {
     
    	return (float)read16u(BQ20Z45_PendingEDV );	
    }
    uint8_t BQ20Z45::StateOfHealth()
    {
     
    	return (float)read(BQ20Z45_StateOfHealth);
    }
    
    uint16_t BQ20Z45::BatteryStatus()
    {
     
    	return read16u(BQ20Z45_BatteryStatus);
    }
    
    uint32_t BQ20Z45::SafetyAlert()
    {
     
    	return read32u(BQ20Z45_SafetyAlert);
    }
    
    uint32_t BQ20Z45::SafetyStatus()
    {
     
    	return read32u(BQ20Z45_SafetyStatus);
    }
    
    uint16_t BQ20Z45::PFAlert()
    {
     
    	return read16u2(BQ20Z45_PFAlert);
    }
    
    uint16_t BQ20Z45::PFStatus()
    {
     
    	return read16u2(BQ20Z45_PFStatus);
    }
    
    uint32_t BQ20Z45::OperationStatus()
    {
     
    	return read32u(BQ20Z45_OperationStatus);
    }
    
    uint16_t BQ20Z45::ChargingStatus()
    {
     
    	return read16u(BQ20Z45_ChargingStatus);
    }
    
   


   


 char BQ20Z45::ManufactureName()
    {
     
    	return readString(BQ20Z45_ManufactureName);
    }

OK, so now here is the .H file with 2 lines at the bottom where I commented out the 2 lines that interact with the readString function. Look at the bottom of the code in the class Public & Private sections.

#ifndef BQ20Z45_h
#define BQ20Z45_h

#include <Wire.h>

#define BQ20Z45_Address           0x0B

#define BQ20Z45_ManAccess         0x00
#define BQ20Z45_RemainCapAlarm    0x01
#define BQ20Z45_RemainTimeAlarm   0x02
#define BQ20Z45_BattMode          0x03
#define BQ20Z45_AtRate            0x04
#define BQ20Z45_AtRateTimeToFull  0x05
#define BQ20Z45_AtRateTimeToEmpty 0x06
#define BQ20Z45_AtRateOK          0x07
#define BQ20Z45_Temp              0x08
#define BQ20Z45_Volt              0x09
#define BQ20Z45_Current           0x0A
#define BQ20Z45_AveCurrent        0x0B
#define BQ20Z45_MaxError          0x0C
#define BQ20Z45_RelativeSOC       0x0D
#define BQ20Z45_AbsoluteSOC       0x0E
#define BQ20Z45_RemCap            0x0F
#define BQ20Z45_FullChargCap      0x10
#define BQ20Z45_RunTime2Empty     0x11
#define BQ20Z45_AveTime2Empty     0x12
#define BQ20Z45_AveTime2Full      0x13
#define BQ20Z45_ChargCurrent      0x14
#define BQ20Z45_ChargVolt         0x15
#define BQ20Z45_BatteryStatus     0x16
#define BQ20Z45_CycleCount        0x17
#define BQ20Z45_DesignCapacity    0x18
#define BQ20Z45_DesignVoltage     0x19
#define BQ20Z45_SpecificationInfo 0x1A
#define BQ20Z45_ManufactureDate   0x1B
#define BQ20Z45_SerialNumber      0x1C
#define BQ20Z45_ManufactureName   0x20
#define BQ20Z45_DeviceName        0x21
#define BQ20Z45_DeviceChemistry   0x22
#define BQ20Z45_ManufactureData   0x23
#define BQ20Z45_HostFETControl    0x2B
#define BQ20Z45_GPIOStauts        0x2C
#define BQ20Z45_GPIOControl       0x2D
#define BQ20Z45_VAUXVoltage       0x2E
#define BQ20Z45_SerialNumber      0x1C

#define BQ20Z45_CellVolt1         0x3F
#define BQ20Z45_CellVolt2         0x3E
#define BQ20Z45_CellVolt3         0x3D
#define BQ20Z45_CellVolt4         0x3C
#define BQ20Z45_CellVolt5         0x3B
#define BQ20Z45_CellVolt6         0x3A
#define BQ20Z45_CellVolt7         0x39
#define BQ20Z45_CellVolt8         0x38

#define BQ20Z45_ExtAveCellVoltage 0x4D
#define BQ20Z45_PendingEDV        0x4E
#define BQ20Z45_StateOfHealth     0x4F
#define BQ20Z45_SafetyAlert       0x50
#define BQ20Z45_SafetyStatus      0x51
#define BQ20Z45_PFAlert           0x52
#define BQ20Z45_PFStatus          0x53
#define BQ20Z45_OperationStatus   0x54
#define BQ20Z45_ChargingStatus    0x55
#define BQ20Z45_GaugingStatus     0x56
#define BQ20Z45_AFEStatus         0x58
#define BQ20Z45_AFEConfig         0x59
#define BQ20Z45_AFEVCx            0x5A
#define BQ20Z45_AFEData           0x5B

#define BQ20Z45_LifetimeDataBlock1 0x60
#define BQ20Z45_LifetimeDataBlock2 0x61
#define BQ20Z45_LifetimeDataBlock3 0x62
#define BQ20Z45_LifetimeDataBlock4 0x63
#define BQ20Z45_LifetimeDataBlock5 0x64
#define BQ20Z45_LifetimeDataBlock6 0x65

#define BQ20Z45_ManufacturerInfo   0x70
#define BQ20Z45_DAStatus           0x71	
#define BQ20Z45_DAStatus           0x72
#define BQ20Z45_CUVSnapshot        0x80
#define BQ20Z45_COVSnapshot        0x81

class BQ20Z45
{
	public:

	
	float GetTemp(void);
    float GetVoltage(void);
    float GetCurrent(void);
	float AverageCurrent(void);
	uint8_t RelativeSOC(void);	
	uint8_t AbsoluteSOC(void);
	float RemainingCapAlarm(void); 
	float RemainingTimeAlarm(void);
	float AtRate(void);
	float AtRateTimeToFull(void);
	float AtRateTimeToEmpty(void);
	float AtRateOK(void);
	uint8_t MaxError(void);
	float RemainingBatteryCapacity(void);
	float FullBatteryCapacity(void);
	float RunTimeTillEmpty(void);
	float AverageTimeTillEmpty(void);
	float AverageTimeTillFull(void);
	float ChargingCurrent(void);
	float ChargingVoltage(void);
	float CycleCount(void);
	float DesignCapacity(void);
    float DesignVoltage(void);
	float CellVoltage1(void);
	float CellVoltage2(void);
    float CellVoltage3(void);
    float CellVoltage4(void);
    float PendingEVD(void);
	uint8_t StateOfHealth(void);
	uint16_t BatteryStatus(void);
    uint32_t SafetyAlert(void);
    uint32_t SafetyStatus(void);
	uint16_t PFAlert(void); 
    uint16_t PFStatus(void);
	uint32_t OperationStatus(void);
	uint16_t ChargingStatus(void);
	char ManufactureName(void);  // Is it correct to define this as a char here?
	
	
	
	protected:
	  
	private:
	  
	void write(uint8_t,uint8_t);
	uint8_t read(uint8_t);	
    uint16_t read16u(uint8_t);
    int16_t read16(uint8_t);
	uint32_t read32u(uint8_t);
	uint16_t read16u2(uint8_t);
    char readString(uint8_t);    // All the other functions from the cpp file were listed above in this private class location. I thought I needed to add the readString here also defined as a char. Not sure if thats right or not. 
};
#endif

Now here is the .ino code. I'm trying to just read and serial print the ManufactuerName register which returns up to 32 ASCII letters from the readString function. I omitted a lot of the serial print code in an attemp to keep it clean but I did leave a few to show how i'm calling the other functions.

// Include the Wire library for I2C access.
#include <Wire.h>

// BQ20Z45 library.
#include <BQ20Z45.h>

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

char strBuffer[33];    // Is this where this code should be place? Inside loop better? 

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

void loop()
{

    Serial.print("Temp: ");
    Serial.println(bms.GetTemp()-273);  
    
    Serial.print("Volt: ");
    Serial.println(bms.GetVoltage()); 
    
    Serial.print("Current: ");
    Serial.println(bms.GetCurrent());
    
    Serial.print("Battery Status Flags: ");
    Serial.println(bms.BatteryStatus(),HEX);
    
    Serial.print("Safety Alerts: ");
    Serial.println(bms.SafetyAlert(),HEX);

 
    
   Serial.print("Manufacturer Name: ");      
   Serial.println(bms.ManufactureName());  //Exactly how should this be setup to properly return the char string from the function? 


    Serial.println();
    delay(3000); // Show new results every second.
  
}

I owe both you guys dinner for all this help :smile: I'll mail you some gift cards to cover a nice meal :wink:

Welcome to 2015!!

Oh yea, @Pra how did you end up so good at the coding? Part of your job or is it just a hobby you know and understand well?

After having read only a part of your mail so far, I've already seen an error that might stop you from building without error.
It would also be helpful, if you could not only provide your code, but also the error messages when compiling. When you can't compile, these error lists are the first thing to look at and are most the time very clear, and should give you at least some clue about the cause of the problem.

In my original code I had it correclty stated and commented, but when you stripped it down to your own version, you also removed the necessary line return len; since the function is declared to return an int, as the introducing comment did state - so it has to return some int. Otherwise you'd have to declare the function void readString(..)

// pass a pointer to a char[] that can take up to 33 chars
// will return the length of the string received
int readString(uint8_t address, char* result)
{
  ...
  return len;
}

The next thing is, that you'd have to add this new function into the class definition in your .h file - I'd do it as public to start with.
And once you've added it to the header file, you'd have to put it into your .cpp file as int BQ20Z45::readString(uint8_t address, char* result).

Next would be the call from your own code, but I'm not sure what's not clear about this

char strBuffer[33];  // must be long enough to take the longest possible string
  ...
  bms.readString(BQ20Z45_Whatever, strBuffer);
  Serial.println(strBuffer);
  ...

Just declare strBuffer as global variable, to start with.
And then just put the other two lines somewhere in your code, just as they are - except from BQ20Z45_Whatever, this you'd have to replace by whatever info you want to retrieve (e.g. 0x50).

The way the string is returned to you is via a variable that you pass in as parameter (in this case a pointer to a memory location that can take up to 33 characters) and the function places its results into this memory location.
The return value you would get by doing myRetValue = readString(...); would be the number of characters your info should have.


Now some formal things

this is not a constructor but a class method (aka function) - a constructor would be BQ20Z45::BQ20Z45().
As stated above if you do return ... this way you would only get the length of the string, but you don't even call the method with the required result parameter - hence the comment // pass a pointer to a char[] that can take up to 33 chars.

If you've got this in you .cpp file

It shouldn't be a surprise that the compiler complains when .h file contains this

Since the headers should tell what the code file will actually implement, it has to be exactly the same function signature (same return type, same name and same parameter list, with the same data types).

I think this should be cleared up now

Call it as outlined above and it should work.

@ScruffR,
Good catch on the ā€œreturn lenā€. My fault, I simply copied, pasted and edited Ryanā€™s posted code in my suggestion. I did notice the closing brace was missing and added that, but never even looked at the return type/value.

@RWB
If you follow what ScruffR has told you here, you should be good to go. Just to add a bit of clarification to C arrays for you:

char     someString[32];          // declares an array of characters 32 characters long
        if (someString[5] == 'a')     // reference the 6th char in the array (starts with an index of 0
                                     // so the array is [0..31]
        somefunction(someString);   // providing the array name without any subscript is a pointer to the 1st char
        somefunction(&someString[0])  // this is the same,a pointer to the first char note the &

These rules apply to any c/c++ array regardless of type (char, int, long, object etc.)

1 Like