Merge branch 'debug-serialusb'

Conflicts:
	libmaple/usb/usb.c
	notes/coding_standard.txt

9 files changed, 215 insertions, 195 deletions

diff --git a/libmaple/usb/README b/libmaple/usb/README
index f3970b6..540b1ea 100644
--- a/libmaple/usb/README
+++ b/libmaple/usb/README
@@ -5,16 +5,19 @@ The USB submodule of libmaple is responsible for:
     the USB isr, resetting the USB disc pin (used to tell the host
     were alive). Additionally, the USB submodule defines the virtual
     com port USB applications that is available to all user sketches
-    via Usb.print() and others.
+    via SerialUSB.print() and others.
 
 To use it:
-   Call Usb.init() to enable the IRQ channel, configure the clocks,
-     pull down usb_disc, and setup the vcom buffers
-   Usb.print/ln, available(), read(), write() implement the same
+
+   [This section is out of date.  Does SerialUSB.begin() do the same
+   thing as the old Usb.init()?]
+
+   SerialUSB.print/ln, available(), read(), write() implement the same
      interface as Serial1/2/3
-   
-   
+
+
 Current Status:
+
     Currently, the USB submodule relies on the low level core library
     provided by ST to access the USB peripheral registers and
     implement the USB transfer protocol for control endpoint
@@ -22,23 +25,27 @@ Current Status:
     unfortunately hard to untangle from this low level dependence, and
     when a new USB core library is written (to nix ST dependence)
     changes will likely have to be made to virtual com application
-    code. Ideally, the new core library should mimick the form of
-    MyUSB (LUFA), since this library (USB for AVR) is growing in
-    popularity and in example applications. Additionally, the USB lib
-    here relies on low level hardware functions that were just ripped
-    out of the bootloader code (for simplicity) but clearly this
-    should be replaced with direct accesses to functions provided
-    elsewhere in libmaple.
-
-    The virtual com port serves two important purposes. 1) is allows
-    serial data transfers between user sketches an a host computer. 2)
-    is allows the host machine to issue a system reset by asserting
-    the DTR signal. After reset, Maple will run the DFU bootloader for
-    a few seconds, during which the user can begin a DFU download
-    operation ('downloads' application binary into RAM/FLASH). This
-    without this virtual com port, it would be necessary to find an
-    alternative means to reset the chip in order to enable the
-    bootloader.
+    code. Ideally, the new core library should mimic the form of MyUSB
+    (LUFA), since this library (USB for AVR) is growing in popularity
+    and in example applications. Additionally, the USB lib here relies
+    on low level hardware functions that were just ripped out of the
+    bootloader code (for simplicity) but clearly this should be
+    replaced with direct accesses to functions provided elsewhere in
+    libmaple.
+
+    The virtual com port serves two important purposes.
+
+      1) It allows serial data transfers between user sketches an a
+         host computer.
+
+      2) It allows the host machine to issue a system reset by
+         asserting the DTR signal.
+
+    After reset, Maple will run the DFU bootloader for a few seconds,
+    during which the user can begin a DFU upload operation (uploads
+    application binary into RAM/FLASH). Thus, without this virtual com
+    port, it would be necessary to find an alternative means to reset
+    the chip in order to enable the bootloader.
 
     If you would like to develop your own USB application for whatever
     reason (uses faster isochronous enpoints for streaming audio, or
@@ -51,19 +58,19 @@ Current Status:
 
     It would be possible to build a compound USB device, that
     implements endpoints for both the virtual COM port as well as some
-    other components (mass sotrage etc.) however this turns out to be
-    a burden from the host driver side, as windows and *nix handle
+    other components (mass storage etc.).  However, this turns out to
+    be a burden from the host driver side, as Windows and *nix handle
     compound USB devices quite differently.
 
-    Be mindful that running the USB application isnt "free." The
+    Be mindful that enabling the USB peripheral isnt "free." The
     device must respond to periodic bus activity (every few
-    milliseconds) by servicing an ISR. Therefore the USB application
+    milliseconds) by servicing an ISR. Therefore, the USB application
     should be disabled inside of timing critical applications. In
     order to disconnect the device from the host, the USB_DISC pin can
