 |
 |
| View previous topic :: View next topic |
| Author |
Message |
Pret
Joined: 18 Jul 2006
Posts: 92
Location: Iasi, Romania
|
| yet another I2C slave issue |
 Posted: Mon May 06, 2013 9:51 am |
 |
|
I had the impression that I've understood how I2C slave works. After a couple of days trying to have a simple communication between two PICs, doubts comes in place.
For testing purposes, I only want to read some data from a slave PIC. Here is my sample code. Stripped it to the relevant code.
| Code: | #include <18F26K22.h>
#use delay(clock = 16MHz, internal)
#use i2c(I2C2, force_sw, master, stream = kbd)
#define KB_ADDRESS 0xA0
void Read()
{
i2c_start(kbd);
i2c_write(kbd, KB_ADDRESS | 1);
uint i;
for(i = 0; i < 5; i++)
{
uint8 b = i2c_read(kbd);
fprintf(dbg, "%2X ", b);
}
i2c_read(kbd, 0); // just for the NAK
i2c_stop(kbd);
return true;
} |
And the classical slave sample:
| Code: |
#include <16F1947.h>
#use delay(clock = 16MHz, internal)
#use i2c(I2C1, slave, address = 0xA0, stream = host)
uint8 index = 0;
#INT_SSP
void i2c_isr()
{
uint8 state = i2c_isr_state(host);
if((state == 0) || (state == 0x80))
{
i2c_read(host);
index = 0;
}
if(state >= 0x80)
{
i2c_write(host, index);
index++;
}
else if(state > 0)
{
i2c_read(host);
}
} |
So, the master should basically print 5 hex numbers, 00 to 04. What do I get (in a loop) is: | Code: | 42 01 02 03 04
42 01 02 03 04
42 01 02 03 04
42 01 02 03 04 |
Where is that '42' is coming from? It turns that my first received value is always 42. I have tried to send letters, fixed numbers, same thing. First value is always 42. On the master side, I've tried software and hardware I2C, but same result. What am I missing?
I'm using CCS 4.138.
Thanks. |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Mon May 06, 2013 12:25 pm |
|
|
| Quote: | | Where is that '42' is coming from? |
I don't know, but we have said many times on here that declaring
variables in mid-code is not supported by CCS. It may work, but
you're taking a chance. See the items in bold below:
| Quote: |
#include <18F26K22.h>
#use delay(clock = 16MHz, internal)
#use i2c(I2C2, force_sw, master, stream = kbd)
#define KB_ADDRESS 0xA0
void Read()
{
i2c_start(kbd);
i2c_write(kbd, KB_ADDRESS | 1);
uint i;
for(i = 0; i < 5; i++)
{
uint8 b = i2c_read(kbd);
fprintf(dbg, "%2X ", b);
}
i2c_read(kbd, 0); // just for the NAK
i2c_stop(kbd);
return true;
} |
|
|
|
Pret
Joined: 18 Jul 2006
Posts: 92
Location: Iasi, Romania
|
|
| Posted: Mon May 06, 2013 3:39 pm |
|
|
| Unfortunately it's not that. I have written thousands (maybe tens of thousands) of code lines in this manner in CCS, had no problem so far. There is a place where it says that mid code variables are not supported? I would be rather surprised... |
|
|
alan
Joined: 12 Nov 2012
Posts: 357
Location: South Africa
|
|
| Posted: Mon May 06, 2013 11:40 pm |
|
|
Don't know whether this will cause it, but the following code will never be executed.
| Code: |
else if (state > 0)
|
as your 1st if already rules that out with
Regards
Alan |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19546
|
|
| Posted: Tue May 07, 2013 12:32 am |
|
|
| alan wrote: | Don't know whether this will cause it, but the following code will never be executed.
| Code: |
else if (state > 0)
|
as your 1st if already rules that out with
Regards
Alan |
No. Think about it. If you have (for instance), state as 1, it'll not be >=0x80, so will go to the 'else', and then since it is not zero, this part will be executed, for everything except 'state==0'.
The code is handling the 'oddity' of state==0, and state==0x80, both of which must trigger a read, and then 0x80, must trigger a write as well.
I'd suspect the master code is the core problem:
| Code: |
#include <18F26K22.h>
#use delay(clock = 16MHz, internal)
#use i2c(I2C2, force_sw, master, stream = kbd)
#define KB_ADDRESS 0xA0
void Read()
{
uint8 b,i;
i2c_start(kbd);
i2c_write(kbd, KB_ADDRESS | 1);
for(i = 0; i < 5; i++)
{
if (i<4)
b = i2c_read(kbd);
else
b = i2c_read(kbd, 0); // NAK
fprintf(dbg, "%2X ", b);
}
i2c_stop(kbd);
}
|
Two specific changes.
First, placing variable declarations where they should be.
Second having the NAK in the data read, rather than as an extra transaction.
Then the original function is declared as void, yet attempted to return a value. Duh....
The original C definitions, allow variables _only_ to be declared at the start of code sections. read K&R.
However a section of a function bracketed, is itself a code section, so the declaration in the 'for' loop for example, is legal. However int i, is not.
Mid declarations are very difficult to do in a chip without a stack, CCS has 'semi accepted' them for about a year, but they have a habit of resulting in values being overwritten by scratch values, so sticking to original C syntax is safer.
Best Wishes |
|
|
Pret
Joined: 18 Jul 2006
Posts: 92
Location: Iasi, Romania
|
|
| Posted: Tue May 07, 2013 1:25 am |
|
|
Thanks guys for the effort.
The weird code from isr slave is actually the one from CCS documentation, as you may already know. The return of Read was a leftover from the stripping process for the demo purpose, wanted to avoid irrelevant code. Guess I've missed that. As well as the additional i2c_read for the NAK. I'm just trying to figure the damn first value...
I've made your code changes Ttelmah, thanks for the hint. Unfortunately, the code only looks prettier. The result is the same.
Regarding the mid declaration, I'm actually surprised. I've checked and I have literally tens of thousands line codes written in this manner. Many of them with lots of int32s and some of them being initialized (withing declaration) with a return of a function. Had never encounter any issue. Maybe the compiler scans any instance of any variable (regardless of their place) and allocate them firstly, in the begging of every routine.
Q1: what happens if no speed keyword (like fast or slow) is being mentioned? What will be the actual speed?
Q2: how can I avoid the deadlock of i2c routines? For instance, if the line is kept low, most of i2c routines stalls.
Thanks,
Cezane |
|
|
RF_Developer
Joined: 07 Feb 2011
Posts: 839
|
|
| Posted: Wed May 08, 2013 2:59 am |
|
|
| Quote: |
Q1: what happens if no speed keyword (like fast or slow) is being mentioned? What will be the actual speed?
|
The actual speed should default to "slow" so as to be compatible with as wide a range of I2C devices as possible. I.e. it falls back to the "legacy" compatible standard.
on other points:
CCS has taken some features of C++ into it's C. These include function overloading, mid-code declaration of variables and optional parameters with defaults, but these are not the only ones (though I can't think of any at the moment).
Despite your positive experience, many of us, myself included have had problems with mid-code declared variables. They are not part of C and therefore they really shouldn't be used in C. They break portability (not that that is really all that important with embedded code, which is generally so tied up with specific hardware that genuine portability is a dream rather than a reality), and cause confusion when read - confusion between C and C++. They are best avoided. Just because mid-code declared varaibles have worked for you in the past is no gaurantee that they will continue to work in the future. Use at your peril!
The history with overloaded functions is a case in point: they work with MOST versions of CCS C, but there was a streak of 4.xxx versions where they DIDN'T work, or didn't work as some users expected, or had experinced it to work in the past. In this case CCS seems to have included overloading as a means to implement some functionality: their maths routines in use it. As such CCS may only test that overloading works as far as THEY need it to work, not in the general case. Again, use overloading at your own risk, and don't moan too much when it doesn't work: it's not part of C.
Default parameters are another of these C++ like extensions that CCS has added to its C. Again, it appears to be so that they can implement some useful, but essentially add-on, convenience, functionality. In this instance the i2c_read uses a default parameter. C++ has a feature that helps make optional parameters ith defaults more practical: C++ has signicantly stronger typing than C, though doesn't go the whole hog as C#, for example, does. This means that C++ can make better decisions about the optional parameters than C can. Even so, in C++ optional parameters must be at the end of the parameter list.
So what does all this mean? Well, in C you can offer up almost any variable as a function parameter and C will, as its definition says it must, convert it as best it can to match the expected parameter type. In your case, how does the compiler know that your stream is not an ack parameter? The answer in C++ would be "because its of type stream rather than int", but C doesn't care, and will merrily convert stream into int and stuff it in the optional ack parameter! Just because YOU, the programmer, know what the steam is, doesn't mean the compiler does, nor that it will understand it in the same way.
My reading of this from the manual:
| Code: |
data = i2c_read();
data = i2c_read(ack);
data = i2c_read(stream, ack);
|
is that the ack parameter is only optional if it is the only parameter to 12c_read(). If you want to specify the stream, you MUST always have both parameters: there is no data = i2c_read(stream); version, and due to weak typing inherent in C, it cannot tell the difference between data = i2c_read(ack); and data = i2c_read(stream);
Now, it is possible that CCS have implemented stronger typing for stream. You have to bear in mind though that pin definitions are just 16 bit integer constants defined with #define, so as pins are simply integers, and not some special pin type, we should assume streams are too.
What I am saying is that your code may have all sorts of unforeseen effects. Its far better, i.e. safer and less liable to come across bugs, to stick to straight C - i.e. not use mid code declared variables and default parameters - than risk the confusion of us, you and the compiler.
Sure, you can moan all you like about how CCS ought to get these extenstions right, and yes in an ideal world they would, but this is not an ideal world - tough, deal with it. Don't moan to us however - we are users, NOT CCS! - and we don't necessarily support anybody who tries. Others, pedants generally, would, and indeed do say that there should be no extensions at all and that everything should be their favoured flavour of C, most often one of the ANSI standards. There's some milage in that arguement too, but ALL true embedded code has to have some extensions to C, making it non-standard, in order to deal with the hardware on which it runs, for example pin assignments, interrupts, integrated peripherals. No standard C, ANSI or otherwise can deal with that stuff, and anyway, which standard is really *the* "correct" one? Shoud all compilers always comply with the very latest standard that no one yet uses, at least at first, or the one of the more common, well established ones? Or should partial compliance, where practical, be acceptable, as is the route CCS has taken?
I've written all the above so that you, and anybody else reading this thread, has a better appreciation of the practicalities of the language extensions you have chosen to use.
So, get rid if the mid-code declarations, rehash the calls to avoid possible confusion of parameters (in many cases as there is often only one I2C peripheral on many PICS) bet rid of the stream stuff altogether. Its something of a big system, abstracted hardware, Windowsy, C++y concept anyway, and here, far from making things simpler, it could well be adding unnecessary confusion.
RF Developer |
|
|
Pret
Joined: 18 Jul 2006
Posts: 92
Location: Iasi, Romania
|
|
| Posted: Wed May 08, 2013 10:43 am |
|
|
Hi there,
In my point of view, the coder should have as many codding features as possible. If CCS one day would say it supports variadic templates and linq I would definitely be delightful. There is no point to get stuck into 20+ years old language. So, if the compiler can give you some third millennium features, you should embrace them. If you are considering portability, well, as you already said, rarely is a practical thing. If you really want to go portable, you may want to use some known coding rules and guidelines, like AUTOSAR.
Using mid-code declarations eases the code readability. I've often encountered the phrase "declare variables as close to their use as possible". And this definitely makes sense. And having this feature is purely related to compiler capabilities. There is no hardware limitation that can forbid that. I would personally use any possible feature that gives me the feel of a high level language, like C#. Claiming the fact that your code is ANSI, doesn't really gives you an advantage if portability is not on the table.
Now, if the compiler claims to support those features but they are not actually working, that's a different topic. If CCS fails to handle mid-code declarations, well, I agree with you guys that it should be avoided. But that would be the only reason to keep them away.
Regarding the i2c_read() routine, I guess we all know how CCS handles its built in routines. They don't even have the declaration in any .h file. Those are simply replaced in place by figuring (guessing?) what you are actually trying to do there. The best example is the printf routine. There is no definition for that. The compiler is optimally replace your call with a custom function. We all know that. Since we are still using CCS means that we are fine with "wizard way" of declaring things like "#use spi" or "#use rtos". That's why we are letting the i2c_read() ambiguity to simply slide. You cannot be rigorous on its parameter types since you are unable to even find them. But this is fine to me, because it gives me portability between 16F, 18F and 24F, usually without any code change. And I believe that's something. And I'm always using streams, even if it's a singularity, because code modules can be moved from one application to another and you may, of course, have others i2c peripherals.
Thanks for your elaborate opinion, RF Developer. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19546
|
|
| Posted: Wed May 08, 2013 11:02 am |
|
|
There is a key bit of understanding 'missing' here. 
Most 'functions' in CCS, are not actually functions....
They are effectively macro declarations for a much larger library of hidden functions. So where printf, in a 'standard' compiler, loads the printf function, which then accepts a wide variety of variables, in CCS, the compiler parses the declared format string, and loads small routines to handle the individual sub components, that the format string requires. This is 'why' CCS doesn't accept variable format strings, but is also essential for producing reasonably sized code that can run in small chips.
90% of the 'functions' in HiTec for example, can't actually be used in small chips.
The big pity is that CCS does not 'document' this. It is a great power of the compiler, but also brings it's own limitations.
Best Wishes |
|
|
RF_Developer
Joined: 07 Feb 2011
Posts: 839
|
|
| Posted: Thu May 09, 2013 2:52 am |
|
|
| Pret wrote: |
Thanks for your elaborate opinion, RF Developer. |
I think you are missing my point. Don't let idealism and dogma get in the way of practicality or even reality. Mid-code declarations may, because they are close to the point of use, sound like a good thing, and perhaps they are. That's not important here however. Its do the damn things work reliably with CCS C? The answer to that, unfortunately, is no, not always. So far, at least, you personally have not encountered any problems. Well, that's great for you; congratulations. Unfortunately, I and may others here, HAVE had problems with them, and don't trust them IN CCS C. Its not about whether they are a good thing in lanugage design, its not even about leveraging the best tools available to you, which I am all in favour of, its about does the bleeding thing work reliably?
Likewise, the documentation, regardless of the mechanisms by which the compiler implements these "pseudo function/macro thingies", strongly suggests, indeed to my reading states, that if you use a steam identifier with i2c_read you must include the ack parameter - it is NOT optional if you're using streams. Yes, it might appear to work, but that is not guarantee that it will continue to work, or always worked. Your code above doesn't always include the ack: you are assuming that becuse its optional in one situation, without a stream parameter, it's optional with. The documentation suggests to me that's a dangerous assumption.
You're asking why your code is not giving you the results you expect. I, and others are trying to help by giving you the results of our experiences. You are just saying to me, and to others, "I haven't had any problems with the stuff you're saying, and therefore I'll ignore you all, and anyway CCS should get this stuff right." Well, yes, they should, but that's in an ideal world. Our experience says that this world, certainly from this CCS users point of view, simply ain't perfect. Stuff happens. CCS C does do stuff wrong. We've found it does and until you've at least seriously tried fixes based on our suggestions; rather than baulking at the mere concept of those suggestions on the basis of programming doctrine, dogma even; its unlikely that we can offer more help.
Mid-code declarations are known to cause unexpected issues, therefore get rid of them.
Optional parameters can be confused, therefore get rid of them. Use stream parameters consistently.
Oh yes, and what you describe as the "classical slave sample" is quite a bit different to that in CCS's documentation. Often its the handling of i2c_isr_state that causes the most difficulties with I2C slaves. Though I've never had to do an I2C slave; plenty of masters, just no slaves.
There, is that elaborate enough for you?
RF Developer
PS For some reason, possibly related to the hardware periherals built into most PICs, I2C support takes up an inordinate amount of support time on this forum. We see the same stuff over and over. This suggests to me that CCS could usefully revamp I2C support to make it easier for casual users to get right, especially with slaves, where the I2C_State seems to cause all sorts of headscratching. If any body has access to V5, are there any improvements in its I2C support? |
|
|
Pret
Joined: 18 Jul 2006
Posts: 92
Location: Iasi, Romania
|
|
| Posted: Thu May 09, 2013 12:09 pm |
|
|
And I think you missed that:
| Pret wrote: | | Now, if the compiler claims to support those features but they are not actually working, that's a different topic. If CCS fails to handle mid-code declarations, well, I agree with you guys that it should be avoided. |
I've never said that I recommend them just because it worked form me so far. And I agree with full i2c params as well. |
|
|
|
|
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
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
