From 4e2ea2d96e62a5c15c196787f816107d0a6c1e45 Mon Sep 17 00:00:00 2001 From: Kurt Eckhardt Date: Thu, 19 Oct 2017 14:57:52 -0700 Subject: [PATCH] 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 | 41 ++++++++++++++++++++++++++++++++-------- antplus.cpp | 1 + enumeration.cpp | 21 +++++++++++--------- examples/Mouse/Mouse.ino | 36 ++++++++++++++++++++++++++++++++--- hid.cpp | 1 + hub.cpp | 1 + keyboard.cpp | 1 + memory.cpp | 30 ++++++++++++++++++++++++++++- midi.cpp | 1 + serial.cpp | 1 + 10 files changed, 113 insertions(+), 21 deletions(-) diff --git a/USBHost_t36.h b/USBHost_t36.h index 25f3950..d44deeb 100644 --- a/USBHost_t36.h +++ b/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; diff --git a/antplus.cpp b/antplus.cpp index e818e1f..dd48306 100644 --- a/antplus.cpp +++ b/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; } diff --git a/enumeration.cpp b/enumeration.cpp index 7775fae..d280706 100644 --- a/enumeration.cpp +++ b/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; } diff --git a/examples/Mouse/Mouse.ino b/examples/Mouse/Mouse.ino index 0bbe298..14955a9 100644 --- a/examples/Mouse/Mouse.ino +++ b/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()); diff --git a/hid.cpp b/hid.cpp index 5a027db..2d320b9 100644 --- a/hid.cpp +++ b/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); } diff --git a/hub.cpp b/hub.cpp index a86547b..e3421f0 100644 --- a/hub.cpp +++ b/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); } diff --git a/keyboard.cpp b/keyboard.cpp index 5dbc92c..2fb11a8 100644 --- a/keyboard.cpp +++ b/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); } diff --git a/memory.cpp b/memory.cpp index 2be4ed6..c325fb3 100644 --- a/memory.cpp +++ b/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); + } +} + + diff --git a/midi.cpp b/midi.cpp index 7e5723e..83068c4 100644 --- a/midi.cpp +++ b/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; diff --git a/serial.cpp b/serial.cpp index 7f9e9b7..e6a8989 100644 --- a/serial.cpp +++ b/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); }