我的一些建议：（大家不要骂我吹毛求疵哦！）
我看了chw75大侠的USBTEST代码，也在我刚刚从Limingth那里拿到的新板子上测试了这个代码，确实能工作了，能点灯了（据说昨天已经能出现"usb mass storage device"标志了），但是我还是希望给大家提一点小小的建议：（申明：我真的无心拿chw75的代码来说话，但是我们正好在这个项目里面，用这个代码说话大家感触比较深一些，以后也可以注意一下！还请chw75大侠不要生气，其实我也是到公司后才发现写代码的规范性的重要性的！ :oops: ）
1.代码里面没有足够的注释，这是最致命名的！几乎所有的函数都没有给解释，在函数里面的关键语句也没有注释！说实话，我们写程序并不是给自己看的，因为作为开发人员，只要程序的运行结果正确就可以啦！但是我们是Open Source,是共同开发（只是可能某些人做的多一些，快一些！），所以我们的代码是给别人看的——别人就不是光看你的程序运行结果是否正确了，还要看你的代码为什么能实现正确的操作？在某些地方，为什么要这样写而不那样写？例如：在D12RdEp()函数中，有：
D12SelEp(Ep);
Delay(20);
if(D12Cmd&1)
{
/*the code here is omited*/
}else
{
/*the code here is omited*/
}
作为开发者，在写这段代码的时候，你肯定是查看了D12的文档，知道在Select Endpoint command之后可以紧跟着读一个字节，读出的这个字节的bit0意味着：
FULL/EMPTY: A ‘1’ indicates the buffer is full, ‘0’ indicates an empty buffer.
所以你的意思是D12Cmd&1运算后如果得到非0值，那么就是这个bit0被置位了，说明在这个选定的端点的buffer中有数据可读，否则就直接返回0表示没有读到数据。但是，老大，作为代码的读者，比如我，也必须走这一遍吗？所有的人拿到这份代码都会走这一遍，可怕啊！为什么偏偏是D12Cmd&1而不是D12Cmd&0x02呢？其实，代码的作者在你自己看了文档，写了这段代码之后，加上必要的注释，说出这个关键点，那么你就节约了很多人的时间！！！不是吗？
D12SelEp(Ep);/*First ,we should select the specific endpoint bufferto read.*/
Delay(20);
if(D12Cmd&1)/*The selected endpoint buffer is full*/
{
/*the code here is omited*/
}
else{
/*the code here is omited*/
}
像上面这样，我想一般的代码读者看了，应该不会再去看文档了，除非他要检查你的代码写对没有？但是由于你的代码已经能正确工作，那就肯定时对的，所以一般也就不会费时间啦！
我是读通信工程专业的，没有软件工程的专业知识，但是我相信这一点软件工程里面应该有，是不是？ 2.标识符，结构体等命名不够规范，这个会影响代码读者的理解！例如，在usbdesc.h中，有个结构体：
typedef __packed struct {
U8 size;
U8 type;
U16 ver;
U8 class;
U8 subclass;
U8 protocol;
U8 pktsize;
U16 vid;
U16 pid;
U16 pver;
U8 stridx0;
U8 stridx1;
U8 stridx2;
U8 itfcnt;
}UsbDevDesc;
改成下面的定义，感觉怎么样呢？
/*
* USB_DEVICE_DESCR
*/
typedef struct usb_device_descr
{
UINT8 length; /* bLength */
UINT8 descriptorType; /* bDescriptorType */
UINT16 bcdUsb; /* bcdUSB - USB release in BCD */
UINT8 deviceClass; /* bDeviceClass */
UINT8 deviceSubClass; /* bDeviceSubClass */
UINT8 deviceProtocol; /* bDeviceProtocol */
UINT8 maxPacketSize0; /* bMaxPacketSize0 */
UINT16 vendor; /* idVendor */
UINT16 product; /* idProduct */
UINT16 bcdDevice; /* bcdDevice - dev release in BCD */
UINT8 manufacturerIndex; /* iManufacturer */
UINT8 productIndex; /* iProduct */
UINT8 serialNumberIndex; /* iSerialNumber */
UINT8 numConfigurations; /* bNumConfigurations */
} WRS_PACK_ALIGN(4) USB_DEVICE_DESCR, *pUSB_DEVICE_DESCR;
这就是大名鼎鼎的Wind River的vxWorks中对USB Device Descriptor的定义！
还有，我觉得上面提到的函数D12RdEp()命名也可以改的归整些，比如D12ReadEndpointBuffer()
如何？可能您会觉得太长了？但是看一下Wind River的一个函数名吧：
usbOhciReleaseBandwidthForCurrentConfiguration()
他这个函数名只用
releband()
就可代替了，但是人家还是那样坚持写了老半天！因为他们的USB Stack可以卖个很高的价钱，vxWorks也能上火星！ 3.这个项目现在的工作状态已经很喜人了，感谢chw75以及很多的其他高手的工作！我是刚拿到板子，就在这里说这些话，实在是很不明智！但是我觉得，以后还有许多开发要做，所以如果从现在做起，我想还是能做得更好的！您看到Linux
2.6的内核与Linux 2.4的内核的区别了吗？其中很大一部分就是规范化和文档化！代码就应该是文档！我昨天晚上看chw75的代码，在我看到的地方，经查阅了文档之后，就接着加入了一些注释！我想我刚来嘛，还有很多不懂，正好通过阅读代码，理解原来代码的编写方式和工作流程，同时把大侠们以前的努力成果注上一把，让后来的人也可以轻松看懂，不知道这个建议大家可否接受？ 感谢CoryXie提的意见,代码中绝大部分是别人的写的,我只是拿来移植一下,而且时间有限,只能业余做一做.说起来很惭愧 :oops:
不过我移植别人代码的时候,这些代码什么五花八门的写法都有,简直就象八国联军,也懒得去注释了,也就只是关注怎么实现功能罢了.
比如象上面提到的unsigned char,我就碰到U8,UCHAR,BYTE,Byte,UINT8等等一系列定义,我也没有办法,只能用typedef在专门的一个文件里去定义了.
以后,如果有时间,我尽量注意,谢谢你的建议!! 我的体会是, 刚从学校出来的,没有在公司经过锻炼的, 比如像我 :cry: , 都很少有写文档的习惯,代码注释也不是特别在意, 有时候顺手会写上一些, 但也是自己看懂就够.在软件开发过程中, 文档的工作应该要比编码需要更多的时间, 特别是开源项目, 前面写 Step by Step 也花费了我将近一个月时间, 虽然那些代码都不太难.
Cory 的建议无疑是对我们的一点鞭策, 不过大家都是业余时间做, 除了网友们的赞一句好也没有别的太多奢望. 另外早发布早修正也是开源软件的一贯风格, 不用担心自己的东西不好而不敢给大家看, lumit4510 第一版飞线的图片还在主页上, 我想发现问题解决问题就是最好的学习, 只有这样才能知道我们是怎样一步一步成长起来的. :-)
页:
[1]