String buffers - Devices contribute buffers

instead of having each HUB have 7 buffers, which can eat up space.  We have each main object contribute currently one string buffer, which than when we initialize a Device_t we try to allocate one for it, likewise we release it when the Device is released.

Hopefully less memory needed.

Also updated such that the HIDInput classes can not retrieve these strings.

Changed test program to now also have list of HIDInput objects and when I detect a new one, I again print out info on it...

USBHost_t36.h

@@ -56,7 +56,7 @@
 // your best effort to read chapter 4 before asking USB questions!
 
 
-#define USBHOST_PRINT_DEBUG
+//#define USBHOST_PRINT_DEBUG
 
 /************************************************/
 /*  Data Types                                  */
@@ -140,13 +140,22 @@ typedef union {
  };
 } setup_t;
 
+typedef struct {
+	enum {STRING_BUF_SIZE=50};
+	enum {STR_ID_MAN=0, STR_ID_PROD, STR_ID_SERIAL, STR_ID_CNT};
+	uint8_t iStrings[STR_ID_CNT];	// Index into array for the three indexes
+	uint8_t buffer[STRING_BUF_SIZE];
+} strbuf_t;
+
+#define DEVICE_STRUCT_STRING_BUF_SIZE 50
+
 // Device_t holds all the information about a USB device
-#define DEVICE_STRUCT_STRING_BUF_SIZE 48
 struct Device_struct {
 	Pipe_t   *control_pipe;
 	Pipe_t   *data_pipes;
 	Device_t *next;
 	USBDriver *drivers;
+	strbuf_t *strbuf;
 	uint8_t  speed; // 0=12, 1=1.5, 2=480 Mbit/sec
 	uint8_t  address;
 	uint8_t  hub_address;
@@ -160,8 +169,6 @@ struct Device_struct {
 	uint16_t idVendor;
 	uint16_t idProduct;
 	uint16_t LanguageID;
-	uint8_t	 string_buf[DEVICE_STRUCT_STRING_BUF_SIZE]; // Probably want a place to allocate fewer of these...
-	uint8_t	 iStrings[3];			// 3 indexes - vendor string, product string, serial number. 
 };
 
 // Pipe_t holes all information about each USB endpoint/pipe
@@ -248,6 +255,7 @@ protected:
 	static void contribute_Devices(Device_t *devices, uint32_t num);
 	static void contribute_Pipes(Pipe_t *pipes, uint32_t num);
 	static void contribute_Transfers(Transfer_t *transfers, uint32_t num);
+	static void contribute_String_Buffers(strbuf_t *strbuf, uint32_t num);
 	static volatile bool enumeration_busy;
 private:
 	static void isr();
@@ -263,6 +271,8 @@ private:
 	static void free_Pipe(Pipe_t *q);
 	static Transfer_t * allocate_Transfer(void);
 	static void free_Transfer(Transfer_t *q);
+	static strbuf_t * allocate_string_buffer(void);
+	static void free_string_buffer(strbuf_t *strbuf);
 	static bool allocate_interrupt_pipe_bandwidth(Pipe_t *pipe,
 		uint32_t maxlen, uint32_t interval);
 	static void add_qh_to_periodic_schedule(Pipe_t *pipe);
@@ -356,9 +366,12 @@ public:
 	uint16_t idVendor() { return (device != nullptr) ? device->idVendor : 0; }
 	uint16_t idProduct() { return (device != nullptr) ? device->idProduct : 0; }
 
-	const uint8_t *manufacturer() { return (device != nullptr) ? &(device->string_buf[device->iStrings[0]]) : nullptr; }
-	const uint8_t *product() { return (device != nullptr) ? &(device->string_buf[device->iStrings[1]]) : nullptr; }
-	const uint8_t *serialNumber() { return (device != nullptr) ? &(device->string_buf[device->iStrings[2]]) : nullptr; }
+	const uint8_t *manufacturer() 
+		{  return  ((device == nullptr) || (device->strbuf == nullptr)) ? nullptr : &device->strbuf->buffer[device->strbuf->iStrings[strbuf_t::STR_ID_MAN]]; }
+	const uint8_t *product() 
+		{  return  ((device == nullptr) || (device->strbuf == nullptr)) ? nullptr : &device->strbuf->buffer[device->strbuf->iStrings[strbuf_t::STR_ID_PROD]]; }
+	const uint8_t *serialNumber()
+		{  return  ((device == nullptr) || (device->strbuf == nullptr)) ? nullptr : &device->strbuf->buffer[device->strbuf->iStrings[strbuf_t::STR_ID_SERIAL]]; }
 
 	// TODO: user-level functions
 	// check if device is bound/active/online
@@ -415,7 +428,6 @@ protected:
 	// wish to claim any device or interface (eg, if getting data
 	// from the HID parser).
 	Device_t *device;
-
 	friend class USBHost;
 };
 
