Cleanup USBSerial

Make the following changes
-Make blocking the first parameter in the constructor
-Add destructor and proper cleanup
-Add ready and wait_ready functions

Author: c1728p9

usb/device/USBSerial/USBCDC.cpp

@@ -18,6 +18,7 @@
 #include "USBCDC.h"
 #include "EndpointResolver.h"
 #include "AsyncOp.h"
+#include "usb_phy_api.h"
 
 static const uint8_t cdc_line_coding_default[7] = {0x80, 0x25, 0x00, 0x00, 0x00, 0x00, 0x08};
 
@@ -58,20 +59,31 @@ class USBCDC::AsyncRead: public AsyncOp {
     bool result;
 };
 
-USBCDC::USBCDC(uint16_t vendor_id, uint16_t product_id, uint16_t product_release, bool connect_blocking)
-    : USBDevice(vendor_id, product_id, product_release)
+USBCDC::USBCDC(bool connect_blocking, uint16_t vendor_id, uint16_t product_id, uint16_t product_release)
+    : USBDevice(get_usb_phy(), vendor_id, product_id, product_release)
 
 {
-    _init(connect_blocking);
+    _init();
+    if (connect_blocking) {
+        connect();
+        wait_ready();
+    } else {
+        init();
+    }
 }
 
-USBCDC::USBCDC(USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint16_t product_release, bool connect_blocking)
+USBCDC::USBCDC(USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint16_t product_release)
     : USBDevice(phy, vendor_id, product_id, product_release)
 {
-    _init(connect_blocking);
+    _init();
 }
 
-void USBCDC::_init(bool connect_blocking)
+USBCDC::~USBCDC()
+{
+    deinit();
+}
+
+void USBCDC::_init()
 {
     memcpy(_cdc_line_coding, cdc_line_coding_default, sizeof(_cdc_line_coding));
 
@@ -91,13 +103,6 @@ void USBCDC::_init(bool connect_blocking)
     _rx_in_progress = false;
     _rx_buf = _rx_buffer;
     _rx_size = 0;
-
-    init();
-
-    USBDevice::connect(false);
-    if (connect_blocking) {
-        wait_connected();
-    }
 }
 
 void USBCDC::callback_reset()
@@ -234,7 +239,17 @@ void USBCDC::_change_terminal_connected(bool connected)
     _terminal_connected = connected;
 }
 
