CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

Why the I2C communication fails after delete a printf? Bug?

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
cleberalbert



Joined: 25 Feb 2014
Posts: 34
Location: Brazil

View user's profile Send private message

Why the I2C communication fails after delete a printf? Bug?
PostPosted: Mon Jul 28, 2014 9:32 pm     Reply with quote

Hi all!

I'm dizzy about it! After I delete a "printf" my program stops working fine...May be a bug?

These are the 2 printfs. I can only delete 1 of them because if I delete both the I2C communication fails. Is not it very strange?
How could I fix up it?

(into the I2C.h file)
Code:

//printf("\n%ld\n",command);
printf("\n%d\n",measuring_unit);


Below is whole the code

Code:

#include <18f4520.h>

#fuses HS
#fuses PUT
#fuses NOWDT                    //No Watch Dog Timer
#fuses NOBROWNOUT               //No brownout reset
#fuses NOLVP                    //No low voltage prgming, B3(PIC16) or B5(PIC18) used for I/O
#fuses NOXINST                  //Extended set extension and Indexed Addressing mode disabled (Legacy mode)

#use delay(crystal=20MHz)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7) //+++ up baud rate?
#use i2c(master, sda=PIN_C4, scl=PIN_C3, FORCE_HW)

#include <I2C.h>

void main(){

int8 measuring_unit=1;
int16 command=18099;

mainI2C (measuring_unit, command);

}


I2C.h file:

Code:
/* Bit defines */
#define EEPROM_SCL PIN_C3
#define EEPROM_SDA PIN_C4
#define MU1_ADDRESS 0x20 // addr=2 reserved, note address masking. MU = Measuring Unit
#define MU2_ADDRESS 0x30 // addr=2 reserved, note address masking. MU = Measuring Unit
#define MU3_ADDRESS 0x40 // addr=2 reserved, note address masking. MU = Measuring Unit
#define I2C_DELAY_US           16 // worked 20 //Worked 32 //75 //100
#define I2C_INTERBYTE_DELAY_US 60 // worked 80

#byte PIC_SSPCON2  = 0xfc5

int8 i2c_nack;

void initI2C (void);
void writeI2C (int16 word, unsigned int slave_addr);
int16 readI2C (unsigned int slave_addr);
void mainI2C ();

void initI2C (void)
{
   output_float(EEPROM_SCL);
   output_float(EEPROM_SDA);
}

void writeI2C (int16 word, unsigned int slave_addr)
{
   i2c_nack=0;
   i2c_start();
   delay_us(I2C_DELAY_US);
   i2c_write(slave_addr);  /* Device Address */
   if (PIC_SSPCON2 & 0x40) i2c_nack++;
   delay_us(I2C_INTERBYTE_DELAY_US);
   i2c_write(word & 0x00ff);            //LSB first
   if (PIC_SSPCON2 & 0x40) i2c_nack++;
   delay_us(I2C_INTERBYTE_DELAY_US);
   i2c_write((word & 0xff00) >> 8);     //MSB second
   if (PIC_SSPCON2 & 0x40) i2c_nack++;
   delay_us(I2C_DELAY_US);
   i2c_stop();
}

int16 readI2C (unsigned int slave_addr)
{
   int8 b1=0, b2=0;
   i2c_nack=0;
   i2c_start();   // restart condition
   delay_us(I2C_DELAY_US);
   i2c_write(slave_addr + 1);
   if (PIC_SSPCON2 & 0x40) i2c_nack++;
   delay_us(I2C_INTERBYTE_DELAY_US);
   b1 = i2c_read(1);
   if (PIC_SSPCON2 & 0x40) i2c_nack++;
   delay_us(I2C_INTERBYTE_DELAY_US);
   b2 = i2c_read(0);
   if (PIC_SSPCON2 & 0x40) i2c_nack++;
   delay_us(I2C_DELAY_US);
   i2c_stop(); 
   return make16(b2, b1);
}

