# dsPIC33CH I2C code syntax problem (SOLVED)

#### GettinBetter

Joined Jun 22, 2018
42
Hi peeps,
I've just picked this up again after 6 months or so as I need to put a new PC together.

I've had it working a few times but not giving the expected logic output.
I've tried to comment it up somewhat to explain what I think it's doing or should be doing.

I'm attempting to write as a block of data two bytes, namely the (operation mode) register address & the register value.

If you guys could flame me with some blatently stupid errors I'd appreciate it.

main.c:
/**

@File Name
main.c

*/

/**
PROJECT NAME: X1632-8Mhz_BNO055_Comms3

Section: Included Files
*/
#include "xc.h"
#include "mcc_generated_files/system.h"
#include "mcc_generated_files/uart1.h"
#include "mcc_generated_files/i2c1.h"
#include <libpic30.h> //Defines __delay32();
#include <stdio.h> // Defines printf("");

int main(void)
{
// initialise the device
SYSTEM_Initialize();

/*    INITIALISES THE FOLLOWING
PIN_MANAGER_Initialize();
CLOCK_Initialize();
INTERRUPT_Initialize();
I2C1_Initialize();
UART1_Initialize();
INTERRUPT_GlobalEnable();
SYSTEM_CORCONModeOperatingSet(CORCON_MODE_PORVALUES);
*/

__delay32(5000);

//  SYSTEMS USED & PROJECT AIMS
printf("Explorer 16/32 v3 DM240001-3, ICD4, dsPIC33CH128MP508 PIM\n\r");
printf("MPLABX v5.4, MCC v4.0.2, XC16 v1.60\n\r");
printf("Project Name: X1632-8Mhz_BNO055_Comms3\n\r");
printf("Simple write to BNO055 Comms via I2C\n\n\r");
// This ^^ all works fine.
/*
* From                            To                  Switching time
* CONFIGMODE              Any operation mode              7ms
* Any operation mode          CONFIGMODE                  19ms
*/

#define CONFIGMODE          (00000000)        // default value = config mode
#define OPR_MODE            (00111101)        //Register 0x3D
#define ACCGYRO_CON         (00000101)        //Register Value 0x05
// In debug mode in MPLABX, when hovering the above shows as "Not recognised"

uint8_t counter;
uint8_t *pdata;     // block to be sent, in this case the address of the register to write to, and the value to write.
uint8_t length;     // number of elements in pdata.
I2C1_MESSAGE_STATUS *pstatus;   //I2C returned status message.

length = 2;        // number of bytes to transfer in the for loop.

while(pstatus != I2C1_MESSAGE_FAIL)     // After I2C1_MasterWrite is called this status is returned. The default is ENUMERATING  and so satisfys the test.
{
for (counter =0; counter < length ;counter++)   //counter to loop increment 'operation mode' Register & value index.
{
//   SET OP_MODE TO ACCGYRO
uint8_t data[2] = {OPR_MODE, ACCGYRO_CON};      //address of the OP_MODE register to write to, and the value to write.

pdata = &data[counter];     // transfer first byte of data.
__delay32(5000);

printf("I2C Message status = %08x \n\r",pdata);         //debug
__delay32(5000);
}
}
printf("I2C Message status = %c \n\r",pstatus);        //debug

}
/**
End of File
*/
Any help would be appreciated.

Regards
GB

#### AlbertHall

Joined Jun 4, 2014
12,190
Mistake 1: using MCC. I never got MCC I2C code to work properly. I wrote some bit bang instead.

Mistake 2:
Followed by:
When you shift 0x28 eight places to the right the answer you get is 0x00!

Then two possible mistakes because these may be OK depending what is in thos header files.
Possible Mistake 3: The value of 'address' is never used.
Possible Mistake 4: You check the value of 'pstatus' but I can't see any place where it gets a value assigned.

#### MrChips

Joined Oct 2, 2009
27,703
#define OPR_MODE (00111101) //Register 0x3D

#define is simply text substitution

#define OPR_MODE 0x3D

and in all such define statements.

#### GettinBetter

Joined Jun 22, 2018
42
Mistake 1: using MCC. I never got MCC I2C code to work properly. I wrote some bit bang instead.

Mistake 2:
Followed by:
When you shift 0x28 eight places to the right the answer you get is 0x00!

Then two possible mistakes because these may be OK depending what is in thos header files.
Possible Mistake 3: The value of 'address' is never used.
Possible Mistake 4: You check the value of 'pstatus' but I can't see any place where it gets a value assigned.

pstatus is returned when I2C1_MasterWrite is called.
I2C1.h Extract:
    [USER=120004]@Param[/USER]

