>
> > 1. I've written two libs: i2c_hw_slave.jal and i2c_hw_slave_isr.jal.
> Only
> > i2c_hw_slave_isr.jal (the ISR) is required by client code. I could have
> merge
> > them, but I thought it might be useful to keep the init/read/write
> functions
> > apart from ISR, for other purpose (like implementing a i2c slave, but not
> > using states). What's your point ?
> I think this is a bit confusing for users, an other file with similar
> code - what should they choose...
> I think it is better to have one file that contains the required code.
Only i2c_hw_slave_isr is required by users (it includes i2c_hw_slave, as a
dependency). The lib splitting is only a matter of lib writers, I'd say.
> 2. In i2c_hw_slave.jal, the init procedure (i2c_hw_slave_init) enables
> > interrupts (global, peripherals and i2c). While this is required by ISR,
> I
> > really wonder if it's a good idea for a lib to enable interrupts while
> user
> > did not require it. Of course, when using an ISR, user know (should)
> > interrupts will be enabled. Maybe I could enable them in ISR. Your point
> ?
>
> I don't understand your point.
> I'd say the user calls i2c_init, so it needs the interrupts.
I would say i2c_hw_slave provides common procedures for an i2c slave
(init/read/write), but has nothing to do with interrupts (you could use this
lib, even if you don't use interrupt). So, if someone needs this common
procedures, but don't want to use interrupts (that is, implement a slave,
but using interrupts, -- is this possible ? --), he could use this lib,
without the ISR.
That said, and since only i2c_hw_slave_isr is needed from a user point of
view, and since it may not useful to do this, I'll merge i2c_hw_slave into
i2c_hw_slave_isr. If needed, we'll split the lib again, while API remains
the same for users. what do you think about this ?
>
> > 3. In the ISR, once a state is detected, the corresponding procedure is
> > called, then the callback. Ex:
> >
> > - state 5 is detected
> > - i2c_hw_slave_proceed_state_(5) is called
> > - i2c_hw_slave_on_master_hangsup(), the corresponding user's callback,
> is
> > called
> >
> > I could have make the corresponding callback directly called (state 5
> =>
> > i2c_hw_slave_on_master_hangsup) but it appears there is some small but
> > important logic users shouldn't care about, but still needs to be
> implemented
> > (ex: for state 5, needs CKP = 1, for state 1, *must* read buffer, ...)
>
> This is just not for the faint of heart users. Users should only go
> this way if the buffer-library (see below) does not meet their
> requirements.
But I understood using buffers also have limitations.
> And they should read the documentation and (comment in
> the) example very carefully.
> (and maybe it is a good idea to note this & point them to the
> buffer-lib in this comment).
OK
> > Joep, you were talking about using a buffer. As I said (and thought, but
> > correct me if I'm wrong), it's a higher level than that. Maybe you could
> > implement this layer on top on this one.
> Almost ;)
> Stack levels are scarse. This is why I wrote my code the way I did.
> Using this code for 3 years learned me that I should have sacrifcied
> one stack level to create a user call-back function outside the
> library. This is what I intend to do. And I will use as much of your
> code as possible, provided it does not cost extra stack levels.
I use "pragma inline" as much as possible (in ISR, and in sample, as
suggested -- see comments --), so it shouldn't eat too much stack. Actually,
compiling 16f88_i2c_hw_slave_echo.jal gives:
Hardware stack depth 5 of 8
and enabling debugging for jalv2 asm shows the following stack usage:
; --- call stack ---
; {root} (depth=0)
; serial_hw_init (depth=1)
; _calculate_and_set_baudrate (depth=2)
; serial_hw_write (depth=1)
; i2c_hw_slave_init (depth=1)
;Temporary Labels
So a max depth of 2, from serial_hardware usage. So where does 5 come from ?
I can remember something about stack usage and ISR, where stack levels
aren't being considered,... oh I can't remember :) What I mean is this "5 of
8" might actually be "2 of 8", according to the debug comments.
I'll give a better search on this.
BTW: don't you think we should add a "pragma inline" in
_calculate_and_set_baudrate(), in serial_hardware lib ? It's called only
once in init, and should never be called elsewhere.
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
-~----------~----~----~----~------~----~------~--~---