void mainI2C (int8 measuring_unit, int16 command)
{
  int16 answer;

initI2C();

//printf("\n%ld\n",command);
printf("\n%d\n",measuring_unit);

switch(measuring_unit){
case 1: measuring_unit=MU1_ADDRESS;
break;
case 2: measuring_unit=MU2_ADDRESS;
break;
case 3: measuring_unit=MU3_ADDRESS;
break;
default:
break;
}

  // Write
  writeI2C(command, measuring_unit);
  if (i2c_nack) {
    printf("\n%d nack(s) on write\r\n", i2c_nack);
  }
  else {
    printf("First command sent successfully from the Processing Unit to the Measuring Unit 0x%x: %ld\n",measuring_unit,command);
  }

  // Read
  answer = readI2C(measuring_unit);
    if (i2c_nack){
    printf("%d nack(s) on read\r\n", i2c_nack);
  }
  else{
  printf("First answer received successfully from the Measuring Unit 0x%x to the Processing Unit: %ld\n",measuring_unit, answer);
  printf("\n");
  }
}
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Mon Jul 28, 2014 9:51 pm     Reply with quote

Experiment. Put a delay in there that's equal to the time it takes the
printf to execute. Or just put in a 10ms delay and see what happens.

---------------
I don't want to look at your code very much because you're not using
CCS functions. You're writing your own hardware i2c routines:
http://www.ccsinfo.com/forum/viewtopic.php?t=52691
Ttelmah



Joined: 11 Mar 2010
Posts: 19221

View user's profile Send private message

PostPosted: Tue Jul 29, 2014 1:02 am     Reply with quote

He is also testing the wrongly for ACK.

Give the bits names, and it becomes obvious. 0x40, is ACKSTAT. Read the data sheet. Note where it says 'master transmit only' for this bit, yet it is being tested after a master receive transaction....
cleberalbert



Joined: 25 Feb 2014
Posts: 34
Location: Brazil

View user's profile Send private message

PostPosted: Tue Jul 29, 2014 12:07 pm     Reply with quote

PCM programmer, Thank you very much! I changed for a delay and now it's working fine.

Ttelmah, I saw. ACKSTAT is an acknowledge status bit and it's loaded with 6 bits. so I should change:

it:
Code:
if (PIC_SSPCON2 & 0x40) i2c_nack++;


for it:
Code:
if (PIC_SSPCON2 & 0b111111) i2c_ack++;


Right?
Ttelmah



Joined: 11 Mar 2010
Posts: 19221

View user's profile Send private message

PostPosted: Tue Jul 29, 2014 12:16 pm     Reply with quote

No. _Read the data sheet_. Look at what the individual bits do. The ACKSTAT bit, is just one bit reflecting the ACK, _when the master is transmitting_. Not on a receive.
As PCM programmer says use the CCS functions. _They_ return the status for each transaction, when one is available. The ACK on a read is controlled by you. This is what the '0', and '1' control in the I2C read function. Setting ACK/NACK. You are receiving the byte, so you acknowledge.
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Tue Jul 29, 2014 12:40 pm     Reply with quote

When I said to substitute a delay for the printf, I meant that as a
trouble-shooting method. Maybe to provide a hint of where the problem
might be. Or a way to think about it further. I didn't mean to happily
accept the delay as a way to make the code "work".

The fact that removing a line of code causes a problem, could mean,

1. The delay time is somehow critical. Removing the delay affects
some hardware process in a bad way.

2. The presence of the ASM code and the ROM space that it uses, causes
parts of the program to shift position in ROM. Maybe the compiler
repartitions the routines in ROM. There may be a bug in the compiler
that is revealed when a line of code is removed.

3. It could be, possibly, that you have a PIC with partially burnt-out
flash memory. Removing the line of code may make the compiler
use the defective block of flash. I admit, this idea is a long shot.

4. There may be a compiler bug with the re-use of RAM with user variables
or the compiler's own scratch variables.

There are ways to research all of this, but is it worth it ? As Ttelmah says
the first thing to do is to cleanup your code by using CCS functions.

5. Or the PIC could be acting erratically because you're missing some
essential hardware on your board, such as bypass capacitors on the Vdd
pins, or a regulated power supply.
Ttelmah



Joined: 11 Mar 2010
Posts: 19221

View user's profile Send private message

PostPosted: Tue Jul 29, 2014 12:44 pm     Reply with quote

As a comment though, it could just be a wake-up timing issue. We have no details of what is at the other end of the I2C bus, it might simply be a chip that takes a few mSec to wake. The data sheet for the chip is the place to start for this.
cleberalbert



Joined: 25 Feb 2014
Posts: 34
Location: Brazil

