I2C Driver Template Header Function Does Not Work

[Peter Trypsteen 0]

Bug description:
The function _I2C_TEMPL
unsigned char i2c_WRITE(unsigned char i2c_data)

Does not work.

Steps to reproduce:
The bug can be reproduced with getting information from a physical connection or a detailed simulator.

Including the i2c_driver.h

letting the use_i2c_SW undefined and then calling the function i2c_write with a valid argument triggers the bug.

It silently fails.

Expected behaviour:
Correctly outputs datapulses on the pins.

Is the problem 100% reproduceable:
Yes.

IDE version: 7.20
Compiler: BoostC
Compiler version: v7 C Compiler for PICmicro

Target device: PICXXXXX (Bug is not limited to a specific device.)
OS: Windows Vista 32 bit SP2

Comments:

Written an independent file with fixed version of the function inspired by the function in i2c_driver.h.

It is tested and works.

 

The bug is solved by in the file i2c_driver.h in the function i2c_WRITE to add an else clause to the if (T_MODE & i2c_HW).

At line 470 append the following at the end of the line:

"else

{"

 

and at line 524 append the following at the end of the line:

"

}"

 

(The empty lines between quotes is for adding a new line.)

Edited September 3, 2013 by Peter Trypsteen

[Pavel 0]

I looked at your fix and unless I missed something it should change the current program flow.

The current code structure looks like this:

...
if( something )
{
    do something   
    return
}
do something else
return

With your change the code will look like:

...
if( something )
{
    do something   
    return
}
else
{
    do something else
    return
}

I fail to see how your fix affect the program flow unless there is another problem somewhere else. Can you comment.

Regards,
Pavel

[Peter Trypsteen 0]

It does change the code execution quite a bit:

 

The current code structure always executes the do something else part, whether if(something) evaluates to true or not..

The code structure with my change only executes the do something else part.if the if(something) evaluates to false.

 

In my microcontroller project, things onlly worked after I made and used my proposed structure.

The other functions also follow the changed structure.

 

 

EDIT: did not take a good look, see my next reply for a better answer

Edited October 11, 2013 by Peter Trypsteen

[Pavel 0]

It does change the code execution quite a bit:

 

The current code structure always executes the do something else part, whether if(something) evaluates to true or not..

No it's not (or we are looking at different code). If the if(something) part evaluates to true it executes the return statement and never gets to the do something else part:

 

unsigned char i2c_WRITE(unsigned char i2c_data)
{
     ...
    if (T_MODE & i2c_HW)   //<<<<<<<< if this evaluates to true
    {
        ...
        return (0); // successful exit       //<<<<< this code exits the function and 
                                             //<<<<< code further down is not executed
    }
 
    // here for i2c software driver
    l_wcol = 0; // clear write collision flag 
    i2c_SSPBUF = i2c_data;
    ...
}

 

 

Regards,

Pavel

[Peter Trypsteen 0]

Didn't take a good look at your example code and you're indeed right about the control flow.

Was thinking about some other code.

 

Somehow it seems to make a difference in something I'm using it in.

[Peter Trypsteen 0]

Other functions in that file add the else too so maybe it's necessary or just good programming practice to do this.

Please implement this small improvement.