[riot-notifications] [RIOT-OS/RIOT] usbus: Add automated test (#12267)

Dylan Laduranty notifications at github.com
Wed Sep 18 11:43:27 CEST 2019


dylad commented on this pull request.

quick round.
I'll try to test whenever I can

> @@ -213,21 +212,19 @@ static int _recv_dev_setup(usbus_t *usbus, usb_setup_t *pkt)
             case USB_SETUP_REQ_SET_ADDRESS:
                 DEBUG("usbus_control: Setting address\n");
                 usbus->addr = (uint8_t)pkt->value;
-                res = 0;
+                res = 1;

Any reason to change this ? Usually we consider 0 to be OK and < 0 not OK

> @@ -307,6 +307,17 @@ static void _usbus_config_ep0(usbus_control_handler_t *ep0_handler)
     usbdev_ep_ready(ep0_handler->out, 0);
 }
 
+uint8_t *usbus_control_get_out_data(usbus_t *usbus, size_t *len)
+{
+    assert((int)usbus->state == (int)USBUS_CONTROL_REQUEST_STATE_OUTDATA);

Is it desireable to mix up `usbus_state_t` with `usbus_setuprq_state_t` ?

> +    }
+}
+
+static void _ep_esr_validation(usbdev_mock_t *dev, usbdev_mock_ep_t *ep)
+{
+    DEBUG("[ep esr]: Data available for stack: %u\n", ep->available);
+    if (req_phase == TEST_REQ_PHASE_IDLE) {
+        DEBUG("[ep esr]: Done with the request\n");
+        /* signal USBDEV_EVENT_ESR to call _test_sequence */
+        dev->usbdev.cb(&dev->usbdev, USBDEV_EVENT_ESR);
+    }
+    ep->ep.dev->epcb(&ep->ep, USBDEV_EVENT_TR_COMPLETE);
+}
+
+/* Handles Control endpoint validation from the usbdev side  */
+/* Called as part of the endpoint ready function             */

put these comments together

> +{
+    (void)ep;
+}
+
+int _ep_get(usbdev_ep_t *ep, usbopt_ep_t opt,
+            void *value, size_t max_len)
+{
+    usbdev_mock_ep_t *testep = (usbdev_mock_ep_t *)ep;
+
+    (void)max_len;
+    switch (opt) {
+        case USBOPT_EP_AVAILABLE:
+            *((size_t *)value) = testep->available;
+            return sizeof(size_t);
+        default:
+            break;

Please add a DEBUG statement

> + *
+ * @param dev   usbdev mock device state
+ * @param ep    usbdev mock device endpoint
+ * @param len   length supplied to the ready call
+ */
+typedef void (*usbdev_mock_ready_cb_t)(usbdev_mock_t *dev,
+                                       usbdev_mock_ep_t *ep,
+                                       size_t len);
+
+/**
+ * @brief usbdev mock device
+ */
+struct usbdev_mock {
+    usbdev_t usbdev;                    /**< Generic usbdev device          */
+    usbdev_mock_ep_t in[1];             /**< IN endpoints                   */
+    usbdev_mock_ep_t out[1];            /**< OUT endpoints                  */

why an array with one element ??

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/12267#pullrequestreview-289787016
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190918/4b59958c/attachment.htm>


More information about the notifications mailing list