[USER=120004]@Param[/USER]
length - The length of the data block to be sent

[USER=120004]@Param[/USER]
*pdata - A pointer to the memory location where received data will
be stored

[USER=120004]@Param[/USER]
*pstatus - A pointer to the status variable that the i2c driver
updates during the execution of the message.

@Returns
I2C1_MESSAGE_STATUS
'address' is defined as a 16 bit unsigned integer, and so I'm thinking that the device address of 0x28 needs shifting to the right ?
or does an 8 bit address look the same in a 16 bit register and the 8 right hand side bits (i.e the MSB) get ignored?

0000000000101000
00101000
and therefore not need shifting at all?

Maybe I should just try it, it won't take a second. But if somebody would be kind enough to explain, that would be nice.

Regards
GB

#### GettinBetter

Joined Jun 22, 2018
42
#define OPR_MODE (00111101) //Register 0x3D

#define is simply text substitution

#define OPR_MODE 0x3D

and in all such define statements.

Ok, thank you I've done this, I was thinking it had to be a binary literal.

#### AlbertHall

Joined Jun 4, 2014
12,190
But if somebody would be kind enough to explain, that would be nice.
The value 0x28 when stored in a 16 variable is 0x0028 and so doesn't need shifting.

#### MrChips

Joined Oct 2, 2009
27,703
Ok, thank you I've done this, I was thinking it had to be a binary literal.
Learn the difference between binary, decimal, and hexadecimal.
As far as the computer is concern it wants binary, always.
Different number representations such as binary, decimal, and hexadecimal are for confused humans.

#### AlbertHall

Joined Jun 4, 2014
12,190
For this compiler at least a binary literal is written as 0b00111101

#### GettinBetter

Joined Jun 22, 2018
42
@ MrChips
I know the difference between Binary, Decimal, & Hex, I'm just confused as to which one to use and when. In this case I used the binary version because the example did.

Trouble I've spotted now is that the Variables window & the intellisense debugging is showing the values in the variables when running in step-through, to be mixed up. The address is showing as the first block of data, and the length is showing the value of the address! Not sure what's going on there. Obviously needs some investigation.

@ Albert
" For this compiler at least a binary literal is written as 0b00111101 "

So that #define could be written as :
#define ACCGYRO_CON 0b00000101

Regards
GB

#### AlbertHall

Joined Jun 4, 2014
12,190
So that #define could be written as :
#define ACCGYRO_CON 0b00000101
Yep.

#### GettinBetter

Joined Jun 22, 2018
42
Almost Working Code:
  Section: Included Files
*/
#include "xc.h"
#include "mcc_generated_files/system.h"
#include "mcc_generated_files/uart1.h"
#include "mcc_generated_files/i2c1.h"
#include <libpic30.h> //Defines __delay32();
#include <stdio.h> // Defines printf("");

#define CONFIGMODE 0b00000000        // default value = config mode
#define OPR_MODE 0b00111101        //Register 0x3D
#define ACCGYRO_CON 0b00000101        //Register Value 0x05

#define DEVICE_TIMEOUT 50
#define RETRY_MAX 100