View user's profile Send private message

PostPosted: Tue Jul 29, 2014 10:21 pm     Reply with quote

PCM programmer and Ttelmah,

Thanks very much for the answers. They are valuable!
So I understood the best way to start is cleaning up my code by using CCS functions. I'll do it right now!
cleberalbert



Joined: 25 Feb 2014
Posts: 34
Location: Brazil

View user's profile Send private message

PostPosted: Thu Jul 31, 2014 1:16 am     Reply with quote

Hi Sirs, I hope you can help me again!

I started to make changes using CCS functions but now I've no idea why the slave code isnt working yet. The return of the i2c_isr_state function is always 0, but the master is sending datas every second.

I'm almost sure that the master code is working fine because it was working right with the ex slave program (ex slave code previously posted which wasnt using CCS functions), but however im also posting it besides the slave code.

May be something about registers?

Slave code:
Code:

#include <18f4520.h>

#fuses HS
#fuses PUT
#fuses NOWDT                    //No Watch Dog Timer
#fuses NOBROWNOUT               //No brownout reset
#fuses NOLVP                    //No low voltage prgming, B3(PIC16) or B5(PIC18) used for I/O
#fuses NOXINST                  //Extended set extension and Indexed Addressing mode disabled (Legacy mode)

#use delay(crystal=20MHz)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7) //+++ up baud rate?
#use i2c(Slave, Slow, sda=PIN_C4, scl=PIN_C3, force_hw, address=0x20)

/* PIC Registers */
#byte SSPBUF   = 0xfc9
#byte SSPADD   = 0xfc8
#byte SSPSTAT  = 0xfc7
#byte SSPCON1  = 0xfc6
#byte SSPCON2  = 0xfc5
#byte PIE1     = 0xf9d
#byte TRISC    = 0xf94

int16 data_to_send=0b1010101010101010;
int16 data_received;

#int_SSP
void i2c_interrupt() {
   //Get state
   int state;
   int receiving_msb;
   int receiving_lsb;
   int I2C_address;
   int writeBuffer[2];

state = i2c_isr_state();
printf("\nState: %d\n",state);

if(state==0) //Address match received with R/W bit clear, perform i2c_read( ) to read the I2C address.
{
I2C_address=i2c_read();
printf("Address match received with R/W bit clear. I2C address: 0x%x\n",I2C_address);   
}
   
if (state==0x80) //Address match received with R/W bit set; perform i2c_read( ) to read the I2C address, and use i2c_write( ) to pre-load the transmit buffer for the next transaction (next I2C read performed by master will read this byte).
{
I2C_address=i2c_read(2);
printf("Address match received with R/W bit set. I2C address: 0x%x\n",I2C_address);
}

if (state>=0x80) //Master is waiting for data
{
writeBuffer[0] = (data_to_send & 0xFF); //LSB first
writeBuffer[1] = ((data_to_send & 0xFF00)>>8);  //MSB secound

i2c_write(writeBuffer[state - 0x80]); //Write appropriate byte, based on how many have already been written
printf("Written to the Master. writeBuffer[state - 0x80]: %d\n",writeBuffer[state - 0x80]);
}

else if(state>0) //Master has sent data; read.
{
receiving_lsb=i2c_read(); //LSB first
printf("\ndata received: %d\n",receiving_lsb);     
}
}

void main() {
   enable_interrupts(INT_SSP); //Enable interrupts
   enable_interrupts(GLOBAL);
 
   i2c_slaveAddr(0x20);
   
   while (TRUE){delay_us(20);}
   
}


Master code (main code)
Code:

#include <18f4520.h>

#fuses HS
#fuses PUT
#fuses NOWDT                    //No Watch Dog Timer
#fuses NOBROWNOUT               //No brownout reset
#fuses NOLVP                    //No low voltage prgming, B3(PIC16) or B5(PIC18) used for I/O
#fuses NOXINST                  //Extended set extension and Indexed Addressing mode disabled (Legacy mode)

#use delay(crystal=20MHz)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7) //+++ up baud rate?
#use i2c(master, sda=PIN_C4, scl=PIN_C3, FORCE_HW)

#include <I2C.h>