@@ -445,6 +457,13 @@ public:
 	operator bool() { return (mydevice != nullptr); }
 	uint16_t idVendor() { return (mydevice != nullptr) ? mydevice->idVendor : 0; }
 	uint16_t idProduct() { return (mydevice != nullptr) ? mydevice->idProduct : 0; }
+	const uint8_t *manufacturer() 
+		{  return  ((mydevice == nullptr) || (mydevice->strbuf == nullptr)) ? nullptr : &mydevice->strbuf->buffer[mydevice->strbuf->iStrings[strbuf_t::STR_ID_MAN]]; }
+	const uint8_t *product() 
+		{  return  ((mydevice == nullptr) || (mydevice->strbuf == nullptr)) ? nullptr : &mydevice->strbuf->buffer[mydevice->strbuf->iStrings[strbuf_t::STR_ID_PROD]]; }
+	const uint8_t *serialNumber()
+		{  return  ((mydevice == nullptr) || (mydevice->strbuf == nullptr)) ? nullptr : &mydevice->strbuf->buffer[mydevice->strbuf->iStrings[strbuf_t::STR_ID_SERIAL]]; }
+
 private:
 	virtual bool claim_collection(Device_t *dev, uint32_t topusage);
 	virtual void hid_input_begin(uint32_t topusage, uint32_t type, int lgmin, int lgmax);
@@ -508,6 +527,7 @@ private:
 	Device_t mydevices[MAXPORTS];
 	Pipe_t mypipes[2] __attribute__ ((aligned(32)));
 	Transfer_t mytransfers[4] __attribute__ ((aligned(32)));
+	strbuf_t mystring_bufs[1];
 	USBDriverTimer debouncetimer;
 	USBDriverTimer resettimer;
 	setup_t setup;
@@ -572,6 +592,7 @@ private:
 	bool use_report_id;
 	Pipe_t mypipes[3] __attribute__ ((aligned(32)));
 	Transfer_t mytransfers[4] __attribute__ ((aligned(32)));
+	strbuf_t mystring_bufs[1];
 };
 
 
@@ -634,6 +655,7 @@ private:
 	bool processing_new_data_ = false;
 	Pipe_t mypipes[2] __attribute__ ((aligned(32)));
 	Transfer_t mytransfers[4] __attribute__ ((aligned(32)));
+	strbuf_t mystring_bufs[1];
 };
 
 class MIDIDevice : public USBDriver {
@@ -777,6 +799,7 @@ private:
 	void (*handleTimeCodeQuarterFrame)(uint16_t data);
 	Pipe_t mypipes[3] __attribute__ ((aligned(32)));
 	Transfer_t mytransfers[7] __attribute__ ((aligned(32)));
+	strbuf_t mystring_bufs[1];
 };
 
 
@@ -810,6 +833,7 @@ private:
 private:
 	Pipe_t mypipes[3] __attribute__ ((aligned(32)));
 	Transfer_t mytransfers[7] __attribute__ ((aligned(32)));
+	strbuf_t mystring_bufs[1];
 	USBDriverTimer txtimer;
 	uint32_t bigbuffer[(BUFFER_SIZE+3)/4];
 	setup_t setup;
@@ -858,6 +882,7 @@ private:
 private:
 	Pipe_t mypipes[2] __attribute__ ((aligned(32)));
 	Transfer_t mytransfers[3] __attribute__ ((aligned(32)));