-void USBCDC::wait_connected()
+bool USBCDC::ready()
+{
+    lock();
+
+    bool ready = _terminal_connected;
+
+    unlock();
+    return ready;
+}
+
+void USBCDC::wait_ready()
 {
     lock();
 

usb/device/USBSerial/USBCDC.h

@@ -29,31 +29,57 @@ class AsyncOp;
 class USBCDC: public USBDevice {
 public:
 
-    /*
-    * Constructor
+    /**
+    * Basic constructor
+    *
+    * Construct this object optionally connecting and blocking until it is ready.
+    *
+    * @note Do not use this constructor in derived classes.
     *
+    * @param connect_blocking true to perform a blocking connect, false to start in a disconnected state
     * @param vendor_id Your vendor_id
     * @param product_id Your product_id
-    * @param product_release Your preoduct_release
-    * @param connect_blocking define if the connection must be blocked if USB not plugged in
+    * @param product_release Your product_release
     */
-    USBCDC(uint16_t vendor_id, uint16_t product_id, uint16_t product_release, bool connect_blocking);
+    USBCDC(bool connect_blocking, uint16_t vendor_id, uint16_t product_id, uint16_t product_release);
 
-    /*
-    * Constructor
+    /**
+    * Fully featured constructor
+    *
+    * Construct this object with the supplied USBPhy and parameters. The user
+    * this object is responsible for calling connect() or init().
+    *
+    * @note Derived classes must use this constructor and call init() or
+    * connect() themselves. Derived classes should also call deinit() in
+    * their destructor. This ensures that no interrupts can occur when the
+    * object is partially constructed or destroyed.
     *
     * @param phy USB phy to use
     * @param vendor_id Your vendor_id
     * @param product_id Your product_id
-    * @param product_release Your preoduct_release
-    * @param connect_blocking define if the connection must be blocked if USB not plugged in
+    * @param product_release Your product_release
     */
-    USBCDC(USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint16_t product_release, bool connect_blocking);
+    USBCDC(USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint16_t product_release);
+
+    /**
+     * Destroy this object
+     *
+     * Any classes which inherit from this class must call deinit
+     * before this destructor runs.
+     */
+    virtual ~USBCDC();
+
+    /**
+     * Check if this class is ready
+     *
+     * @return true if a terminal is connected, false otherwise
+     */
+    bool ready();
 
     /**
      * Block until the terminal is connected
      */
-    void wait_connected();
+    void wait_ready();
 
     /*
     * Send a buffer
@@ -162,7 +188,7 @@ class USBCDC: public USBDevice {
     virtual void callback_set_configuration(uint8_t configuration);
     virtual void callback_set_interface(uint16_t interface, uint8_t alternate);
 
-    void _init(bool connect_blocking);
+    void _init();
 
     void _change_terminal_connected(bool connected);
     void _connect_wake_all();

usb/device/USBSerial/USBSerial.cpp

@@ -16,10 +16,40 @@
 
 #include "stdint.h"
 #include "USBSerial.h"
+#include "usb_phy_api.h"
+
+
+USBSerial::USBSerial(bool connect_blocking, uint16_t vendor_id, uint16_t product_id, uint16_t product_release):
+        USBCDC(get_usb_phy(), vendor_id, product_id, product_release)
+{
+    _settings_changed_callback = 0;
+
+    if (connect_blocking) {
+        connect();
+        wait_ready();
+    } else {
+        init();
+    }
+}
+
+USBSerial::USBSerial(USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint16_t product_release):
+        USBCDC(phy, vendor_id, product_id, product_release)
+{
+    _settings_changed_callback = 0;
+}
+
+USBSerial::~USBSerial()
+{
+    deinit();
+}
 
 int USBSerial::_putc(int c)
 {
-    return send((uint8_t *)&c, 1) ? 1 : 0;
+    if (send((uint8_t *)&c, 1)) {
+        return c;
+    } else {
+        return -1;
+    }
 }
 
 int USBSerial::_getc()

usb/device/USBSerial/USBSerial.h

@@ -45,33 +45,46 @@ class USBSerial: public USBCDC, public Stream {
 public:
 
     /**
-    *   Constructor
+    * Basic constructor
     *
+    * Construct this object optionally connecting and blocking until it is ready.
+    *
+    * @note Do not use this constructor in derived classes.
+    *
+    * @param connect_blocking true to perform a blocking connect, false to start in a disconnected state
     * @param vendor_id Your vendor_id (default: 0x1f00)
     * @param product_id Your product_id (default: 0x2012)
-    * @param product_release Your preoduct_release (default: 0x0001)
-    * @param connect_blocking define if the connection must be blocked if USB not plugged in
+    * @param product_release Your product_release (default: 0x0001)
     *
     */
-    USBSerial(uint16_t vendor_id = 0x1f00, uint16_t product_id = 0x2012, uint16_t product_release = 0x0001, bool connect_blocking = true): USBCDC(vendor_id, product_id, product_release, connect_blocking)
-    {
-        _settings_changed_callback = 0;
-    };
+    USBSerial(bool connect_blocking=true, uint16_t vendor_id=0x1f00, uint16_t product_id=0x2012, uint16_t product_release=0x0001);
 
     /**
-    *   Constructor
+    * Fully featured constructor
+    *
+    * Construct this object with the supplied USBPhy and parameters. The user
+    * this object is responsible for calling connect() or init().
+    *
+    * @note Derived classes must use this constructor and call init() or
+    * connect() themselves. Derived classes should also call deinit() in
+    * their destructor. This ensures that no interrupts can occur when the
+    * object is partially constructed or destroyed.
     *
     * @param phy USB phy to use
     * @param vendor_id Your vendor_id (default: 0x1f00)
     * @param product_id Your product_id (default: 0x2012)
-    * @param product_release Your preoduct_release (default: 0x0001)
-    * @param connect_blocking define if the connection must be blocked if USB not plugged in
+    * @param product_release Your product_release (default: 0x0001)
     *
     */
-    USBSerial(USBPhy *phy, uint16_t vendor_id = 0x1f00, uint16_t product_id = 0x2012, uint16_t product_release = 0x0001, bool connect_blocking = true): USBCDC(phy, vendor_id, product_id, product_release, connect_blocking)
-    {
-        _settings_changed_callback = 0;
-    };
+    USBSerial(USBPhy *phy, uint16_t vendor_id=0x1f00, uint16_t product_id=0x2012, uint16_t product_release=0x0001);
+
+    /**
+     * Destroy this object
+     *
+     * Any classes which inherit from this class must call deinit
+     * before this destructor runs.
+     */
+    virtual ~USBSerial();
 
     /**
     * Send a character. You can use puts, printf.