Hi Seb, 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?
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)? Finally, I do know you can't debate about taste, but i'll give it a try anyway ;) 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. The main isr only checks certain vars and then dispatches to the procedures mentioned. 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. Joep 2009/1/19, Sebastien Lelong <[email protected]>: > Hi guys, > > I'm about to modify i2c hardware slave ISR API, and would require your > feedback, 'cause I have doubts about how clear (or not) the API is. ISR > implements a finite state machine (well, here my post :) > http://jallib.blogspot.com/2009/01/step-by-step-building-i2c-slave-with.html). > Each state calls a procedure which must be defined by users. > > Previous callbacks names were: > > - i2c_hw_slave_on_master_talks for state 1 > - i2c_hw_slave_on_master_writes for state 2 > - i2c_hw_slave_on_master_reads for state 3 > - i2c_hw_slave_on_master_stillreads for state 4 > - i2c_hw_slave_on_master_hangsup for state 5 > > I was not happy with this, mainly because state 1 is a new write operation, > and callbacks is not explicit about it. I also first thought there were a > symmetry between write and read operation, so callbacks should reflect it, > but that's not completely true: when master is willing to write, it must go > to state 1, then state 2, whereas reading only require state 3 (and > optionally state 4). > > So I was thinking about renaming state 1 callback from > i2c_hw_slave_on_master_talks to > i2c_hw_slave_on_master_begins_to_write (well, that's a long > name, but it's explicitly, and "write once read thousands" principle would > allow this...) > > What do you think about this ? Is this pure nit-picking or philosophically > non-pragmatic ? In a word, who cares ? (except me, of course :)) > > > Thanks for your feedback ! > Seb > > > > 2009/1/18 Sebastien Lelong <[email protected]> > > > Hi Joep, > > > > > > > > > > > > > > I've seen your postings on the blog and I like them :) > > > Do we have an idea if many people visit this blog or use jallib? > > > > > > Thanks. I don't know how many use jallib (download counter says ~ 100 to > 150). Blog stats are very low, few follow it, but there are some, and it may > take time. Once I've finished the 3rd and last part of my howto implement an > i2c slave, I'm thinking about posting something on jallist. Do you think we > should announce on jallist too ? Or it is too much ? > > > > > > > > > > Today I had a brief look at the i2c slave lib and I agree with you > > > that my lib should be on top of this one and carefully selected inline > > > statements should be able prevent exhaustion. > > > > > > OK, but be patient, I need to rename some callbacks. This is just > renaming, no signrature change (I hope). This is because currently it's not > clear if master wants to read or writes. eg. i2c_hw_slave_on_master_talks > should become > > i2c_hw_slave_on_master_starts_write, and > i2c_hw_slave_on_master_reads should become > i2c_hw_slave_on_master_starts_read. These are subtleties, > but callbacks name should show the symmetry between read and write > operation. I'll take of this probably tomorrow. > > > > > > > > > > > > Cheers, > > Seb > > -- > > Sébastien Lelong > > http://www.sirloon.net > > http://sirbot.org > > > > > > -- > 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 -~----------~----~----~----~------~----~------~--~---
