From a33cba585f485b68dca651c68b8023522040f468 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 17 May 2013 14:34:06 +1000 Subject: [PATCH 01/13] Allow USB product and manufacturer strings to be supplied in boards.txt --- boards.txt | 13 ++++--- cores/arduino/USBCore.cpp | 72 +++++++++++++++++++++++++-------------- 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/boards.txt b/boards.txt index 893e880..1ec00c0 100644 --- a/boards.txt +++ b/boards.txt @@ -188,10 +188,11 @@ leonardo.build.mcu=atmega32u4 leonardo.build.f_cpu=16000000L leonardo.build.vid=0x2341 leonardo.build.pid=0x8036 +leonardo.build.usb_product="Arduino Leonardo" leonardo.build.board=AVR_LEONARDO leonardo.build.core=arduino leonardo.build.variant=leonardo -leonardo.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} +leonardo.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} ############################################################## @@ -217,10 +218,11 @@ micro.build.mcu=atmega32u4 micro.build.f_cpu=16000000L micro.build.vid=0x2341 micro.build.pid=0x8037 +micro.build.usb_product=Arduino Micro micro.build.board=AVR_MICRO micro.build.core=arduino micro.build.variant=micro -micro.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} +micro.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} ############################################################## @@ -246,10 +248,11 @@ esplora.build.mcu=atmega32u4 esplora.build.f_cpu=16000000L esplora.build.vid=0x2341 esplora.build.pid=0x803c +esplora.build.usb_product=Arduino Esplora esplora.build.board=AVR_ESPLORA esplora.build.core=arduino esplora.build.variant=leonardo -esplora.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} +esplora.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} ############################################################## @@ -407,15 +410,15 @@ LilyPadUSB.bootloader.extended_fuses=0xce LilyPadUSB.bootloader.file=caterina-LilyPadUSB/Caterina-LilyPadUSB.hex LilyPadUSB.bootloader.unlock_bits=0x3F LilyPadUSB.bootloader.lock_bits=0x2F - LilyPadUSB.build.mcu=atmega32u4 LilyPadUSB.build.f_cpu=8000000L LilyPadUSB.build.vid=0x1B4F LilyPadUSB.build.pid=0x9208 +LilyPadUSB.build.usb_product=LilyPad USB LilyPadUSB.build.board=AVR_LILYPAD_USB LilyPadUSB.build.core=arduino LilyPadUSB.build.variant=leonardo -LilyPadUSB.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} +LilyPadUSB.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} ############################################################## diff --git a/cores/arduino/USBCore.cpp b/cores/arduino/USBCore.cpp index d3e0170..680cba7 100644 --- a/cores/arduino/USBCore.cpp +++ b/cores/arduino/USBCore.cpp @@ -39,8 +39,8 @@ volatile u8 RxLEDPulse; /**< Milliseconds remaining for data Rx LED pulse */ //================================================================== extern const u16 STRING_LANGUAGE[] PROGMEM; -extern const u16 STRING_IPRODUCT[] PROGMEM; -extern const u16 STRING_IMANUFACTURER[] PROGMEM; +extern const u8 STRING_PRODUCT[] PROGMEM; +extern const u8 STRING_MANUFACTURER[] PROGMEM; extern const DeviceDescriptor USB_DeviceDescriptor PROGMEM; extern const DeviceDescriptor USB_DeviceDescriptorA PROGMEM; @@ -49,31 +49,34 @@ const u16 STRING_LANGUAGE[2] = { 0x0409 // English }; -const u16 STRING_IPRODUCT[17] = { - (3<<8) | (2+2*16), -#if USB_PID == 0x8036 - 'A','r','d','u','i','n','o',' ','L','e','o','n','a','r','d','o' +#ifndef USB_PRODUCT +// Use a hardcoded product name if none is provided +#if USB_PID == 0x8036 +#define USB_PRODUCT "Arduino Leonardo" #elif USB_PID == 0x8037 - 'A','r','d','u','i','n','o',' ','M','i','c','r','o',' ',' ',' ' +#define USB_PRODUCT "Arduino Micro" #elif USB_PID == 0x803C - 'A','r','d','u','i','n','o',' ','E','s','p','l','o','r','a',' ' +#define USB_PRODUCT "Arduino Esplora" #elif USB_PID == 0x9208 - 'L','i','l','y','P','a','d','U','S','B',' ',' ',' ',' ',' ',' ' +#define USB_PRODUCT "LilyPad USB" #else - 'U','S','B',' ','I','O',' ','B','o','a','r','d',' ',' ',' ',' ' +#define USB_PRODUCT "USB IO Board" +#endif #endif -}; -const u16 STRING_IMANUFACTURER[12] = { - (3<<8) | (2+2*11), +const u8 STRING_PRODUCT[] PROGMEM = USB_PRODUCT; + #if USB_VID == 0x2341 - 'A','r','d','u','i','n','o',' ','L','L','C' +#define USB_MANUFACTURER "Arduino LLC" #elif USB_VID == 0x1b4f - 'S','p','a','r','k','F','u','n',' ',' ',' ' -#else - 'U','n','k','n','o','w','n',' ',' ',' ',' ' +#define USB_MANUFACTURER "SparkFun" +#elif !defined(USB_MANUFACTURER) +// Fall through to unknown if no manufacturer name was provided in a macro +#define USB_MANUFACTURER "Unknown" #endif -}; + +const u8 STRING_MANUFACTURER[] PROGMEM = USB_MANUFACTURER; + #ifdef CDC_ENABLED #define DEVICE_CLASS 0x02 @@ -416,6 +419,22 @@ int USB_SendControl(u8 flags, const void* d, int len) return sent; } +// Send a USB descriptor string. The string is stored in PROGMEM as a +// plain ASCII string but is sent out as UTF-16 with the correct 2-byte +// prefix +static bool USB_SendStringDescriptor(const u8*string_P, u8 string_len) { + SendControl(2 + string_len * 2); + SendControl(3); + for(u8 i = 0; i < string_len; i++) { + bool r = SendControl(pgm_read_byte(&string_P[i])); + r &= SendControl(0); // high byte + if(!r) { + return false; + } + } + return true; +} + // Does not timeout or cross fifo boundaries // Will only work for transfers <= 64 bytes // TODO @@ -476,7 +495,6 @@ bool SendDescriptor(Setup& setup) return HID_GetDescriptor(t); #endif - u8 desc_length = 0; const u8* desc_addr = 0; if (USB_DEVICE_DESCRIPTOR_TYPE == t) { @@ -486,20 +504,22 @@ bool SendDescriptor(Setup& setup) } else if (USB_STRING_DESCRIPTOR_TYPE == t) { - if (setup.wValueL == 0) + if (setup.wValueL == 0) { desc_addr = (const u8*)&STRING_LANGUAGE; - else if (setup.wValueL == IPRODUCT) - desc_addr = (const u8*)&STRING_IPRODUCT; - else if (setup.wValueL == IMANUFACTURER) - desc_addr = (const u8*)&STRING_IMANUFACTURER; + } + else if (setup.wValueL == IPRODUCT) { + return USB_SendStringDescriptor(STRING_PRODUCT, strlen(USB_PRODUCT)); + } + else if (setup.wValueL == IMANUFACTURER) { + return USB_SendStringDescriptor(STRING_MANUFACTURER, strlen(USB_MANUFACTURER)); + } else return false; } if (desc_addr == 0) return false; - if (desc_length == 0) - desc_length = pgm_read_byte(desc_addr); + u8 desc_length = pgm_read_byte(desc_addr); USB_SendControl(TRANSFER_PGM,desc_addr,desc_length); return true; From 96286247b6f4e8871628f586391e508e665b3c29 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 24 Apr 2013 17:49:24 +1000 Subject: [PATCH 02/13] boards.txt: Refactor the default usb build flags into a generic property in platform.txt --- boards.txt | 8 ++++---- platform.txt | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/boards.txt b/boards.txt index 1ec00c0..8ee4561 100644 --- a/boards.txt +++ b/boards.txt @@ -192,7 +192,7 @@ leonardo.build.usb_product="Arduino Leonardo" leonardo.build.board=AVR_LEONARDO leonardo.build.core=arduino leonardo.build.variant=leonardo -leonardo.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} +leonardo.build.extra_flags={build.usb_flags} ############################################################## @@ -222,7 +222,7 @@ micro.build.usb_product=Arduino Micro micro.build.board=AVR_MICRO micro.build.core=arduino micro.build.variant=micro -micro.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} +micro.build.extra_flags={build.usb_flags} ############################################################## @@ -252,7 +252,7 @@ esplora.build.usb_product=Arduino Esplora esplora.build.board=AVR_ESPLORA esplora.build.core=arduino esplora.build.variant=leonardo -esplora.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} +esplora.build.extra_flags={build.usb_flags} ############################################################## @@ -418,7 +418,7 @@ LilyPadUSB.build.usb_product=LilyPad USB LilyPadUSB.build.board=AVR_LILYPAD_USB LilyPadUSB.build.core=arduino LilyPadUSB.build.variant=leonardo -LilyPadUSB.build.extra_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} +LilyPadUSB.build.extra_flags={build.usb_flags} ############################################################## diff --git a/platform.txt b/platform.txt index eaf9469..a0e35c3 100644 --- a/platform.txt +++ b/platform.txt @@ -86,3 +86,9 @@ tools.avrdude.bootloader.params.verbose=-v -v -v -v tools.avrdude.bootloader.params.quiet=-q -q tools.avrdude.bootloader.pattern="{cmd.path}" "-C{config.path}" {bootloader.verbose} -p{build.mcu} -c{protocol} {program.extra_params} "-Uflash:w:{runtime.ide.path}/hardware/arduino/avr/bootloaders/{bootloader.file}:i" -Ulock:w:{bootloader.lock_bits}:m + +# USB Default Flags +# Default blank usb manufacturer will be filled it at compile time +# - from numeric vendor ID, set to Unknown otherwise +build.usb_manufacturer= +build.usb_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} From 0340b90366350b61983a3619805f65539fabbfc8 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 17 May 2013 15:01:15 +1000 Subject: [PATCH 03/13] Fix whitespace (tabify), oops --- cores/arduino/USBCore.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cores/arduino/USBCore.cpp b/cores/arduino/USBCore.cpp index 680cba7..9a50bda 100644 --- a/cores/arduino/USBCore.cpp +++ b/cores/arduino/USBCore.cpp @@ -504,22 +504,22 @@ bool SendDescriptor(Setup& setup) } else if (USB_STRING_DESCRIPTOR_TYPE == t) { - if (setup.wValueL == 0) { + if (setup.wValueL == 0) { desc_addr = (const u8*)&STRING_LANGUAGE; - } + } else if (setup.wValueL == IPRODUCT) { - return USB_SendStringDescriptor(STRING_PRODUCT, strlen(USB_PRODUCT)); - } + return USB_SendStringDescriptor(STRING_PRODUCT, strlen(USB_PRODUCT)); + } else if (setup.wValueL == IMANUFACTURER) { - return USB_SendStringDescriptor(STRING_MANUFACTURER, strlen(USB_MANUFACTURER)); - } + return USB_SendStringDescriptor(STRING_MANUFACTURER, strlen(USB_MANUFACTURER)); + } else return false; } if (desc_addr == 0) return false; - u8 desc_length = pgm_read_byte(desc_addr); + u8 desc_length = pgm_read_byte(desc_addr); USB_SendControl(TRANSFER_PGM,desc_addr,desc_length); return true; From ff47a782f5c72e7bdf9432a67326439fefa5cd0e Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 17 May 2013 15:02:51 +1000 Subject: [PATCH 04/13] Remove hardcoded product names (all provided for in boards.txt) --- cores/arduino/USBCore.cpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/cores/arduino/USBCore.cpp b/cores/arduino/USBCore.cpp index 9a50bda..c46c75f 100644 --- a/cores/arduino/USBCore.cpp +++ b/cores/arduino/USBCore.cpp @@ -50,19 +50,9 @@ const u16 STRING_LANGUAGE[2] = { }; #ifndef USB_PRODUCT -// Use a hardcoded product name if none is provided -#if USB_PID == 0x8036 -#define USB_PRODUCT "Arduino Leonardo" -#elif USB_PID == 0x8037 -#define USB_PRODUCT "Arduino Micro" -#elif USB_PID == 0x803C -#define USB_PRODUCT "Arduino Esplora" -#elif USB_PID == 0x9208 -#define USB_PRODUCT "LilyPad USB" -#else +// If no product is provided, use USB IO Board #define USB_PRODUCT "USB IO Board" #endif -#endif const u8 STRING_PRODUCT[] PROGMEM = USB_PRODUCT; From 43392fb2b45370f01bcba887d350a0da0bdf4167 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 17 Jul 2013 14:42:41 +0200 Subject: [PATCH 05/13] Added quoting to usb_product key to preserve double quotes. See #1422. --- platform.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform.txt b/platform.txt index a0e35c3..6bf3cdb 100644 --- a/platform.txt +++ b/platform.txt @@ -91,4 +91,4 @@ tools.avrdude.bootloader.pattern="{cmd.path}" "-C{config.path}" {bootloader.verb # Default blank usb manufacturer will be filled it at compile time # - from numeric vendor ID, set to Unknown otherwise build.usb_manufacturer= -build.usb_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} -DUSB_PRODUCT={build.usb_product} +build.usb_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} -DUSB_MANUFACTURER={build.usb_manufacturer} '-DUSB_PRODUCT={build.usb_product}' From bda85506eae7807ac4ad96ee9df5c1760761002f Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 22 Jul 2013 12:30:25 +0200 Subject: [PATCH 06/13] Fixed usb_products on some AVR boards --- boards.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boards.txt b/boards.txt index 8ee4561..ebcbf21 100644 --- a/boards.txt +++ b/boards.txt @@ -218,7 +218,7 @@ micro.build.mcu=atmega32u4 micro.build.f_cpu=16000000L micro.build.vid=0x2341 micro.build.pid=0x8037 -micro.build.usb_product=Arduino Micro +micro.build.usb_product="Arduino Micro" micro.build.board=AVR_MICRO micro.build.core=arduino micro.build.variant=micro @@ -248,7 +248,7 @@ esplora.build.mcu=atmega32u4 esplora.build.f_cpu=16000000L esplora.build.vid=0x2341 esplora.build.pid=0x803c -esplora.build.usb_product=Arduino Esplora +esplora.build.usb_product="Arduino Esplora" esplora.build.board=AVR_ESPLORA esplora.build.core=arduino esplora.build.variant=leonardo @@ -414,7 +414,7 @@ LilyPadUSB.build.mcu=atmega32u4 LilyPadUSB.build.f_cpu=8000000L LilyPadUSB.build.vid=0x1B4F LilyPadUSB.build.pid=0x9208 -LilyPadUSB.build.usb_product=LilyPad USB +LilyPadUSB.build.usb_product="LilyPad USB" LilyPadUSB.build.board=AVR_LILYPAD_USB LilyPadUSB.build.core=arduino LilyPadUSB.build.variant=leonardo From a056282246abd36e7b9ccc8dc60ff2abd481a1dd Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 18 Apr 2013 18:50:10 +0200 Subject: [PATCH 07/13] Use uint8_t for HardwareSerial ringbuffer pointers Since the buffers aren't bigger than 64 bytes, these values can be smaller. This saves a few bytes of ram, but also saves around 50 bytes of program space, since the values can now be loaded using a single instruction. To prevent problems when people manually increase the buffer size, a compile-time check is added. Closes: #1078 --- cores/arduino/HardwareSerial.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cores/arduino/HardwareSerial.cpp b/cores/arduino/HardwareSerial.cpp index eb2365f..7b056e9 100644 --- a/cores/arduino/HardwareSerial.cpp +++ b/cores/arduino/HardwareSerial.cpp @@ -62,10 +62,14 @@ struct ring_buffer { unsigned char buffer[SERIAL_BUFFER_SIZE]; - volatile unsigned int head; - volatile unsigned int tail; + volatile uint8_t head; + volatile uint8_t tail; }; +#if SERIAL_BUFFER_SIZE > 256 +#error Serial buffer size too big for head and tail pointers +#endif + #if defined(USBCON) ring_buffer rx_buffer = { { 0 }, 0, 0}; ring_buffer tx_buffer = { { 0 }, 0, 0}; From 714874dd8c4d4bdf9acb648f55c9f482c1901098 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 18 Apr 2013 18:52:48 +0200 Subject: [PATCH 08/13] Move buffers into HardwareSerial This removes the need for doing an extra pointer dereference on every access to the buffers, shrinking the code by around 100 bytes. The members for these buffers must be public for now, since the interrupt handlers also need to access them. These can later be made private again. Furthermore, the struct ring_buffer was removed. This allows the all head and tail pointers to be put into the HardwareSerial struct before the actual buffers, so the pointers all end up in the first 32 bytes of the struct that can be accessed using a single instruction (ldd). References: #947 --- cores/arduino/HardwareSerial.cpp | 125 ++++++++++--------------------- cores/arduino/HardwareSerial.h | 26 ++++++- 2 files changed, 63 insertions(+), 88 deletions(-) diff --git a/cores/arduino/HardwareSerial.cpp b/cores/arduino/HardwareSerial.cpp index 7b056e9..e40bfbc 100644 --- a/cores/arduino/HardwareSerial.cpp +++ b/cores/arduino/HardwareSerial.cpp @@ -49,59 +49,17 @@ #endif #endif -// Define constants and variables for buffering incoming serial data. We're -// using a ring buffer (I think), in which head is the index of the location -// to which to write the next incoming character and tail is the index of the -// location from which to read. -#if (RAMEND < 1000) - #define SERIAL_BUFFER_SIZE 16 -#else - #define SERIAL_BUFFER_SIZE 64 -#endif - -struct ring_buffer +inline void store_char(unsigned char c, HardwareSerial *s) { - unsigned char buffer[SERIAL_BUFFER_SIZE]; - volatile uint8_t head; - volatile uint8_t tail; -}; - -#if SERIAL_BUFFER_SIZE > 256 -#error Serial buffer size too big for head and tail pointers -#endif - -#if defined(USBCON) - ring_buffer rx_buffer = { { 0 }, 0, 0}; - ring_buffer tx_buffer = { { 0 }, 0, 0}; -#endif -#if defined(UBRRH) || defined(UBRR0H) - ring_buffer rx_buffer = { { 0 }, 0, 0 }; - ring_buffer tx_buffer = { { 0 }, 0, 0 }; -#endif -#if defined(UBRR1H) - ring_buffer rx_buffer1 = { { 0 }, 0, 0 }; - ring_buffer tx_buffer1 = { { 0 }, 0, 0 }; -#endif -#if defined(UBRR2H) - ring_buffer rx_buffer2 = { { 0 }, 0, 0 }; - ring_buffer tx_buffer2 = { { 0 }, 0, 0 }; -#endif -#if defined(UBRR3H) - ring_buffer rx_buffer3 = { { 0 }, 0, 0 }; - ring_buffer tx_buffer3 = { { 0 }, 0, 0 }; -#endif - -inline void store_char(unsigned char c, ring_buffer *buffer) -{ - int i = (unsigned int)(buffer->head + 1) % SERIAL_BUFFER_SIZE; + int i = (unsigned int)(s->_rx_buffer_head + 1) % SERIAL_BUFFER_SIZE; // if we should be storing the received character into the location // just before the tail (meaning that the head would advance to the // current location of the tail), we're about to overflow the buffer // and so we don't write the character or advance the head. - if (i != buffer->tail) { - buffer->buffer[buffer->head] = c; - buffer->head = i; + if (i != s->_rx_buffer_tail) { + s->_rx_buffer[s->_rx_buffer_head] = c; + s->_rx_buffer_head = i; } } @@ -126,7 +84,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer) #if defined(UDR0) if (bit_is_clear(UCSR0A, UPE0)) { unsigned char c = UDR0; - store_char(c, &rx_buffer); + store_char(c, &Serial); } else { unsigned char c = UDR0; }; @@ -152,7 +110,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer) { if (bit_is_clear(UCSR1A, UPE1)) { unsigned char c = UDR1; - store_char(c, &rx_buffer1); + store_char(c, &Serial1); } else { unsigned char c = UDR1; }; @@ -167,7 +125,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer) { if (bit_is_clear(UCSR2A, UPE2)) { unsigned char c = UDR2; - store_char(c, &rx_buffer2); + store_char(c, &Serial2); } else { unsigned char c = UDR2; }; @@ -182,7 +140,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer) { if (bit_is_clear(UCSR3A, UPE3)) { unsigned char c = UDR3; - store_char(c, &rx_buffer3); + store_char(c, &Serial3); } else { unsigned char c = UDR3; }; @@ -222,7 +180,7 @@ ISR(USART0_UDRE_vect) ISR(USART_UDRE_vect) #endif { - if (tx_buffer.head == tx_buffer.tail) { + if (Serial._tx_buffer_head == Serial._tx_buffer_tail) { // Buffer empty, so disable interrupts #if defined(UCSR0B) cbi(UCSR0B, UDRIE0); @@ -232,8 +190,8 @@ ISR(USART_UDRE_vect) } else { // There is more data in the output buffer. Send the next byte - unsigned char c = tx_buffer.buffer[tx_buffer.tail]; - tx_buffer.tail = (tx_buffer.tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = Serial._tx_buffer[Serial._tx_buffer_tail]; + Serial._tx_buffer_tail = (Serial._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; #if defined(UDR0) UDR0 = c; @@ -250,14 +208,14 @@ ISR(USART_UDRE_vect) #ifdef USART1_UDRE_vect ISR(USART1_UDRE_vect) { - if (tx_buffer1.head == tx_buffer1.tail) { + if (Serial1._tx_buffer_head == Serial1._tx_buffer_tail) { // Buffer empty, so disable interrupts cbi(UCSR1B, UDRIE1); } else { // There is more data in the output buffer. Send the next byte - unsigned char c = tx_buffer1.buffer[tx_buffer1.tail]; - tx_buffer1.tail = (tx_buffer1.tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = Serial1._tx_buffer[Serial1._tx_buffer_tail]; + Serial1._tx_buffer_tail = (Serial1._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; UDR1 = c; } @@ -267,14 +225,14 @@ ISR(USART1_UDRE_vect) #ifdef USART2_UDRE_vect ISR(USART2_UDRE_vect) { - if (tx_buffer2.head == tx_buffer2.tail) { + if (Serial2._tx_buffer_head == Serial2._tx_buffer_tail) { // Buffer empty, so disable interrupts cbi(UCSR2B, UDRIE2); } else { // There is more data in the output buffer. Send the next byte - unsigned char c = tx_buffer2.buffer[tx_buffer2.tail]; - tx_buffer2.tail = (tx_buffer2.tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = Serial2._tx_buffer[Serial2._tx_buffer_tail]; + Serial2._tx_buffer_tail = (Serial2._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; UDR2 = c; } @@ -284,31 +242,30 @@ ISR(USART2_UDRE_vect) #ifdef USART3_UDRE_vect ISR(USART3_UDRE_vect) { - if (tx_buffer3.head == tx_buffer3.tail) { + if (Serial3._tx_buffer_head == Serial3._tx_buffer_tail) { // Buffer empty, so disable interrupts cbi(UCSR3B, UDRIE3); } else { // There is more data in the output buffer. Send the next byte - unsigned char c = tx_buffer3.buffer[tx_buffer3.tail]; - tx_buffer3.tail = (tx_buffer3.tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = Serial3._tx_buffer[Serial3._tx_buffer_tail]; + Serial3._tx_buffer_tail = (Serial3._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; UDR3 = c; } } #endif - // Constructors //////////////////////////////////////////////////////////////// -HardwareSerial::HardwareSerial(ring_buffer *rx_buffer, ring_buffer *tx_buffer, +HardwareSerial::HardwareSerial( volatile uint8_t *ubrrh, volatile uint8_t *ubrrl, volatile uint8_t *ucsra, volatile uint8_t *ucsrb, volatile uint8_t *ucsrc, volatile uint8_t *udr, uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udrie, uint8_t u2x) { - _rx_buffer = rx_buffer; - _tx_buffer = tx_buffer; + _tx_buffer_head = _tx_buffer_tail = 0; + _rx_buffer_head = _rx_buffer_tail = 0; _ubrrh = ubrrh; _ubrrl = ubrrl; _ucsra = ucsra; @@ -416,7 +373,7 @@ try_again: void HardwareSerial::end() { // wait for transmission of outgoing data - while (_tx_buffer->head != _tx_buffer->tail) + while (_tx_buffer_head != _tx_buffer_tail) ; cbi(*_ucsrb, _rxen); @@ -425,31 +382,31 @@ void HardwareSerial::end() cbi(*_ucsrb, _udrie); // clear any received data - _rx_buffer->head = _rx_buffer->tail; + _rx_buffer_head = _rx_buffer_tail; } int HardwareSerial::available(void) { - return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer->head - _rx_buffer->tail) % SERIAL_BUFFER_SIZE; + return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer_head - _rx_buffer_tail) % SERIAL_BUFFER_SIZE; } int HardwareSerial::peek(void) { - if (_rx_buffer->head == _rx_buffer->tail) { + if (_rx_buffer_head == _rx_buffer_tail) { return -1; } else { - return _rx_buffer->buffer[_rx_buffer->tail]; + return _rx_buffer[_rx_buffer_tail]; } } int HardwareSerial::read(void) { // if the head isn't ahead of the tail, we don't have any characters - if (_rx_buffer->head == _rx_buffer->tail) { + if (_rx_buffer_head == _rx_buffer_tail) { return -1; } else { - unsigned char c = _rx_buffer->buffer[_rx_buffer->tail]; - _rx_buffer->tail = (unsigned int)(_rx_buffer->tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = _rx_buffer[_rx_buffer_tail]; + _rx_buffer_tail = (unsigned int)(_rx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; return c; } } @@ -463,16 +420,16 @@ void HardwareSerial::flush() size_t HardwareSerial::write(uint8_t c) { - int i = (_tx_buffer->head + 1) % SERIAL_BUFFER_SIZE; + int i = (_tx_buffer_head + 1) % SERIAL_BUFFER_SIZE; // If the output buffer is full, there's nothing for it other than to // wait for the interrupt handler to empty it a bit // ???: return 0 here instead? - while (i == _tx_buffer->tail) + while (i == _tx_buffer_tail) ; - _tx_buffer->buffer[_tx_buffer->head] = c; - _tx_buffer->head = i; + _tx_buffer[_tx_buffer_head] = c; + _tx_buffer_head = i; sbi(*_ucsrb, _udrie); // clear the TXC bit -- "can be cleared by writing a one to its bit location" @@ -489,9 +446,9 @@ HardwareSerial::operator bool() { // Preinstantiate Objects ////////////////////////////////////////////////////// #if defined(UBRRH) && defined(UBRRL) - HardwareSerial Serial(&rx_buffer, &tx_buffer, &UBRRH, &UBRRL, &UCSRA, &UCSRB, &UCSRC, &UDR, RXEN, TXEN, RXCIE, UDRIE, U2X); + HardwareSerial Serial(&UBRRH, &UBRRL, &UCSRA, &UCSRB, &UCSRC, &UDR, RXEN, TXEN, RXCIE, UDRIE, U2X); #elif defined(UBRR0H) && defined(UBRR0L) - HardwareSerial Serial(&rx_buffer, &tx_buffer, &UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UCSR0C, &UDR0, RXEN0, TXEN0, RXCIE0, UDRIE0, U2X0); + HardwareSerial Serial(&UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UCSR0C, &UDR0, RXEN0, TXEN0, RXCIE0, UDRIE0, U2X0); #elif defined(USBCON) // do nothing - Serial object and buffers are initialized in CDC code #else @@ -499,13 +456,13 @@ HardwareSerial::operator bool() { #endif #if defined(UBRR1H) - HardwareSerial Serial1(&rx_buffer1, &tx_buffer1, &UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UCSR1C, &UDR1, RXEN1, TXEN1, RXCIE1, UDRIE1, U2X1); + HardwareSerial Serial1(&UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UCSR1C, &UDR1, RXEN1, TXEN1, RXCIE1, UDRIE1, U2X1); #endif #if defined(UBRR2H) - HardwareSerial Serial2(&rx_buffer2, &tx_buffer2, &UBRR2H, &UBRR2L, &UCSR2A, &UCSR2B, &UCSR2C, &UDR2, RXEN2, TXEN2, RXCIE2, UDRIE2, U2X2); + HardwareSerial Serial2(&UBRR2H, &UBRR2L, &UCSR2A, &UCSR2B, &UCSR2C, &UDR2, RXEN2, TXEN2, RXCIE2, UDRIE2, U2X2); #endif #if defined(UBRR3H) - HardwareSerial Serial3(&rx_buffer3, &tx_buffer3, &UBRR3H, &UBRR3L, &UCSR3A, &UCSR3B, &UCSR3C, &UDR3, RXEN3, TXEN3, RXCIE3, UDRIE3, U2X3); + HardwareSerial Serial3(&UBRR3H, &UBRR3L, &UCSR3A, &UCSR3B, &UCSR3C, &UDR3, RXEN3, TXEN3, RXCIE3, UDRIE3, U2X3); #endif #endif // whole file diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index a73117f..a091e03 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -27,13 +27,19 @@ #include "Stream.h" -struct ring_buffer; +// Define constants and variables for buffering incoming serial data. We're +// using a ring buffer (I think), in which head is the index of the location +// to which to write the next incoming character and tail is the index of the +// location from which to read. +#if (RAMEND < 1000) + #define SERIAL_BUFFER_SIZE 16 +#else + #define SERIAL_BUFFER_SIZE 64 +#endif class HardwareSerial : public Stream { private: - ring_buffer *_rx_buffer; - ring_buffer *_tx_buffer; volatile uint8_t *_ubrrh; volatile uint8_t *_ubrrl; volatile uint8_t *_ucsra; @@ -46,8 +52,20 @@ class HardwareSerial : public Stream uint8_t _udrie; uint8_t _u2x; bool transmitting; + public: - HardwareSerial(ring_buffer *rx_buffer, ring_buffer *tx_buffer, + volatile uint8_t _rx_buffer_head; + volatile uint8_t _rx_buffer_tail; + volatile uint8_t _tx_buffer_head; + volatile uint8_t _tx_buffer_tail; + + // Don't put any members after these buffers, since only the first + // 32 bytes of this struct can be accessed quickly using the ldd + // instruction. + unsigned char _rx_buffer[SERIAL_BUFFER_SIZE]; + unsigned char _tx_buffer[SERIAL_BUFFER_SIZE]; + + HardwareSerial( volatile uint8_t *ubrrh, volatile uint8_t *ubrrl, volatile uint8_t *ucsra, volatile uint8_t *ucsrb, volatile uint8_t *ucsrc, volatile uint8_t *udr, From d1da7ef303a1cb41df3e72d0a50febcb34650148 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 18 Apr 2013 18:46:14 +0200 Subject: [PATCH 09/13] Make private members of HardwareSerial protected This allows users to create subclasses. Closes: #947 --- cores/arduino/HardwareSerial.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index a091e03..0f62262 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -39,7 +39,7 @@ class HardwareSerial : public Stream { - private: + protected: volatile uint8_t *_ubrrh; volatile uint8_t *_ubrrl; volatile uint8_t *_ucsra; From 090d53a74e1ebe256523608c4b7fd90304d611b9 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 26 Jul 2013 12:50:17 +0200 Subject: [PATCH 10/13] Fixed compile problem for Leonardo after 0bd6a2d20fb9664255b20e0db11dd4586ebe9007 --- cores/arduino/USBAPI.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cores/arduino/USBAPI.h b/cores/arduino/USBAPI.h index eb2e593..1a4183b 100644 --- a/cores/arduino/USBAPI.h +++ b/cores/arduino/USBAPI.h @@ -25,6 +25,8 @@ extern USBDevice_ USBDevice; //================================================================================ // Serial over CDC (Serial1 is the physical port) +struct ring_buffer; + class Serial_ : public Stream { private: @@ -193,4 +195,4 @@ void USB_Flush(uint8_t ep); #endif -#endif /* if defined(USBCON) */ \ No newline at end of file +#endif /* if defined(USBCON) */ From cddc83bb19e760145ea312f187371f44a6a921e8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 26 Jul 2013 13:50:34 +0200 Subject: [PATCH 11/13] Applied HardwareSerial updates to robot's core. --- cores/robot/HardwareSerial.cpp | 121 +++++++++++---------------------- cores/robot/HardwareSerial.h | 28 ++++++-- cores/robot/USBAPI.h | 4 +- 3 files changed, 67 insertions(+), 86 deletions(-) diff --git a/cores/robot/HardwareSerial.cpp b/cores/robot/HardwareSerial.cpp index eb2365f..e40bfbc 100644 --- a/cores/robot/HardwareSerial.cpp +++ b/cores/robot/HardwareSerial.cpp @@ -49,55 +49,17 @@ #endif #endif -// Define constants and variables for buffering incoming serial data. We're -// using a ring buffer (I think), in which head is the index of the location -// to which to write the next incoming character and tail is the index of the -// location from which to read. -#if (RAMEND < 1000) - #define SERIAL_BUFFER_SIZE 16 -#else - #define SERIAL_BUFFER_SIZE 64 -#endif - -struct ring_buffer +inline void store_char(unsigned char c, HardwareSerial *s) { - unsigned char buffer[SERIAL_BUFFER_SIZE]; - volatile unsigned int head; - volatile unsigned int tail; -}; - -#if defined(USBCON) - ring_buffer rx_buffer = { { 0 }, 0, 0}; - ring_buffer tx_buffer = { { 0 }, 0, 0}; -#endif -#if defined(UBRRH) || defined(UBRR0H) - ring_buffer rx_buffer = { { 0 }, 0, 0 }; - ring_buffer tx_buffer = { { 0 }, 0, 0 }; -#endif -#if defined(UBRR1H) - ring_buffer rx_buffer1 = { { 0 }, 0, 0 }; - ring_buffer tx_buffer1 = { { 0 }, 0, 0 }; -#endif -#if defined(UBRR2H) - ring_buffer rx_buffer2 = { { 0 }, 0, 0 }; - ring_buffer tx_buffer2 = { { 0 }, 0, 0 }; -#endif -#if defined(UBRR3H) - ring_buffer rx_buffer3 = { { 0 }, 0, 0 }; - ring_buffer tx_buffer3 = { { 0 }, 0, 0 }; -#endif - -inline void store_char(unsigned char c, ring_buffer *buffer) -{ - int i = (unsigned int)(buffer->head + 1) % SERIAL_BUFFER_SIZE; + int i = (unsigned int)(s->_rx_buffer_head + 1) % SERIAL_BUFFER_SIZE; // if we should be storing the received character into the location // just before the tail (meaning that the head would advance to the // current location of the tail), we're about to overflow the buffer // and so we don't write the character or advance the head. - if (i != buffer->tail) { - buffer->buffer[buffer->head] = c; - buffer->head = i; + if (i != s->_rx_buffer_tail) { + s->_rx_buffer[s->_rx_buffer_head] = c; + s->_rx_buffer_head = i; } } @@ -122,7 +84,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer) #if defined(UDR0) if (bit_is_clear(UCSR0A, UPE0)) { unsigned char c = UDR0; - store_char(c, &rx_buffer); + store_char(c, &Serial); } else { unsigned char c = UDR0; }; @@ -148,7 +110,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer) { if (bit_is_clear(UCSR1A, UPE1)) { unsigned char c = UDR1; - store_char(c, &rx_buffer1); + store_char(c, &Serial1); } else { unsigned char c = UDR1; }; @@ -163,7 +125,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer) { if (bit_is_clear(UCSR2A, UPE2)) { unsigned char c = UDR2; - store_char(c, &rx_buffer2); + store_char(c, &Serial2); } else { unsigned char c = UDR2; }; @@ -178,7 +140,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer) { if (bit_is_clear(UCSR3A, UPE3)) { unsigned char c = UDR3; - store_char(c, &rx_buffer3); + store_char(c, &Serial3); } else { unsigned char c = UDR3; }; @@ -218,7 +180,7 @@ ISR(USART0_UDRE_vect) ISR(USART_UDRE_vect) #endif { - if (tx_buffer.head == tx_buffer.tail) { + if (Serial._tx_buffer_head == Serial._tx_buffer_tail) { // Buffer empty, so disable interrupts #if defined(UCSR0B) cbi(UCSR0B, UDRIE0); @@ -228,8 +190,8 @@ ISR(USART_UDRE_vect) } else { // There is more data in the output buffer. Send the next byte - unsigned char c = tx_buffer.buffer[tx_buffer.tail]; - tx_buffer.tail = (tx_buffer.tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = Serial._tx_buffer[Serial._tx_buffer_tail]; + Serial._tx_buffer_tail = (Serial._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; #if defined(UDR0) UDR0 = c; @@ -246,14 +208,14 @@ ISR(USART_UDRE_vect) #ifdef USART1_UDRE_vect ISR(USART1_UDRE_vect) { - if (tx_buffer1.head == tx_buffer1.tail) { + if (Serial1._tx_buffer_head == Serial1._tx_buffer_tail) { // Buffer empty, so disable interrupts cbi(UCSR1B, UDRIE1); } else { // There is more data in the output buffer. Send the next byte - unsigned char c = tx_buffer1.buffer[tx_buffer1.tail]; - tx_buffer1.tail = (tx_buffer1.tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = Serial1._tx_buffer[Serial1._tx_buffer_tail]; + Serial1._tx_buffer_tail = (Serial1._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; UDR1 = c; } @@ -263,14 +225,14 @@ ISR(USART1_UDRE_vect) #ifdef USART2_UDRE_vect ISR(USART2_UDRE_vect) { - if (tx_buffer2.head == tx_buffer2.tail) { + if (Serial2._tx_buffer_head == Serial2._tx_buffer_tail) { // Buffer empty, so disable interrupts cbi(UCSR2B, UDRIE2); } else { // There is more data in the output buffer. Send the next byte - unsigned char c = tx_buffer2.buffer[tx_buffer2.tail]; - tx_buffer2.tail = (tx_buffer2.tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = Serial2._tx_buffer[Serial2._tx_buffer_tail]; + Serial2._tx_buffer_tail = (Serial2._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; UDR2 = c; } @@ -280,31 +242,30 @@ ISR(USART2_UDRE_vect) #ifdef USART3_UDRE_vect ISR(USART3_UDRE_vect) { - if (tx_buffer3.head == tx_buffer3.tail) { + if (Serial3._tx_buffer_head == Serial3._tx_buffer_tail) { // Buffer empty, so disable interrupts cbi(UCSR3B, UDRIE3); } else { // There is more data in the output buffer. Send the next byte - unsigned char c = tx_buffer3.buffer[tx_buffer3.tail]; - tx_buffer3.tail = (tx_buffer3.tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = Serial3._tx_buffer[Serial3._tx_buffer_tail]; + Serial3._tx_buffer_tail = (Serial3._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; UDR3 = c; } } #endif - // Constructors //////////////////////////////////////////////////////////////// -HardwareSerial::HardwareSerial(ring_buffer *rx_buffer, ring_buffer *tx_buffer, +HardwareSerial::HardwareSerial( volatile uint8_t *ubrrh, volatile uint8_t *ubrrl, volatile uint8_t *ucsra, volatile uint8_t *ucsrb, volatile uint8_t *ucsrc, volatile uint8_t *udr, uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udrie, uint8_t u2x) { - _rx_buffer = rx_buffer; - _tx_buffer = tx_buffer; + _tx_buffer_head = _tx_buffer_tail = 0; + _rx_buffer_head = _rx_buffer_tail = 0; _ubrrh = ubrrh; _ubrrl = ubrrl; _ucsra = ucsra; @@ -412,7 +373,7 @@ try_again: void HardwareSerial::end() { // wait for transmission of outgoing data - while (_tx_buffer->head != _tx_buffer->tail) + while (_tx_buffer_head != _tx_buffer_tail) ; cbi(*_ucsrb, _rxen); @@ -421,31 +382,31 @@ void HardwareSerial::end() cbi(*_ucsrb, _udrie); // clear any received data - _rx_buffer->head = _rx_buffer->tail; + _rx_buffer_head = _rx_buffer_tail; } int HardwareSerial::available(void) { - return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer->head - _rx_buffer->tail) % SERIAL_BUFFER_SIZE; + return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer_head - _rx_buffer_tail) % SERIAL_BUFFER_SIZE; } int HardwareSerial::peek(void) { - if (_rx_buffer->head == _rx_buffer->tail) { + if (_rx_buffer_head == _rx_buffer_tail) { return -1; } else { - return _rx_buffer->buffer[_rx_buffer->tail]; + return _rx_buffer[_rx_buffer_tail]; } } int HardwareSerial::read(void) { // if the head isn't ahead of the tail, we don't have any characters - if (_rx_buffer->head == _rx_buffer->tail) { + if (_rx_buffer_head == _rx_buffer_tail) { return -1; } else { - unsigned char c = _rx_buffer->buffer[_rx_buffer->tail]; - _rx_buffer->tail = (unsigned int)(_rx_buffer->tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = _rx_buffer[_rx_buffer_tail]; + _rx_buffer_tail = (unsigned int)(_rx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; return c; } } @@ -459,16 +420,16 @@ void HardwareSerial::flush() size_t HardwareSerial::write(uint8_t c) { - int i = (_tx_buffer->head + 1) % SERIAL_BUFFER_SIZE; + int i = (_tx_buffer_head + 1) % SERIAL_BUFFER_SIZE; // If the output buffer is full, there's nothing for it other than to // wait for the interrupt handler to empty it a bit // ???: return 0 here instead? - while (i == _tx_buffer->tail) + while (i == _tx_buffer_tail) ; - _tx_buffer->buffer[_tx_buffer->head] = c; - _tx_buffer->head = i; + _tx_buffer[_tx_buffer_head] = c; + _tx_buffer_head = i; sbi(*_ucsrb, _udrie); // clear the TXC bit -- "can be cleared by writing a one to its bit location" @@ -485,9 +446,9 @@ HardwareSerial::operator bool() { // Preinstantiate Objects ////////////////////////////////////////////////////// #if defined(UBRRH) && defined(UBRRL) - HardwareSerial Serial(&rx_buffer, &tx_buffer, &UBRRH, &UBRRL, &UCSRA, &UCSRB, &UCSRC, &UDR, RXEN, TXEN, RXCIE, UDRIE, U2X); + HardwareSerial Serial(&UBRRH, &UBRRL, &UCSRA, &UCSRB, &UCSRC, &UDR, RXEN, TXEN, RXCIE, UDRIE, U2X); #elif defined(UBRR0H) && defined(UBRR0L) - HardwareSerial Serial(&rx_buffer, &tx_buffer, &UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UCSR0C, &UDR0, RXEN0, TXEN0, RXCIE0, UDRIE0, U2X0); + HardwareSerial Serial(&UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UCSR0C, &UDR0, RXEN0, TXEN0, RXCIE0, UDRIE0, U2X0); #elif defined(USBCON) // do nothing - Serial object and buffers are initialized in CDC code #else @@ -495,13 +456,13 @@ HardwareSerial::operator bool() { #endif #if defined(UBRR1H) - HardwareSerial Serial1(&rx_buffer1, &tx_buffer1, &UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UCSR1C, &UDR1, RXEN1, TXEN1, RXCIE1, UDRIE1, U2X1); + HardwareSerial Serial1(&UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UCSR1C, &UDR1, RXEN1, TXEN1, RXCIE1, UDRIE1, U2X1); #endif #if defined(UBRR2H) - HardwareSerial Serial2(&rx_buffer2, &tx_buffer2, &UBRR2H, &UBRR2L, &UCSR2A, &UCSR2B, &UCSR2C, &UDR2, RXEN2, TXEN2, RXCIE2, UDRIE2, U2X2); + HardwareSerial Serial2(&UBRR2H, &UBRR2L, &UCSR2A, &UCSR2B, &UCSR2C, &UDR2, RXEN2, TXEN2, RXCIE2, UDRIE2, U2X2); #endif #if defined(UBRR3H) - HardwareSerial Serial3(&rx_buffer3, &tx_buffer3, &UBRR3H, &UBRR3L, &UCSR3A, &UCSR3B, &UCSR3C, &UDR3, RXEN3, TXEN3, RXCIE3, UDRIE3, U2X3); + HardwareSerial Serial3(&UBRR3H, &UBRR3L, &UCSR3A, &UCSR3B, &UCSR3C, &UDR3, RXEN3, TXEN3, RXCIE3, UDRIE3, U2X3); #endif #endif // whole file diff --git a/cores/robot/HardwareSerial.h b/cores/robot/HardwareSerial.h index a73117f..0f62262 100644 --- a/cores/robot/HardwareSerial.h +++ b/cores/robot/HardwareSerial.h @@ -27,13 +27,19 @@ #include "Stream.h" -struct ring_buffer; +// Define constants and variables for buffering incoming serial data. We're +// using a ring buffer (I think), in which head is the index of the location +// to which to write the next incoming character and tail is the index of the +// location from which to read. +#if (RAMEND < 1000) + #define SERIAL_BUFFER_SIZE 16 +#else + #define SERIAL_BUFFER_SIZE 64 +#endif class HardwareSerial : public Stream { - private: - ring_buffer *_rx_buffer; - ring_buffer *_tx_buffer; + protected: volatile uint8_t *_ubrrh; volatile uint8_t *_ubrrl; volatile uint8_t *_ucsra; @@ -46,8 +52,20 @@ class HardwareSerial : public Stream uint8_t _udrie; uint8_t _u2x; bool transmitting; + public: - HardwareSerial(ring_buffer *rx_buffer, ring_buffer *tx_buffer, + volatile uint8_t _rx_buffer_head; + volatile uint8_t _rx_buffer_tail; + volatile uint8_t _tx_buffer_head; + volatile uint8_t _tx_buffer_tail; + + // Don't put any members after these buffers, since only the first + // 32 bytes of this struct can be accessed quickly using the ldd + // instruction. + unsigned char _rx_buffer[SERIAL_BUFFER_SIZE]; + unsigned char _tx_buffer[SERIAL_BUFFER_SIZE]; + + HardwareSerial( volatile uint8_t *ubrrh, volatile uint8_t *ubrrl, volatile uint8_t *ucsra, volatile uint8_t *ucsrb, volatile uint8_t *ucsrc, volatile uint8_t *udr, diff --git a/cores/robot/USBAPI.h b/cores/robot/USBAPI.h index eb2e593..1a4183b 100644 --- a/cores/robot/USBAPI.h +++ b/cores/robot/USBAPI.h @@ -25,6 +25,8 @@ extern USBDevice_ USBDevice; //================================================================================ // Serial over CDC (Serial1 is the physical port) +struct ring_buffer; + class Serial_ : public Stream { private: @@ -193,4 +195,4 @@ void USB_Flush(uint8_t ep); #endif -#endif /* if defined(USBCON) */ \ No newline at end of file +#endif /* if defined(USBCON) */ From 4de497b72504c62cd41b9930db4a53fb2280e6c8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Sat, 27 Jul 2013 12:06:42 +0200 Subject: [PATCH 12/13] Move buffers into USB CDC (look #947 and #1369 for reference) --- cores/arduino/CDC.cpp | 41 +++++++++++------------------------------ cores/arduino/USBAPI.h | 12 ++++++++++-- cores/robot/CDC.cpp | 41 +++++++++++------------------------------ cores/robot/USBAPI.h | 12 ++++++++++-- 4 files changed, 42 insertions(+), 64 deletions(-) diff --git a/cores/arduino/CDC.cpp b/cores/arduino/CDC.cpp index 701e483..fb25a96 100644 --- a/cores/arduino/CDC.cpp +++ b/cores/arduino/CDC.cpp @@ -23,21 +23,6 @@ #if defined(USBCON) #ifdef CDC_ENABLED -#if (RAMEND < 1000) -#define SERIAL_BUFFER_SIZE 16 -#else -#define SERIAL_BUFFER_SIZE 64 -#endif - -struct ring_buffer -{ - unsigned char buffer[SERIAL_BUFFER_SIZE]; - volatile int head; - volatile int tail; -}; - -ring_buffer cdc_rx_buffer = { { 0 }, 0, 0}; - typedef struct { u32 dwDTERate; @@ -140,8 +125,7 @@ void Serial_::end(void) void Serial_::accept(void) { - ring_buffer *buffer = &cdc_rx_buffer; - int i = (unsigned int)(buffer->head+1) % SERIAL_BUFFER_SIZE; + int i = (unsigned int)(_rx_buffer_head+1) % SERIAL_BUFFER_SIZE; // if we should be storing the received character into the location // just before the tail (meaning that the head would advance to the @@ -149,42 +133,39 @@ void Serial_::accept(void) // and so we don't write the character or advance the head. // while we have room to store a byte - while (i != buffer->tail) { + while (i != _rx_buffer_tail) { int c = USB_Recv(CDC_RX); if (c == -1) break; // no more data - buffer->buffer[buffer->head] = c; - buffer->head = i; + _rx_buffer[_rx_buffer_head] = c; + _rx_buffer_head = i; - i = (unsigned int)(buffer->head+1) % SERIAL_BUFFER_SIZE; + i = (unsigned int)(_rx_buffer_head+1) % SERIAL_BUFFER_SIZE; } } int Serial_::available(void) { - ring_buffer *buffer = &cdc_rx_buffer; - return (unsigned int)(SERIAL_BUFFER_SIZE + buffer->head - buffer->tail) % SERIAL_BUFFER_SIZE; + return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer_head - _rx_buffer_tail) % SERIAL_BUFFER_SIZE; } int Serial_::peek(void) { - ring_buffer *buffer = &cdc_rx_buffer; - if (buffer->head == buffer->tail) { + if (_rx_buffer_head == _rx_buffer_tail) { return -1; } else { - return buffer->buffer[buffer->tail]; + return _rx_buffer[_rx_buffer_tail]; } } int Serial_::read(void) { - ring_buffer *buffer = &cdc_rx_buffer; // if the head isn't ahead of the tail, we don't have any characters - if (buffer->head == buffer->tail) { + if (_rx_buffer_head == _rx_buffer_tail) { return -1; } else { - unsigned char c = buffer->buffer[buffer->tail]; - buffer->tail = (unsigned int)(buffer->tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = _rx_buffer[_rx_buffer_tail]; + _rx_buffer_tail = (unsigned int)(_rx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; return c; } } diff --git a/cores/arduino/USBAPI.h b/cores/arduino/USBAPI.h index 1a4183b..cabecf3 100644 --- a/cores/arduino/USBAPI.h +++ b/cores/arduino/USBAPI.h @@ -27,10 +27,14 @@ extern USBDevice_ USBDevice; struct ring_buffer; +#if (RAMEND < 1000) +#define SERIAL_BUFFER_SIZE 16 +#else +#define SERIAL_BUFFER_SIZE 64 +#endif + class Serial_ : public Stream { -private: - ring_buffer *_cdc_rx_buffer; public: void begin(uint16_t baud_count); void end(void); @@ -43,6 +47,10 @@ public: virtual size_t write(uint8_t); using Print::write; // pull in write(str) and write(buf, size) from Print operator bool(); + + volatile uint8_t _rx_buffer_head; + volatile uint8_t _rx_buffer_tail; + unsigned char _rx_buffer[SERIAL_BUFFER_SIZE]; }; extern Serial_ Serial; diff --git a/cores/robot/CDC.cpp b/cores/robot/CDC.cpp index 701e483..fb25a96 100644 --- a/cores/robot/CDC.cpp +++ b/cores/robot/CDC.cpp @@ -23,21 +23,6 @@ #if defined(USBCON) #ifdef CDC_ENABLED -#if (RAMEND < 1000) -#define SERIAL_BUFFER_SIZE 16 -#else -#define SERIAL_BUFFER_SIZE 64 -#endif - -struct ring_buffer -{ - unsigned char buffer[SERIAL_BUFFER_SIZE]; - volatile int head; - volatile int tail; -}; - -ring_buffer cdc_rx_buffer = { { 0 }, 0, 0}; - typedef struct { u32 dwDTERate; @@ -140,8 +125,7 @@ void Serial_::end(void) void Serial_::accept(void) { - ring_buffer *buffer = &cdc_rx_buffer; - int i = (unsigned int)(buffer->head+1) % SERIAL_BUFFER_SIZE; + int i = (unsigned int)(_rx_buffer_head+1) % SERIAL_BUFFER_SIZE; // if we should be storing the received character into the location // just before the tail (meaning that the head would advance to the @@ -149,42 +133,39 @@ void Serial_::accept(void) // and so we don't write the character or advance the head. // while we have room to store a byte - while (i != buffer->tail) { + while (i != _rx_buffer_tail) { int c = USB_Recv(CDC_RX); if (c == -1) break; // no more data - buffer->buffer[buffer->head] = c; - buffer->head = i; + _rx_buffer[_rx_buffer_head] = c; + _rx_buffer_head = i; - i = (unsigned int)(buffer->head+1) % SERIAL_BUFFER_SIZE; + i = (unsigned int)(_rx_buffer_head+1) % SERIAL_BUFFER_SIZE; } } int Serial_::available(void) { - ring_buffer *buffer = &cdc_rx_buffer; - return (unsigned int)(SERIAL_BUFFER_SIZE + buffer->head - buffer->tail) % SERIAL_BUFFER_SIZE; + return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer_head - _rx_buffer_tail) % SERIAL_BUFFER_SIZE; } int Serial_::peek(void) { - ring_buffer *buffer = &cdc_rx_buffer; - if (buffer->head == buffer->tail) { + if (_rx_buffer_head == _rx_buffer_tail) { return -1; } else { - return buffer->buffer[buffer->tail]; + return _rx_buffer[_rx_buffer_tail]; } } int Serial_::read(void) { - ring_buffer *buffer = &cdc_rx_buffer; // if the head isn't ahead of the tail, we don't have any characters - if (buffer->head == buffer->tail) { + if (_rx_buffer_head == _rx_buffer_tail) { return -1; } else { - unsigned char c = buffer->buffer[buffer->tail]; - buffer->tail = (unsigned int)(buffer->tail + 1) % SERIAL_BUFFER_SIZE; + unsigned char c = _rx_buffer[_rx_buffer_tail]; + _rx_buffer_tail = (unsigned int)(_rx_buffer_tail + 1) % SERIAL_BUFFER_SIZE; return c; } } diff --git a/cores/robot/USBAPI.h b/cores/robot/USBAPI.h index 1a4183b..cabecf3 100644 --- a/cores/robot/USBAPI.h +++ b/cores/robot/USBAPI.h @@ -27,10 +27,14 @@ extern USBDevice_ USBDevice; struct ring_buffer; +#if (RAMEND < 1000) +#define SERIAL_BUFFER_SIZE 16 +#else +#define SERIAL_BUFFER_SIZE 64 +#endif + class Serial_ : public Stream { -private: - ring_buffer *_cdc_rx_buffer; public: void begin(uint16_t baud_count); void end(void); @@ -43,6 +47,10 @@ public: virtual size_t write(uint8_t); using Print::write; // pull in write(str) and write(buf, size) from Print operator bool(); + + volatile uint8_t _rx_buffer_head; + volatile uint8_t _rx_buffer_tail; + unsigned char _rx_buffer[SERIAL_BUFFER_SIZE]; }; extern Serial_ Serial; From 99bb4a573fa71a4e6c30cad144284d12b3f7edd0 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 30 Jul 2013 10:39:41 +0200 Subject: [PATCH 13/13] Applied USB CDC updates to robot's core. --- cores/robot/USBCore.cpp | 70 +++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/cores/robot/USBCore.cpp b/cores/robot/USBCore.cpp index d3e0170..c46c75f 100644 --- a/cores/robot/USBCore.cpp +++ b/cores/robot/USBCore.cpp @@ -39,8 +39,8 @@ volatile u8 RxLEDPulse; /**< Milliseconds remaining for data Rx LED pulse */ //================================================================== extern const u16 STRING_LANGUAGE[] PROGMEM; -extern const u16 STRING_IPRODUCT[] PROGMEM; -extern const u16 STRING_IMANUFACTURER[] PROGMEM; +extern const u8 STRING_PRODUCT[] PROGMEM; +extern const u8 STRING_MANUFACTURER[] PROGMEM; extern const DeviceDescriptor USB_DeviceDescriptor PROGMEM; extern const DeviceDescriptor USB_DeviceDescriptorA PROGMEM; @@ -49,31 +49,24 @@ const u16 STRING_LANGUAGE[2] = { 0x0409 // English }; -const u16 STRING_IPRODUCT[17] = { - (3<<8) | (2+2*16), -#if USB_PID == 0x8036 - 'A','r','d','u','i','n','o',' ','L','e','o','n','a','r','d','o' -#elif USB_PID == 0x8037 - 'A','r','d','u','i','n','o',' ','M','i','c','r','o',' ',' ',' ' -#elif USB_PID == 0x803C - 'A','r','d','u','i','n','o',' ','E','s','p','l','o','r','a',' ' -#elif USB_PID == 0x9208 - 'L','i','l','y','P','a','d','U','S','B',' ',' ',' ',' ',' ',' ' -#else - 'U','S','B',' ','I','O',' ','B','o','a','r','d',' ',' ',' ',' ' +#ifndef USB_PRODUCT +// If no product is provided, use USB IO Board +#define USB_PRODUCT "USB IO Board" #endif -}; -const u16 STRING_IMANUFACTURER[12] = { - (3<<8) | (2+2*11), +const u8 STRING_PRODUCT[] PROGMEM = USB_PRODUCT; + #if USB_VID == 0x2341 - 'A','r','d','u','i','n','o',' ','L','L','C' +#define USB_MANUFACTURER "Arduino LLC" #elif USB_VID == 0x1b4f - 'S','p','a','r','k','F','u','n',' ',' ',' ' -#else - 'U','n','k','n','o','w','n',' ',' ',' ',' ' +#define USB_MANUFACTURER "SparkFun" +#elif !defined(USB_MANUFACTURER) +// Fall through to unknown if no manufacturer name was provided in a macro +#define USB_MANUFACTURER "Unknown" #endif -}; + +const u8 STRING_MANUFACTURER[] PROGMEM = USB_MANUFACTURER; + #ifdef CDC_ENABLED #define DEVICE_CLASS 0x02 @@ -416,6 +409,22 @@ int USB_SendControl(u8 flags, const void* d, int len) return sent; } +// Send a USB descriptor string. The string is stored in PROGMEM as a +// plain ASCII string but is sent out as UTF-16 with the correct 2-byte +// prefix +static bool USB_SendStringDescriptor(const u8*string_P, u8 string_len) { + SendControl(2 + string_len * 2); + SendControl(3); + for(u8 i = 0; i < string_len; i++) { + bool r = SendControl(pgm_read_byte(&string_P[i])); + r &= SendControl(0); // high byte + if(!r) { + return false; + } + } + return true; +} + // Does not timeout or cross fifo boundaries // Will only work for transfers <= 64 bytes // TODO @@ -476,7 +485,6 @@ bool SendDescriptor(Setup& setup) return HID_GetDescriptor(t); #endif - u8 desc_length = 0; const u8* desc_addr = 0; if (USB_DEVICE_DESCRIPTOR_TYPE == t) { @@ -486,20 +494,22 @@ bool SendDescriptor(Setup& setup) } else if (USB_STRING_DESCRIPTOR_TYPE == t) { - if (setup.wValueL == 0) + if (setup.wValueL == 0) { desc_addr = (const u8*)&STRING_LANGUAGE; - else if (setup.wValueL == IPRODUCT) - desc_addr = (const u8*)&STRING_IPRODUCT; - else if (setup.wValueL == IMANUFACTURER) - desc_addr = (const u8*)&STRING_IMANUFACTURER; + } + else if (setup.wValueL == IPRODUCT) { + return USB_SendStringDescriptor(STRING_PRODUCT, strlen(USB_PRODUCT)); + } + else if (setup.wValueL == IMANUFACTURER) { + return USB_SendStringDescriptor(STRING_MANUFACTURER, strlen(USB_MANUFACTURER)); + } else return false; } if (desc_addr == 0) return false; - if (desc_length == 0) - desc_length = pgm_read_byte(desc_addr); + u8 desc_length = pgm_read_byte(desc_addr); USB_SendControl(TRANSFER_PGM,desc_addr,desc_length); return true;