int main(void)
{
// initialise the device
SYSTEM_Initialize();

/*    INITIALISES THE FOLLOWING
PIN_MANAGER_Initialize();
CLOCK_Initialize();
INTERRUPT_Initialize();
I2C1_Initialize();
UART1_Initialize();
INTERRUPT_GlobalEnable();
SYSTEM_CORCONModeOperatingSet(CORCON_MODE_PORVALUES);
*/

__delay32(50000);

//  SYSTEMS USED & PROJECT AIMS
printf("\n\n\r Explorer 16/32 v3 DM240001-3, ICD4, dsPIC33CH128MP508 PIM\n\r");
__delay32(5000);
printf("MPLABX v5.4, MCC v4.0.2, XC16 v1.60\n\r");
__delay32(5000);
printf("Project Name: X1632-8Mhz_BNO055_Comms3\n\r");
__delay32(5000);
printf("Simple write to BNO055 Comms via I2C  \n\n\r");
__delay32(5000);
__delay32(5000);

/*
* From                            To                  Switching time
* CONFIGMODE              Any operation mode              7ms
* Any operation mode          CONFIGMODE                  19ms
*/

uint8_t counter;    // Loop counter
uint8_t *pdata;     // block to be sent, in this case it's the address of the register to write to, and the value to write.
uint8_t length;     // number of elements in pdata.
I2C1_MESSAGE_STATUS *pstatus;   //I2C returned status message.
uint16_t    retryTimeOut, slaveTimeOut;

length = 2;        // number of bytes to transfer in the for loop.
address = (BNO55_I2C_ADDR); // Shifted (in the I2C1 TRB function) to the LSB as only writing 7 bit address.
retryTimeOut = 0;
slaveTimeOut = 0;
// I2C1_MESSAGE_STATUS status = I2C1_MESSAGE_PENDING;

while(pstatus != I2C1_MESSAGE_FAIL)     // After I2C1_MasterWrite is called this status is returned. The default is ENUMERATING  and so satisfys the test.
{
//   SET OP_MODE(0x3D) TO ACCGYRO(0x05)
uint8_t data[2] = {OPR_MODE, ACCGYRO_CON};      //address of the OP_MODE register to write to, and the value to write.
pdata = data[0];     // Initialise the source of the data.

for (counter =0; counter < length ;counter++)   //counter to loop increment 'operation mode' Register & value index.
{
__delay32(5000);
// wait for the message to be sent or status has changed.
while(pstatus == I2C1_MESSAGE_PENDING)
{
__delay32(5000);

// timeout checking
// check for max retry and skip this byte
if (slaveTimeOut == DEVICE_TIMEOUT)
break;
else
slaveTimeOut++;
}

printf("\n\n\r I2C Message status = %X \n\n\r",pstatus);     //debug
__delay32(5000);
__delay32(5000);
printf("Data  = %08x \n\n\r",pdata);                    //debug
__delay32(5000);

pdata= data[1];

//    if (pstatus == I2C1_MESSAGE_COMPLETE)
//    break;
}
if (retryTimeOut == RETRY_MAX)
break;
else
retryTimeOut++;

}
while(1)
{

};
}

The above code gives me the (attached) output on the analyser, the device address is working fine. The register address and value are also sent and acknowledged, however the values are not correct, and although the vales are passed correctly the other parts i.e. transmission block building, and inserting into the transmission queue, seem to muddle up the correct values.

Anyone else come across this issue, and know what's going on?

#### AlbertHall

Joined Jun 4, 2014
12,190
for (counter =0; counter < length ;counter++) //counter to loop increment 'operation mode' Register & value index.
This code executes the 'for' loop 'length' times. Within the loop I2C1_MasterWrite has length as a parameter and so presumably sends 'length' bytes of data.
Seems wrong to me.

#### GettinBetter

Joined Jun 22, 2018
42
This code executes the 'for' loop 'length' times. Within the loop I2C1_MasterWrite has length as a parameter and so presumably sends 'length' bytes of data.
Seems wrong to me.
Yes That was my intention, and I'm assuming (and correct me if I'm wrong) that the 'register address' and 'register value' total two bytes, I'm also assuming that after the first byte is sent, a restart is created to send the second byte. I thought that was what the logic analyser output is telling me with the 'ACK's ?
Which is why I thought it's the values being sent are wrong, assuming 0x00 is an acceptable value, but not what I want.
Regards
GB

Last edited:

#### AlbertHall

Joined Jun 4, 2014
12,190
Your analyser print shows only one I2C transaction (from start to stop). Your code should generate two such transactions. Can you find and show both transactions?

#### GettinBetter

Joined Jun 22, 2018
42
Your analyser print shows only one I2C transaction (from start to stop). Your code should generate two such transactions. Can you find and show both transactions?
The only evidence I have of the output is via the printf statement, as all the logic waveforms look the same (with the zero -zero waveforms). The printf statement outputs on my terminal show 0x3D and 0x05 being recorded in the pdata variable.

Question: Isn't the first transmission in the waveform the address (0x50 i.e. 0x28 <1) which has an 'ACK' the second part should have been the register address (0x3D) which also has an 'ACK' with the third part being the register value (0x05), isn't that a complete 'Write' function? Why should that happen twice?

Regards
GB

Last edited:

#### AlbertHall

Joined Jun 4, 2014
12,190
Why should that happen twice?
Because that is what your program does with that for loop. The logic analyser should capture the whole transaction if you set the sampling time long enough, say 10 seconds.

#### GettinBetter

Joined Jun 22, 2018
42
Because that is what your program does with that for loop. The logic analyser should capture the whole transaction if you set the sampling time long enough, say 10 seconds.
It creates that waveform 100 times (i.e. MAX_RETRY_VALUE ) then stops but they are all the same, with the '0x50-0x00-0x00' waveform.

Isn't the I2C1_MasterWrite function creating a 'restart' for the loop, just before the +0.5ms (i.e. the start of the register address byte ) & the +0.7ms (the start of the register value byte), or am I reading this wrong?

#### AlbertHall

Joined Jun 4, 2014
12,190
This line is wrong:
pdata = data[0]; // Initialise the source of the data.

