"David Bourgeois" <[EMAIL PROTECTED]> writes:
> Hi Damien and the others,
>
> I'm finally looking at the daemon more seriously as I'm adding the
> sleep and ID stuff now. There are a couple things I'd like to
> discuss.
>
> First, I'm writing some code in the daemon and as I'm still a beginner
> at linux programming, I would appreciate any kind of feedback
> regarding my commits, would it be the style, the kind of functions I
> use, the structure, anything I could do better is welcome to point
> out.
Well, since you asked... In the recent firmware commits:
* Several lines exceed the 80 column limit (as stated in the coding
style guidelines) quite a bit (100+).
* There's a *big* "if else if else if ..." on command[0] which could
be replaced by a switch.
* Some types have all uppercase names, but this should be reserved
for macro constants only. Besides, the naming of some enums types
and their values could be more consistent.
That's from the top of my head, I'll post more as I read the code.
> For the current code, it seems that by removing the USB status thread
> at revision 322, the cmd_status_flag (which should probably have been
> a mutex) is never reset when the usb_write_TuxDroid() function is
> called. To understand what's happening, I need to describe how the
> USB/RF protocol works.
That's something I'll have to check. When I removed the threads, I had
a strange feeling about the locking that I wasn't able to pinpoint at
that time, so I may have overlooked something...
Right now, what happens is that select() has a timeout of 50000 µs,
and when it times out, it calls update_tux_status():
...
r = select(fdmax + 1, &rset, NULL, NULL, &timeout);
if (r == 0)
{
if (!update_tux_status())
...
This triggers the following calls:
update_tux_status()
\_ usb_get_status_TuxDroid()
\_ update_system_status_table()
=> cmd_status_flag = 0
Two things worth of note:
(1) Something I intended to fix but forgot about: currently, if
select() always has data to process, the timeout may well never
happen. We have to ensure that update_tux_status() is called
every 50000 µs, *no matter what*.
(2) I got the 50000 µs from the pre-r322 code. Not sure how it fits
the 2ms you mention below.
That's just after a quick look, I'll have to read the code and my
commit more thoroughly.
> Each 2ms, a frame is sent from the RF module(I) (in the dongle) to the
> USB chip. This frame contains a few RF and dongle status (in which
> the RF ACK is), the microphone sound data and 4 bytes of raw status
> from tux. Here's what's happening when sending some raw data to tux:
[...]
> I don't necessarily want to revert the thread and to improve the
> communication I think we should divide usb_write_TuxDroid() in 2
> functions, one that would send the raw, the other that would wait for
> the ack
I'd rather avoid using threads, as it eludes portability problems (see
the NSLU2 problem). We can do just the same with a state machine using
select().
> 1. either they're called alternately and we can't send a raw if the
> ack is not received (as of now)
> 2. either we can send a second command while waiting for the ack of
> the first one; we can't send more than 2 commands while waiting for
> the ack otherwise it's possible that it will overwrite the previous
> command.
>
> I'd like to implement 2. and also add a buffer for the incoming
> commands (from the API or anything else).
Solution 2 would indeed be more optimized, but is it necessary ? I
mean, do we have performance problems, could we do with 2 commands
flying things we can't do know ?
> As of now the API sends one command to the daemon, waits for the ACK
> and store it in a variable which is not used by the applications,
> this explains why the daemon is still working. The ACK is always
> TIMEOUT but that doesn't matter for the application.
>
> Any thought about this before I start working on it? (Yes I know I
> have to add the protocols on the wiki ;-) )
> Finally, I find the file structure quite strange, there's only main.c
> in the root folder, then all files are under /libs and all names
> start with USBDaemon. I don't want to change this right now but any
> idea on what a good structure and naming conventions would be?
Agreed, we'll have to shuffle things around quite a bit ;-)
Damien
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
tux-droid-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tux-droid-user
