Basic example of CDC

[hoihu]

Hi @projectgus !

I was tinkering around with CDC and was able to implement a device driver that closely matches with the C-implementation (in terms of interfaces and descriptors). So basically on wireshark they show up the same way.

I also added a simple example with a read and write function. It's very early days I guess, so I haven't done any work into how that could be integrated into MicroPython and how it should be configured (perhaps, similar as on the STM32's USB API https://docs.micropython.org/en/latest/library/pyb.USB_VCP.html?)

If you find some time to review, would be great. It's definitely very cool to write a device driver in pure python.

[hoihu]

If you want I can rebase on your latest updates.

[hoihu]

@projectgus ping

[projectgus]

Hey @hoihu,

Sorry I'd seen your branch but I missed that you'd opened this PR, thanks for the "ping"!

Your approach looks really good. Last week I pulled your code into my updated branch and tinkered with it a bit. There's obviously a lot of functionality needed to reach feature parity with the TinyUSB CDC driver (for example), but it's an excellent start and already useful.

What are your plans with this from here? If you're keen to work on it and have time then I can provide some guidance on API integration, etc. On the other hand if you'd rather I use your work here as a base and I keep building on it, that's an option as well.

[hoihu]

@projectgus
Thanks for your time and the reply.

What are your plans with this from here?

I thought about adding a ringbuffer, then a stream interface so it can be used with StreamReader resp. StreamWriter (as explained https://github.com/peterhinch/micropython-async/blob/master/v3/docs/TUTORIAL.md#64-writing-streaming-device-drivers ).

Interesting would be to try configuring as many CDC interfaces as possible. Should be plenty on the rp2040 :)

Yet another thing would be to support the REPL on one of those interfaces.

On the other hand if you'd rather I use your work here as a base and I keep building on it, that's an option as well.

I'm definitely fine with either way. Next month I'll be offline though.. If you could give me some hints about integration and your views about it, would be great. If you find some time to do the integration and push this PR forward, even better ;)

[projectgus]

Hey @hoihu,

Sorry again for the slow reply. I've been trying to think of specific advice to give, but at this point it's actually a bit challenging because the underlying usbd code is in active development again. The latest set of changes may break the CDC class next time you update to it, sorry! (some callbacks were renamed), and more breaking changes are likely.

That said, I think your list of intended features is a very good place to start.

I also have a branch with your CDC work and I've been looking at what else is needed to make it compliant - so far I noted handlers for get/set line state, handlers for driver open/reset events, etc.

If I get some time to keep working on this then I'll make sure to push my changes so I don't waste your time duplicating any effort. I'll watch to see if you have time to do the same, although I know it's going to be more challenging for you as you're aiming at a moving target.

[hoihu]

so far I noted handlers for get/set line state,

I pushed an update with several bug fixes. And there are now handlers to set line coding and control line state. I tested it with pyserial and opening the com port with various baud rates / stop bits etc. I also added a repr for CDCControlInterface that prints the most relevant settings. Example:

>> print(ctrl_cdc)
38400/8/E/1 rts=True dtr=True 

If you could have a quick look into it, would be great.

[hoihu]

I was also looking into supporting SERIAL_STATE. But I did not figure out how I can send a notification... I tried something along these lines:

    def _send_serial_state(self):
        # collect cts, dcd, dsr and send over endpoint
        # as notification to the host
        data = self.dcd | self.dsr << 1 | self.cts << 4
        self.submit_xfer(self.ep_in, ustruct.pack('<BBHHHH',
                                                  0xa1,   # bmRequestType
                                                  _SERIAL_STATE,   # bNotification
                                                  0,
                                                  self.itf_id,
                                                  2,   # wLength
                                                  data))   # UART State Bitmap

but on pyserial the cts , dcd or dsr bits still show up as False, regardless what the settings was. @projectgus do you have any clue where the issue is?

[hoihu]

btw: line break should now also work

[hoihu]

chatGPT tells me notifications are control requests. In the comment here:

def _submit_xfer(self, ep_addr, data, done_cb=None):
        # Singleton function to submit a USB transfer (of any type except control).

says "except control"... so maybe control transfers cannot be sent from device -> host?

[projectgus]

Great progress, @hoihu !

chatGPT tells me notifications are control requests.

ChatGPT might have been hallucinating, I think. 😁 .

As per CDC spec chapter "3.4.1 Communications Class Interface", the "notification element" is an additional optional endpoint, either interrupt or bulk in endpoint. So provided _send_serial_state is implemented on the control interface (with the existing interrupt endpoint), that looks broadly right to me. I'll try it out as soon as I have some time.

[projectgus]

@hoihu I've cherry-picked your changes for "line coding state" and "line state" into https://github.com/projectgus/micropython-lib/tree/feature/usbd_python_cdc, thanks for those! There's an additional commit there that changes the API a bit, to work towards the single "CDC" object as the public interface.

Basic testing is working fine for me now, but I haven't started testing timeouts or trying to integrate with async code yet.

I was also looking into supporting SERIAL_STATE

This looks quite neat, but to be totally honest I'm not sure how much demand there is for it. The TinyUSB cdc_device.c driver doesn't support it at all, for example.

If there are use cases then it might end up being worth splitting the CDC support into a "minimal" CDC that's really just a pipe to push bytes back and forth, and a "full" CDC that has all of the additional state handlers.