data[0] is the value stored in location 0, not a pointer.
Should be pdata = data;

#### GettinBetter

Joined Jun 22, 2018
42
This line is wrong:
pdata = data[0]; // Initialise the source of the data.

data[0] is the value stored in location 0, not a pointer.
Should be pdata = data;
Duly noted and corrected, thank you.

Updated 19-12-20:
/**
PROJECT NAME: X1632-8Mhz_BNO055_Comms3

Section: Included Files
*/
#include "xc.h"
#include "mcc_generated_files/system.h"
#include "mcc_generated_files/uart1.h"
#include "mcc_generated_files/i2c1.h"
#include <libpic30.h> //Defines __delay32();
#include <stdio.h> // Defines printf("");

#define CONFIGMODE 0b00000000        // default value = config mode
#define OPR_MODE 0b00111101        //Register 0x3D
#define ACCGYRO_CON 0b00000101        //Register Value 0x05

#define DEVICE_TIMEOUT 50
#define RETRY_MAX 100

int main(void)
{
// initialise the device
SYSTEM_Initialize();

/*    INITIALISES THE FOLLOWING
PIN_MANAGER_Initialize();
CLOCK_Initialize();
INTERRUPT_Initialize();
I2C1_Initialize();
UART1_Initialize();
INTERRUPT_GlobalEnable();
SYSTEM_CORCONModeOperatingSet(CORCON_MODE_PORVALUES);
*/

__delay32(50000);

//  SYSTEMS USED & PROJECT AIMS
printf("\n\n\r Explorer 16/32 v3 DM240001-3, ICD4, dsPIC33CH128MP508 PIM\n\r");
__delay32(5000);
printf("MPLABX v5.4, MCC v4.0.2, XC16 v1.60\n\r");
__delay32(5000);
printf("Project Name: X1632-8Mhz_BNO055_Comms3\n\r");
__delay32(5000);
printf("Simple write to BNO055 Comms via I2C  \n\n\r");
__delay32(5000);
__delay32(5000);

/*
* From                            To                  Switching time
* CONFIGMODE              Any operation mode              7ms
* Any operation mode          CONFIGMODE                  19ms
*/

uint8_t counter;    // Loop counter
uint8_t *pdata;     // block to be sent, in this case it's the address of the register to write to, and the value to write.
uint8_t length;     // number of elements in pdata.
I2C1_MESSAGE_STATUS *pstatus;   //I2C returned status message.
uint16_t    retryTimeOut, slaveTimeOut;

length = 2;        // number of bytes to transfer in the for loop.

address = (BNO55_I2C_ADDR); // Shifted (in the I2C1 TRB function) to the LSB as only writing 7 bit address.
retryTimeOut = 0;
slaveTimeOut = 0;

//   SET OP_MODE(0x3D) TO ACCGYRO(0x05)
uint8_t data[]= {OPR_MODE, ACCGYRO_CON};      //address of the OP_MODE register to write to, and the value to write.

while(pstatus != I2C1_MESSAGE_FAIL)     // After I2C1_MasterWrite is called this status is returned. The default is ENUMERATING  and so satisfys the test.
{

pdata = data;     // Initialise the source of the data.

for (counter = 0; counter < length ;counter++)   //counter to loop increment 'operation mode' Register & value index.
{
__delay32(5000);
// wait for the message to be sent or status has changed.
while(pstatus == I2C1_MESSAGE_PENDING)
{
__delay32(5000);

// timeout checking
// check for max retry and skip this byte
if (slaveTimeOut == DEVICE_TIMEOUT)
break;
else
slaveTimeOut++;
}

printf("\n\n\r I2C Message status = %X \n\n\r",pstatus);     //debug
__delay32(5000);
__delay32(5000);
printf("Data  = %08x \n\n\r",*pdata);                    //debug
__delay32(5000);

pdata++;

}
if (retryTimeOut == RETRY_MAX)
break;
else
retryTimeOut++;
if (pstatus == I2C1_MESSAGE_COMPLETE)
break;

}
while(1)
{

}
return 1;
}
/**
End of File
*
*/
So with a few other changes I get the following on terminal and logic analyser. As I mentioned before, the values at the variables are correct, and the address is correct. On terminal there are only two bytes of data (as expected) and the status shows 'message complete', so IMO the values aren't getting sent because they aren't going through the transmission block or message queue properly.... well maybe. It could be my crappy code.

I'll continue to attempt debug, but if anyone has any suggestions I'd be glad to hear them.

Regards
GB

Last edited:

#### AlbertHall

Joined Jun 4, 2014
12,190
Please post the code of the I2C1_MasterWrite function.