Wrappering the I2C functions produces undesirable effects (N

[Stuart]

Wrappering the I2C function produces undesirable effects.

//------------------------------------------------------------------------------
// Function : W_I2C_stop
// Description : wrappered I2C_stop function to prevent inline compilation
// Parameters : None
// Returns : None
//------------------------------------------------------------------------------
void W_I2C_stop(void)
{
i2c_stop();
}

******
This section of code does not work, the repeated start bit is not interpreted as a repeated start bit
by the compiler and is instead interpreted as a standard start bit.

...
W_I2C_start();
W_I2C_write(RTC_8583_I2C_WRITE_ADDRESS);
W_I2C_write(address);
W_I2C_start();
W_I2C_write(RTC_8583_I2C_READ_ADDRESS);
retval = W_I2C_read(1);
dummy = W_I2C_read(0);
W_I2C_stop();
...

The Reason? According to the manual:
"If another I2C_start() is called in the SAME FUNCTION before an I2C_stop() is called, then a special
restart condition is issued.

This does not lend itself very well to a C environment where abstraction and wrappered functions are used regularly. Instead it encourages users to use simple inline C which is a far cry from what is capable with a real C compiler.

*******

The following code snippet works after finding the "fault" with the i2c_start() function:
I2C_start();
I2C_write(RTC_8583_I2C_WRITE_ADDRESS);
I2C_write(address);
I2C_start(); /*>>>>>>>>Noddy inline code*/
I2C_write(RTC_8583_I2C_READ_ADDRESS);
retval = I2C_read(1);
I2C_stop();

As I have said, this encourages people to write Noddy code which is fine for home users but not very effective for professional development.

Stuart
___________________________
This message was ported from CCS's old forum
Original Post ID: 144516346

[Sherpa Doug]

:=This does not lend itself very well to a C environment where abstraction and wrappered functions are used regularly. Instead it encourages users to use simple inline C which is a far cry from what is capable with a real C compiler.
:=
:=Stuart

I started programming with the 16C5x PICs where the stack is only two levels deep. This means that main() can call one function, and that function can call one other, and that is it! If you add a wrapper layer you blow the stack and you can never get back to main() without a goto or a processor reset.

From my experience wrappering might be used in PC or mainframe C programming, but not in embedded code. Function calls cost machine cycles that we often can ill afford. If I can reduce program size by 2\% that means I can use a 2\% slower clock to get the job done, which saves battery size, cost weight, maybe heat. It all adds to market share! Maybe I am a bit anal... but that is why I am in this business.

___________________________
This message was ported from CCS's old forum
Original Post ID: 144516350

[Stuart]

I forgot to mention that I am using the PIC18LF242, which has 31 deep stack...plenty for this type of application.

Even though the compiler/PIC is limited does not mean that we have to write ugly code. I'll admit that a 2 deep stack is laughable and even 8 was bad enough, but now that Microchip has an almost real processor ('C' friendly) doesn't mean that we have to be forced into writing bad code.

Programming is not about getting something to work, it also involves writing maintainable/portable code. And wrappering is one of the most important features of doing this.

By the way, I also used 16C54's years ago but then again I used assembly for that...but I have progressed from there, so why haven't the compilers?

Stuart
___________________________
This message was ported from CCS's old forum
Original Post ID: 144516351

[R.J.Hamlett]

:=Wrappering the I2C function produces undesirable effects.
:=
:=//------------------------------------------------------------------------------
:=// Function : W_I2C_stop
:=// Description : wrappered I2C_stop function to prevent inline compilation
:=// Parameters : None
:=// Returns : None
:=//------------------------------------------------------------------------------
:=void W_I2C_stop(void)
:={
:= i2c_stop();
:=}
:=
:=******
:=This section of code does not work, the repeated start bit is not interpreted as a repeated start bit
:=by the compiler and is instead interpreted as a standard start bit.
:=
:=...
:=W_I2C_start();
:=W_I2C_write(RTC_8583_I2C_WRITE_ADDRESS);
:=W_I2C_write(address);
:=W_I2C_start();
:=W_I2C_write(RTC_8583_I2C_READ_ADDRESS);
:=retval = W_I2C_read(1);
:=dummy = W_I2C_read(0);
:=W_I2C_stop();
:=...
:=
:=The Reason? According to the manual:
:="If another I2C_start() is called in the SAME FUNCTION before an I2C_stop() is called, then a special
:=restart condition is issued.
:=
:=This does not lend itself very well to a C environment where abstraction and wrappered functions are used regularly. Instead it encourages users to use simple inline C which is a far cry from what is capable with a real C compiler.
:=
:=*******
:=
:=The following code snippet works after finding the "fault" with the i2c_start() function:
:=I2C_start();
:=I2C_write(RTC_8583_I2C_WRITE_ADDRESS);
:=I2C_write(address);
:=I2C_start(); /*>>>>>>>>Noddy inline code*/
:=I2C_write(RTC_8583_I2C_READ_ADDRESS);
:=retval = I2C_read(1);
:=I2C_stop();
:=
:=As I have said, this encourages people to write Noddy code which is fine for home users but not very effective for professional development.
:=
:=Stuart
I think it is perhaps worth reflecting, that the same use of 'wrappering', will cause problems when writing device drivers, in MS C, if you perform two 'timing critical' functions, and then use wrappers, that the compiler fails to optimise out.
At the end of the day, The I2C 'operators', are low level components which are designed to be used together as a 'set'. It is perfectly fair and reasonable to wrapper an entire function produced using these, but to expect wrappering the components to work, is asking too much.
There is nothing to stop you writing a 'wraperable' version of the code, but to do so, you have to remember that though the stack is reasonably deep on the latter chips, there is no equivalent data stack, for variables.
The use of wrappers, was originally developed, 'to make functions re-useable across projects'. Though with the bigger chips, the PIC is now possibly reaching the size where this applies, wrappering is _not_ good for code efficiency, and would not normally be advised for 'low level' operations such as hardware I/O. One of the prerequisites for 'reuse' of this sort, is that you know exactly what the function does, and that the function resolves to a well defined scope. This is definately not the case with a component function like the I2C handlers, and and such makes these automatically unsuitable for reuse in this way. However it could well be the case with a complete 'handler' written using these as a set.
The whole point of wrappering, is to provide a defined entry 'shell', to functions that may well be poorly structured, and to conceal this from the external application. You are not doing this at all, with the code as shown. You are also making the assumption, that wrappering like this will prevent the functions from being 'inline'. In fact you do not use the #seperate directive to prevent this, and the behaviour of your wrapper will differ for the functions that are actually #define expansions, from those that are really stand alone functions.
I would say that if you want to provide a wrapperable version of this code, you should write the routines yourself, and build your own software stack, to hold temporary variables (such as the flag for whether I2C start has allready occurred). However a far more reasonable approach, is to treat this type of function, as a driver component, that should only be used as a complete set, and then wrapper the entire transaction, which meets the normal resolvability requirments for wrappering.

Best Wishes
___________________________
This message was ported from CCS's old forum
Original Post ID: 144516354

[Sherpa Doug]

:=By the way, I also used 16C54's years ago but then again I used assembly for that...but I have progressed from there, so why haven't the compilers?
:=
:=Stuart

I still design new products with 16C54's and 16C58's for autonomous underwater robotic vehicles. They are still as good as they ever were!

___________________________
This message was ported from CCS's old forum
Original Post ID: 144516361

[Stuart]

I know about the #seperate pragma. But wrappering is used for a multitude of reasons, providing debug info, portabilty, ease of maintenance, readability etc. But in this day and age, portability counts a lot...especially for platform to platform migration.

I2C is static so there are no timing critical issues involved so it shouldn't have to be run as a "set".

If I were to provide an I2C module for resale as is here, it would include the "Flag" that you mention, actually a private module state variable, not to force people to have to use a special sequence.

Even an i2c_start(0), i2c_start(1) option would have been 100\% of a better option.

Best bet is to use the compiler for compiling which it seems to do pretty well and write your own interrupt driven drivers to override the provided (fairly flakey) ones.

Don't get me wrong, I am not flaming anyone here or starting a fight. It is just that this compiler has not progressed as far as it could have, especially with the PIC18 series...

Stuart
___________________________
This message was ported from CCS's old forum
Original Post ID: 144516377

[Neutone]

:=I know about the #seperate pragma. But wrappering is used for a multitude of reasons, providing debug info, portabilty, ease of maintenance, readability etc. But in this day and age, portability counts a lot...especially for platform to platform migration.
:=
:=I2C is static so there are no timing critical issues involved so it shouldn't have to be run as a "set".
:=
:=If I were to provide an I2C module for resale as is here, it would include the "Flag" that you mention, actually a private module state variable, not to force people to have to use a special sequence.
:=
:=Even an i2c_start(0), i2c_start(1) option would have been 100\% of a better option.
:=
:=Best bet is to use the compiler for compiling which it seems to do pretty well and write your own interrupt driven drivers to override the provided (fairly flakey) ones.
:=
:=Don't get me wrong, I am not flaming anyone here or starting a fight. It is just that this compiler has not progressed as far as it could have, especially with the PIC18 series...
:=
:=Stuart

I have on several occations asked for new features such as i2c_start(0), i2c_start(1). All the features I have requested refelcted a lack in fuctionality that could only be resolved in assembly. All the features I have requested have been implemented in some from. Discribe the problem to CCS and suggest the solution you would like. I believe CCS will work with you.
___________________________
This message was ported from CCS's old forum
Original Post ID: 144516378

[R.J.Hamlett]

:=:=I know about the #seperate pragma. But wrappering is used for a multitude of reasons, providing debug info, portabilty, ease of maintenance, readability etc. But in this day and age, portability counts a lot...especially for platform to platform migration.
:=:=
:=:=I2C is static so there are no timing critical issues involved so it shouldn't have to be run as a "set".
:=:=
:=:=If I were to provide an I2C module for resale as is here, it would include the "Flag" that you mention, actually a private module state variable, not to force people to have to use a special sequence.
:=:=
:=:=Even an i2c_start(0), i2c_start(1) option would have been 100\% of a better option.
:=:=
:=:=Best bet is to use the compiler for compiling which it seems to do pretty well and write your own interrupt driven drivers to override the provided (fairly flakey) ones.
:=:=
:=:=Don't get me wrong, I am not flaming anyone here or starting a fight. It is just that this compiler has not progressed as far as it could have, especially with the PIC18 series...
:=:=
:=:=Stuart
:=
:=I have on several occations asked for new features such as i2c_start(0), i2c_start(1). All the features I have requested refelcted a lack in fuctionality that could only be resolved in assembly. All the features I have requested have been implemented in some from. Discribe the problem to CCS and suggest the solution you would like. I believe CCS will work with you.

I disagree that the lacks can only be solved in assembler. The compiler has now reached the point where it is perfectly possible to write a full I2C routine in C, with the features you want.
In fact I have to say, that for SSP (which I tend to use rather more than I2C), I have done exactly this because of the 'illogic' of the standard functions.
What I was saying about the use of the functions as a 'set', was not that there was any hardware issues involved in splitting them, but that trying to wrapper the functions _missed_ one of the important parts of wrapper definitions, that the function should have a fully defined and understood scope. Given that the routines 'hide' the ability to have two modes dependant on whether the interface is allready live, implies that they have a hidden flag. Since you do not know the location, or the context (local or global), of this, it places a constraint on the use of the functions, except as a 'set'. It is also worth realising, that this is not a 'compiler' problem, but a fault with the functions as written (you could generate functions with similar problems, in just about any compiler). It unfortunately, reflects the fact that these routines appear to have basically been left 'as is', from the earlier 16F family, where the approach made a lot more sense.
There has been talk in the past on this group, that it would be worth the group as a whole starting to 'pool' some of the standard functions that we allready have (possibly re-writing slightly, to use standard register set definition names, to improve overall portability), which would then allow users to employ the 'core' compiler, without the libraries (which are the part that most commonly causes problems, but also the part that makes most users choose the compiler...).

Best Wishes
___________________________
This message was ported from CCS's old forum
Original Post ID: 144516384