Hi Joep,

> I had a closer look at your lib and see you do 'SSPCON1_CKP = 1' in
> state 5. And I don't do this in my lib. What's the difference if you
> put this in?


AN734 seems to have a bug, that's at least what some refers. In AN, there's
no mention about CKP = 1, but in the PIC datasheet, there is. See also:
http://www.astrosurf.com/soubie/pic_as_an_i2c_slave.htm (particularly
section "
Microchip Application Note AN734"). Having this should not break code where
it's not required (IIRC)


> Further, my lib has an issue: If the master sends an addrss and a few
> bytes, each byte gets an interrupt and my library stores these bytes
> in an array. But when the master sends a stop, I don't get an other
> interrupt. And I do need an interrupt at message complete (i.c. the
> stop) to handle the message receveid.
> I handle 'message complete' now when I receive a new address, which is
> usually the following attemp of the master to read from the same
> slave.
> So to trigger immediate processing (and that is what you mostly want),
> I always need to read a byte back . Effective, but not realy efficient
> and definite not elegant!
> Do you know how I could trigger the message processing any other way
> (assuming you don't know the number of bytes the master will write to
> the slave)?


You could try to init with interrupts: i2c_hw_slave_init(address,true): true
for "yes I want start/stop interrupts". The problem you'll have is this
won't be recognize as a know state (see xor mask), and you'll reach "on
error" callback. I could maybe (maybe) handle this by adding some tests.
Before diving into this, I understand your lib has a limitation, so I'd ask:
why don't fix it ?

BTW, remember the discussion about having two i2c hw slave lib ? One for
init, read and write procedure, another fir the ISR handling ? Note in my
blog posts (part 2), I define my own ISR, by enabling Start/Stop interrupts.
This couldn't be possible if ISR was in the same lib as the init procedure.
Flexibility... :)


> Finally, I do know you can't debate about taste, but i'll give it a
> try anyway ;)


Yeah, give a try !!!

In your slave_isr lib, you created a procedure for each state. Those
> procedures contain very little code, good comment and call an other
> procedure.


OK


>
> The main isr only checks certain vars and then dispatches to the
> procedures mentioned.


OK ("Mmmh, I think I know what he's gonna say...")


> I would ask you to concider put all the code of the isr program into
> one (isr) procedure. This would probably generate the same code, but
> on source level it would reduce 5+ lines of code per procedure and
> gives me more overview then all the separte procedures, who do call
> (similar named ;) procedures themself.
>

OK, ("Phew, I thought it was something else :)")

If you mean, for instance, moving i2c_hw_slave_proceed_state_1's code
directly the ISR, I'd say yes, this may make the code easier to read. I tend
split a lot, this helps me extract some fully functional piece of code, and
help me focus on what it should do. In other words, I don't like procedures
with a lot of lines. Small chunk of code is what I like. But I'll modify
this, as you suggested.


Nice try, BTW :)

Cheers,
Seb
-- 
Sébastien Lelong
http://www.sirloon.net
http://sirbot.org

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"jallib" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/jallib?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to