void main(){

int8 measuring_unit=1;
int16 command=18099;
int16 received;

while(true){
received = mainI2C (measuring_unit, command);
  printf("First answer received successfully from the Measuring Unit %d to the Processing Unit: %ld\n",measuring_unit, received);
  printf("\n");
delay_ms(1000);
}
}



I2C.h
Code:

/* Bit defines */
#define EEPROM_SCL PIN_C3
#define EEPROM_SDA PIN_C4
#define MU1_ADDRESS 0x20 // addr=2 reserved, note address masking. MU = Measuring Unit
#define MU2_ADDRESS 0x30 // addr=2 reserved, note address masking. MU = Measuring Unit
#define MU3_ADDRESS 0x40 // addr=2 reserved, note address masking. MU = Measuring Unit
#define I2C_DELAY_US           16 // worked 20 //Worked 32 //75 //100
#define I2C_INTERBYTE_DELAY_US 60 // worked 80

#byte PIC_SSPCON2  = 0xfc5

void initI2C (void);
void writeI2C (int16 word, unsigned int slave_addr);
int16 readI2C (unsigned int slave_addr);
int16 mainI2C ();

void initI2C (void)
{
   output_float(EEPROM_SCL);
   output_float(EEPROM_SDA);
}

void writeI2C (int16 word, unsigned int slave_addr)
{
   i2c_start();
   delay_us(I2C_DELAY_US);
   i2c_write(slave_addr);  /* Device Address */
   delay_us(I2C_DELAY_US);
   i2c_write(word & 0x00ff);            //LSB first
   delay_us(I2C_DELAY_US);
   i2c_write((word & 0xff00) >> 8);     //MSB second
   delay_us(I2C_DELAY_US);
   i2c_stop();
   delay_us(I2C_DELAY_US);
}

int16 readI2C (unsigned int slave_addr)
{
   int8 b1=0, b2=0;
   i2c_start();   // restart condition
   delay_us(I2C_DELAY_US);
   i2c_write(slave_addr + 1);
   delay_us(I2C_INTERBYTE_DELAY_US);
   b1 = i2c_read(1);
   delay_us(I2C_INTERBYTE_DELAY_US);
   b2 = i2c_read(0);
   delay_us(I2C_DELAY_US);
   i2c_stop(); 
   return make16(b2, b1);
}

int16 mainI2C (int8 unit, int16 message){

int16 answer;

initI2C();
delay_ms(10);

switch(unit){ //
case 1: unit=MU1_ADDRESS;
break;
case 2: unit=MU2_ADDRESS;
break;
case 3: unit=MU3_ADDRESS;
break;
default:
break;
}
  // Write
  writeI2C(message, unit);
  printf("First message sent successfully from the Processing Unit to the Measuring Unit 0x%x: %ld\n",unit,message);

  // Read
  answer = readI2C(unit);
  return answer;
}
Ttelmah



Joined: 11 Mar 2010
Posts: 19221

View user's profile Send private message

PostPosted: Thu Jul 31, 2014 2:06 am     Reply with quote

There are several things wrong.

First the printf's. These are much too slow to include in a slave ISR. Output a bit pattern on a whole port, to say where you are, or clock a pulse for a few uSec maximum. Since these take far longer than the time between the bytes being sent in the master, the peripheral will become hung.

Then 'static'. Any variable used in a function, that you do not want cleared every time you enter the function needs to be declared as static. Things that need to hold bytes for successive calls, like the lsb, need to be static.

Then general comment, when using the hardware serial, you _must_ always either use the keyword 'ERRORS', _or_ your own code must handle RS232 error states. Otherwise the UART can become hung.

Then you never actually assemble 'data_received'.

Then don't waste time fiddling with moving bytes to the write buffer. Just use the make8 function, or a union. Difference between dozens of instructions, masking and rotating 16 bits, and a single byte fetch.

Learn how to indent code for clarity.

I didn't even start looking at the master code....
cleberalbert



Joined: 25 Feb 2014
Posts: 34
Location: Brazil

View user's profile Send private message

PostPosted: Thu Jul 31, 2014 2:56 pm     Reply with quote

Thanks very much! It works now...

To who needs, I posted in the Code Library:
http://www.ccsinfo.com/forum/viewtopic.php?p=189386#189386

This program is to provide 16 bits I2C communication between 2 PICs 18F. The code was done on CCS 5.018, it's commented and working. Tested with PIC18F4520
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group