Well, I had no idea what I was getting myself into. I never expected that it would take five times longer to debug the PIC18 support. I thought this was a going concern but it just did not work properly on the stack I received. Geez, what a headache. The good news is that in trying to track down the major problem I found many other little things that were not quite right and would have caused problems later on.
As it was that was not the killer. Right now I just don't want to mention what it was other than to say it took me completely by surprise and in the end once I figured it out it was so bloody obvious! Someone somewhere really fudged the stack and I do not believe it was "honken."
So, now I have here what should be a good base for both PIC families and even expandable for the up and coming dsPIC33E and PIC24E families and even the PIC32 parts if someone wants to continue development in that direction.
I will rip out all the debug code etc and package it up asap. I need to do other things and I know there are some eager people waiting to get there hands on the latest and now working version. Given that I am now finalizing a version for both the PIC18 and PIC24 families give me a few days to do this.
Among the good things that have happened is that I have ported it to MPLAB X and there is no need for specific linker files and therefore less worries about microchip license issues. However MPLAB X is not quite at "showtime" for the PIC18 parts so I will include the MPLAB 8.xx version too.
Anyway, it's 4:20AM and I need to sleep. More soon.
Yeah, I have the BPv4 working and I'm very busy tidying up the code. There have been some problems to sort out and really I am at the point that I am not going to pursue them myself at this time. Some things can wait for v.2 of the stack. There is a minimum that I will accept as "job done" for now and to achieve this I have actually put aside a lot of my work and gone back to a bare basic framework. Anyway, the thing is that it is working and I aught not try to be too much of a perfectionist. It only holds things up.
Right now I am using MPLAB X and it is a fantastic improvement over MPLAB 8.xx. When I send Ian the project it will be as a MPLAB X project. I wish I had of made the change earlier. It really does make navigating through the code so much easier. In fact the way I have it set up is that it will show all PIC24 bug fixes in the task window so people can see what I have done specifically for the PIC24.
@Ian. What I wanted where the BPv4 firmware files. You send me the board files. Even with a new ISP (the reason why I have been absent from the forum - no internet) google code still throws an error and not just for the dangerousprototypes stuff. It is like the whole of Australia has been banned or something. It doesn't matter now as I nutted through the circuit diagrams and worked things out.
Anyway. I'll keep at it and send what I can as soon as I can. There is much else for me to be doing...
[quote author="Thomas"]Can you post the source code for the working pic24F stack? It would be easier for us with a bp v4 to work on the CDC portion of the code if we can get it to enumerate.[/quote]
I look forward to posting the stack as soon as I can. I have dropped everything to work on this and there are some house keeping things I must do. The BPv4 files I needed have just arrived and I will check my work against this and also check that I have not broken the PIC18 support.
I am about 92.563% done but as all experienced engineers know it is that last 7.437% that is the hardest to complete.
Meantime I have assured Ian that it is coming. Ian sent me some hardware so I know that I have obligations here but they are not the only ones I have.
Rest assured that I am working on it and fixing it up to a presentable level and adding comments as to why I have reordered somethings, particularly the EP initialization. There where a few spots where you had TODO and question marks and some of these are addressed.
I think all the problems with the PIC24 are specifically marked with a comment:
// JTR PIC24 fixups
I think I marked them all and I do hope so. This is so the mysteries are solved for all those interested and can even be back ported to the stack as it currently is though I really feel this will be a backward step.
I am sort of sorry that I did not get it to work on the stack as it was. It would have made for a lot less work presenting my findings.
Currently I have it working on the early BPv4 board Ian sent me. I whipped up some alternate code for the CDC test along the lines of what I suggested before. A simple state machine that copies the OUT EP into the IN EP and tracks the requirement for a ZLP. Works well.
Note! There are no changes required to your CDC section for PIC24 support. Anything done to advance the PIC18 CDC functions will work with the PIC24. There is no need for anyone working on that part of the code to wait for my code.
After much toil and a sophisicated debugging system that consisted of a resister and a LED I am happy to say....
I now have my PIC24FJ64GB002 mock-up enumerating with the modified Honken stack! Yay!
There were something like 6-8 show stopper bugs and about half of them have been point out on this forum. The last issue I found very much to my surprise was the ACTIVE interrupt stopping the stack from working. Why I do not know and there are still somethings I am investigating. I will have to go back through my changes and verify and document all the issues I found. That may have to wait for tomorrow. But for now I have the stack working to the point that it is asking for a driver.
I will port the code back to the PIC24FJ256GB106 so that it is BP4 compatible and send Ian my code.
I have not done anything with the CDC component and there are still issues here. Particularly what Rebecca pointed out needs to be looked at and fixed if indeed it is an issue.
Also the CDC component needs some work but this is true for its PIC18 support too.
Anyway, this all is some progress and the BPv4 takes a big step forward.
[quote author="rebeccapalmer"][quote author="JTR"]Yes, this is true however I read the compiler documentation and it said the the low byte of an int would be stored in a byte variable and the PIC24 only use the lower 8-bits of this SPR.[/quote] That would work for a single register, but USB_UEP is a pointer, so USB_UEP will be endpoint i/2 instead of i.[/quote]
I was thinking something like that would be the case however even so this will not affect the enumeration for the PIC24 as it only needs to correctly address UEP0. Even if UEP1 writes into the MSB of UEP0 this will not affect UEP0 as these are don't care bits.
For certain it need to be fixed to get the CDC component to work.
[quote author="rebeccapalmer"]I have found what may be the problem: usb_uep_t is unsigned char but the PIC24's U1EPx registers are 16 bit. (Though due to a suspected compiler bug, at present I can't even test if it compiles with this fix.) [/quote]
Yes, this is true however I read the compiler documentation and it said the the low byte of an int would be stored in a byte variable and the PIC24 only use the lower 8-bits of this SPR. I also looked at the dissassembled code and it all looked right to me. I don't believe this is a bug but I would be happy for someone to double check it.
As the pointer is written to in the device descriptor handler. Now the SETUP packet handler executes but the device still doesn't enumerate. [/quote]
Yes, good catch. I found that bug too @ 3AM this morning while running the stack through MPSIM.
I simply dropped the ROM keyword and it seemed to me to have fixed it but really I was not sure. Thanks for that solution.
Unfortunately my mobile carrier has some contract with industry for bulk usage in the wee hours and I cannot log on for a couple of hours and then sleep called. I am seriously thinking of joining the class action against my carrier but that is a different story.
@ Ian the BP4 arrived here today. Thank-You. I had built a breadboard with a PIC24FJ64BA002 on it and tested it with the microchip stack. However the PICKIT 2 does not support this device in MPLAB so I could not run the debugger. Now I will be able too recompile ofr the GB206 and hope to track down some more gremlins.
Look if you are confident do what you wish. There are trade offs to whatever you do and maybe you have the best idea. If you get something working using the SOF fine by me. Like I said currently as it is I have doubts that it is fully sorted. That does not mean it will not work fine with some further TLC.
I am concentrating on getting the PIC24 to enumerate. The CDC component can be added on when this is working.
Why would you like to have this in the class driver? If you don't mind me asking. The reason I made an echo test program in the first place was that it was the smallest use of CDC ACM I could think of.
The copying could be handled easy enough, just hand the IN EP descriptor the address for the OUT EP buffer and don't set the UOWN flag of EP OUT until confirmation of transmit of the IN EP.
This will make the OUT EP reply with NAK's until you are finished using it's buffer. Or, perhaps a better solution, you could swap the BDADDR with each other.
But then again, I wouldn't leave that part of code in a class driver for generic use.
The API of the cdc.c was modeled after the UART lib of mcc18, and I agree it needs a little TLC.[/quote]
The reason why I suggested what I did is in fact the reason you yourself gave. The CDC class requires a ZLP when the last transfer is equal to the size of the bulk endpoint. You said so yourself.
There is no way around the fact that some sort of mechanism like a state machine for the CDC BULK IN transfers is required. This is true regardless of what code the final CDC stack ends up running with. We need this coded as you have already said yourself.
As for copying the whole IN buffer to the OUT buffer, the reason for this is that I by passes other potential bugs. It is initially for testing purposes and may or may not be part of the final CDC stack. I have suspicions that the problems Ian found with the 18F25J50 relate to the way the echoing is currently been done. That is using the SOF interrupt and the possibility that there is an issue here with timing etc.
Note. I have strong doubts that using the SOF is a good idea. You would have to mask it while you were loading the IN buffer and that eliminates its use for other purposes. And yes, I have a CDC stack where it is used for other timing purposes.
My proposal is the simplest solution for testing the basic operation of the under lying standard USB stack and a proof of concept for the CDC component. Based on the evidence before us we know that the stack can enumerate on the 18F24J50 but the CDC echo is flawed.
Keep in mind that I have done all this before and know a way. Sure it is not the only way just one way that has proven itself to work. Accept or reject as you see fit.
[quote author="rsdio"][Quick answer: Look at common Low Speed Classes such as keyboard and mouse. I seem to recall that there is one Low Speed Device, or possibly all Low Speed devices, where the requirement is no more than 8 bytes for ep0. In fact, Low Speed may not allow endpoints besides ep0. My memory is a little fuzzy because I don't deal with Low Speed. [/quote]
Likewise. I never considered this stack to be aimed as low speed devices. There are a raft of limitations for low speed devices it is true. I am sure I once knew what they were but like you I have no interest in them.
[quote author="rsdio"]
Quote
EP1 IN MAX_PACKET_SIZE = 10 (Yes 10, that is the size of the CDC LINE_NOTIFICATION block)(CDC bulk endpoints)
Actually, if you really want to send 8 bytes most or all of the time, then it's a good practice to define the endpoint with 9 or 10 bytes. This way, when you send 8 bytes, the USB doesn't require an additional 0-byte packet to signal the end. If you do end up sending 10 bytes, then you will need that 0-byte transaction, which will eat up a significant percentage of your available USB bandwidth, but 8-byte packets will be sufficient all on their own. I learned this trick indirectly via answers on Apple's USB developer mailing list, in response to my reports of what I thought was strange behavior. [/quote]
Yes, under some circumstances this would be trick and it is a good idea that I will file for future reference.
But the truth is a "terminating" (as opposed to a status transfer) ZLP though is not often required. In all my work with USB I have only known it to be required with CDC when the bulk IN transfer byte count is the same as the endpoint size. That is not the same as saying it is not required in other cases, just that I have not seen it.
[quote author="rsdio"]
Quote
An experienced C coder could do in an hour what it would take me a whole day to do.
If you don't mind my asking, what sort of programming experience do you have? I haven't looked at your code, but you sure seem to know what you're talking about. I'm just surprised that you see yourself as an inefficient C programmer - although I must admit that I might have missed a big chunk of this thread or another. Are you one of those guru assembly programmers that happens to be rusty at C? [/quote]
Assembler guru. I could never master C because of the strong syntax and for reasons only a neuroscientist could understand. :)
I have written a USB stack in assembler for the PIC18 that includes a USB bootloader, 128-bit block encryption, FULL CHP9 implementation and a "Control box" that handles all transfers for EP0. The FULL USB stack is not just for the bootloader. It can be used for user apps. No need of a duplicate stack. You can bootload any class into the PIC as the inbuilt stack provides many hooks, callbacks and variables at fixed locations. All this fits in the 0x800 BYTE boot block of the 18F14K50 or 18Fxx5x and was pain stacking researched. I studied the Diolan stack, microchip stack, The obscure ANA stack and of course Brad Minch's stack and read everything I could find of Tsuneo Chinzei's postings across numerous forums.
On top of this I have written a full CDC stack, HID, CDC + HID and generic bulk applications in assembler all based on the above kernel and even managed to squeeze a USB stack and bootloader into the 0x400 byte boot block of the 18F13K50.
[quote author="rsdio"]
Quote
As soon as I am certain I have not fudged something with all the little fix-ups here and there I will email the stack onward so that others can see what and why I have changed a few cosmetic things. The stack proper though is mostly untouched. It was pretty well written I must say.
A friendly suggestion: You should generally avoid making cosmetic changes to public code - and this is especially true if you consider yourself to be an inefficient C coder. Perhaps all your changes are dead-on improvements, but if you're reluctant in any area then I'd say it's probably best to change as little as possible and let cosmetic alterations wait until a future code review.[/quote]
Ok, "cosmetic" was a diplomatic word. I am not trying to hijack this stack, just tidy it up at the edges without being intrusive. Cosmetic really means bug fixes and putting things where in my experience they belong in order to keep the stack open for other uses. Chances are that I am eminently qualified to do this. I have pointed out what part of the stack I am addressing as a "hint" for others to concentrate on other areas, like the CDC class itself so we can combine efforts. And as it turns out when earlier on I pointed out that I needed some changes to be made in the C code I did not get any so I will do as I see fit to advance this work and others can accept or reject it as they see fit. As it is I have at least three "honken" stacks here that have had way more then "cosmetic" changes made by others. I call it growth.
No one has to accept my work and I have not at any point supplanted the existing "public" work in the SVN. My changes are all marked in the source code and there are comments as to why I have changed things. At the very least it will serve as a reference if people want to stick to the stack as it is.
BTW. You may or may not of figured out who I am. I just quietly slipped in here to help out without trying to "rain on the parade." I hardly consider myself to be expert, just more experienced. You do know me from another forum and it is even possible (not sure) that I helped with this stack before it made its way here to DP. :)
@ Honken. Sorry mate, yes I meant to point that out too. Indeed I found that the suspend (IDLE) interrupt was firing before enumeration so I just disabled it until we can sort that out later.
@rsdio. I am not aware that any class dictates the EP0 max size. As far as I know it is 8, 16, 32 or 64 regardless of the class. If you can show otherwise this would be new info and important for the stack.
@All. I have changed the stack so that: EP0 MAX_PACKET_SIZE = 8 EP1 IN MAX_PACKET_SIZE = 10 (Yes 10, that is the size of the CDC LINE_NOTIFICATION block) EP1 OUT not used, not armed, no buffer space required. EP2 IN/OUT MAX_PACKET_SIZE = 64 (CDC bulk endpoints)
No other endpoints are armed or required for the CDC class.
These endpoint sizes are almost universally accepted as being correct and/or the most functional for the CDC-ACM class.
So where are we at?
There are two main issues with the stack that are being worked on or at least need to be worked on.
1) The CDC code is really under done. This is holding back the stack for the PIC18 family. 2) So far it is not enumeration on the PIC24 family.
Re the PIC18:
I have advanced the underlying USB stack now to the point that I cannot go any further with testing it without the CDC code being brought up to speed. I have the enumeration sequence and endpoint arming all sorted out to the point the CDC class is ready to go. However I am not really looking to work on the CDC class itself unless I really have to. An experienced C coder could do in an hour what it would take me a whole day to do. The code that is needed immediately is code that can copy straight from the CDC bulk OUT EP to the CDC bulk IN EP AND a state machine to track the CDC transfers, particularly the ZLP requirement. The microchip system is simple and as good as any.
Re the PIC24.
I have found and corrected a number of bugs with this and I have the new code compiling and looking like it is ready. I have some PIC24F64GB002 PICs here and I will knock up a bread board to start testing the PIC24 enumeration with that.
Hey Ian, did you ship that BP prototype yet?
It would be far easier for me to troubleshoot the PIC24 changes I have made rather than trying to do it via remote control but if I luck out with the hardware that is what we may need to do.
As soon as I am certain I have not fudged something with all the little fix-ups here and there I will email the stack onward so that others can see what and why I have changed a few cosmetic things. The stack proper though is mostly untouched. It was pretty well written I must say.
[quote author="JTR"][quote author="rebeccapalmer"][ To define PIC_24F or PIC_18F for every processor I use: [code]#if defined __PIC24F__ || defined __PIC24FK__ //we don't care about 24H/30/33 because they don't have USB anyway [/quote]
Best I can tell none of the PIC24FK series have USB either.[/quote]
Future product dsPIC33EP512MU810 has USB. I guess there will be more USB dsPIC33 parts to come. Who knows we may also see a 60MIPs PIC24 variant too.
