vpelletier / python-functionfs

Pythonic API for linux's functionfs
GNU General Public License v3.0
40 stars 13 forks source link

composite gadget with generic bulk in/out and acm on raspberry pi zero w #5

Closed monkey-jsun closed 6 years ago

monkey-jsun commented 6 years ago

Hi, all,

This is a great stuff! I've been looking for something like this in the past a few days. Thanks.

I tried the test program and it works like a charm.

Like the subject says, my goal is to have a composite gadget with generic bulk in/out endpoints and ACM. I tried to add acm function by adding the following configfs commands:

mkdir -p functions/acm.usb0 ln -s functions/acm.usb0 configs/c.1/

However, after adding these statements, 'ls /sys/class/udc > UDC' would fail. Take either one out, it would succeed. Apparent they don't like each other. Any ideas?

Also, another thing is really puzzling to me. While I do have add CONFIG_USB_FUNCTIONFS in order for the test program to work, I don't have to add CONFIG_USB_CONFIGFS to kernel. However, I do have to add the following to /boot/config.txt file though.

dtoverlay=dwc2

Any thoughts? Really appreciate it.

My kernel is v4.9.61.

vpelletier commented 6 years ago

Hello,

These questions look like a general issue with functionfs/configfs/geting USB gadget to work, rather than some issue specific to this python library which rather helps declaring descriptors and implementing the function. So while the theme is close enough, I may not be the best source of advice.

Apparent they don't like each other.

IIRC (not played with gadget in a while), the former makes the function available to local kernel, while the latter actually exposes it to usb as part of a configuration. So the former alone working and the function not being actually be exposed to host would make perfect sense. The latter working alone would be more surprising (I would be less surprised if the kernel refused to create the link), but maybe it ignores broken symlinks.

I don't have to add CONFIG_USB_CONFIGFS

The kernel I built for my intel edison has both as modules, but AFAIK CONFIG_USB_FUNCTIONFS is deprecated. Checking loaded modules after runing the first batch of shell commands for the test, I see usb_f_fs module loaded, but not g_ffs (which comes from CONFIG_USB_FUNCTIONFS):

# lsmod | grep fs
usb_f_fs               32768  3
libcomposite           49152  10 usb_f_fs
udc_core               40960  3 usb_f_fs,libcomposite,dwc3
configfs               32768  3 usb_f_fs,libcomposite

Relevant .config chunk:

CONFIG_USB_GADGET=m
CONFIG_USB_GADGET_VBUS_DRAW=2
CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS=2
CONFIG_USB_DUMMY_HCD=m
CONFIG_USB_LIBCOMPOSITE=m
CONFIG_USB_F_ACM=m
CONFIG_USB_F_SS_LB=m
CONFIG_USB_U_SERIAL=m
CONFIG_USB_U_ETHER=m
CONFIG_USB_F_SERIAL=m
CONFIG_USB_F_OBEX=m
CONFIG_USB_F_NCM=m
CONFIG_USB_F_ECM=m
CONFIG_USB_F_EEM=m
CONFIG_USB_F_SUBSET=m
CONFIG_USB_F_RNDIS=m
CONFIG_USB_F_MASS_STORAGE=m
CONFIG_USB_F_FS=m
CONFIG_USB_F_HID=m
CONFIG_USB_F_PRINTER=m
CONFIG_USB_CONFIGFS=m
CONFIG_USB_CONFIGFS_SERIAL=y
CONFIG_USB_CONFIGFS_ACM=y
CONFIG_USB_CONFIGFS_OBEX=y
CONFIG_USB_CONFIGFS_NCM=y
CONFIG_USB_CONFIGFS_ECM=y
CONFIG_USB_CONFIGFS_ECM_SUBSET=y
CONFIG_USB_CONFIGFS_RNDIS=y
CONFIG_USB_CONFIGFS_EEM=y
CONFIG_USB_CONFIGFS_MASS_STORAGE=y
CONFIG_USB_CONFIGFS_F_LB_SS=y
CONFIG_USB_CONFIGFS_F_FS=y
CONFIG_USB_CONFIGFS_F_HID=y
CONFIG_USB_CONFIGFS_F_PRINTER=y
CONFIG_USB_FUNCTIONFS=m
CONFIG_USB_FUNCTIONFS_GENERIC=y

dtoverlay=dwc2

I do not own a rpi-zero, so although I do have a few rpis and toyed with devicetree overlays, take the following with a grain of salt.

This makes the raspberry pi kernel use the dwc2 kernel module instead of the default one. I believe the dwc2 module to be more feature-complete, while the default one takes advantage of some ARM-specific kind-of-DMA, making it faster than dwc2 on rpis.

dwc2 may be (I did not check !) the only module exposing UDC (USB device controler) feature, while default one may be host-only (really, just guessing, please do check). If this is correct, it seems a reasonable step, and quite independent from functionfs/configfs settings.

monkey-jsun commented 6 years ago

Thanks for your reply, Vincent.

I can confirm that after I turned on CONFIG_USB_CONFIGFS and its related macros (e.g., CONFIG_USB_CONFIGFS_F_FS etc) and turned off all built-in gadget functions, the test still works. Here is my related kernel configs:

CONFIG_USB_GADGET=y CONFIG_USB_CONFIGFS=y CONFIG_USB_CONFIGFS_SERIAL=y CONFIG_USB_CONFIGFS_ACM=y CONFIG_USB_CONFIGFS_RNDIS=y CONFIG_USB_CONFIGFS_EEM=y CONFIG_USB_CONFIGFS_F_LB_SS=y CONFIG_USB_CONFIGFS_F_FS=y