+	strbuf_t mystring_bufs[1];
 	//USBDriverTimer txtimer;
 	USBDriverTimer updatetimer;
 	Pipe_t *rxpipe;

antplus.cpp

@@ -53,6 +53,7 @@ void AntPlus::init()
 {
 	contribute_Pipes(mypipes, sizeof(mypipes)/sizeof(Pipe_t));
 	contribute_Transfers(mytransfers, sizeof(mytransfers)/sizeof(Transfer_t));
+	contribute_String_Buffers(mystring_bufs, sizeof(mystring_bufs)/sizeof(strbuf_t));
 	driver_ready_for_device(this);
 	callbackFunc = NULL;
 }

enumeration.cpp

@@ -119,6 +119,7 @@ Device_t * USBHost::new_Device(uint32_t speed, uint32_t hub_addr, uint32_t hub_p
 		free_Device(dev);
 		return NULL;
 	}
+	dev->strbuf = allocate_string_buffer();  // try to allocate a string buffer; 
 	dev->control_pipe->callback_function = &enumeration;
 	dev->control_pipe->direction = 1; // 1=IN
 	// Here is where the enumeration process officially begins.
@@ -203,10 +204,6 @@ void USBHost::enumeration(const Transfer_t *transfer)
 			dev->enum_state = 4;
 			return;
 		case 4: // parse Language ID
-			dev->iStrings[0] = 0;  // Set indexes into string buffer to say not there...
-			dev->iStrings[1] = 0;
-			dev->iStrings[2] = 0;
-			dev->string_buf[0] = 0;	// have trailing NULL..
 			if (enumbuf[4] < 4 || enumbuf[5] != 3) {
 				dev->enum_state = 11;
 			} else {
@@ -294,28 +291,31 @@ void USBHost::enumeration(const Transfer_t *transfer)
 }
 
 void  USBHost::convertStringDescriptorToASCIIString(uint8_t string_index, Device_t *dev, const Transfer_t *transfer) {
+	strbuf_t *strbuf = dev->strbuf; 
+	if (!strbuf) return;	// don't have a buffer
+
 	uint8_t *buffer = (uint8_t*)transfer->buffer;
-	uint8_t buf_index = string_index? dev->iStrings[string_index]+1 : 0;
+	uint8_t buf_index = string_index? strbuf->iStrings[string_index]+1 : 0;
 
 	// Try to verify - The first byte should be length and the 2nd byte should be 0x3
 	if (!buffer || (buffer[1] != 0x3)) {
 		return;	// No string so can simply return
 	}
 
-	dev->iStrings[string_index] = buf_index;	// remember our starting positio
+	strbuf->iStrings[string_index] = buf_index;	// remember our starting positio
 	uint8_t count_bytes_returned = buffer[0];
 	if ((buf_index + count_bytes_returned/2) >= DEVICE_STRUCT_STRING_BUF_SIZE)
 		count_bytes_returned = (DEVICE_STRUCT_STRING_BUF_SIZE - buf_index) * 2;
 
 	// Now copy into our storage buffer. 
 	for (uint8_t i = 2; (i < count_bytes_returned) && (buf_index < (DEVICE_STRUCT_STRING_BUF_SIZE -1)); i += 2) {
-		dev->string_buf[buf_index++] = buffer[i];
+		strbuf->buffer[buf_index++] = buffer[i];
 	} 
-	dev->string_buf[buf_index] = 0;	// null terminate. 
+	strbuf->buffer[buf_index] = 0;	// null terminate. 
 
 	// Update other indexes to point to null character
 	while (++string_index < 3) {
-		dev->iStrings[string_index] = buf_index;	// point to trailing NULL character
+		strbuf->iStrings[string_index] = buf_index;	// point to trailing NULL character
 	}
 }
 
@@ -461,6 +461,9 @@ void USBHost::disconnect_Device(Device_t *dev)
 				prev_dev->next = p->next;
 			}
 			println("removed Device_t from devlist");