-    be asserted (on Maple v1,2,3 this is GPIOC,12). Alternatively, the
-    NVIC can be directly configured to disable the USB LP/HP IRQ's
+    be asserted (on Maple this is GPIO C12). Alternatively, the NVIC
+    can be directly configured to disable the USB LP/HP IRQ's.
 
-    This library should exposed through usb.h, do not include any
+    This library should exposed through usb.h; do not include any
     other files direcly in your application.
 
     The files inside of usb_lib were provided by ST and are subject to
@@ -72,21 +79,22 @@ Current Status:
 
 Integration with libmaple:
 
-    The current USB lib is ported directly from the maple bootloader
-    code, adapted to be a virtual com rather than a DFU device. That
-    means several functions are redefined locally that could have been
-    pulled from elsewhere in libmaple. Thus, ths USB module depends
-    absolutely zero on libmaple, it even ensures that clocks are
-    configured correctly for its operation.
+    The current USB lib is ported from the Maple bootloader code,
+    adapted to be a virtual com rather than a DFU device. That means
+    several functions are redefined locally that could have been
+    pulled from elsewhere in libmaple.  Thus, ths USB module doesn't
+    have too many dependencies on libmaple.  It even ensures that
+    clocks are configured correctly for its operation.  However, over
+    time, some libmaple dependencies have crept in.
 
 Todo:
 
     - write custom low level USB stack to strip out any remaining
       dependence on ST code
     - remove dependence on hardware.c, since any functions here really
-      should have their own analogues elsewhere inside libmaple
+      should have their own analogs elsewhere inside libmaple
     - add a high level USB application library that would allow users
       to make their own HID/Mass Storage/Audio/Video devices.
-    - implement a Usb.link(SerialX) that forces a passthrough
+    - implement a SerialUSB.link(SerialX) that forces a passthrough
       the host computer virtual com to SerialX, and utilizes the
       line_config commands correctly (sets baud etc)
diff --git a/libmaple/usb/descriptors.h b/libmaple/usb/descriptors.h
index 6652942..6f7d08b 100644
--- a/libmaple/usb/descriptors.h
+++ b/libmaple/usb/descriptors.h
@@ -1,4 +1,27 @@
-/* insert license */
+/* *****************************************************************************
+ * The MIT License
+ *
+ * Copyright (c) 2010 LeafLabs LLC.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ * ****************************************************************************/
+
 #ifndef __DESCRIPTORS_H
 #define __DESCRIPTORS_H
 