Right now I'm trying to implement something really simple like the synchronous chat device program in https://github.com/kopasiak/simple_usb_chat/tree/master/device_synch_chat. How should I do the same with your framework? Your test program seems a little too complicated to deduce that. Would appreciate your pointers.

vpelletier commented 6 years ago

Thanks for testing, README updated in 36e4c1cf63dc3f43e4c8881a25c299544e4efad4 .

About sync chat example, a very quick read of the C source makes me feel it should be rather easy to translate it to python: lines up to 211 are descriptor data and stuff implemented in python-functionfs (writing descriptors to ep0, opening produced endpoint files), which should become similar to FunctionFSTestDevice.__init__ in the test, minus the "guess how many endpoint I can allocate" code. handle_ep0 (handling events from ep0) is also implemented in python-functionfs, and as no special usb requests are implemented you will not have to customise ep0 handler.

What remains is:

This should produce fairly short code (gut feeling: under 150 lines, most of which will be descriptor declaration). Threads may be an unnecessary complication, although they may make blocking IO more forgiving (ie, rather than having the program stuck, you'll get a "poping from empty queue" error or so).

[EDIT] You may need to override onEnable method, to start your threads. In my test, I wanted the code to survive multiple enable/disable event pairs without having to re-spawn the threads, so I added the run_lock mechanism so they do not actually exit (as python refuses to restart exited threads).

monkey-jsun commented 6 years ago

Thanks, Vincent. I'm trying to morph your devce.py into a simple chat program.

I'm sorry but I still have a few things I don't quite understand about your test program.

How does read/write happen on endpoints? In your test program, I don't see a simple read() or write(). Instead there is a vague reference to method(), which is not clear to me how I can use it for read/write.

Also, what echo_buf and echo payload related here?

Would really appreciate. I hope I can complete chat and contribute back to your framework.

monkey-jsun commented 6 years ago

EDIT: please ignore this question. It turns out the other program mysterious add 2 to message body length, which I did not notice. After fixing this extra 2 byte issue, things seem to work.

For whatever it is worth, I muscled my way through and managed to receive first message from host (using the C based host chat program). Currently I'm blocked sending size back to host. It seems host always receives wrong size.

See my modified main part of device.py below.

def main(path):
    with FunctionFSTestDevice(path) as function:
        print('Servicing functionfs events forever...')
        try:
            inputfs = function.getEndpoint(2)
            outputfs = function.getEndpoint(1)
            #outputfs = open("/mnt/ep1", "wb")
            request=inputfs.read(2)
            [size]=struct.unpack("H",request)
            print("message size is %d" %(size))
            message = inputfs.read(size)
            print("message body is %s" %(message.decode("utf-8")))

            #function.processEventsForever()
            #function.processEvents()

            buf = struct.pack("<H", 3)
            print(byte2hex(buf))
            ret=outputfs.write(buf)
            print("write size = %d" % (ret))
            buf = b"ok\x00"
            print(byte2hex(buf))
            outputfs.write(buf)
        except KeyboardInterrupt:
            pass

Output from host

sudo ./host_schat 
Chat started. You may say something or type \exit to exit...
host> hello
jsun : message send size is 8
jsun : message receive size is 1
Unable to receive message
Unable to receive message

Output from device console

message size is 8
message body is hello
0300
write size = 2
6f6b00
vpelletier commented 6 years ago

One difference I noticed is that C code adds 2 to length before sending. Which can explain why host prints 1 when you send 3.

Then, as host requests a one-byte read but receives 3 bytes, libusb must be erroring, saying it discarded bytes (LIBUSB_ERROR_OVERFLOW).

AFAIK host should provide a buffer always (an integer multiple of) the size of endpoint transfer size, and handle truncation only after, taking into account both the number of bytes actually received at usb level and what it expexted to receive.

monkey-jsun commented 6 years ago

Vincent, now that I got a basic code working, I start to think doing it properly. Here is my thought.

  1. Having a dedicated thread to call processEventsForever()
  2. Having a dedicated thread to handle each endpoint and provide an abstraction for sending and receiving.

2 is not necessary, but probably good for full duplex case and localizing exception handling. It will make the sample code more useful and reusable.

What do you think?

vpelletier commented 6 years ago

Actually, I started implementing this example app as a test for libaio/eventfd development from #6, so I should be able to show you how I would implement it. Which includes moving more code into python-functionfs to avoid repeating descriptors 3 times for simple setups.

monkey-jsun commented 6 years ago

That is great. When can we have a preview?

On Nov 21, 2017 3:55 AM, "Vincent Pelletier" notifications@github.com wrote:

Actually, I started implementing this example app as a test for libaio/eventfd development from #8, so I should be able to show you hiw I would implement it. Which includes moving more code into python-functionfs to avoid repeating descriptors 3 times for simple setups.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vpelletier/python-functionfs/issues/5#issuecomment-346004651, or mute the thread https://github.com/notifications/unsubscribe-auth/ALvYW0hUCkjdjoZj7wJqihTshUBXJLxSks5s4roUgaJpZM4QbNsS .

vpelletier commented 6 years ago

Just pushed it.

It is not protocol-compatible with the chat program (no length prefix), instead it became a minimal netcat-ish pair of programs, piping their stdin to one endpoint and another endpoint to their stdout. I only did minimal testing, and especially no performance testing.

I still have to report a kernel panic I sumbled upon while working on this (something about cancelling AIO operations inside dwc3 gadget driver - conditions are not very clear yet).

vpelletier commented 6 years ago

Closing old issues.