+			if (p->strbuf != nullptr ) {
+				free_string_buffer(p->strbuf);
+			}
 			free_Device(p);
 			break;
 		}

examples/Mouse/Mouse.ino

@@ -8,6 +8,7 @@ USBHost myusb;
 USBHub hub1(myusb);
 USBHub hub2(myusb);
 USBHub hub3(myusb);
+USBHub hub4(myusb);
 KeyboardController keyboard1(myusb);
 KeyboardController keyboard2(myusb);
 KeyboardHIDExtrasController hidextras(myusb);
@@ -19,15 +20,23 @@ USBHIDParser hid5(myusb);
 MouseController mouse1(myusb);
 JoystickController joystick1(myusb);
 
-USBDriver *drivers[] = {&hub1, &hub2, &hub3, &keyboard1, &keyboard2, &hid1, &hid2, &hid3, &hid4, &hid5};
-#define CNT_DEVICES (sizeof(drivers)/sizeof(drivers[1]))
-const char * driver_names[CNT_DEVICES] = {"Hub1","Hub2","Hub3", "KB1", "KB2", "HID1", "HID2", "HID3", "HID4", "HID5" };
+USBDriver *drivers[] = {&hub1, &hub2, &hub3, &hub4, &keyboard1, &keyboard2, &hid1, &hid2, &hid3, &hid4, &hid5};
+#define CNT_DEVICES (sizeof(drivers)/sizeof(drivers[0]))
+const char * driver_names[CNT_DEVICES] = {"Hub1","Hub2", "Hub3", "Hub4" "KB1", "KB2", "HID1", "HID2", "HID3", "HID4", "HID5" };
 bool driver_active[CNT_DEVICES] = {false, false, false, false};
 
+// Lets also look at HID Input devices
+USBHIDInput *hiddrivers[] = {&mouse1, &joystick1};
+#define CNT_HIDDEVICES (sizeof(hiddrivers)/sizeof(hiddrivers[0]))
+const char * hid_driver_names[CNT_DEVICES] = {"Mouse1","Joystick1"};
+bool hid_driver_active[CNT_DEVICES] = {false, false};
+
+
 void setup()
 {
   while (!Serial) ; // wait for Arduino Serial Monitor
   Serial.println("\n\nUSB Host Testing");
+  Serial.println(sizeof(USBHub), DEC);
   myusb.begin();
   keyboard1.attachPress(OnPress);
   keyboard2.attachPress(OnPress);
@@ -59,6 +68,27 @@ void loop()
     }
   }
 
+  for (uint8_t i = 0; i < CNT_HIDDEVICES; i++) {
+    if (*hiddrivers[i] != hid_driver_active[i]) {
+      if (hid_driver_active[i]) {
+        Serial.printf("*** HID Device %s - disconnected ***\n", hid_driver_names[i]);
+        hid_driver_active[i] = false;
+      } else {
+        Serial.printf("*** HID Device %s %x:%x - connected ***\n", hid_driver_names[i], hiddrivers[i]->idVendor(), hiddrivers[i]->idProduct());
+        hid_driver_active[i] = true;
+
+        const uint8_t *psz = hiddrivers[i]->manufacturer();
+        if (psz && *psz) Serial.printf("  manufacturer: %s\n", psz);
+        psz = hiddrivers[i]->product();
+        if (psz && *psz) Serial.printf("  product: %s\n", psz);
+        psz = hiddrivers[i]->serialNumber();
+        if (psz && *psz) Serial.printf("  Serial: %s\n", psz);
+      }
+    }
+  }
+
+
+
   if(mouse1.available()) {
     Serial.print("Mouse: buttons = ");
     Serial.print(mouse1.getButtons());

hid.cpp

@@ -39,6 +39,7 @@ void USBHIDParser::init()
 {
 	contribute_Pipes(mypipes, sizeof(mypipes)/sizeof(Pipe_t));
 	contribute_Transfers(mytransfers, sizeof(mytransfers)/sizeof(Transfer_t));
+	contribute_String_Buffers(mystring_bufs, sizeof(mystring_bufs)/sizeof(strbuf_t));
 	driver_ready_for_device(this);
 }
 

hub.cpp

@@ -37,6 +37,7 @@ void USBHub::init()
 	contribute_Devices(mydevices, sizeof(mydevices)/sizeof(Device_t));
 	contribute_Pipes(mypipes, sizeof(mypipes)/sizeof(Pipe_t));
 	contribute_Transfers(mytransfers, sizeof(mytransfers)/sizeof(Transfer_t));
+	contribute_String_Buffers(mystring_bufs, sizeof(mystring_bufs)/sizeof(strbuf_t));
 	driver_ready_for_device(this);
 }
 

