provide unique USB serial number

[Ho-Ro]

inspired by erikkaashoek/tinySA#85 create a 12 char string based on the Arduino algorithm https://github.com/limbongofficial/STM32_Core-Arduino/blob/master/cores/arduino/stm32/usb/usbd_desc.c#L326-L370

[DiSlord]

Most good solution generate Serial number on ask inside USB driver (not need ~huge buffer in RAM)
also for generation better use plot_printf:
plot_printf(buf, "%04x%08x", (*(uint32_t *)0x1FFFF7AC + *(uint32_t *)0x1FFFF7B4)&0xFFFF, *(uint32_t *)0x1FFFF7B0);

or use any function for convert digit to different radix (in your solution used radix = 16 for hex)

[DiSlord]

usb serial string ID is here:

/*
 * USB Device Descriptor.
 */
static const uint8_t vcom_device_descriptor_data[18] = {
  USB_DESC_DEVICE       (0x0110,        /* bcdUSB (1.1).                    */
                         0x02,          /* bDeviceClass (CDC).              */
                         0x00,          /* bDeviceSubClass.                 */
                         0x00,          /* bDeviceProtocol.                 */
                         0x40,          /* bMaxPacketSize.                  */
                         0x0483,        /* idVendor (ST).                   */
                         0x5740,        /* idProduct.                       */
                         0x0200,        /* bcdDevice.                       */
                         1,             /* iManufacturer.                   */
                         2,             /* iProduct.                        */
                         3,             /* iSerialNumber.                   */  <<--here is string ID
                         1)             /* bNumConfigurations.              */
};

after host ask this string and deivce process it in:
device_handler (ask USB_REQ_GET_DESCRIPTOR)
function call
get_descriptor from

const USBConfig usbcfg = {
  usb_event,
  get_descriptor,
  sduRequestsHook,
  sof_handler
};

/*
 * Handles the GET_DESCRIPTOR callback. All required descriptors must be
 * handled here.
 */
static const USBDescriptor *get_descriptor(USBDriver *usbp,
                                           uint8_t dtype,
                                           uint8_t dindex,
                                           uint16_t lang) {

  (void)usbp;
  (void)lang;
  switch (dtype) {
  case USB_DESCRIPTOR_DEVICE:
    return &vcom_device_descriptor;
  case USB_DESCRIPTOR_CONFIGURATION:
    return &vcom_configuration_descriptor;
  case USB_DESCRIPTOR_STRING:
    if (dindex < 4)
      return &vcom_strings[dindex];
  }
  return NULL;
}

dindex = 3 is ask serial string, you can use lcd_spi buffer (end) for generate serial and send

[Ho-Ro]

Good hint, please check my latest commit.
I did not use printf because the usb strings are using utf-16 wchar, so I do it step by step.

[DiSlord]

