Add supervisor.set_usb_identification(manufacturer, product, vid, pid) function

[s-ol]

This adds supervisor.set_usb_identification(manufacturer: Optional[str] = None, product: Optional[str] = None, vid: int = -1, pid: int = -1)

This function can be called to override the build-type settings for USB_VID, USB_PID, USB_MANUFACTURER_NAME, USB_PRODUCT_NAME for the next boot process from boot.py.

Closes #4404

[dhalbert]

Thanks for taking on this work! A number of people have asked for it.

I made some detailed comments. I would also note that we may want to get this to fit on the SAMD21 builds, which are very tight on code space for right now. If that's not possible, we need to make this optional to save the code space.

[s-ol]

@dhalbert do you have any idea what that could be about? Are those last two bytes just enough to hit a stack size limit maybe?

For reference, I'm testing this on the solderparty RP2040 stamp. I don't have anything set up to debug the crash currently - USB doesn't come up so I just know that it "didn't work".

[dhalbert]

@dhalbert do you have any idea what that could be about? Are those last two bytes just enough to hit a stack size limit maybe?

I don't know, but I was going to look later. I think there may be a write past the end of something. Your shorter ones may be working by accident. Also, does odd vs even size make a difference (e.g. 128 vs 127?)

[s-ol]

Also, does odd vs even size make a difference (e.g. 128 vs 127?)

Ah, that seems to be it actually! At 128 it works. Increasing CIRCUITPY_DEFAULT_STACK_SIZE didn't help. Not sure what this means? Is this some alignment thing?

[dhalbert]

Are these buffers where the UTF-16 values go? Then I'd think they need to be a multiple of two bytes.

[s-ol]

No, these buffers are used to hold the "unexpanded" ASCII values. They are memcpyd into from mp_buffer_info_t.buf and then the whole usb_identification_t struct is memcpyd again into the temporary storage in main.c. The ascii data is then read bytewise by pointer in usb_desc.c.

[s-ol]

I've fixed a wrong sizeof() as indicated by some builds, and also this alignment issue pointed out by the old 8086_commander CI build:

../../supervisor/shared/usb/usb.c: In function 'usb_return_boot_py_data':
../../supervisor/shared/usb/usb.c:161:27: error: cast increases required alignment of target type [-Werror=cast-align]
  161 |     usb_build_descriptors((usb_identification_t *)temp_storage);
      |                           ^

But it still will only work with even lengths. I'm not sure how to diagnose this further, but I actually just realized that the length of 128 makes perfect sense: the final USB descriptor can hold 127 UTF-16 chars, but in this phase we actually need space for a null byte (since we are not storing the size out-of-band).

I guess that mp_get_buffer does not guarantee null termination of the content? This seems to be the correct way to ensure zero-termination:

    mp_get_buffer_raise(args.manufacturer.u_obj, &info, MP_BUFFER_READ);
    mp_arg_validate_length_range(info.len, 0, 127, MP_QSTR_manufacturer);
    memcpy(identification->manufacturer_name, info.buf, info.len);
    identification->manufacturer_name[info.len] = 0;

[s-ol]

It actually turns out that the maximum length of USB descriptor strings is 126 characters. Since the USB descriptor size field is only one byte, the maximum length of the whole descriptor is actually 255 and not 256 bytes as I had wrongly assumed so far.

[s-ol]

Hey @dhalbert, this is ready functionality-wise at this point; it does look like I pushed some builds over the edge though (~50 bytes too much for most failures). There are some that seem a bit odd though, for example trinket_m0_haxpress at 732 bytes over?

As you mentioned above, this probably means that this feature should go behind a feature flag?

[dhalbert]

I have some ideas for post-7.3.0 for shrinking some builds a bit by consolidating similar error messages, but yes, we probably need to conditionalize it.

[dhalbert]

@s-ol I have not forgotten this, but am deferring it until after I finish working on 7.3.0 final.

[dhalbert]

In the long run, this functionality is probably better put in the usb module (which right now has a somewhat different charter), but we can do a transition for that across major versions.

[s-ol]

In the long run, this functionality is probably better put in the usb module

Yeah, I think we discussed it in a line-comment thread above originally. It definitely would fit in a usb. namespace, but since the usb module right now is for a host api that would require some restructuring there and I'm not sure if and how PyUSB api-compatibility would work.

addressed

[dhalbert]

OK, will merge after the latest tweaks I made!

Tested this on Linux with a Metro M4, using lsusb. One interesting thing is that the manufacturer string is overriden by a known VID name. I had to try several before I got the mfr name I supplied to go through (e.g. try 0x0001 for the VID, and you'll get "Fry's Electronics")

Thank you @s-ol, and sorry for the delay!