keyboard.cpp

@@ -96,6 +96,7 @@ void KeyboardController::init()
 {
 	contribute_Pipes(mypipes, sizeof(mypipes)/sizeof(Pipe_t));
 	contribute_Transfers(mytransfers, sizeof(mytransfers)/sizeof(Transfer_t));
+	contribute_String_Buffers(mystring_bufs, sizeof(mystring_bufs)/sizeof(strbuf_t));
 	driver_ready_for_device(this);
 }
 

memory.cpp

@@ -53,7 +53,7 @@
 static Device_t * free_Device_list = NULL;
 static Pipe_t * free_Pipe_list = NULL;
 static Transfer_t * free_Transfer_list = NULL;
-
+static strbuf_t * free_strbuf_list = NULL;
 // A small amount of non-driver memory, just to get things started
 // TODO: is this really necessary?  Can these be eliminated, so we
 // use only memory from the drivers?
@@ -107,6 +107,25 @@ void USBHost::free_Transfer(Transfer_t *transfer)
 	free_Transfer_list = transfer;
 }
 
+strbuf_t * USBHost::allocate_string_buffer(void)
+{
+	strbuf_t *strbuf = free_strbuf_list;
+	if (strbuf) {
+		free_strbuf_list = *(strbuf_t **)strbuf;
+		strbuf->iStrings[strbuf_t::STR_ID_MAN] = 0;  // Set indexes into string buffer to say not there...
+		strbuf->iStrings[strbuf_t::STR_ID_PROD] = 0;
+		strbuf->iStrings[strbuf_t::STR_ID_SERIAL] = 0;
+		strbuf->buffer[0] = 0;	// have trailing NULL..
+	} 
+	return strbuf;
+}
+
+void USBHost::free_string_buffer(strbuf_t *strbuf) 
+{
+	*(strbuf_t **)strbuf = free_strbuf_list;
+	free_strbuf_list = strbuf;
+}
+
 void USBHost::contribute_Devices(Device_t *devices, uint32_t num)
 {
 	Device_t *end = devices + num;
@@ -132,3 +151,12 @@ void USBHost::contribute_Transfers(Transfer_t *transfers, uint32_t num)
 	}
 }
 
+void USBHost::contribute_String_Buffers(strbuf_t *strbufs, uint32_t num)
+{
+	strbuf_t *end = strbufs + num;
+	for (strbuf_t *str = strbufs ; str < end; str++) {
+		free_string_buffer(str);
+	}
+}
+
+

midi.cpp

@@ -31,6 +31,7 @@ void MIDIDevice::init()
 {
 	contribute_Pipes(mypipes, sizeof(mypipes)/sizeof(Pipe_t));
 	contribute_Transfers(mytransfers, sizeof(mytransfers)/sizeof(Transfer_t));
+	contribute_String_Buffers(mystring_bufs, sizeof(mystring_bufs)/sizeof(strbuf_t));
 	handleNoteOff = NULL;
 	handleNoteOn = NULL;
 	handleVelocityChange = NULL;

serial.cpp

@@ -35,6 +35,7 @@ void USBSerial::init()
 {
 	contribute_Pipes(mypipes, sizeof(mypipes)/sizeof(Pipe_t));
 	contribute_Transfers(mytransfers, sizeof(mytransfers)/sizeof(Transfer_t));
+	contribute_String_Buffers(mystring_bufs, sizeof(mystring_bufs)/sizeof(strbuf_t));
 	driver_ready_for_device(this);
 }