I add d5377c3
USB UID optional (selected in CONFIG->EXPERT SETTINGS->USB DEVICE UID

[Ho-Ro]

I add d5377c3

Line 207 looks strange, because spi_buffer is an array of uint16_t for -H and the sizeof operator returns the size in bytes

uint16_t *buf    = ((uint16_t *)&spi_buffer[sizeof(spi_buffer)]) - USB_SERIAL_STRING_SIZE - 4; // 16 byte align

And more strange - the compiler warns, but only when I set USE_LTO = no. Disabling LTO improves the stability of my -H, with USE_LTO = yes I get random freezes (also for previous versions since d768592). :(

usbcfg.c: In function 'getSerialStringDescriptor':
usbcfg.c:222:6: warning: array subscript 4080 is outside array bounds of 'pixel_t[2048]' {aka 'short unsigned int[2048]'} [-Warray-bounds]
  222 |   buf[0] = size | (USB_DESCRIPTOR_STRING << 8);
      |   ~~~^~~
In file included from usbcfg.c:18:
nanovna.h:1234:16: note: while referencing 'spi_buffer'
 1234 | extern pixel_t spi_buffer[SPI_BUFFER_SIZE];
      |                ^~~~~~~~~~

So IMHO line 207 should look like this:

uint16_t *buf    = ((uint16_t *)&spi_buffer[SPI_BUFFER_SIZE]) - USB_SERIAL_STRING_SIZE - 4; // 16 byte align

or

uint16_t *buf    = ((uint16_t *)&spi_buffer[ sizeof(spi_buffer) / sizeof(pixel_t) ]) - USB_SERIAL_STRING_SIZE - 4; // 16 byte align

[Ho-Ro]

In the next lines you use two times id1 and do not use id0 += id2!

  uint32_t id0 = *(uint32_t *)0x1FFFF7AC; // MCU id0 address
  uint32_t id1 = *(uint32_t *)0x1FFFF7B0; // MCU id1 address
  uint32_t id2 = *(uint32_t *)0x1FFFF7B4; // MCU id2 address
  uint64_t uid = id1;
  id0+= id2;
  uid = id1 | (uid<<32);                  // generate unique 64bit ID

[DiSlord]

Thanks, fixed 9f35ce5

[DiSlord]

About LTO, yes I see before strange NanoVNA H freeze (but not H4), after long time I enable this and run my H ~2-3 hours and no freeze, so I hope all good

Link Time Optimization (LTO) gives GCC the capability of dumping its internal representation (GIMPLE) to disk, so that all the different compilation units that make up a single executable can be optimized as a single module. This expands the scope of inter-procedural optimizations to encompass the whole program (or, rather, everything that is visible at link time).

[Ho-Ro]

Last question, why |= this can mask a lot of zeros in the sum of id0 and id2?

  uint64_t uid = id1;
  id0+= id2;
  uid|= id0 | (uid<<32);                  // generate unique 64bit ID

better, then id1 and id0+id2 do not interfere.

  uint64_t uid = id1;
  uid = (id0 + id2) | (uid<<32);          // generate unique 64bit ID

I close this PR here because it's solved now and mostly done.

[Ho-Ro]

With LTO it makes still problems with my -H (HW ver 3.4 w/o SD card). It freezes few minutes to ~1 hour after switching on (with or without any activity). Tested with arm gcc 8.2.1 and 9.2.1.
Only HW change: I have added a 32768 Hz clock XTAL to the CPU which gives a working RTC, but I suspect this may not be the cause of the random freezes.

Without LTO a random freeze can happen after one or two day idling at a stupid 5V power supply without any USB activities. This is known, see the issue #26.

[Ho-Ro]

And another small improvement, the radix5 encoding requires 64/5 = 12.8 (i.e. 13) char, your solution cuts the last 4 bits. My change adds the missing bits and aligns correctly for all string sizes: SerialNumber: MRTO7QH2N2D85

--- a/usbcfg.c
+++ b/usbcfg.c
@@ -201,17 +201,16 @@ static const USBDescriptor vcom_strings[] = {
 #ifdef __USB_UID__
 // Use unique serial string generated from MCU id
 #define UID_RADIX                5                 // Radix conversion constant (5 bit, use 0..9 and A..V)
-#define USB_SERIAL_STRING_SIZE  (64 / UID_RADIX)   // Result string size
+#define USB_SERIAL_STRING_SIZE  ((64 + UID_RADIX -1) / UID_RADIX)   // Result string size
 USBDescriptor *getSerialStringDescriptor(void) {
   uint16_t i;
-  uint16_t *buf    = ((uint16_t *)&spi_buffer[ARRAY_COUNT(spi_buffer)]) - USB_SERIAL_STRING_SIZE - 4; // 16 byte align
+  uint16_t *buf    = ((uint16_t *)&spi_buffer[ARRAY_COUNT(spi_buffer)]) - ((USB_SERIAL_STRING_SIZE + 3) & ~3); // 32 bit align
   USBDescriptor *d = ((USBDescriptor *)buf) - 1;
   uint32_t id0 = *(uint32_t *)0x1FFFF7AC; // MCU id0 address
   uint32_t id1 = *(uint32_t *)0x1FFFF7B0; // MCU id1 address
   uint32_t id2 = *(uint32_t *)0x1FFFF7B4; // MCU id2 address
   uint64_t uid = id1;
-  id0+= id2;
-  uid|= id0 | (uid<<32);                  // generate unique 64bit ID
+  uid = (id0 + id2) | (uid<<32);          // generate unique 64bit ID
   // Prepare serial string descriptor from 64 bit ID
   for(i = 1; i < USB_SERIAL_STRING_SIZE + 1; i++) {
     uint16_t c = uid & ((1<<UID_RADIX) - 1);

[Ho-Ro]

@DiSlord What is your opinion on my last suggestion? I will then also adjust my PR for the tinySA accordingly.