xcore / sc_ethernet

10/100 MII Ethernet MAC for XMOS microcontrollers
http://xcore.github.com/sc_ethernet/index.html
Other
37 stars 29 forks source link

The pakage format is puzzling #21

Closed interactive-matter closed 11 years ago

interactive-matter commented 12 years ago

While I am at it I am also complaining about the packet format for the ethernet packages: a) it oscilates between MTU/4 int values and MTU chars - it is understandable, but is it possible to stick to one of them? b) accessing the different header informations is a pain (for me high level guy ;) ). I do not really understand which package format this is. and by that I am not really able to understand which data is accessed in the different handler routines (currently I am asking myself why rxbuf[23] is not 0x01 even though I am bombarding my XMOS with ping packages. I would really appreciate if we could introduce some macros that explain which parts of the header are accessed.

In the old version (perhaps also the new version) of app_led_tile the package was mapped to a struct and those struct members then used to access the header - much more understandable. Unfortunalty something solely for C, not availaible in XC - at least from what I understand.

ajwlucas commented 12 years ago

The packet size in ints vs bytes is something we've identified as confusing (and have confused ourselves sometimes). Unfortunately this comes back to the fact that we often need to convert between data stored as int arrays to packets stored as byte arrays.

I think we could make it clearer in variable names whether the size is in words or in bytes.

interactive-matter commented 12 years ago

Yes,

I can understand that (that it puzzles you too and that one time you need int and one time you need chars). I would suggest to stick to one format in the API and if some function needs another format internally it has to convert it. I think ints are more efficient. Any way I think streamlining this across the API ccan reduce a lot of noise in the code. But that is just another mid term feature request ;)