diff --git a/libmaple/usb/usb.c b/libmaple/usb/usb.c
index 39bf31a..d875785 100644
--- a/libmaple/usb/usb.c
+++ b/libmaple/usb/usb.c
@@ -332,82 +332,68 @@ void usbWaitReset(void) {
  * This function will quickly copy up to 64 bytes of data (out of an
  * arbitrarily large buffer) into the USB peripheral TX buffer and return the
  * number placed in that buffer. It is up to usercode to divide larger packets
- * into 64-byte chunks to guarantee delivery. Use usbGetCountTx() to determine
- * whether the bytes were ACTUALLY recieved by the host or just transfered to
- * the buffer.
+ * into 64-byte chunks to guarantee delivery.
  *
- * The function will return -1 if it doesn't think that the USB host is
- * "connected", but it can't detect this state robustly. "Connected" in this
- * context means that an actual program on the Host operating system is
- * connected to the virtual COM/ttyACM device and is recieving the bytes; the
- * Host operating system is almost always configured and keeping this endpoint
- * alive, but the bytes never get read out of the endpoint buffer.
  *
- * The behavior of this function is subtle and frustrating; it has gone through
- * many simpler and cleaner implementation that frustratingly don't work cross
- * platform.
- *
- * */
-uint16 usbSendBytes(uint8* sendBuf, uint16 len) {
-
-  uint16 loaded = 0;
-
-  if (bDeviceState != CONFIGURED || (!usbGetDTR() && !usbGetRTS())) {
-    // Indicates to caller to stop trying, were not configured/connected
-    // The DTR and RTS lines are handled differently on major platforms, so
-    // the above logic is unreliable
-    return 0;
-  }
+ */
+void usbBlockingSendByte(char ch) {
+    while (countTx);
+    UserToPMABufferCopy(&ch,VCOM_TX_ADDR,1);
+    _SetEPTxCount(VCOM_TX_ENDP,1);
+    _SetEPTxValid(VCOM_TX_ENDP);
+    countTx = 1;
+    while (countTx);
+}
+uint32 usbSendBytes(uint8* sendBuf, uint32 len) {
+  /* any checks on connection (via dtr/rts) done upstream in wirish or
+     by user */
 
-  // Due to a variety of shit this is how we roll; all buffering etc is pushed
-  // upstream
+  /* last xmit hasnt finished, abort */
   if (countTx) {
     return 0;
   }
 
   // We can only put VCOM_TX_EPSIZE bytes in the buffer
-  if(len > VCOM_TX_EPSIZE) {
-    loaded = VCOM_TX_EPSIZE;
-  } else {
-    loaded = len;
+  if(len > VCOM_TX_EPSIZE/2) {
+    len = VCOM_TX_EPSIZE/2;
   }
 
   // Try to load some bytes if we can
-  if (loaded) {
-    UserToPMABufferCopy(sendBuf,VCOM_TX_ADDR + countTx, loaded);
-    _SetEPTxCount(VCOM_TX_ENDP, countTx+loaded);
-    countTx += loaded;
+  if (len) {
+    UserToPMABufferCopy(sendBuf,VCOM_TX_ADDR, len);
+    _SetEPTxCount(VCOM_TX_ENDP, len);
+    countTx += len;
     _SetEPTxValid(VCOM_TX_ENDP);
   }
 
-  return loaded;
+  return len;
 }
 
 /* returns the number of available bytes are in the recv FIFO */
-uint8 usbBytesAvailable(void) {
-  return VCOM_RX_EPSIZE - maxNewBytes;
+uint32 usbBytesAvailable(void) {
+  return newBytes;
 }
 
 /* copies len bytes from the local recieve FIFO (not
    usb packet buffer) into recvBuf and deq's the fifo.
    will only copy the minimum of len or the available
    bytes. returns the number of bytes copied */
-uint8 usbReceiveBytes(uint8* recvBuf, uint8 len) {
-  if (len > VCOM_RX_EPSIZE - maxNewBytes) {
-    len = VCOM_RX_EPSIZE - maxNewBytes;
+uint32 usbReceiveBytes(uint8* recvBuf, uint32 len) {
+  if (len > newBytes) {
+      len = newBytes;
   }
 
   int i;
   for (i=0;i<len;i++) {
-    recvBuf[i] = (uint8)(vcomBufferRx[recvBufOut]);
-    recvBufOut = (recvBufOut + 1) % VCOM_RX_EPSIZE;
+    recvBuf[i] = (uint8)(vcomBufferRx[i]);
   }
 
-  maxNewBytes += len;
+  newBytes -= len;
 
   /* re-enable the rx endpoint which we had set to receive 0 bytes */
-  if (maxNewBytes - len == 0) {
-    SetEPRxCount(VCOM_RX_ENDP,maxNewBytes);
+  if (newBytes == 0) {
+    SetEPRxCount(VCOM_RX_ENDP,VCOM_RX_EPSIZE);
+    SetEPRxStatus(VCOM_RX_ENDP,EP_RX_VALID);
   }
 
   return len;
diff --git a/libmaple/usb/usb.h b/libmaple/usb/usb.h
index ffba9ff..0ed02e5 100644
--- a/libmaple/usb/usb.h
+++ b/libmaple/usb/usb.h
@@ -73,9 +73,10 @@ void usb_lpIRQHandler(void);
 void usbWaitReset(void);
 
 /* blocking functions for send/receive */
-uint16 usbSendBytes(uint8* sendBuf,uint16 len);
-uint8 usbBytesAvailable(void);
-uint8 usbReceiveBytes(uint8* recvBuf, uint8 len);
+void   usbBlockingSendByte(char ch);
+uint32 usbSendBytes(uint8* sendBuf,uint32 len);
+uint32 usbBytesAvailable(void);
+uint32 usbReceiveBytes(uint8* recvBuf, uint32 len);
 uint8 usbGetDTR(void);
 uint8 usbGetRTS(void);
 uint8 usbIsConnected(void);
diff --git a/libmaple/usb/usb_callbacks.c b/libmaple/usb/usb_callbacks.c
index 250acea..ccb0fdd 100644
--- a/libmaple/usb/usb_callbacks.c
+++ b/libmaple/usb/usb_callbacks.c
@@ -32,12 +32,12 @@ USB_Line_Coding line_coding = {
  datatype:    0x08
 };
 
-uint8 vcomBufferRx[VCOM_RX_EPSIZE];
-volatile uint8 countTx    = 0;
-volatile uint8 recvBufIn  = 0;
-volatile uint8 recvBufOut = 0;
-volatile uint8 maxNewBytes   = VCOM_RX_EPSIZE;
-
+uint8 vcomBufferRx[VCOM_RX_BUFLEN];
+volatile uint32 countTx    = 0;
+volatile uint32 recvBufIn  = 0;
+volatile uint32 recvBufOut = 0;
+volatile uint32 maxNewBytes   = VCOM_RX_BUFLEN;
+volatile uint32 newBytes = 0;
 RESET_STATE reset_state = DTR_UNSET;
 uint8       line_dtr_rts = 0;
 
@@ -46,7 +46,7 @@ void vcomDataTxCb(void) {
 
     /* allows usbSendBytes to stop blocking */
 
-
+    /* assumes tx transactions are atomic 64 bytes (nearly certain they are) */
     countTx = 0;
 }
 
@@ -56,84 +56,65 @@ void vcomDataTxCb(void) {
    in the fifo. this number will be reincremented after calls
    to usbReceiveBytes */
 void vcomDataRxCb(void) {
-    /* do whatever after data has been received from host */
-
-    /* setEPRxCount on the previous cycle should garuntee
-       we havnt received more bytes than we can fit */
-    uint8 newBytes = GetEPRxCount(VCOM_RX_ENDP);
-    /* assert (newBytes <= maxNewBytes); */
-
-    /* todo, not checking very carefully for edge cases. USUALLY,
-       if we emit the reset pulse and send 4 bytes, then newBytes 
-       should be 4. But its POSSIBLE that this would be violated
-       in some cases */
-
-    /* magic number, {0x31, 0x45, 0x41, 0x46} is "1EAF" */
-    char chkBuf[4];
-    char cmpBuf[4] = {0x31, 0x45, 0x41, 0x46};
-    if (reset_state == DTR_NEGEDGE) {
-        reset_state = DTR_LOW;
-
-        if  (newBytes >= 4) {
-            unsigned int target = (unsigned int)usbWaitReset | 0x1;
-
-            PMAToUserBufferCopy(chkBuf,VCOM_RX_ADDR,4);
-
-            int i;
-            USB_Bool cmpMatch = TRUE;
-            for (i=0; i<4; i++) {
-                if (chkBuf[i] != cmpBuf[i]) {
-                    cmpMatch = FALSE;
-                }
-            }
-
-            if (cmpMatch) {
-                asm volatile("mov r0, %[stack_top]      \n\t"             // Reset the stack
-                             "mov sp, r0                \n\t"
-                             "mov r0, #1                \n\t"
-                             "mov r1, %[target_addr]    \n\t"
-                             "mov r2, %[cpsr]           \n\t"
-                             "push {r2}                 \n\t"             // Fake xPSR
-                             "push {r1}                 \n\t"             // Target address for PC
-                             "push {r0}                 \n\t"             // Fake LR
-                             "push {r0}                 \n\t"             // Fake R12
-                             "push {r0}                 \n\t"             // Fake R3
-                             "push {r0}                 \n\t"             // Fake R2
-                             "push {r0}                 \n\t"             // Fake R1
-                             "push {r0}                 \n\t"             // Fake R0
-                             "mov lr, %[exc_return]     \n\t"
-                             "bx lr"
-                             :
-                             : [stack_top] "r" (STACK_TOP),
-                               [target_addr] "r" (target),
-                               [exc_return] "r" (EXC_RETURN),
-                               [cpsr] "r" (DEFAULT_CPSR)
-                             : "r0", "r1", "r2");
-                /* should never get here */
-            }
-        }
+  /* do whatever after data has been received from host */
+
+  /* setEPRxCount on the previous cycle should garuntee
+     we havnt received more bytes than we can fit */
+  newBytes = GetEPRxCount(VCOM_RX_ENDP);
+  SetEPRxStatus(VCOM_RX_ENDP,EP_RX_NAK);
+
+  /* todo, not checking very carefully for edge cases. USUALLY,
+     if we emit the reset pulse and send 4 bytes, then newBytes
+     should be 4. But its POSSIBLE that this would be violated
+     in some cases */
+
+  /* magic number, {0x31, 0x45, 0x41, 0x46} is "1EAF" */
+  char chkBuf[4];
+  char cmpBuf[4] = {0x31, 0x45, 0x41, 0x46};
+  if (reset_state == DTR_NEGEDGE) {
+    reset_state = DTR_LOW;
+
+    if  (newBytes >= 4) {
+      unsigned int target = (unsigned int)usbWaitReset | 0x1;
+
+      PMAToUserBufferCopy(chkBuf,VCOM_RX_ADDR,4);
+
+      int i;
+      USB_Bool cmpMatch = TRUE;
+      for (i=0; i<4; i++) {
+          if (chkBuf[i] != cmpBuf[i]) {
+              cmpMatch = FALSE;
+          }
+      }
+
+      if (cmpMatch) {
+          asm volatile("mov r0, %[stack_top]      \n\t"             // Reset the stack
+                       "mov sp, r0                \n\t"
+                       "mov r0, #1                \n\t"
+                       "mov r1, %[target_addr]    \n\t"
+                       "mov r2, %[cpsr]           \n\t"
+                       "push {r2}                 \n\t"             // Fake xPSR
+                       "push {r1}                 \n\t"             // Target address for PC
+                       "push {r0}                 \n\t"             // Fake LR
+                       "push {r0}                 \n\t"             // Fake R12
+                       "push {r0}                 \n\t"             // Fake R3
+                       "push {r0}                 \n\t"             // Fake R2
+                       "push {r0}                 \n\t"             // Fake R1
+                       "push {r0}                 \n\t"             // Fake R0
+                       "mov lr, %[exc_return]     \n\t"
+                       "bx lr"
+                       :
+                       : [stack_top] "r" (STACK_TOP),
+                         [target_addr] "r" (target),
+                         [exc_return] "r" (EXC_RETURN),
+                         [cpsr] "r" (DEFAULT_CPSR)
+                       : "r0", "r1", "r2");
+          /* should never get here */
+      }
     }
+  }
 
-
-
-    if (recvBufIn + newBytes < VCOM_RX_EPSIZE) {
-        PMAToUserBufferCopy(&vcomBufferRx[recvBufIn],VCOM_RX_ADDR,newBytes);
-        recvBufIn += newBytes;
-    } else {
-        /* we have to copy the data in two chunks because we roll over
-           the edge of the circular buffer */
-        uint8 tailBytes = VCOM_RX_EPSIZE - recvBufIn;
-        uint8 remaining = newBytes - tailBytes;
-
-        PMAToUserBufferCopy(&vcomBufferRx[recvBufIn],VCOM_RX_ADDR,tailBytes);
-        PMAToUserBufferCopy(&vcomBufferRx[0],        VCOM_RX_ADDR,remaining);
-
-        recvBufIn = (recvBufIn + newBytes ) % VCOM_RX_EPSIZE;
-    }
-  
-    maxNewBytes    -= newBytes;
-    SetEPRxCount(VCOM_RX_ENDP,maxNewBytes);
-    SetEPRxValid(VCOM_RX_ENDP);
+  PMAToUserBufferCopy(&vcomBufferRx[0],VCOM_RX_ADDR,newBytes);
 }
 
 void vcomManagementCb(void) {
@@ -177,7 +158,7 @@ void usbReset(void) {
     _SetEPRxAddr(ENDP0,VCOM_CTRL_RX_ADDR);
     _SetEPTxAddr(ENDP0,VCOM_CTRL_TX_ADDR);
     Clear_Status_Out(ENDP0);
-  
+
     SetEPRxCount(ENDP0, pProperty->MaxPacketSize);
     SetEPRxValid(ENDP0);
 
@@ -232,11 +213,11 @@ RESULT usbDataSetup(uint8 request) {
 
     if (Type_Recipient == (CLASS_REQUEST | INTERFACE_RECIPIENT)) {
         switch (request) {
-        case (GET_LINE_CODING): 
+        case (GET_LINE_CODING):
             CopyRoutine = vcomGetSetLineCoding;
             last_request = GET_LINE_CODING;
             break;
-        case (SET_LINE_CODING): 
+        case (SET_LINE_CODING):
             CopyRoutine = vcomGetSetLineCoding;
             last_request = SET_LINE_CODING;
             break;
@@ -284,7 +265,7 @@ RESULT usbNoDataSetup(u8 request) {
                     reset_state = DTR_NEGEDGE;
                 } else {
                     reset_state = DTR_HIGH;
-                }	
+                }
                 break;
 
             case DTR_NEGEDGE:
diff --git a/libmaple/usb/usb_callbacks.h b/libmaple/usb/usb_callbacks.h
index a94de11..20d2c13 100644
--- a/libmaple/usb/usb_callbacks.h
+++ b/libmaple/usb/usb_callbacks.h
@@ -34,11 +34,12 @@ typedef enum {
 
 extern RESET_STATE reset_state;  /* tracks DTR/RTS */
 extern uint8       line_dtr_rts;  
-extern volatile uint8 countTx;
-extern uint8 vcomBufferRx[VCOM_RX_EPSIZE];  /* no reason this has to be VCOM_RX_EPSIZE, could be bigger */
-extern volatile uint8 recvBufIn;   /* the FIFO in index to the recvbuffer */
-extern volatile uint8 recvBufOut;  /* the FIFO out index to the recvbuffer */
-extern volatile uint8 maxNewBytes;
+extern volatile uint32 countTx;
+extern uint8 vcomBufferRx[VCOM_RX_BUFLEN];  /* no reason this has to be VCOM_RX_EPSIZE, could be bigger */
+extern volatile uint32 recvBufIn;   /* the FIFO in index to the recvbuffer */
+extern volatile uint32 recvBufOut;  /* the FIFO out index to the recvbuffer */
+extern volatile uint32 maxNewBytes;
+extern volatile uint32 newBytes;
 
 void vcomDataTxCb(void);
 void vcomDataRxCb(void);
diff --git a/libmaple/usb/usb_config.h b/libmaple/usb/usb_config.h
index ba05d42..e5f3979 100644
--- a/libmaple/usb/usb_config.h
+++ b/libmaple/usb/usb_config.h
@@ -26,6 +26,7 @@
 #define VCOM_RX_EPNUM             0x03
 #define VCOM_RX_ADDR              0x110
 #define VCOM_RX_EPSIZE            0x40
+#define VCOM_RX_BUFLEN            (VCOM_RX_EPSIZE*3)
 
 #define bMaxPacketSize            0x40  /* 64B, maximum for USB FS Devices */
 
diff --git a/notes/coding_standard.txt b/notes/coding_standard.txt
index 0e250d5..5cb96c3 100644
--- a/notes/coding_standard.txt
+++ b/notes/coding_standard.txt
@@ -60,7 +60,7 @@ Whitespace/Indentation
 
 - Files end in exactly one newline. [The presence of a newline at EOF
   is already done by `c-require-final-newline' in recent versions of
-  emacs.]
+  Emacs.]
 
 - Exactly two newlines separate source paragraphs (you do separate
   your code into paragraphs, don't you?).
@@ -140,9 +140,8 @@ Naming conventions
 - Structs: pick a style from "Variables" or "Classes" depending on how
   you mean it (since it might be either a simple record type, in which
   case do like c variables, or you might be faking an object in c, in
-  which case do like classes).  If it's in a typedef, should also
-  probably put _t at the end, but maybe you won't, and I don't really
-  feel too strongly about it.
+  which case do like classes).  If it's in a typedef, don't feel
+  obliged to put "_t" at the end of the name; we don't.
 
 - Macros and constants: all caps, separated by underscores.  Variables
   with the "const" qualifier aren't considered "constants" for the
@@ -169,7 +168,7 @@ Naming conventions
 Documentation
 -------------
 
-- Document your code!
+- Document your code.  This should go without saying.
 
 - For complicated peripherals, it would be nice if you put longer-form
   comments into this directory (notes/), with a comment in the
@@ -185,6 +184,21 @@ Documentation
   michaeljones breathe repos on github and potentially figure out
   why).
 
+- Doxygen comments generally just belong on types, functions,
+  etc. that are part of the public user-facing API.  This generally
+  means that if there's ReST documentation for it under docs/source/,
+  it needs Doxygen comments, and that ReST should use Breathe to pull
+  that Doxygen comment out. (For more info on this, see docs/README).
+
+  There are some exceptions to this rule since Breathe isn't totally
+  mature yet and Sphinx's C++ domain is still in flux.  In these
+  cases, document the code "manually" in ReST.
+
+  This should be avoided if at all possible, since it creates a
+  maintenance burden of documenting things in two places at once, and
+  provides an opportunity for bad documentation to slip in, when the
+  code comments fall out of sync with the ReST docs.
+
 General Formatting
 ------------------
 
@@ -201,14 +215,9 @@ General Formatting
   Then put the file somewhere in your load-path, and:
 
     (require 'lineker)
-    (dolist (hook programming-mode-hooks)
+    (dolist (hook '(c-mode-hook c++-mode-hook))
       (add-hook hook (lambda () (lineker-mode 1))))
 
-  Where "programming-mode-hooks" is a list that should at least
-  include c-mode-hook and c++-mode-hook; e.g.:
-
-    (setq programming-mode-hooks '(c-mode-hook
-                                   c++-mode-hook
-                                   python-mode-hook
-                                   ...
-                                   ))
+  There are only a few exceptional situations.  The most important one
+  is when specifying a lookup table like PIN_MAP where it'd be ugly to
+  split each entry over multiple lines.
diff --git a/wirish/usb_serial.cpp b/wirish/usb_serial.cpp
index 5c8a65f..e2cdee3 100644
--- a/wirish/usb_serial.cpp
+++ b/wirish/usb_serial.cpp
@@ -101,17 +101,27 @@ uint32 USBSerial::available(void) {
     return usbBytesAvailable();
 }
 
+/* blocks forever until len_bytes is received */
 uint32 USBSerial::read(void *buf, uint32 len) {
     if (!buf) {
         return 0;
     }
 
-    return usbReceiveBytes((uint8*)buf, len);
+    uint32 bytes_in = 0;
+    while (len > 0) {
+        uint32 new_bytes = usbReceiveBytes((uint8*)((uint8*)buf+bytes_in), len);
+        len -= new_bytes;
+        bytes_in += new_bytes;
+    }
+
+    return len;
 }
 
+/* blocks forever until 1 byte is received */
 uint8 USBSerial::read(void) {
     uint8 ch;
-    usbReceiveBytes(&ch, 1);
+
+    while (usbReceiveBytes(&ch, 1) == 0);
     return ch;
 }